libata: clear SError after link resume

SError used to be cleared in ->postreset.  This has small hotplug race
condition.  If a device is plugged in after reset is complete but
postreset hasn't run yet, its hotplug event gets lost when SError is
cleared.  This patch makes sata_link_resume() clear SError.  This
kills the race condition and makes a lot of sense as some PMP and host
PHYs don't work properly without SError cleared.

This change makes sata_pmp_std_{pre|post}_reset()'s unnecessary as
they become identical to ata_std counterparts.  It also simplifies
sata_pmp_hardreset() and ahci_vt8251_hardreset().

Signed-off-by: Tejun Heo <htejun@gmail.com>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 0f553aaa6f79a5c1170d76c3352efc13cb905433..a69bcca4eb1b32c679e80cfdc4c47512d1aaab12 100644 (file)
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1377,7 +1377,6 @@ static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
                                 unsigned long deadline)
 {
        struct ata_port *ap = link->ap;
-       u32 serror;
        bool online;
        int rc;
 
@@ -1388,10 +1387,6 @@ static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
        rc = sata_link_hardreset(link, sata_ehc_deb_timing(&link->eh_context),
                                 deadline, &online, NULL);
 
-       /* vt8251 needs SError cleared for the port to operate */
-       ahci_scr_read(ap, SCR_ERROR, &serror);
-       ahci_scr_write(ap, SCR_ERROR, serror);
-
        ahci_start_engine(ap);
 
        DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c4fd4afbf34951c108ec22ab7e63542f16892692..e00b620f161ae46de49def383fb83c85f34f4b21 100644 (file)
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -90,9 +90,9 @@ const struct ata_port_operations sata_port_ops = {
 const struct ata_port_operations sata_pmp_port_ops = {
        .inherits               = &sata_port_ops,
 
-       .pmp_prereset           = sata_pmp_std_prereset,
+       .pmp_prereset           = ata_std_prereset,
        .pmp_hardreset          = sata_pmp_std_hardreset,
-       .pmp_postreset          = sata_pmp_std_postreset,
+       .pmp_postreset          = ata_std_postreset,
        .error_handler          = sata_pmp_error_handler,
 };
 
@@ -3493,7 +3493,7 @@ int sata_link_debounce(struct ata_link *link, const unsigned long *params,
 int sata_link_resume(struct ata_link *link, const unsigned long *params,
                     unsigned long deadline)
 {
-       u32 scontrol;
+       u32 scontrol, serror;
        int rc;
 
        if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
@@ -3509,7 +3509,25 @@ int sata_link_resume(struct ata_link *link, const unsigned long *params,
         */
        msleep(200);
 
-       return sata_link_debounce(link, params, deadline);
+       if ((rc = sata_link_debounce(link, params, deadline)))
+               return rc;
+
+       /* Clear SError.  PMP and some host PHYs require this to
+        * operate and clearing should be done before checking PHY
+        * online status to avoid race condition (hotplugging between
+        * link resume and status check).
+        */
+       if (!(rc = sata_scr_read(link, SCR_ERROR, &serror)))
+               rc = sata_scr_write(link, SCR_ERROR, serror);
+       if (rc == 0 || rc == -EINVAL) {
+               unsigned long flags;
+
+               spin_lock_irqsave(link->ap->lock, flags);
+               link->eh_info.serror = 0;
+               spin_unlock_irqrestore(link->ap->lock, flags);
+               rc = 0;
+       }
+       return rc;
 }
 
 /**
@@ -3701,18 +3719,11 @@ int sata_std_hardreset(struct ata_link *link, unsigned int *class,
  */
 void ata_std_postreset(struct ata_link *link, unsigned int *classes)
 {
-       u32 serror;
-
        DPRINTK("ENTER\n");
 
        /* print link status */
        sata_print_link_status(link);
 
-       /* clear SError */
-       if (sata_scr_read(link, SCR_ERROR, &serror) == 0)
-               sata_scr_write(link, SCR_ERROR, serror);
-       link->eh_info.serror = 0;
-
        DPRINTK("EXIT\n");
 }
 
@@ -6296,9 +6307,7 @@ EXPORT_SYMBOL_GPL(ata_pci_device_resume);
 #endif /* CONFIG_PCI */
 
 EXPORT_SYMBOL_GPL(sata_pmp_qc_defer_cmd_switch);
-EXPORT_SYMBOL_GPL(sata_pmp_std_prereset);
 EXPORT_SYMBOL_GPL(sata_pmp_std_hardreset);
-EXPORT_SYMBOL_GPL(sata_pmp_std_postreset);
 EXPORT_SYMBOL_GPL(sata_pmp_error_handler);
 
 EXPORT_SYMBOL_GPL(__ata_ehi_push_desc);

diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index 7f1a87f01ab2e0bd61d5d281ceb3b975e743be8a..2f8a9577c26d3d2b83817ed301712fd4a1e2e67a 100644 (file)
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -175,49 +175,6 @@ int sata_pmp_scr_write(struct ata_link *link, int reg, u32 val)
        return 0;
 }
 
-/**
- *     sata_pmp_std_prereset - prepare PMP link for reset
- *     @link: link to be reset
- *     @deadline: deadline jiffies for the operation
- *
- *     @link is about to be reset.  Initialize it.
- *
- *     LOCKING:
- *     Kernel thread context (may sleep)
- *
- *     RETURNS:
- *     0 on success, -errno otherwise.
- */
-int sata_pmp_std_prereset(struct ata_link *link, unsigned long deadline)
-{
-       struct ata_eh_context *ehc = &link->eh_context;
-       const unsigned long *timing = sata_ehc_deb_timing(ehc);
-       int rc;
-
-       /* if we're about to do hardreset, nothing more to do */
-       if (ehc->i.action & ATA_EH_HARDRESET)
-               return 0;
-
-       /* resume link */
-       rc = sata_link_resume(link, timing, deadline);
-       if (rc) {
-               /* phy resume failed */
-               ata_link_printk(link, KERN_WARNING, "failed to resume link "
-                               "for reset (errno=%d)\n", rc);
-               return rc;
-       }
-
-       /* clear SError bits including .X which blocks the port when set */
-       rc = sata_scr_write(link, SCR_ERROR, 0xffffffff);
-       if (rc) {
-               ata_link_printk(link, KERN_ERR,
-                               "failed to clear SError (errno=%d)\n", rc);
-               return rc;
-       }
-
-       return 0;
-}
-
 /**
  *     sata_pmp_std_hardreset - standard hardreset method for PMP link
  *     @link: link to be reset
@@ -238,33 +195,13 @@ int sata_pmp_std_prereset(struct ata_link *link, unsigned long deadline)
 int sata_pmp_std_hardreset(struct ata_link *link, unsigned int *class,
                           unsigned long deadline)
 {
-       const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
-       bool online;
        u32 tmp;
        int rc;
 
        DPRINTK("ENTER\n");
 
-       /* do hardreset */
-       rc = sata_link_hardreset(link, timing, deadline, &online, NULL);
-       if (rc) {
-               ata_link_printk(link, KERN_ERR,
-                               "COMRESET failed (errno=%d)\n", rc);
-               goto out;
-       }
-
-       /* clear SError bits including .X which blocks the port when set */
-       rc = sata_scr_write(link, SCR_ERROR, 0xffffffff);
-       if (rc) {
-               ata_link_printk(link, KERN_ERR, "failed to clear SError "
-                               "during hardreset (errno=%d)\n", rc);
-               goto out;
-       }
+       rc = sata_std_hardreset(link, class, deadline);
 
-       /* if device is present, follow up with srst to wait for !BSY */
-       if (online)
-               rc = -EAGAIN;
- out:
        /* if SCR isn't accessible, we need to reset the PMP */
        if (rc && rc != -EAGAIN && sata_scr_read(link, SCR_STATUS, &tmp))
                rc = -ERESTART;
@@ -273,34 +210,6 @@ int sata_pmp_std_hardreset(struct ata_link *link, unsigned int *class,
        return rc;
 }
 
-/**
- *     ata_std_postreset - standard postreset method for PMP link
- *     @link: the target ata_link
- *     @classes: classes of attached devices
- *
- *     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.
- *
- *     LOCKING:
- *     Kernel thread context (may sleep)
- */
-void sata_pmp_std_postreset(struct ata_link *link, unsigned int *class)
-{
-       u32 serror;
-
-       DPRINTK("ENTER\n");
-
-       /* clear SError */
-       if (sata_scr_read(link, SCR_ERROR, &serror) == 0)
-               sata_scr_write(link, SCR_ERROR, serror);
-
-       /* print link status */
-       sata_print_link_status(link);
-
-       DPRINTK("EXIT\n");
-}
-
 /**
  *     sata_pmp_read_gscr - read GSCR block of SATA PMP
  *     @dev: PMP device

diff --git a/include/linux/libata.h b/include/linux/libata.h
index c060cd3cba66b4adaf49ded9a2411405a199a151..b9188371b12acc59096a35dda0dc736fad7463f7 100644 (file)
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1025,10 +1025,8 @@ static inline int ata_acpi_cbl_80wire(struct ata_port *ap,
  * PMP - drivers/ata/libata-pmp.c
  */
 extern int sata_pmp_qc_defer_cmd_switch(struct ata_queued_cmd *qc);
-extern int sata_pmp_std_prereset(struct ata_link *link, unsigned long deadline);
 extern int sata_pmp_std_hardreset(struct ata_link *link, unsigned int *class,
                                  unsigned long deadline);
-extern void sata_pmp_std_postreset(struct ata_link *link, unsigned int *class);
 extern void sata_pmp_error_handler(struct ata_port *ap);
 
 /*