libata: retry link resume if necessary
PATCH 1/1 libata: retry link resume if necessary
Interestingly, when SIDPR is used in ata_piix, writes to DET in SControl sometimes get ignored leading to detection failure. Update sata_link_resume() such that it reads back SControl after clearing DET and retry if it's not clear.
---
Greg, although this patch should be included in -stable release in time, I think it should receive some more testing in devel branch for at least a couple of -rc cycles as it changes very fundamental part of hardreset protocol. Is there a protocol for delayed inclusion into -stable? If not, I'll try to remember and ping you back after some time but I'm almost sure that I'm gonna forget this. :-)
Thanks.
drivers/ata/libata-core.c | 38 +++++++++++++++++++++++++++++++------- include/linux/libata.h | 3 +++ 2 files changed, 34 insertions(+), 7 deletions(-)
drivers/ata/libata-core.c +31 -7
@@ -3790,21 +3790,45 @@ 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) { +int tries =ATA_LINK_RESUME_TRIES ;u32 scontrol, serror;int rc;if ((rc =sata_scr_read (link,SCR_CONTROL , &scontrol)))return rc; - scontrol = (scontrol &0x0f0 ) |0x300 ; +/* +* Writes to SControl sometimes get ignored under certain +* controllers (ata_piix SIDPR). Make sure DET actually is +* cleared. +*/ +do { + scontrol = (scontrol &0x0f0 ) |0x300 ; +if ((rc =sata_scr_write (link,SCR_CONTROL , scontrol))) +return rc; +/* +* Some PHYs react badly if SStatus is pounded +* immediately after resuming. Delay 200ms before +* debouncing. +*/ +msleep (200 ); -if ((rc =sata_scr_write (link,SCR_CONTROL , scontrol))) -return rc; +/* is SControl restored correctly? */ +if ((rc =sata_scr_read (link,SCR_CONTROL , &scontrol))) +return rc; + }while ((scontrol &0xf0f ) !=0x300 && --tries); -/* Some PHYs react badly if SStatus is pounded immediately -* after resuming. Delay 200ms before debouncing. -*/ -msleep (200 ); +if ((scontrol &0xf0f ) !=0x300 ) { +ata_link_printk (link,KERN_ERR , +"failed to resume link (SControl %X)\n" , + scontrol); +return 0 ; + } + +if (tries <ATA_LINK_RESUME_TRIES ) +ata_link_printk (link,KERN_WARNING , +"link resume succeeded after %d retries\n" , +ATA_LINK_RESUME_TRIES - tries);if ((rc =sata_link_debounce (link, params, deadline)))return rc;
include/linux/libata.h +3 -0
@@ -354,6 +354,9 @@ enum {/* max tries if error condition is still set after ->error_handler */ ATA_EH_MAX_TRIES =5 , +/* sometimes resuming a link requires several retries */ +ATA_LINK_RESUME_TRIES =5 , +/* how hard are we gonna try to probe/recover devices */ ATA_PROBE_MAX_TRIES =3 , ATA_EH_DEV_TRIES = 3,
Quoted 9 lines · Tejun Heo
Interestingly, when SIDPR is used in
ata_piix, writes to DET in SControl sometimes get ignored leading to detection failure. Updatesata_link_resume()such that it reads back SControl after clearing DET and retry if it's not clear.Signed-off-by Tejun Heo <[email protected]>Reported-by fengxiangjun <[email protected]>Reported-by Jim Faulkner <[email protected]>
Quoted 7 lines
---
Greg, although this patch should be included in -stable release in time, I think it should receive some more testing in devel branch for at least a couple of -rc cycles as it changes very fundamental part of hardreset protocol. Is there a protocol for delayed inclusion into -stable? If not, I'll try to remember and ping you back after some time but I'm almost sure that I'm gonna forget this. :-)
I'll drop this into my "todo-stable" queue, and ping you back on this in a few weeks. Sound good?
thanks,
greg k-h
Hello,
Quoted 18 lines · Greg KH
Quoted 9 lines · Tejun Heo
Interestingly, when SIDPR is used in
ata_piix, writes to DET in SControl sometimes get ignored leading to detection failure. Updatesata_link_resume()such that it reads back SControl after clearing DET and retry if it's not clear.Signed-off-by Tejun Heo <[email protected]>Reported-by fengxiangjun <[email protected]>Reported-by Jim Faulkner <[email protected]>Quoted 7 lines
---
Greg, although this patch should be included in -stable release in time, I think it should receive some more testing in devel branch for at least a couple of -rc cycles as it changes very fundamental part of hardreset protocol. Is there a protocol for delayed inclusion into -stable? If not, I'll try to remember and ping you back after some time but I'm almost sure that I'm gonna forget this. :-)
Quoted 2 lines
I'll drop this into my "todo-stable" queue, and ping you back on this in a few weeks. Sound good?
Yeap, sounds good.
Thanks.
Signature · 1 line
tejun
Jeff Garzik
AppliedQuoted 17 lines · Tejun Heo
Interestingly, when SIDPR is used in
ata_piix, writes to DET in SControl sometimes get ignored leading to detection failure. Updatesata_link_resume()such that it reads back SControl after clearing DET and retry if it's not clear.Signed-off-by Tejun Heo<[email protected]>Reported-by fengxiangjun<[email protected]>Reported-by Jim Faulkner<[email protected]>---
Greg, although this patch should be included in -stable release in time, I think it should receive some more testing in devel branch for at least a couple of -rc cycles as it changes very fundamental part of hardreset protocol. Is there a protocol for delayed inclusion into -stable? If not, I'll try to remember and ping you back after some time but I'm almost sure that I'm gonna forget this. :-)
Quoted 5 lines
Thanks.
drivers/ata/libata-core.c | 38 +++++++++++++++++++++++++++++++------- include/linux/libata.h | 3 +++ 2 files changed, 34 insertions(+), 7 deletions(-)
applied