On Thu, 17 Jan 2013, Felipe Balbi wrote: > > Bits 31 & 30 from PORTSC register were allocated by our SOC designers > > to inform the host controller about the PHY type to be used. > > Wow, that's something you should never do. PORTSC register belongs to > the EHCI controller and those bits are reserved for future use and they > *MUST* return zero. I wouldn't be surprised if current EHCI driver > assumes those bits will be zero and/or makes sure they're set to zero > when writing to PORTSC register. In fact, those bits _have_ been assigned an official purpose in the EHCI-1.1 addendum. Presumably the Tegra hardware only supports EHCI-1.0. > What if after configuring those two bits, ehci-hcd.ko clears them by > accident ? Your platform won't work. That won't happen; ehci-hcd is careful to preserve the values of PORTSC register bits it doesn't use. But this still isn't a good idea. > > As type of PHY is a property related to PHY DT nodes, PHY driver will get this info. > > (Will remove phy_type from controller DT node soon, after all patches get merged.) > > However as PORTSC register is in controller domain, added tegra_ehci_set_pts() API > > to serve the purpose. > > > > As per Tegra USB PHY design, need to wait for PHY clock to stabilize once we > > set/clear PHCD bit. Hence this is being accessed from PHY driver. To serve this > > purpose added tegra_ehci_set_phcd(). > > > > On further analysis, seems tegra_ehci_set_wakeon_events() can be > > removed. > > > > If you are okay with above explanation, I can send updated patch for > > review. > > not sure I want to sign-off such a patch. I understand it's how your HW > was done, but this shouldn't have been done, really. > > Alan, what do you think ? Are you ok with PHY driver overwritting PORTSC > register ? It's okay provided it is done correctly -- i.e., in a way that won't interfere with the normal EHCI operation. Alan Stern -- 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
