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

[Marek Vasut]

On 2/10/22 15:49, Adam Ford wrote:
> On Mon, Feb 7, 2022 at 6:15 AM Adam Ford <aford173 at gmail.com> wrote:
>>
>> On Mon, Feb 7, 2022 at 5:50 AM Marek Vasut <marex at denx.de> wrote:
>>>
>>> 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.
>>
>> Sure.  If I run ums 0 mmc 2, I see the peripheral is enumerating on
>> the U-Boot side:
>>
>> U-Boot 2022.01-00678-ge83818c782-dirty (Feb 06 2022 - 18:55:39 -0600)
>>
>> CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
>> Reset cause: POR
>> Model: Beacon EmbeddedWorks i.MX8M Mini Development Kit
>> DRAM:  2 GiB
>> Core:  99 devices, 21 uclasses, devicetree: separate
>> WDT:   Started watchdog at 30280000 with servicing (60s timeout)
>> MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
>> Loading Environment from MMC... OK
>> In:    serial at 30890000
>> Out:   serial at 30890000
>> Err:   serial at 30890000
>> Net:   eth0: ethernet at 30be0000
>> Hit any key to stop autoboot:  0
>> u-boot=> ums 0 mmc 2
>> UMS: LUN 0, dev mmc 2, hwpart 0, sector 0x0, count 0x3a3e000
>> imx8m_power_domain_on 0
>> imx8m_power_domain_on 2
>> -
>>
>> And the power domains enable in the right order.
>>
>> On the host side, I see:
>>
>> usb 3-2: new high-speed USB device number 24 using xhci_hcd
>> usb 3-2: New USB device found, idVendor=0525, idProduct=a4a5, bcdDevice= 2.21
>> usb 3-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
>> usb 3-2: Product: USB download gadget
>>   usb 3-2: Manufacturer: U-Boot
>> usb-storage 3-2:1.0: USB Mass Storage device detected
>>   usb-storage 3-2:1.0: Quirks match for vid 0525 pid a4a5: 10000
>> scsi host6: usb-storage 3-2:1.0
>>   scsi 6:0:0:0: Direct-Access     Linux    UMS disk 0       ffff PQ: 0 ANSI: 2
>> sd 6:0:0:0: Attached scsi generic sg2 type 0
>> sd 6:0:0:0: [sdb] 61071360 512-byte logical blocks: (31.3 GB/29.1 GiB)
>> sd 6:0:0:0: [sdb] Write Protect is off
>> sd 6:0:0:0: [sdb] Mode Sense: 0f 00 00 00
>> sd 6:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't
>> support DPO or FUA
>> sd 6:0:0:0: [sdb] Attached SCSI removable disk
> 
> Marek,
> 
> Is this sufficient, or do you need explicit printf's inside the driver
> to show the device is in peripheral mode?

It's in the MR queue.