Alexander Shishkin a écrit : > Matthieu CASTET <matthieu.castet@xxxxxxxxxx> writes: > >> Alexander Shishkin a crit : >>> Matthieu CASTET <matthieu.castet@xxxxxxxxxx> writes: >>> >>>> Alexander Shishkin a crit : >>>>> + ehci->regs = ci->hw_bank.cap + >>>>> + HC_LENGTH(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase)); >>>>> + ehci->hcs_params = ehci_readl(ehci, &ehci->caps->hcs_params); >>>>> + ehci->has_hostpc = 1; >>>>> + ehci->sbrn = 0x20; >>>> Why do you set has_hostpc = 1 ? >>>> This won't work with normal chipidea core ? >>> ....because this way we get proper mode setting from ehci_reset on >>> lpm-capable chipideas, see below. >>> >>>>> + >>>>> + ret = hw_device_reset(ci, USBMODE_CM_HC); >>>>> + if (ret) >>>>> + goto rm_hcd; >>>> Why do you need to do that ? ehci_reset will do it (with tdi_reset). >>> Actually, it will only do that for non lpm-capable controllers, which >>> have USBMODE register in the place where tdi_reset() expects it to >>> be. On lpm-capable controllers, USBMODE is in fact where USBMODE_EX >>> (from ehci-def.h) points to, so tdi_reset() sort of shoots at nothing in >>> particular. Writes to the USBMODE location for such devices are silently >>> ignored (maybe we need to tweak tdi_reset() to not do that anyway to be >>> no a safe side, but that's another matter). >>> >>> However, ehci_reset() will still do the right thing if has_hostpc is >>> set, so you're right, this reset call is actually not needed. >>> >>> Having said all that, it seems obvious that has_hostpc setting should >>> depend on wheather we are lpm-capable or not. What do you think? >>> >> If has_hostpc is really chipidea with lpm I think we should : >> - rename it to something understandable >> - autodetect it like we does in udc (using HCCPARAMS). May be it could be >> replaced with ehci_is_TDI(ehci) && HCC_LPM(hcc_params) ? > > Well, that's pretty much how it ends up in the chipidea code, I set > has_hostpc from the lpm flag, which is read from HCCPARAMS in the > driver's probe. I'm about to resend the patchset with this change. > >> - fix tdi_reset and tdi_in_host_mode to use the correct register (USBMODE or >> USBMODE_EX according to has_hostpc). May be we should move the code [1] in >> tdi_reset ? > > Sounds sensible to me. At least it will make the code a bit more > understandable. But I'd like to do this outside of the chipidea patchset > (since the tdi_reset() currently does no harm). > >> Is there a public datasheet to lpm chipidea controller ? > > Unfortunately, I don't really know. I suspect not. Ok, not lpm version of chipidea core got also PHCD bit for controlling "PHY Low Power" [1] Do you know if the hostpc code in ehci-hub can also be used for those controller (minus the different register mapping) ? [1] see http://www.nxp.com/documents/user_manual/UM10314.pdf p130 PHCD PHY low power suspend - clock disable (PLPSCD) R/W 0 In host mode, the PHY can be put into Low Power Suspend Clock Disable when the downstream device has been put into suspend mode or when no downstream device is connected. Low power suspend is completely under the control of software. 1 Writing a 1 disables the PHY clock. Reading a 1 indicates the status of the PHY clock (disabled). 0 Writing a 0 enables the PHY clock. Reading a 0 indicates the status of the PHY clock (enabled). -- 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
