From 1871ee134b73fb4cadab75752a7152ed2813c751 Mon Sep 17 00:00:00 2001 From: Kevin Hao Date: Sat, 12 Jul 2014 12:08:24 +0800 Subject: libata: support the ata host which implements a queue depth less than 32 The sata on fsl mpc8315e is broken after the commit 8a4aeec8d2d6 ("libata/ahci: accommodate tag ordered controllers"). The reason is that the ata controller on this SoC only implement a queue depth of 16. When issuing the commands in tag order, all the commands in tag 16 ~ 31 are mapped to tag 0 unconditionally and then causes the sata malfunction. It makes no senses to use a 32 queue in software while the hardware has less queue depth. So consider the queue depth implemented by the hardware when requesting a command tag. Fixes: 8a4aeec8d2d6 ("libata/ahci: accommodate tag ordered controllers") Cc: stable@vger.kernel.org Signed-off-by: Kevin Hao Acked-by: Dan Williams Signed-off-by: Tejun Heo --- drivers/ata/libata-core.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 18d97d5c7d9..d19c37a7abc 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4787,6 +4787,10 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words) * ata_qc_new - Request an available ATA command, for queueing * @ap: target port * + * Some ATA host controllers may implement a queue depth which is less + * than ATA_MAX_QUEUE. So we shouldn't allocate a tag which is beyond + * the hardware limitation. + * * LOCKING: * None. */ @@ -4794,14 +4798,16 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words) static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap) { struct ata_queued_cmd *qc = NULL; - unsigned int i, tag; + unsigned int i, tag, max_queue; + + max_queue = ap->scsi_host->can_queue; /* no command while frozen */ if (unlikely(ap->pflags & ATA_PFLAG_FROZEN)) return NULL; - for (i = 0; i < ATA_MAX_QUEUE; i++) { - tag = (i + ap->last_tag + 1) % ATA_MAX_QUEUE; + for (i = 0, tag = ap->last_tag + 1; i < max_queue; i++, tag++) { + tag = tag < max_queue ? tag : 0; /* the last tag is reserved for internal command. */ if (tag == ATA_TAG_INTERNAL) @@ -6169,6 +6175,16 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; + /* + * The max queue supported by hardware must not be greater than + * ATA_MAX_QUEUE. + */ + if (sht->can_queue > ATA_MAX_QUEUE) { + dev_err(host->dev, "BUG: the hardware max queue is too large\n"); + WARN_ON(1); + return -EINVAL; + } + /* host must have been started */ if (!(host->flags & ATA_HOST_STARTED)) { dev_err(host->dev, "BUG: trying to register unstarted host\n"); -- cgit v1.2.3-70-g09d2 From eec7e1c16d2b65e38137686dd9b7e102c2150905 Mon Sep 17 00:00:00 2001 From: Alexey Asemov Date: Tue, 15 Jul 2014 10:28:42 +0400 Subject: libata: EH should handle AMNF error condition as a media error libata-eh.c should handle AMNF error condition (error byte bit 0, usually code 0x01) in libata-eh.c along with UNC as a media error so SCSI stack can handle it properly (translation code 0x01 is already present in libata-scsi.c) but was never passed down due to lack of handling in EH. While using linux-based machine (AMD 6550M-based notebook, PCI IDs for the controller are 1022:7801 subsys 1025:059d) and ddrescue to salvage data from failing hard drive (WD7500BPVT 2.5" 750G SATA2), I've found that pure AMNF 0x01 error code generates generic "device error" that is retried several times by SCSI stack instead of "media error" that is passed up to software. So we may assume deprecated AMNF error code is surely not dead yet, and it's better for it to be handled properly. As we may see it is used by modern enough devices, and used properly: drive returned AMNF only when IDs for track cannot be read completely due to dying head or positioning, otherwise it returned UNC(orrectables). Not handling it causes wrong generic error code ("device error") reporting down the stack, can damage failing drives further because of excessive retries, and slows salvaging down a lot. Also, there is handling code in libata-scsi.c for 0x01 AMNF error already. https://bugzilla.kernel.org/show_bug.cgi?id=80031 tj: Shortened $SUBJ and moved its content to the first paragraph. Signed-off-by: Alexey Asemov Signed-off-by: Tejun Heo --- drivers/ata/libata-eh.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 6760fc4e85b..dad83df555c 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1811,7 +1811,7 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc, case ATA_DEV_ATA: if (err & ATA_ICRC) qc->err_mask |= AC_ERR_ATA_BUS; - if (err & ATA_UNC) + if (err & (ATA_UNC | ATA_AMNF)) qc->err_mask |= AC_ERR_MEDIA; if (err & ATA_IDNF) qc->err_mask |= AC_ERR_INVALID; @@ -2556,11 +2556,12 @@ static void ata_eh_link_report(struct ata_link *link) } if (cmd->command != ATA_CMD_PACKET && - (res->feature & (ATA_ICRC | ATA_UNC | ATA_IDNF | - ATA_ABORTED))) - ata_dev_err(qc->dev, "error: { %s%s%s%s}\n", + (res->feature & (ATA_ICRC | ATA_UNC | ATA_AMNF | + ATA_IDNF | ATA_ABORTED))) + ata_dev_err(qc->dev, "error: { %s%s%s%s%s}\n", res->feature & ATA_ICRC ? "ICRC " : "", res->feature & ATA_UNC ? "UNC " : "", + res->feature & ATA_AMNF ? "AMNF " : "", res->feature & ATA_IDNF ? "IDNF " : "", res->feature & ATA_ABORTED ? "ABRT " : ""); #endif -- cgit v1.2.3-70-g09d2 From a2a9e02b5b67a7a32a14ab6c4c331a1a0c23a1db Mon Sep 17 00:00:00 2001 From: Andrey Utkin Date: Thu, 17 Jul 2014 15:13:23 +0300 Subject: drivers/ata/pata_ep93xx.c: use signed int type for result of platform_get_irq() [linux-3.16-rc5/drivers/ata/pata_ep93xx.c:929]: (style) Checking if unsigned variable 'irq' is less than zero. Source code is irq = platform_get_irq(pdev, 0); if (irq < 0) { but unsigned int irq; $ fgrep platform_get_irq `find . -name \*.h -print` ./include/linux/platform_device.h:extern int platform_get_irq(struct platform_device *, unsigned int); Now using "int" type instead of "unsigned int" for "irq" variable. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=80401 Reported-by: David Binderman Signed-off-by: Andrey Utkin Signed-off-by: Tejun Heo --- drivers/ata/pata_ep93xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c index 6ad5c072ce3..4d37c5415fc 100644 --- a/drivers/ata/pata_ep93xx.c +++ b/drivers/ata/pata_ep93xx.c @@ -915,7 +915,7 @@ static int ep93xx_pata_probe(struct platform_device *pdev) struct ep93xx_pata_data *drv_data; struct ata_host *host; struct ata_port *ap; - unsigned int irq; + int irq; struct resource *mem_res; void __iomem *ide_base; int err; -- cgit v1.2.3-70-g09d2 From b32bfc06aefab61acc872dec3222624e6cd867ed Mon Sep 17 00:00:00 2001 From: Romain Degez Date: Fri, 11 Jul 2014 18:08:13 +0200 Subject: ahci: add support for the Promise FastTrak TX8660 SATA HBA (ahci mode) Add support of the Promise FastTrak TX8660 SATA HBA in ahci mode by registering the board in the ahci_pci_tbl[]. Note: this HBA also provide a hardware RAID mode when activated in BIOS but specific drivers from the manufacturer are required in this case. Signed-off-by: Romain Degez Tested-by: Romain Degez Signed-off-by: Tejun Heo Cc: stable@vger.kernel.org --- drivers/ata/ahci.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index dae5607e111..4cd52a4541a 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -456,6 +456,7 @@ static const struct pci_device_id ahci_pci_tbl[] = { /* Promise */ { PCI_VDEVICE(PROMISE, 0x3f20), board_ahci }, /* PDC42819 */ + { PCI_VDEVICE(PROMISE, 0x3781), board_ahci }, /* FastTrak TX8660 ahci-mode */ /* Asmedia */ { PCI_VDEVICE(ASMEDIA, 0x0601), board_ahci }, /* ASM1060 */ -- cgit v1.2.3-70-g09d2