On 11-10-06 06:10 PM, Tejun Heo wrote: > Hello, > > On Thu, Oct 06, 2011 at 01:44:27PM -0700, Gwendal Grignou wrote: >> Yes. Until the device sends the async D2H Reg FIS indicating the drive >> spun up, the MPM does not accept the SoftRest FIS from the controller. >> On most controller, that fine, the controller state machine keeps >> retrying, but on Sil3132 it stops after a second or so and send an >> error back to the driver. > > I see. > >>> So, now libata waits for full reset period before proceeding to reset >>> PMP. Hmmm... yeah, it makes sense. Unfortunately, the only way to >>> achieve spinup wait in this case is waiting blindly and libata's reset >>> timeouts are configured to accomodate drive spinup times. PMP SCR >>> failure kinda destroys those blind wait periods. >> >> Yes, I totally agree this blind wait is not clean. Normally we would >> wait until an event occurs [async FIS] and have the timeout just for >> the error case. >> Here we wait [10s] because we think the device is spinning up. > > Yes but that's exactly how the reset timeouts are set up. They're > supposed to provide reasonable spinup timeouts when the proper wait > mechanisms can't do so and here it becomes a problem because the blind > timeouts are circumvented by SCR read failure handling. > >>> I'm not too sure about ATA_LFLAG_WAIT_SRST. I don't think making the >>> new behavior default would hurt. >> >> I see your point. But if there is no PMP, Sil3132 is behaving, there >> is no need of this logic. > > Yes, sure, the behavior is necessary iff PMP is attached as that's > only time SCR read failure can occur anyway and I think it would > generally be a good idea to always enforce the blind timeouts if PMP > is attached, so no need for ATA_LFLAG_WAIT_SRST. I wonder if this same logic is more generally applicable for PMP rather than sil3132 specific? What I've read thus far in this thread sounds very much like the issues I see here with PMPs not working on JMB and Marvell controllers (where they USED to work fine). Gwendal, have you got a version of that patch which applies the same change globally rather than only for sil3132? I'd like to try it out here. Thanks -- 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
