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

Thu Feb 10 15:49:52 CET 2022

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?

adam