From 68dbbe7d5b4fde736d104cbbc9a2fce875562012 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Thu, 4 Nov 2021 17:31:58 +0900 Subject: libata: fix read log timeout value Some ATA drives are very slow to respond to READ_LOG_EXT and READ_LOG_DMA_EXT commands issued from ata_dev_configure() when the device is revalidated right after resuming a system or inserting the ATA adapter driver (e.g. ahci). The default 5s timeout (ATA_EH_CMD_DFL_TIMEOUT) used for these commands is too short, causing errors during the device configuration. Ex: ... ata9: SATA max UDMA/133 abar m524288@0x9d200000 port 0x9d200400 irq 209 ata9: SATA link up 6.0 Gbps (SStatus 133 SControl 300) ata9.00: ATA-9: XXX XXXXXXXXXXXXXXX, XXXXXXXX, max UDMA/133 ata9.00: qc timeout (cmd 0x2f) ata9.00: Read log page 0x00 failed, Emask 0x4 ata9.00: Read log page 0x00 failed, Emask 0x40 ata9.00: NCQ Send/Recv Log not supported ata9.00: Read log page 0x08 failed, Emask 0x40 ata9.00: 27344764928 sectors, multi 16: LBA48 NCQ (depth 32), AA ata9.00: Read log page 0x00 failed, Emask 0x40 ata9.00: ATA Identify Device Log not supported ata9.00: failed to set xfermode (err_mask=0x40) ata9: SATA link up 6.0 Gbps (SStatus 133 SControl 300) ata9.00: configured for UDMA/133 ... The timeout error causes a soft reset of the drive link, followed in most cases by a successful revalidation as that give enough time to the drive to become fully ready to quickly process the read log commands. However, in some cases, this also fails resulting in the device being dropped. Fix this by using adding the ata_eh_revalidate_timeouts entries for the READ_LOG_EXT and READ_LOG_DMA_EXT commands. This defines a timeout increased to 15s, retriable one time. Reported-by: Geert Uytterhoeven Tested-by: Geert Uytterhoeven Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal --- drivers/ata/libata-eh.c | 8 ++++++++ include/linux/libata.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index bf9c4b6c5c3d..1d4a6f1e88cd 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -93,6 +93,12 @@ static const unsigned long ata_eh_identify_timeouts[] = { ULONG_MAX, }; +static const unsigned long ata_eh_revalidate_timeouts[] = { + 15000, /* Some drives are slow to read log pages when waking-up */ + 15000, /* combined time till here is enough even for media access */ + ULONG_MAX, +}; + static const unsigned long ata_eh_flush_timeouts[] = { 15000, /* be generous with flush */ 15000, /* ditto */ @@ -129,6 +135,8 @@ static const struct ata_eh_cmd_timeout_ent ata_eh_cmd_timeout_table[ATA_EH_CMD_TIMEOUT_TABLE_SIZE] = { { .commands = CMDS(ATA_CMD_ID_ATA, ATA_CMD_ID_ATAPI), .timeouts = ata_eh_identify_timeouts, }, + { .commands = CMDS(ATA_CMD_READ_LOG_EXT, ATA_CMD_READ_LOG_DMA_EXT), + .timeouts = ata_eh_revalidate_timeouts, }, { .commands = CMDS(ATA_CMD_READ_NATIVE_MAX, ATA_CMD_READ_NATIVE_MAX_EXT), .timeouts = ata_eh_other_timeouts, }, { .commands = CMDS(ATA_CMD_SET_MAX, ATA_CMD_SET_MAX_EXT), diff --git a/include/linux/libata.h b/include/linux/libata.h index 2884383f1718..5331557316e8 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -394,7 +394,7 @@ enum { /* This should match the actual table size of * ata_eh_cmd_timeout_table in libata-eh.c. */ - ATA_EH_CMD_TIMEOUT_TABLE_SIZE = 6, + ATA_EH_CMD_TIMEOUT_TABLE_SIZE = 7, /* Horkage types. May be set by libata or controller on drives (some horkage may be drive/controller pair dependent */ -- cgit v1.2.3-70-g09d2 From 51839e25d43dc6c91d653995a41a3dfc52a21e33 Mon Sep 17 00:00:00 2001 From: Xu Wang Date: Fri, 5 Nov 2021 01:50:21 +0000 Subject: ata: sata_highbank: Remove unnecessary print function dev_err() The print function dev_err() is redundant because platform_get_irq() already prints an error. Signed-off-by: Xu Wang Signed-off-by: Damien Le Moal --- drivers/ata/sata_highbank.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c index 8440203e835e..b29d3f1d64b0 100644 --- a/drivers/ata/sata_highbank.c +++ b/drivers/ata/sata_highbank.c @@ -469,10 +469,8 @@ static int ahci_highbank_probe(struct platform_device *pdev) } irq = platform_get_irq(pdev, 0); - if (irq < 0) { - dev_err(dev, "no irq\n"); + if (irq < 0) return irq; - } if (!irq) return -EINVAL; -- cgit v1.2.3-70-g09d2 From 636f6e2af4fb916b9f1b432964294b6979c34002 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Tue, 9 Nov 2021 08:45:25 +0900 Subject: libata: add horkage for missing Identify Device log ACS-3 introduced the ATA Identify Device Data log as mandatory. A warning message currently signals to the user if a device does not report supporting this log page in the log directory page, regardless of the ATA version of the device. Furthermore, this warning will appear for all attempts at accessing this missing log page during device revalidation. Since it is useless to constantly access the log directory and warn about this lack of support once we have discovered that the device does not support this log page, introduce the horkage flag ATA_HORKAGE_NO_ID_DEV_LOG to mark a device as lacking support for the Identify Device Data log page. Set this flag when ata_log_supported() returns false in ata_identify_page_supported(). The warning is printed only if the device ATA level is 10 or above (ACS-3 or above), and only once on device scan. With this flag set, the log directory page is not accessed again to test for Identify Device Data log page support. Signed-off-by: Damien Le Moal Reviewed-by: Christoph Hellwig Reviewed-by: Martin K. Petersen --- drivers/ata/libata-core.c | 13 ++++++++++++- include/linux/libata.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 3018ca84a3d8..8a0ccb190d76 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2052,8 +2052,19 @@ static bool ata_identify_page_supported(struct ata_device *dev, u8 page) struct ata_port *ap = dev->link->ap; unsigned int err, i; + if (dev->horkage & ATA_HORKAGE_NO_ID_DEV_LOG) + return false; + if (!ata_log_supported(dev, ATA_LOG_IDENTIFY_DEVICE)) { - ata_dev_warn(dev, "ATA Identify Device Log not supported\n"); + /* + * IDENTIFY DEVICE data log is defined as mandatory starting + * with ACS-3 (ATA version 10). Warn about the missing log + * for drives which implement this ATA level or above. + */ + if (ata_id_major_version(dev->id) >= 10) + ata_dev_warn(dev, + "ATA Identify Device Log not supported\n"); + dev->horkage |= ATA_HORKAGE_NO_ID_DEV_LOG; return false; } diff --git a/include/linux/libata.h b/include/linux/libata.h index 5331557316e8..2a8404b26083 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -427,6 +427,7 @@ enum { ATA_HORKAGE_MAX_SEC_1024 = (1 << 25), /* Limit max sects to 1024 */ ATA_HORKAGE_MAX_TRIM_128M = (1 << 26), /* Limit max trim size to 128M */ ATA_HORKAGE_NO_NCQ_ON_ATI = (1 << 27), /* Disable NCQ on ATI chipset */ + ATA_HORKAGE_NO_ID_DEV_LOG = (1 << 28), /* Identify device log missing */ /* DMA mask for user DMA control: User visible values; DO NOT renumber */ -- cgit v1.2.3-70-g09d2 From 1b87bda1f29a91720a410ac0819866a3cf0df32d Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Thu, 11 Nov 2021 12:03:27 +0900 Subject: libata: libahci: declare ahci_shost_attr_group as static ahci_shost_attr_group is referenced only in drivers/ata/libahci.c. Declare it as static. Fixes: c3f69c7f629f ("scsi: ata: Switch to attribute groups") Cc: Bart Van Assche Signed-off-by: Damien Le Moal Reviewed-by: Christoph Hellwig --- drivers/ata/libahci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index 28430c093a7f..8a6835bfd18a 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -131,7 +131,7 @@ const struct attribute_group *ahci_shost_groups[] = { }; EXPORT_SYMBOL_GPL(ahci_shost_groups); -struct attribute *ahci_sdev_attrs[] = { +static struct attribute *ahci_sdev_attrs[] = { &dev_attr_sw_activity.attr, &dev_attr_unload_heads.attr, &dev_attr_ncq_prio_supported.attr, -- cgit v1.2.3-70-g09d2