Hey, On Tue, Jan 14, 2014 at 07:57:19AM -0800, Loc Ho wrote: > > No, they don't and the comments in your driver don't really explain > > what's going on. Why are we having retry loops inside hardreset > > itself? This can prolong recovery time significantly in corner cases. > > Why is this necessary? > > There are two retry in the hardreset. The first retry is due to the > requirement to have slightly modified PHY setting for different speed > of the attached disk. The default is Gen3. If it is Gen1 or Gen2 disk > attached, HW will require slightly modified PHY setting. The second > retry is due to the fact that link may not link up the first time due > to HW errata. For the initial version, I will remove the second retry > and submit an separate patch. And please explain what's going on in more detail in the comments. > > * If your driver needs to append or prepent something to the existing > > one, export the existing one and then wrap that one with the extra > > logic around it. Do not copy function body unnecessarily. Even for > > the port_interrupt, it looks like you can just call the flush after > > invoking the normal ahci_handle_port_interrupt(), no? > > As mentioned, the flush requires immediately after reading the CI. > Otherwise, there is still an chance that the command is completed and > the OS notified the upper layer while the data is still in flight. For > the initial version, I will remove the flush (IRQ wrapper) and submit > separate patch. The function is called with ap->lock held. Nothing can happen inbetween. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
