On Thu, Feb 3, 2022 at 2:59 PM Tim Harvey <thar...@gateworks.com> wrote: > > On Tue, Jan 25, 2022 at 12:44 AM Marcel Ziswiler > <marcel.ziswi...@toradex.com> wrote: > > > > On Mon, 2022-01-17 at 18:21 -0600, 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> > > > --- > > > 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 the clocks are enabled, but before > > > the data is needed. > > > > > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > > > index 1bd6147c76..cf44e53ff7 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); > > > @@ -573,9 +573,8 @@ static int ehci_usb_of_to_plat(struct udevice *dev) > > > case USB_DR_MODE_PERIPHERAL: > > > plat->init_type = USB_INIT_DEVICE; > > > break; > > > - case USB_DR_MODE_OTG: > > > - case USB_DR_MODE_UNKNOWN: > > > - return ehci_usb_phy_mode(dev); > > > + default: > > > + plat->init_type = USB_INIT_UNKNOWN; > > > }; > > > > > > return 0; > > > @@ -677,6 +676,20 @@ static int ehci_usb_probe(struct udevice *dev) > > > mdelay(1); > > > #endif > > > > > > + /* > > > + * If the device tree didn't specify host or device, > > > + * the default is USB_INIT_UNKNOWN, so we need to check > > > + * the register. For imx8mm and imx8mn, the clocks need to be > > > + * running first, so we defer the check until they are. > > > + */ > > > + if (priv->init_type == USB_INIT_UNKNOWN) { > > > + ret = ehci_usb_phy_mode(dev); > > > + if (ret) > > > + goto err_clk; > > > > Hm, unfortunately, that label is gated by an ifdef leading to the following > > on colibri_imx7: > > > > drivers/usb/host/ehci-mx6.c: In function ‘ehci_usb_probe’: > > drivers/usb/host/ehci-mx6.c:688:4: error: label ‘err_clk’ used but not > > defined > > 688 | goto err_clk; > > | ^~~~ > > make[1]: *** [scripts/Makefile.build:254: drivers/usb/host/ehci-mx6.o] > > Error 1 > > > > I have to admit that maybe we should indeed run it with DM_REGULATOR but > > that won't change anything on this > > issue. I guess one should really not have any gotos to a label err_clk > > being ifdefed from somewhere not > > ifdefed. > > > > Adam, > > Can you re-submit this with an ifdef to handle the correct goto when > DM_REGULATOR is undefined? > > It looks like Fabio figured out his issue with his board and it was > unrelated to this (fixed by 'mmc: fsl_esdhc_imx: fix watermark level > in dma') so I think Marcel's point is the only blocker on this patch > unless there is more discussion needed with Michael or some feedback > needed from Marek?
Tim, Not a problem. I was waiting for feedback from Marek in case there were more issues, but I can fix the goto reference and resubmit. I'm build-testing the colibri_imx7 now. I'm just moving the 'err_clk' reference out of the #ifdef so the same reference can be used. It doesn't look like it should be inside the #ifdef anyway. You should see something shortly, and I'll add you to the CC list. adam > > Best regards, > > Tim