[PATCH] sata_via: Apply WD workaround only when needed on VT6421

[PATCH] sata_via: Apply WD workaround only when needed on VT6421

[Ondrej Zary]

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@rainbow-software.org>
---
 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