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

From: Michael Walle <michael@walle.cc>
To: aford173@gmail.com
Cc: festevam@denx.de, marex@denx.de, sjg@chromium.org,
	u-boot@lists.denx.de, Tim Harvey <tharvey@gateworks.com>,
	Michael Walle <michael@walle.cc>
Subject: Re: [PATCH V2] usb: ehci-mx6: Enable OTG detection on imx8mm and imx8mn
Date: Tue,  4 Jan 2022 09:31:48 +0100	[thread overview]
Message-ID: <20220104083148.1328623-1-michael@walle.cc> (raw)
In-Reply-To: <20211223140756.304527-1-aford173@gmail.com>

Hi,

> 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.
> 
> The init_type in both priv and plat data are the same, so it doesn't
> make sense to configure the data in the plat data and copy the
> data to priv when priv can be configured directly.  Instead, rename
> ehci_usb_of_to_plat to ehci_usb_dr_mode and call it from the
> probe functions after the clocks are enabled, but before the data
> is required.
> 
> With that added, the additional checks for imx8mm and imx8mn will
> allow reading the register to automatically determine the state
> (host or device) of the OTG controller.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
> 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..f2a34b7f06 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c

..

> @@ -639,10 +639,8 @@ static int mx6_parse_dt_addrs(struct udevice *dev)
>  
>  static int ehci_usb_probe(struct udevice *dev)
>  {
> -	struct usb_plat *plat = dev_get_plat(dev);
>  	struct usb_ehci *ehci = dev_read_addr_ptr(dev);
>  	struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
> -	enum usb_init_type type = plat->init_type;
>  	struct ehci_hccr *hccr;
>  	struct ehci_hcor *hcor;
>  	int ret;
> @@ -660,8 +658,6 @@ static int ehci_usb_probe(struct udevice *dev)
>  		return ret;
>  
>  	priv->ehci = ehci;
> -	priv->init_type = type;

I'm not sure this is correct. There is also this:
https://elixir.bootlin.com/u-boot/v2022.01-rc4/source/drivers/usb/host/usb-uclass.c#L407

Which won't work anymore. usb_setup_ehci_gadget() is called from
usb_gadget_register_driver() in ci_udc.c. The latter is the one used
on the imx, right? But I might be wrong. I'm still trying to figure
out how this all works together, because I also try to get OTG
running on the dwc3 driver. It looks like the ci_udc.c is special
here, and I wonder how a transition to UCLASS_USB_GADGET_GENERIC
might look like for this driver.

-michael