On Sat, May 19, 2012 at 09:20:19PM +0800, Ming Lei wrote: > On Sat, May 19, 2012 at 12:43 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, 18 May 2012, Alan Stern wrote: > > > > IMO, it is not a good idea. > > Firstly, after writing RESET on command register, the default value of > CMD register > should be restored to '00080000h (00080B00h if Asynchronous Schedule Park > Capability is a one)'[1], and both 'Interrupt Threshold Control' and > 'Park mode' has > been set in ehci_init(), so using ehci->command after ehci_reset is correct. > > Secondly, remaining other bits from HW is not good since HW may be buggy > and the default value after reset may be uncertain, and it is better > to not depend > on them. For some newer hardware, if the reserved fields are to be > used, just do > it in ehci_init like setting lpm bits. > > Finally, before the commit 3d9545cc375d117554a9b35dfddadf9189c62775( > EHCI: maintain the ehci->command value properly), the ehci->command is used > just directly in ehci_run and the other HW bits are not kept from HW, > so it is better > to be consistent as before to avoid introducing possible regression. > > [1], 4.1 Host Controller Initialization of EHCI Specification 1.0 > > > Would anyone like to code that up? > > Looks removing the line of 'ehci->command = ehci_readl(ehci, > &ehci->regs->command);' in ehci_reset is OK. > At first, I also think just remove ehci->command store code at ehci_reset is ok. But, after thinking a bit more, we may need to make clearly what value of ehci->command is correct? 1. Its default value after command reset or (|) the value changed at ehci_init; 2. Use the software decided value after ehci_init. Choosing 1 or 2 depends on - Does the hardware reset value (expect for the value we will change at ehci_init) is correct or not? - Is it possible the ehci_init will override some correct default value at usbcmd? The reason why I thought Alan's proposal patch is correct is I thought (1) was correct. But just like Ming mentioned before Alan's patch (EHCI: maintain the ehci->command value properly), we are OK with our EHCI host controller which value is software decided, so it is better use (2) to align with current code. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html