On 2/10/22 15:49, Adam Ford wrote:
On Mon, Feb 7, 2022 at 6:15 AM Adam Ford <aford...@gmail.com> wrote:

On Mon, Feb 7, 2022 at 5:50 AM Marek Vasut <ma...@denx.de> wrote:

On 2/7/22 12:13, Adam Ford wrote:
On Mon, Feb 7, 2022 at 2:47 AM Marek Vasut <ma...@denx.de> wrote:

On 2/7/22 01:51, Adam Ford wrote:
On Sun, Feb 6, 2022 at 3:59 PM Marek Vasut <ma...@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 <aford...@gmail.com>
Tested-by: Tim Harvey <thar...@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@30280000 with servicing (60s timeout)
MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
Loading Environment from MMC... OK
In:    serial@30890000
Out:   serial@30890000
Err:   serial@30890000
Net:   eth0: ethernet@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.

Reply via email to