On Wed, Jan 31, 2007 at 07:44:43PM +0900, Tejun Heo wrote: > Gary Hade wrote: > > Some of my random thoughts: > > There does appear to be this invalid assumption that 0xFF status > > always implies device-not-present. The status register access > > restrictions in ATA/ATAPI-7 V1 5.14.2 include the statement "The > > contents of this register, except for BSY, shall be ignored when > > BSY is set to one." which the code does not honor. There is apparently > > past experience that 0xFF status implies device-not-present for some > > controllers (the odd clowns :) but I have no idea how common these are. > > The 0xff is the value we get when there is no device present and the > motherboard manufacturer forgot to pull down the ATA bus. It's not very > uncommon in cheap PATA world and, following the weird tradition, some > SATA controllers choose to emulate 0xff if there is no device attached > (link not established). Not sure how many of them does it but intel's > SATA chipset is one of them, so we're pretty much stuck with it. > > ie. In many P/SATA setups, your patch would add 2 extra secs of waiting > for empty ports. > > > We obviously can't get rid of the check but since we cannot clear > > the read-only status register and there appears to be no specification > > dictated upper limit on how long it should take for a software reset to > > complete it just seems like we need to wait long enough to support the > > slowest known device which may be the GoVault. > > Agreed but still hesitant to ack the patch. :-) > > I'm gonna work on parallel probing for libata. I think we can easily > hide extra 2 secs of waiting with parallel probing. It will take some > time but that seems to be the 'right' thing to do especially considering > the fact that 150ms sleep has been enough for gazillions of ATA devices > during last decade except for this GoVault drive. > > I'll leave this thread in my to-do folder and apply your patch after > parallel probing is in place (optimistic ETA 1 month). How does that sound? Thanks. Solution sounds great. ETA may be an issue since we need to get the device enabled in a near-term release. I'll let you know. Polling version included below. GoVault appeared to be the only currently known device that would benefit from polling so I didn't think the extra code was necessary. After considering Jeff's comment, it makes sense prepare for the possibility of those future slow but not quite so slow devices. Gary -- Gary Hade System x Enablement IBM Linux Technology Center 503-578-4503 IBM T/L: 775-4503 garyhade@xxxxxxxxxx http://www.ibm.com/linux/ltc Controllers such as the ICH6R/ICH6RW may set the status to 0xFF when software reset is initiated even when the device is present. Since some removable media devices can take longer than 150ms to complete the 0xFF status check can fail even when the device is present. For example, a software reset for the Quantum GoVault removable hard can take as long as 2 seconds to complete. This patch eliminates incorrect software reset failures for slower than normal software reset responders by adding an additional 2 second wait when a 0xFF status is detected following the current 150ms wait. Signed-off-by: Gary Hade <garyhade@xxxxxxxxxx> --- linux-2.6.20-rc6/drivers/ata/libata-core.c.orig 2007-01-24 18:19:28.000000000 -0800 +++ linux-2.6.20-rc6/drivers/ata/libata-core.c 2007-01-31 12:00:52.000000000 -0800 @@ -2653,6 +2653,8 @@ static unsigned int ata_bus_softreset(st unsigned int devmask) { struct ata_ioports *ioaddr = &ap->ioaddr; + u8 status; + unsigned long timeout; DPRINTK("ata%u: bus reset via SRST\n", ap->id); @@ -2683,11 +2685,22 @@ static unsigned int ata_bus_softreset(st */ msleep(150); + /* For those controllers where the status could start out at + * 0xFF even though the device is present we wait up to 2 seconds + * longer for slower removable media devices to respond. + */ + status = ata_check_status(ap); + timeout = jiffies + 2*HZ; + while (status == 0xFF && time_before(jiffies, timeout)) { + msleep(50); + status = ata_check_status(ap); + } + /* Before we perform post reset processing we want to see if * the bus shows 0xFF because the odd clown forgets the D7 * pulldown resistor. */ - if (ata_check_status(ap) == 0xFF) + if (status == 0xFF) return 0; ata_bus_post_reset(ap, devmask); - 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
