From 56497bd5f1340cb97d94cab953b050be6f729790 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 15 Feb 2006 15:01:42 +0900 Subject: [PATCH 1/6] [PATCH] libata: fix comment regarding setting cable type The comment above ata_std_postreset() specified that setting cable type is the responsibility of postreset(), which isn't possible / optimal depending on controller / driver. This patch kills the comment. Setting cable type is responsibility of ->probe_reset. libata doesn't care whether it's done in probeinit, reset or postreset. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/scsi/libata-core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index 76621463b499..62340f4ef004 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c @@ -2112,7 +2112,6 @@ int sata_std_hardreset(struct ata_port *ap, int verbose, unsigned int *class) * This function is invoked after a successful reset. Note that * the device might have been reset more than once using * different reset methods before postreset is invoked. - * postreset is also reponsible for setting cable type. * * This function is to be used as standard callback for * ata_drive_*_reset(). @@ -2124,7 +2123,7 @@ void ata_std_postreset(struct ata_port *ap, unsigned int *classes) { DPRINTK("ENTER\n"); - /* set cable type */ + /* set cable type if it isn't already set */ if (ap->cbl == ATA_CBL_NONE && ap->flags & ATA_FLAG_SATA) ap->cbl = ATA_CBL_SATA; From 573db6b8f93b5e359abd9a05e620fd9a589b0209 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 15 Feb 2006 15:01:42 +0900 Subject: [PATCH 2/6] [PATCH] ata_piix: convert pata to new reset mechanism Convert ata_piix pata ->phy_reset to new reset mechanism. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/scsi/ata_piix.c | 43 ++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c index 4cc1108f721a..d79c252a3f60 100644 --- a/drivers/scsi/ata_piix.c +++ b/drivers/scsi/ata_piix.c @@ -131,7 +131,7 @@ enum { static int piix_init_one (struct pci_dev *pdev, const struct pci_device_id *ent); -static void piix_pata_phy_reset(struct ata_port *ap); +static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes); static void piix_sata_phy_reset(struct ata_port *ap); static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev); static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev); @@ -207,7 +207,7 @@ static const struct ata_port_operations piix_pata_ops = { .exec_command = ata_exec_command, .dev_select = ata_std_dev_select, - .phy_reset = piix_pata_phy_reset, + .probe_reset = piix_pata_probe_reset, .bmdma_setup = ata_bmdma_setup, .bmdma_start = ata_bmdma_start, @@ -258,8 +258,7 @@ static struct ata_port_info piix_port_info[] = { /* ich5_pata */ { .sht = &piix_sht, - .host_flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST | - PIIX_FLAG_CHECKINTR, + .host_flags = ATA_FLAG_SLAVE_POSS | PIIX_FLAG_CHECKINTR, .pio_mask = 0x1f, /* pio0-4 */ #if 0 .mwdma_mask = 0x06, /* mwdma1-2 */ @@ -284,7 +283,7 @@ static struct ata_port_info piix_port_info[] = { /* piix4_pata */ { .sht = &piix_sht, - .host_flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST, + .host_flags = ATA_FLAG_SLAVE_POSS, .pio_mask = 0x1f, /* pio0-4 */ #if 0 .mwdma_mask = 0x06, /* mwdma1-2 */ @@ -366,30 +365,42 @@ cbl40: } /** - * piix_pata_phy_reset - Probe specified port on PATA host controller - * @ap: Port to probe + * piix_pata_probeinit - probeinit for PATA host controller + * @ap: Target port * - * Probe PATA phy. + * Probeinit including cable detection. * * LOCKING: * None (inherited from caller). */ +static void piix_pata_probeinit(struct ata_port *ap) +{ + piix_pata_cbl_detect(ap); + ata_std_probeinit(ap); +} -static void piix_pata_phy_reset(struct ata_port *ap) +/** + * piix_pata_probe_reset - Perform reset on PATA port and classify + * @ap: Port to reset + * @classes: Resulting classes of attached devices + * + * Reset PATA phy and classify attached devices. + * + * LOCKING: + * None (inherited from caller). + */ +static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes) { struct pci_dev *pdev = to_pci_dev(ap->host_set->dev); if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->hard_port_no])) { - ata_port_disable(ap); printk(KERN_INFO "ata%u: port disabled. ignoring.\n", ap->id); - return; + return 0; } - piix_pata_cbl_detect(ap); - - ata_port_probe(ap); - - ata_bus_reset(ap); + return ata_drive_probe_reset(ap, piix_pata_probeinit, + ata_std_softreset, NULL, + ata_std_postreset, classes); } /** From ccbe6d5ee0eb3182675ef1c84322810fd884586d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 15 Feb 2006 15:01:42 +0900 Subject: [PATCH 3/6] [PATCH] ata_piix: convert sata to new reset mechanism Convert ata_piix sata ->phy_reset to new reset mechanism. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/scsi/ata_piix.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c index d79c252a3f60..c662bf531514 100644 --- a/drivers/scsi/ata_piix.c +++ b/drivers/scsi/ata_piix.c @@ -132,7 +132,7 @@ static int piix_init_one (struct pci_dev *pdev, const struct pci_device_id *ent); static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes); -static void piix_sata_phy_reset(struct ata_port *ap); +static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes); static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev); static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev); @@ -235,7 +235,7 @@ static const struct ata_port_operations piix_sata_ops = { .exec_command = ata_exec_command, .dev_select = ata_std_dev_select, - .phy_reset = piix_sata_phy_reset, + .probe_reset = piix_sata_probe_reset, .bmdma_setup = ata_bmdma_setup, .bmdma_start = ata_bmdma_start, @@ -272,8 +272,8 @@ static struct ata_port_info piix_port_info[] = { /* ich5_sata */ { .sht = &piix_sht, - .host_flags = ATA_FLAG_SATA | ATA_FLAG_SRST | - PIIX_FLAG_COMBINED | PIIX_FLAG_CHECKINTR, + .host_flags = ATA_FLAG_SATA | PIIX_FLAG_COMBINED | + PIIX_FLAG_CHECKINTR, .pio_mask = 0x1f, /* pio0-4 */ .mwdma_mask = 0x07, /* mwdma0-2 */ .udma_mask = 0x7f, /* udma0-6 */ @@ -297,8 +297,7 @@ static struct ata_port_info piix_port_info[] = { /* ich6_sata */ { .sht = &piix_sht, - .host_flags = ATA_FLAG_SATA | ATA_FLAG_SRST | - PIIX_FLAG_COMBINED_ICH6 | + .host_flags = ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 | PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS, .pio_mask = 0x1f, /* pio0-4 */ .mwdma_mask = 0x07, /* mwdma0-2 */ @@ -309,8 +308,7 @@ static struct ata_port_info piix_port_info[] = { /* ich6_sata_ahci */ { .sht = &piix_sht, - .host_flags = ATA_FLAG_SATA | ATA_FLAG_SRST | - PIIX_FLAG_COMBINED_ICH6 | + .host_flags = ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 | PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS | PIIX_FLAG_AHCI, .pio_mask = 0x1f, /* pio0-4 */ @@ -448,28 +446,25 @@ static int piix_sata_probe (struct ata_port *ap) } /** - * piix_sata_phy_reset - Probe specified port on SATA host controller - * @ap: Port to probe + * piix_sata_probe_reset - Perform reset on SATA port and classify + * @ap: Port to reset + * @classes: Resulting classes of attached devices * - * Probe SATA phy. + * Reset SATA phy and classify attached devices. * * LOCKING: * None (inherited from caller). */ - -static void piix_sata_phy_reset(struct ata_port *ap) +static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes) { if (!piix_sata_probe(ap)) { - ata_port_disable(ap); printk(KERN_INFO "ata%u: SATA port has no device.\n", ap->id); - return; + return 0; } - ap->cbl = ATA_CBL_SATA; - - ata_port_probe(ap); - - ata_bus_reset(ap); + return ata_drive_probe_reset(ap, ata_std_probeinit, + ata_std_softreset, NULL, + ata_std_postreset, classes); } /** From 9ae61c6cb69f5251d160576c324948805f97e901 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 20 Feb 2006 23:48:37 +0900 Subject: [PATCH 4/6] [PATCH] libata: fix WARN_ON() condition in *_fill_sg() For ATAPI commands, padding can reduce qc->n_elem by one and thus to zero making assert(qc->n_elem > 0)'s in ata_fill_sg() and qs_fill_sg() fail for legal commands. This patch fixes the assert()'s to take qc->pad_len into account. Although the condition check seems a bit excessive, as this part of code isn't still stable yet, I think it's worth to keep those. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/scsi/libata-core.c | 2 +- drivers/scsi/sata_qstor.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index 7ddd5a69352a..bbac87a13d57 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c @@ -2570,7 +2570,7 @@ static void ata_fill_sg(struct ata_queued_cmd *qc) unsigned int idx; assert(qc->__sg != NULL); - assert(qc->n_elem > 0); + assert(qc->n_elem > 0 || qc->pad_len > 0); idx = 0; ata_for_each_sg(sg, qc) { diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c index de05e2883f9c..80480f0fb2b8 100644 --- a/drivers/scsi/sata_qstor.c +++ b/drivers/scsi/sata_qstor.c @@ -277,7 +277,7 @@ static unsigned int qs_fill_sg(struct ata_queued_cmd *qc) u8 *prd = pp->pkt + QS_CPB_BYTES; assert(qc->__sg != NULL); - assert(qc->n_elem > 0); + assert(qc->n_elem > 0 || qc->pad_len > 0); nelem = 0; ata_for_each_sg(sg, qc) { From cc1887f3d8ae8ea61efa1a75af8ec0467b9dd546 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 20 Feb 2006 23:48:38 +0900 Subject: [PATCH 5/6] [PATCH] libata: fix qc->n_elem == 0 case handling in ata_qc_next_sg This patch makes ata_for_each_sg() start with pad_sgent when qc->n_elem is zero. Previously, ata_for_each_sg() unconditionally started with qc->__sg, handling the first sg to fill_sg() routines even when the entry was invalid. And while at it, unwind ?: in ata_qc_next_sg() into if statement. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- include/linux/libata.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/include/linux/libata.h b/include/linux/libata.h index 9e5db2949c58..c91be5e64ede 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -556,6 +556,16 @@ ata_sg_is_last(struct scatterlist *sg, struct ata_queued_cmd *qc) return 0; } +static inline struct scatterlist * +ata_qc_first_sg(struct ata_queued_cmd *qc) +{ + if (qc->n_elem) + return qc->__sg; + if (qc->pad_len) + return &qc->pad_sgent; + return NULL; +} + static inline struct scatterlist * ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc) { @@ -563,11 +573,13 @@ ata_qc_next_sg(struct scatterlist *sg, struct ata_queued_cmd *qc) return NULL; if (++sg - qc->__sg < qc->n_elem) return sg; - return qc->pad_len ? &qc->pad_sgent : NULL; + if (qc->pad_len) + return &qc->pad_sgent; + return NULL; } #define ata_for_each_sg(sg, qc) \ - for (sg = qc->__sg; sg; sg = ata_qc_next_sg(sg, qc)) + for (sg = ata_qc_first_sg(qc); sg; sg = ata_qc_next_sg(sg, qc)) static inline unsigned int ata_tag_valid(unsigned int tag) { From 2e242fa994428bd1a40b6a7e97430413246d0a16 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 20 Feb 2006 23:48:38 +0900 Subject: [PATCH 6/6] [PATCH] libata: make ata_sg_setup_one() trim zero length sg This patch makes ata_sg_setup_one() trim sg entry (thus making qc->n_elem zero) if padding results in zero length sg entry. Signed-off-by: Tejun Heo Signed-off-by: Jeff Garzik --- drivers/scsi/libata-core.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c index bbac87a13d57..5f1d7580218d 100644 --- a/drivers/scsi/libata-core.c +++ b/drivers/scsi/libata-core.c @@ -2514,7 +2514,7 @@ static void ata_sg_clean(struct ata_queued_cmd *qc) assert(sg != NULL); if (qc->flags & ATA_QCFLAG_SINGLE) - assert(qc->n_elem == 1); + assert(qc->n_elem <= 1); VPRINTK("unmapping %u sg elements\n", qc->n_elem); @@ -2537,7 +2537,7 @@ static void ata_sg_clean(struct ata_queued_cmd *qc) kunmap_atomic(addr, KM_IRQ0); } } else { - if (sg_dma_len(&sg[0]) > 0) + if (qc->n_elem) dma_unmap_single(ap->host_set->dev, sg_dma_address(&sg[0]), sg_dma_len(&sg[0]), dir); @@ -2715,6 +2715,7 @@ static int ata_sg_setup_one(struct ata_queued_cmd *qc) int dir = qc->dma_dir; struct scatterlist *sg = qc->__sg; dma_addr_t dma_address; + int trim_sg = 0; /* we must lengthen transfers to end on a 32-bit boundary */ qc->pad_len = sg->length & 3; @@ -2734,13 +2735,15 @@ static int ata_sg_setup_one(struct ata_queued_cmd *qc) sg_dma_len(psg) = ATA_DMA_PAD_SZ; /* trim sg */ sg->length -= qc->pad_len; + if (sg->length == 0) + trim_sg = 1; DPRINTK("padding done, sg->length=%u pad_len=%u\n", sg->length, qc->pad_len); } - if (!sg->length) { - sg_dma_address(sg) = 0; + if (trim_sg) { + qc->n_elem--; goto skip_map; } @@ -2753,9 +2756,9 @@ static int ata_sg_setup_one(struct ata_queued_cmd *qc) } sg_dma_address(sg) = dma_address; -skip_map: sg_dma_len(sg) = sg->length; +skip_map: DPRINTK("mapped buffer of %d bytes for %s\n", sg_dma_len(sg), qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");