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
