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>
This commit is contained in:
Tejun Heo 2008-04-07 22:47:19 +09:00 committed by Jeff Garzik
parent 57c9efdfb3
commit ac371987a8
4 changed files with 23 additions and 112 deletions

View file

@ -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);

View file

@ -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);

View file

@ -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;
}
rc = sata_std_hardreset(link, class, deadline);
/* 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;
}
/* 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

View file

@ -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);
/*