Re: [PATCH #upstream-fixes] libata: clear link-eh_info.serror from ata_std_postreset()

2007-12-17 Thread Jeff Garzik

Tejun Heo wrote:

link-eh_info.serror is used to cache SError for controllers which
need it cleared from interrupt handler to clear IRQ.  It also should
be cleared after reset just like SError itself.

Make ata_std_postreset() clear link-eh_info.serror too and update
sata_sil such that it doesn't care about bookkeeping the value.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
 drivers/ata/libata-core.c |1 +
 drivers/ata/sata_sil.c|   11 +--
 2 files changed, 2 insertions(+), 10 deletions(-)


applied #upstream-fixes


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH #upstream-fixes] libata: clear link-eh_info.serror from ata_std_postreset()

2007-12-07 Thread Tejun Heo
Jeff Garzik wrote:
 Tejun Heo wrote:
 link-eh_info.serror is used to cache SError for controllers which
 need it cleared from interrupt handler to clear IRQ.  It also should
 be cleared after reset just like SError itself.

 Make ata_std_postreset() clear link-eh_info.serror too and update
 sata_sil such that it doesn't care about bookkeeping the value.

 Signed-off-by: Tejun Heo [EMAIL PROTECTED]
 ---
  drivers/ata/libata-core.c |1 +
  drivers/ata/sata_sil.c|   11 +--
  2 files changed, 2 insertions(+), 10 deletions(-)

 Index: work/drivers/ata/libata-core.c
 ===
 --- work.orig/drivers/ata/libata-core.c
 +++ work/drivers/ata/libata-core.c
 @@ -3923,6 +3923,7 @@ void ata_std_postreset(struct ata_link *
  /* clear SError */
  if (sata_scr_read(link, SCR_ERROR, serror) == 0)
  sata_scr_write(link, SCR_ERROR, serror);
 +link-eh_info.serror = 0;
 
 IMO it would make more sense to record the state of the hardware
 following sata_scr_write() than simply zeroing the cache value.
 
 Just a gut feeling... it seems like having a manufactured value (zero)
 rather than the last known from-the-hardware value could lead to
 inconsistencies.
 
 Comments?

link-eh_info.serror is always used as complement to the hardware SError
value.  It's a temporary storage to dump/accumulate SError value when
for whatever reason the hardware can't hold the value.
link-eh_info.serror and the hardware SError value are always OR'd
before being used, so no reason to dup the value.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH #upstream-fixes] libata: clear link-eh_info.serror from ata_std_postreset()

2007-12-07 Thread Tejun Heo
link-eh_info.serror is used to cache SError for controllers which
need it cleared from interrupt handler to clear IRQ.  It also should
be cleared after reset just like SError itself.

Make ata_std_postreset() clear link-eh_info.serror too and update
sata_sil such that it doesn't care about bookkeeping the value.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
 drivers/ata/libata-core.c |1 +
 drivers/ata/sata_sil.c|   11 +--
 2 files changed, 2 insertions(+), 10 deletions(-)

Index: work/drivers/ata/libata-core.c
===
--- work.orig/drivers/ata/libata-core.c
+++ work/drivers/ata/libata-core.c
@@ -3923,6 +3923,7 @@ void ata_std_postreset(struct ata_link *
/* clear SError */
if (sata_scr_read(link, SCR_ERROR, serror) == 0)
sata_scr_write(link, SCR_ERROR, serror);
+   link-eh_info.serror = 0;
 
/* is double-select really necessary? */
if (classes[0] != ATA_DEV_NONE)
Index: work/drivers/ata/sata_sil.c
===
--- work.orig/drivers/ata/sata_sil.c
+++ work/drivers/ata/sata_sil.c
@@ -394,16 +394,7 @@ static void sil_host_intr(struct ata_por
 * it's PHYRDY CHG.
 */
if (serror  SERR_PHYRDY_CHG) {
-   /* Trigger hotplug and accumulate SError only
-* if the port isn't already frozen.
-* Otherwise, PHY events during hardreset
-* makes controllers with broken SIEN repeat
-* probing needlessly.
-*/
-   if (!(ap-pflags  ATA_PFLAG_FROZEN)) {
-   ata_ehi_hotplugged(ap-link.eh_info);
-   ap-link.eh_info.serror |= serror;
-   }
+   ap-link.eh_info.serror |= serror;
goto freeze;
}
 
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html