Currently, workaround for broken WD drives is applied always, slowing down all drives. And it has a bug - it's not applied after resume. Apply the workaround only if the error really appears (SErr == 0x1000500). This allows unaffected drives to run at full speed (provided that no affected drive is connected to the controller). Also make sure the workaround is re-applied on resume. Tested on VT6421. As SCR registers access is known to cause problems on VT6420 (and I don't have it to test), keep the workaround applied always on VT6420. Unaffected drive (Hitachi HDS721680PLA380): Before: $ hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 160 MB in 3.01 seconds = 53.16 MB/sec After: $ hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 200 MB in 3.01 seconds = 66.47 MB/sec Affected drive (WDC WD5003ABYX-18WERA0): Before: $ hdparm -t --direct /dev/sda /dev/sda: Timing O_DIRECT disk reads: 180 MB in 3.02 seconds = 59.51 MB/sec After: $ hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 156 MB in 3.03 seconds = 51.48 MB/sec $ hdparm -t --direct /dev/sdb /dev/sdb: Timing O_DIRECT disk reads: 180 MB in 3.02 seconds = 59.64 MB/sec The first hdparm is slower because of the error: [ 50.408042] ata5: Incompatible drive: enabling workaround. This slows down transfer rate to ~60 MB/s [ 50.728052] ata5: SATA link up 1.5 Gbps (SStatus 113 SControl 310) [ 50.744834] ata5.00: configured for UDMA/133 Signed-off-by: Ondrej Zary <linux@xxxxxxxxxxxxxxxxxxxx> --- drivers/ata/sata_via.c | 79 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 72 insertions(+), 7 deletions(-) diff --git a/drivers/ata/sata_via.c b/drivers/ata/sata_via.c index 17d31fc..a58511d 100644 --- a/drivers/ata/sata_via.c +++ b/drivers/ata/sata_via.c @@ -73,7 +73,14 @@ enum { SATA_EXT_PHY = (1 << 6), /* 0==use PATA, 1==ext phy */ }; +struct svia_priv { + bool wd_workaround; +}; + static int svia_init_one(struct pci_dev *pdev, const struct pci_device_id *ent); +#ifdef CONFIG_PM_SLEEP +static int svia_pci_device_resume(struct pci_dev *pdev); +#endif static int svia_scr_read(struct ata_link *link, unsigned int sc_reg, u32 *val); static int svia_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val); static int vt8251_scr_read(struct ata_link *link, unsigned int scr, u32 *val); @@ -85,6 +92,7 @@ static void vt6420_bmdma_start(struct ata_queued_cmd *qc); static int vt6421_pata_cable_detect(struct ata_port *ap); static void vt6421_set_pio_mode(struct ata_port *ap, struct ata_device *adev); static void vt6421_set_dma_mode(struct ata_port *ap, struct ata_device *adev); +static void vt6421_error_handler(struct ata_port *ap); static const struct pci_device_id svia_pci_tbl[] = { { PCI_VDEVICE(VIA, 0x5337), vt6420 }, @@ -105,7 +113,7 @@ static struct pci_driver svia_pci_driver = { .probe = svia_init_one, #ifdef CONFIG_PM_SLEEP .suspend = ata_pci_device_suspend, - .resume = ata_pci_device_resume, + .resume = svia_pci_device_resume, #endif .remove = ata_pci_remove_one, }; @@ -137,6 +145,7 @@ static struct ata_port_operations vt6421_sata_ops = { .inherits = &svia_base_ops, .scr_read = svia_scr_read, .scr_write = svia_scr_write, + .error_handler = vt6421_error_handler, }; static struct ata_port_operations vt8251_ops = { @@ -536,7 +545,36 @@ static int vt8251_prepare_host(struct pci_dev *pdev, struct ata_host **r_host) return 0; } -static void svia_configure(struct pci_dev *pdev, int board_id) +static void svia_wd_fix(struct pci_dev *pdev) +{ + u8 tmp8; + + pci_read_config_byte(pdev, 0x52, &tmp8); + pci_write_config_byte(pdev, 0x52, tmp8 | BIT(2)); +} + +static void vt6421_error_handler(struct ata_port *ap) +{ + struct svia_priv *hpriv = ap->host->private_data; + struct pci_dev *pdev = to_pci_dev(ap->host->dev); + u32 serror; + + /* see svia_configure() for description */ + if (!hpriv->wd_workaround) { + svia_scr_read(&ap->link, SCR_ERROR, &serror); + if (serror == 0x1000500) { + ata_port_warn(ap, "Incompatible drive: enabling workaround. This slows down transfer rate to ~60 MB/s"); + svia_wd_fix(pdev); + hpriv->wd_workaround = true; + ap->link.eh_context.i.flags |= ATA_EHI_QUIET; + } + } + + ata_sff_error_handler(ap); +} + +static void svia_configure(struct pci_dev *pdev, int board_id, + struct svia_priv *hpriv) { u8 tmp8; @@ -593,11 +631,15 @@ static void svia_configure(struct pci_dev *pdev, int board_id) * https://bugzilla.kernel.org/show_bug.cgi?id=15173 * http://article.gmane.org/gmane.linux.ide/46352 * http://thread.gmane.org/gmane.linux.kernel/1062139 + * + * As the fix slows down data transfer, apply it only if the error + * actually appears - see vt6421_error_handler() + * Apply the fix always on vt6420 as we don't know if SCR_ERROR can be + * read safely. */ - if (board_id == vt6420 || board_id == vt6421) { - pci_read_config_byte(pdev, 0x52, &tmp8); - tmp8 |= 1 << 2; - pci_write_config_byte(pdev, 0x52, tmp8); + if (board_id == vt6420) { + svia_wd_fix(pdev); + hpriv->wd_workaround = true; } } @@ -608,6 +650,7 @@ static int svia_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) struct ata_host *host = NULL; int board_id = (int) ent->driver_data; const unsigned *bar_sizes; + struct svia_priv *hpriv; ata_print_version_once(&pdev->dev, DRV_VERSION); @@ -647,11 +690,33 @@ static int svia_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) if (rc) return rc; - svia_configure(pdev, board_id); + hpriv = devm_kzalloc(&pdev->dev, sizeof(*hpriv), GFP_KERNEL); + if (!hpriv) + return -ENOMEM; + host->private_data = hpriv; + + svia_configure(pdev, board_id, hpriv); pci_set_master(pdev); return ata_host_activate(host, pdev->irq, ata_bmdma_interrupt, IRQF_SHARED, &svia_sht); } +#ifdef CONFIG_PM_SLEEP +static int svia_pci_device_resume(struct pci_dev *pdev) +{ + struct ata_host *host = pci_get_drvdata(pdev); + int rc; + + rc = ata_pci_device_do_resume(pdev); + if (rc) + return rc; + + svia_wd_fix(pdev); + ata_host_resume(host); + + return 0; +} +#endif + module_pci_driver(svia_pci_driver); -- Ondrej Zary -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html