On Fri, Mar 24, 2006 at 11:03:55PM -0500, Jeff Garzik wrote: > Tejun Heo wrote: > >ata_set_mode() used to disable whole port on failure. This patch adds > >@disable_on_err which makes ata_set_mode() disable failing devices > >when non-zero, and simply return when zero. Due to the port-wide > >characteristic of ATA xfer mode configuration, ata_mode_set() is the > >final place to determine device offlining; thus, the @disable_on_err > >mechanism to tell it which action to take on failure. > > > >With this patch, only failing devices are disabled not the whole port. > >Transfer mode configuration must consider all devices on the port > >regardless of failure status; otherwise, device selection timing can > >be violoated resulting in malfunction. This patch makes > >ata_dev_xfermask() consider disabled but present devices such that > >device timing selection timing is honored. > > > >Signed-off-by: Tejun Heo <htejun@xxxxxxxxx> > > ACK, but dropped due to dropping patch #1 > I'll remove ata_dev_present() for the time being. It's used in only one place anyway. We can do with ata_dev_enabled() || ata_dev_disabled() for the time being and resurrect ata_dev_present() after one or two releases, I think. > > >As said in the above comment, this patch makes sure that present but > >disabled devices are taken into account when determining transfer > >mode. If IDENTIFY data is present, it is used; otherwise, PIO0 is > >forced. I think this should be enough. > > Several follow-up comments: > > * Ideally, I think libata should re-read the identify data from the > device. This (a) makes sure PIO is working, and (b) tells us for > certain what mode the device is. That's fine for a follow-up patch > though, since few will exercise this code anyway. Agreed, we can revalidate the device and then try to configure the next lower transfer mode until we succeed. For the time being, I just wanna move forward with the current code so that EH changes can be submitted sooner than later. > * skipping ->post_set_mode() in this error case being discussing is > probably unwise. We don't skip ->post_set_mode() unless @disable_on_err is zero, in which case the upper layer is responsible for handling the error condition. Upper layer will usually reset and reconfigure the whole thing again, so omitting ->post_set_mode() should be fine there (we need to force PIO0 before IDENTIFY'ing though). > BTW, got any PATA hardware lying about? Since you're wandering into > xfer mode territory, it would better to test PATA than SATA, as xfer > mode matters more in the PATA realm. Intel PATA should be fairly easy > to find, covered by ata_piix, and all the docs are on developer.intel.com. Yeap, my test machine's ICH7 can do ata_piix and my notebook has ICH6M. -- tejun - : 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
