Hi, Martin Blumenstingl <[email protected]> writes: >>>>>>> When searching the web I did not come across any SoC that uses a >>>>>>> configuration with more than one port enabled. >>>>>>> >>>>>>> On my Amlogic Meson GXM device (consumer device, no development board) >>>>>>> I see the following USB2 PHY register configuration (full register >>>>>>> dump from the kernel that was shipped with the device is attached): >>>>>>> GUSB2PHYCFG(0) = 0x40102500 >>>>>>> GUSB2PHYCFG(1) = 0x40102540 >>>>>>> GUSB2PHYCFG(2) = 0x40102540 >>>>>> >>>>>> multiple PHYs are only used by the host block (xHCI). Don't touch these >>>>>> and just let xHCI core handle the ports. >>>>> could you be more specific with "xHCI core" - do you mean the core in >>>>> the dwc3 IP or drivers/usb/host/xhci-*? >>>>> However, we still have a "problem" here: the USB PHYs for each >>>>> "enabled" port have to be turned on (if I leave only one USB PHY >>>>> disabled then none of the ports is working). unfortunately the current >>>>> code (both, in dwc3 and drivers/usb/host/*) assumes that there's >>>>> either 0 or 1 PHY for each HCD. >> >> true. But even so, that PHY handle is only needed for devices which >> weren't properly configured at coreConsultant time. > I don't understand that - there are two separate modules involved > here: 1. the dwc3 IP block 2. Amlogic's own USB2 PHYs (they did not > choose Synopsys DesignWare PHYs) > (some background info: the USB2 PHYs are in "reset mode" by default) > So even if the dwc3 IP block is configured correctly at coreConsultant > time we still need to configure the PHYs. From "USB controller driver" > (could be dwc3, or some generic hcd.c code, etc.) this means that > phy_init() and phy_power_on() needs to be called for each of the three > "Amlogic USB2 PHYs". the current code however only calls these for the > first PHY (leaving the others in reset mode = disabled).
argh, good point. In that case, we need to figure out the best way to
pass all these handles to xHCI-plat
>>>>>>> Then vendor kernel sources (a 3.14 kernel) are adding the resets for
>>>>>>> GUSB2PHYCFG([1-3]) in dwc3_core_soft_reset().
>>>>>>
>>>>>> That shouldn't be necessary, actually. If it is, it means the HW was
>>>>>> poorly integrated. In that case, we _can_ add the other resets, but I
>>>>>> need confirmation that they are needed by means of a public errata
>>>>>> document.
>>>>>>
>>>>>>> A mainline 4.9+(Meson GXL USB PHY patches + dwc3/xhci-plat DMA patches
>>>>>>> from linux-usb) kernel works fine even with just applying the reset to
>>>>>>> GUSB2PHYCFG(0).
>>>>>>
>>>>>> there you go
>>>>> does that mean that the reset of GUSB2PHYCFG(0) (which is part of the
>>>>> current dwc3 code in dwc3_phy_setup) is done only because of the
>>>>> quirks/erratas? in other words: do you mean that one would not have to
>>>>> reset GUSB2PHYCFG(0) if there were no erratas?
>>>>
>>>> no, it's done for peripheral configuration of dwc3. Look at the code
>>>> more carefully:
>>> sorry for the confusion - the word "reset" should be "configuration".
>>> The function I am looking at is dwc3_phy_setup(): apart from toggling
>>> some "errata bits" it also configures the PHY modes. I am wondering if
>>> I need to do this setup (DWC3_GUSB2PHYCFG_ULPI_UTMI and
>>> DWC3_GUSB2PHYCFG_PHYIF_MASK related bits) not only for the *first*
>>> port ("DWC3_GUSB2PHYCFG(0)") but also for the "other" ports (index 1
>>> and 2 in above case, where the roothub has 3 ports)
>>
>> Ideally, that should've been set at coreConsultant (RTL instantiation)
>> time. If it wasn't, then we need to add a quirk for these SoCs
>> accordingly. We _do_ need documentation that this quirk is, indeed,
>> needed.
> That is an explanation I'm fine with: we trust the (default) values
> which are in hardware until we have documentation that we need a
> quirk. I do not have access to Amlogic's documentation so I can't
> provide that - but we can probably leave it as it is as it "worked for
> me".
awesome, so we need *only* phy_init() / phy_power_on() for the other
PHYs, right?
--
balbi
signature.asc
Description: PGP signature
