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 <htejun@xxxxxxxxx> >> --- >> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
