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

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

[Adam Ford]

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.

Simply adding checks in ehci_usb_phy_mode for 8mm and
8mn in ehci_usb_phy_mode is not enough, because ehci_usb_of_to_plat
is run before the clock is enabled which results in a hang.

Enable the USB clock in ehci_usb_of_to_plat and add checks in
ehci_usb_phy_mode for 8mm and 8mn to enable auto detection of
the OTG mode on i.MX8M Mini and Nano.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 1bd6147c76..fa0798171b 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);
@@ -561,11 +561,30 @@ static int ehci_usb_phy_mode(struct udevice *dev)
 
 static int ehci_usb_of_to_plat(struct udevice *dev)
 {
+#if CONFIG_IS_ENABLED(CLK)
+	int ret = 0;
+	struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
+
+	ret = clk_get_by_index(dev, 0, &priv->clk);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_enable(&priv->clk);
+	if (ret)
+		return ret;
+#endif
+
 	struct usb_plat *plat = dev_get_plat(dev);
 	enum usb_dr_mode dr_mode;
 
 	dr_mode = usb_get_dr_mode(dev_ofnode(dev));
 
+#if CONFIG_IS_ENABLED(CLK)
+	ret = clk_disable(&priv->clk);
+	if (ret)
+		return ret;
+#endif
+
 	switch (dr_mode) {
 	case USB_DR_MODE_HOST:
 		plat->init_type = USB_INIT_HOST;
-- 
2.32.0

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

[Marek Vasut]

On 12/22/21 13:52, 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.
> 
> Simply adding checks in ehci_usb_phy_mode for 8mm and
> 8mn in ehci_usb_phy_mode is not enough, because ehci_usb_of_to_plat
> is run before the clock is enabled which results in a hang.
> 
> Enable the USB clock in ehci_usb_of_to_plat and add checks in
> ehci_usb_phy_mode for 8mm and 8mn to enable auto detection of
> the OTG mode on i.MX8M Mini and Nano.

I was under the impression that of_to_plat() was meant to parse DT into 
driver local data, so frobbing with clock there could be a problem, 
right ? +CC Simon.

> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index 1bd6147c76..fa0798171b 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);
> @@ -561,11 +561,30 @@ static int ehci_usb_phy_mode(struct udevice *dev)
>   
>   static int ehci_usb_of_to_plat(struct udevice *dev)
>   {
> +#if CONFIG_IS_ENABLED(CLK)
> +	int ret = 0;
> +	struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
> +
> +	ret = clk_get_by_index(dev, 0, &priv->clk);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = clk_enable(&priv->clk);
> +	if (ret)
> +		return ret;
> +#endif
> +
>   	struct usb_plat *plat = dev_get_plat(dev);
>   	enum usb_dr_mode dr_mode;
>   
>   	dr_mode = usb_get_dr_mode(dev_ofnode(dev));
>   
> +#if CONFIG_IS_ENABLED(CLK)
> +	ret = clk_disable(&priv->clk);
> +	if (ret)
> +		return ret;
> +#endif
> +
>   	switch (dr_mode) {
>   	case USB_DR_MODE_HOST:
>   		plat->init_type = USB_INIT_HOST;

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

[Adam Ford]

On Wed, Dec 22, 2021 at 1:31 PM Marek Vasut <marex@denx.de> wrote:

> On 12/22/21 13:52, 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.
> >
> > Simply adding checks in ehci_usb_phy_mode for 8mm and
> > 8mn in ehci_usb_phy_mode is not enough, because ehci_usb_of_to_plat
> > is run before the clock is enabled which results in a hang.
> >
> > Enable the USB clock in ehci_usb_of_to_plat and add checks in
> > ehci_usb_phy_mode for 8mm and 8mn to enable auto detection of
> > the OTG mode on i.MX8M Mini and Nano.
>
> I was under the impression that of_to_plat() was meant to parse DT into
> driver local data, so frobbing with clock there could be a problem,
> right ? +CC Simon.
>

If that's true, we'll likely need to move the functions from
ehci_usb_of_to_plat to the probe function to run after the clocks are
enabled, because the call to usb_get_dr_mode hangs without the clocks
running.  usb_get_dr_mode tells the driver if it's a host or device mode.

We could make a temporary clock instead of passing the clock to the priv
structure.  I intentionally shut the clock off as soon as we're finished
reading the register so it didn't collide with the existing functions.

adam

>
> > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> > index 1bd6147c76..fa0798171b 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);
> > @@ -561,11 +561,30 @@ static int ehci_usb_phy_mode(struct udevice *dev)
> >
> >   static int ehci_usb_of_to_plat(struct udevice *dev)
> >   {
> > +#if CONFIG_IS_ENABLED(CLK)
> > +     int ret = 0;
> > +     struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
> > +
> > +     ret = clk_get_by_index(dev, 0, &priv->clk);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = clk_enable(&priv->clk);
> > +     if (ret)
> > +             return ret;
> > +#endif
> > +
> >       struct usb_plat *plat = dev_get_plat(dev);
> >       enum usb_dr_mode dr_mode;
> >
> >       dr_mode = usb_get_dr_mode(dev_ofnode(dev));
> >
> > +#if CONFIG_IS_ENABLED(CLK)
> > +     ret = clk_disable(&priv->clk);
> > +     if (ret)
> > +             return ret;
> > +#endif
> > +
> >       switch (dr_mode) {
> >       case USB_DR_MODE_HOST:
> >               plat->init_type = USB_INIT_HOST;
>

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

[Marek Vasut]

On 12/22/21 21:08, Adam Ford wrote:
> On Wed, Dec 22, 2021 at 1:31 PM Marek Vasut <marex@denx.de> wrote:
> 
>> On 12/22/21 13:52, 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.
>>>
>>> Simply adding checks in ehci_usb_phy_mode for 8mm and
>>> 8mn in ehci_usb_phy_mode is not enough, because ehci_usb_of_to_plat
>>> is run before the clock is enabled which results in a hang.
>>>
>>> Enable the USB clock in ehci_usb_of_to_plat and add checks in
>>> ehci_usb_phy_mode for 8mm and 8mn to enable auto detection of
>>> the OTG mode on i.MX8M Mini and Nano.
>>
>> I was under the impression that of_to_plat() was meant to parse DT into
>> driver local data, so frobbing with clock there could be a problem,
>> right ? +CC Simon.
>>
> 
> If that's true, we'll likely need to move the functions from
> ehci_usb_of_to_plat to the probe function to run after the clocks are
> enabled, because the call to usb_get_dr_mode hangs without the clocks
> running.  usb_get_dr_mode tells the driver if it's a host or device mode.

Yes and this does not work, because you need to know whether the IP is 
in host and gadget mode before probe happens I think. Indeed, that's why 
OTG is unsupported thus far. But please do dig in and try to find a 
proper solution.

> We could make a temporary clock instead of passing the clock to the priv
> structure.  I intentionally shut the clock off as soon as we're finished
> reading the register so it didn't collide with the existing functions.

[...]