[PATCH V4] usb: ehci-mx6: Enable OTG detection on imx8mm and imx8mn

[Marek Vasut]

On 2/7/22 12:13, Adam Ford wrote:
> On Mon, Feb 7, 2022 at 2:47 AM Marek Vasut <marex at denx.de> wrote:
>>
>> On 2/7/22 01:51, Adam Ford wrote:
>>> On Sun, Feb 6, 2022 at 3:59 PM Marek Vasut <marex at denx.de> wrote:
>>>>
>>>> On 2/3/22 22:20, Adam Ford wrote:
>>>>> The imx8mm and imx8mn appear compatible with imx7d-usb
>>>>> flags in the OTG driver.  If the dr_mode is defined as
>>>>> host or peripheral, the device appears to operate correctly,
>>>>> however the auto host/peripheral detection results in an error.
>>>>>
>>>>> The solution isn't just adding checks for imx8mm and imx8mn to
>>>>> the check for imx7, because the USB clock needs to be running
>>>>> to read from the USBNC_PHY_STATUS_OFFSET register or it will hang.
>>>>>
>>>>> Marek requested that I not enable the clocks in ehci_usb_of_to_plat,
>>>>> so I modified that function to return an unknown state if the
>>>>> device tree does not explicitly state whether it is a host
>>>>> or a peripheral.
>>>>>
>>>>> When the driver probes, it looks to see if it's in the unknown
>>>>> state, and only then will it read the register to auto-detect.
>>>>>
>>>>> Signed-off-by: Adam Ford <aford173 at gmail.com>
>>>>> Tested-by: Tim Harvey <tharvey at gateworks.com>
>>>>> ---
>>>>> V4:  Fix 'err_clk' reference, so it is not encapsulated in
>>>>>         an ifdef making it available at all times.
>>>>> V3:  Keep ehci_usb_of_to_plat but add the ability to return
>>>>>         and unknown state instead of reading the register.
>>>>>         If the probe determines the states is unknown, it will
>>>>>         query the register after the clocks have been enabled.
>>>>>         Because of the slight behavior change, I removed any
>>>>>         review or tested tags.
>>>>>
>>>>> V2:  Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it
>>>>>         from the probe after t
>>>>>
>>>>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
>>>>> index 1bd6147c76..060b02accc 100644
>>>>> --- a/drivers/usb/host/ehci-mx6.c
>>>>> +++ b/drivers/usb/host/ehci-mx6.c
>>>>> @@ -543,7 +543,7 @@ static int ehci_usb_phy_mode(struct udevice *dev)
>>>>>                         plat->init_type = USB_INIT_DEVICE;
>>>>>                 else
>>>>>                         plat->init_type = USB_INIT_HOST;
>>>>> -     } else if (is_mx7()) {
>>>>> +     } else if (is_mx7() || is_imx8mm() || is_imx8mn()) {
>>>>>                 phy_status = (void __iomem *)(addr +
>>>>>                                               USBNC_PHY_STATUS_OFFSET);
>>>>>                 val = readl(phy_status);
>>>>
>>>> Is the USB GPCv2 power domain always ON at this point ? If it isn't,
>>>> then this access will lock up the whole SoC.
>>>
>>> The GPCv2 needs to be enabled if USB is going to be used, but I would
>>> expect that to happen even without this patch.  I ran into that during
>>> my early testing.
>>>
>>>>
>>>> You can check that easily by adding printf() into
>>>> drivers/power/domain/imx8m-power-domain.c imx8m_power_domain_on() , and
>>>> verify that printf() prints something before this ehci_usb_phy_mode()
>>>> code is called.
>>>
>>> I added a printf to show when a power domain is enabled.  It does
>>> appear that power domains for HSIO, USBOTG1 and USBOTG2 are all
>>> enabled before the scan on the respective USB bus happens, so I think
>>> we're safe from hanging.
>>
>> Isn't the ehci_usb_of_to_plat() called way before you even initiate the
>> scan, somewhere during U-Boot startup ? So no, you might end up hanging
>> the hardware ?
> 
> If you look at what this patch is doing, ehci_usb_of_to_plat is no
> longer making any calls to the USB registers.  ehci_usb_of_to_plat is
> just an exercise in reading the device tree for the desired mode.  If
> it's not host mode or peripheral mode it returns unknown.  The part
> that reads the registers now is delayed until after the clocks are
> enabled inside the probe function which is called when USB is started
> and the power-domains are available.

Ah, I missed that part of the change.

One more thing, can you double-check that the "if (ctrl->init == 
USB_INIT_DEVICE)" checks in drivers/usb/host/ehci-hcd.c still work as 
they should ? If so, then this patch is fine.