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; > > ehci_setup already does : > ehci->regs = (void __iomem *)ehci->caps + > HC_LENGTH(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase)); > dbg_hcs_params(ehci, "reset"); > dbg_hcc_params(ehci, "reset"); > > /* cache this readonly data; minimize chip reads */ > ehci->hcs_params = ehci_readl(ehci, &ehci->caps->hcs_params); > > ehci->sbrn = HCD_USB2; That's a leftover and shouldn't be there, except for the has_hostpc... > 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? Regards, -- Alex -- 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
