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

[Tim Harvey <tharvey@gateworks.com>]

On Thu, Dec 23, 2021 at 6:08 AM Adam Ford <aford173@gmail.com> 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.
>
> 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
> @@ -513,7 +513,7 @@ static const struct ehci_ops mx6_ehci_ops = {
>
>  static int ehci_usb_phy_mode(struct udevice *dev)
>  {
> -       struct usb_plat *plat = dev_get_plat(dev);
> +       struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
>         void *__iomem addr = dev_read_addr_ptr(dev);
>         void *__iomem phy_ctrl, *__iomem phy_status;
>         const void *blob = gd->fdt_blob;
> @@ -540,18 +540,18 @@ static int ehci_usb_phy_mode(struct udevice *dev)
>                 val = readl(phy_ctrl);
>
>                 if (val & USBPHY_CTRL_OTG_ID)
> -                       plat->init_type = USB_INIT_DEVICE;
> +                       priv->init_type = USB_INIT_DEVICE;
>                 else
> -                       plat->init_type = USB_INIT_HOST;
> -       } else if (is_mx7()) {
> +                       priv->init_type = USB_INIT_HOST;
> +       } else if (is_mx7() || is_imx8mm() || is_imx8mn()) {
>                 phy_status = (void __iomem *)(addr +
>                                               USBNC_PHY_STATUS_OFFSET);
>                 val = readl(phy_status);
>
>                 if (val & USBNC_PHYSTATUS_ID_DIG)
> -                       plat->init_type = USB_INIT_DEVICE;
> +                       priv->init_type = USB_INIT_DEVICE;
>                 else
> -                       plat->init_type = USB_INIT_HOST;
> +                       priv->init_type = USB_INIT_HOST;
>         } else {
>                 return -EINVAL;
>         }
> @@ -559,19 +559,19 @@ static int ehci_usb_phy_mode(struct udevice *dev)
>         return 0;
>  }
>
> -static int ehci_usb_of_to_plat(struct udevice *dev)
> +static int ehci_usb_dr_mode(struct udevice *dev)
>  {
> -       struct usb_plat *plat = dev_get_plat(dev);
> +       struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
>         enum usb_dr_mode dr_mode;
>
>         dr_mode = usb_get_dr_mode(dev_ofnode(dev));
>
>         switch (dr_mode) {
>         case USB_DR_MODE_HOST:
> -               plat->init_type = USB_INIT_HOST;
> +               priv->init_type = USB_INIT_HOST;
>                 break;
>         case USB_DR_MODE_PERIPHERAL:
> -               plat->init_type = USB_INIT_DEVICE;
> +               priv->init_type = USB_INIT_DEVICE;
>                 break;
>         case USB_DR_MODE_OTG:
>         case USB_DR_MODE_UNKNOWN:
> @@ -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;
> -       priv->phy_type = usb_get_phy_mode(dev_ofnode(dev));
>
>  #if CONFIG_IS_ENABLED(CLK)
>         ret = clk_get_by_index(dev, 0, &priv->clk);
> @@ -677,6 +673,11 @@ static int ehci_usb_probe(struct udevice *dev)
>         mdelay(1);
>  #endif
>
> +       ret = ehci_usb_dr_mode(dev);
> +       if (ret)
> +               goto err_clk;
> +       priv->phy_type = usb_get_phy_mode(dev_ofnode(dev));
> +
>  #if CONFIG_IS_ENABLED(DM_REGULATOR)
>         ret = device_get_supply_regulator(dev, "vbus-supply",
>                                           &priv->vbus_supply);
> @@ -700,7 +701,7 @@ static int ehci_usb_probe(struct udevice *dev)
>  #if CONFIG_IS_ENABLED(DM_REGULATOR)
>         if (priv->vbus_supply) {
>                 ret = regulator_set_enable(priv->vbus_supply,
> -                                          (type == USB_INIT_DEVICE) ?
> +                                          (priv->init_type == USB_INIT_DEVICE) ?
>                                            false : true);
>                 if (ret && ret != -ENOSYS) {
>                         printf("Error enabling VBUS supply (ret=%i)\n", ret);
> @@ -785,11 +786,9 @@ U_BOOT_DRIVER(usb_mx6) = {
>         .name   = "ehci_mx6",
>         .id     = UCLASS_USB,
>         .of_match = mx6_usb_ids,
> -       .of_to_plat = ehci_usb_of_to_plat,
>         .probe  = ehci_usb_probe,
>         .remove = ehci_usb_remove,
>         .ops    = &ehci_usb_ops,
> -       .plat_auto      = sizeof(struct usb_plat),
>         .priv_auto      = sizeof(struct ehci_mx6_priv_data),
>         .flags  = DM_FLAG_ALLOC_PRIV_DMA,
>  };
> --
> 2.32.0
>

Adam,

How are you testing this for IMX8MM or IMX8MN?

Best Regards,

Tim