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

[Marek Vasut]

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 ?

(you can verify that by adding more printf()s)