[U-Boot] [PATCH V2] usb: ehci-mx6: Fix bus enumeration for DM case

[Igor Opaniuk]

Hi Marek,

On Mon, Jun 24, 2019 at 8:08 PM Marek Vasut <marex at denx.de> wrote:
>
> The EHCI iMX6 driver is only partly converted to DT probing and
> still uses a tremendous amount of hard-coded addresses. Worse,
> the driver uses hard-coded SoC-model-specific base addresses, which
> are derived from values protected by SoC-specific macros, hence the
> driver is also compiled for a specific SoC model. Even worse, the
> driver depends on specific sequential indexing of the controllers,
> from which it derives offsets in the PHY and ANATOP register sets.
>
> However, when the driver is probed from DT, the indexing is not
> correct. In fact, each controller has index 0. This patch derives
> the index for DT probing case from the controller base addresses,
> which is not the way this should be done, however it is the least
> intrusive approach, favorable this close to release.
>
> The necessary steps to convert this driver fully to DT probing are
> described inside the patch, however this should be done in the next
> release and depends on iMX clock driver patches.
>
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Abel Vesa <abel.vesa at nxp.com>
> Cc: Adam Ford <aford173 at gmail.com>
> Cc: Fabio Estevam <festevam at gmail.com>
> Cc: Ludwig Zenz <lzenz at dh-electronics.com>
> Cc: Lukasz Majewski <lukma at denx.de>
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Vagrant Cascadian <vagrant at debian.org>
> ---
> V2: Derive the controller index from it's base address
> ---
>  drivers/usb/host/ehci-mx6.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index 33abfeada0..e9e6ed596d 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -503,6 +503,42 @@ static int ehci_usb_ofdata_to_platdata(struct udevice *dev)
>         return 0;
>  }
>
> +static int ehci_usb_bind(struct udevice *dev)
> +{
> +       /*
> +        * TODO:
> +        * This driver is only partly converted to DT probing and still uses
> +        * a tremendous amount of hard-coded addresses. To make things worse,
> +        * the driver depends on specific sequential indexing of controllers,
> +        * from which it derives offsets in the PHY and ANATOP register sets.
> +        *
> +        * Here we attempt to calculate these indexes from DT information as
> +        * well as we can. The USB controllers on all existing iMX6/iMX7 SoCs
> +        * are placed next to each other, at addresses incremented by 0x200.
> +        * Thus, the index is derived from the multiple of 0x200 offset from
> +        * the first controller address.
I'm afraid I'm a bit late, as the patch is already applied but
this statement is not true for iMX7S/iMX7D SoCs.

If you look into arch/arm/dts/imx7s.dtsi and
arch/arm/dts/imx7d.dtsi, you'll find that
addresses are not incremented by 0x200:

usbotg1: usb at 30b10000 {
    compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
    reg = <0x30b10000 0x200>;
              ^^^^^^^^^^^^
....
usbotg2: usb at 30b20000 {
    compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
    reg = <0x30b20000 0x200>;
              ^^^^^^^^^^^
....

usbh: usb at 30b30000 {
    compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
    reg = <0x30b30000 0x200>;
                 ^^^^^^^^^^^
....

Which causes probing issues:

Colibri iMX7 # usb start
starting USB...
Bus usb at 30b10000: USB EHCI 1.00
Bus usb at 30b20000: probe failed, error -22
scanning bus usb at 30b10000 for devices... 1 USB Device(s) found
       scanning usb for storage devices... 0 Storage Device(s) found

> +        *
> +        * However, to complete conversion of this driver to DT probing, the
> +        * following has to be done:
> +        * - DM clock framework support for iMX must be implemented
> +        * - usb_power_config() has to be converted to clock framework
> +        *   -> Thus, the ad-hoc "index" variable goes away.
> +        * - USB PHY handling has to be factored out into separate driver
> +        *   -> Thus, the ad-hoc "index" variable goes away from the PHY
> +        *      code, the PHY driver must parse it's address from DT. This
> +        *      USB driver must find the PHY driver via DT phandle.
> +        *   -> usb_power_config() shall be moved to PHY driver
> +        * With these changes in place, the ad-hoc indexing goes away and
> +        * the driver is fully converted to DT probing.
> +        */
> +       fdt_size_t size;
> +       fdt_addr_t addr = devfdt_get_addr_size_index(dev, 0, &size);
> +
> +       dev->req_seq = (addr - USB_BASE_ADDR) / size;
> +
> +       return 0;
> +}
> +
>  static int ehci_usb_probe(struct udevice *dev)
>  {
>         struct usb_platdata *plat = dev_get_platdata(dev);
> @@ -564,6 +600,7 @@ U_BOOT_DRIVER(usb_mx6) = {
>         .id     = UCLASS_USB,
>         .of_match = mx6_usb_ids,
>         .ofdata_to_platdata = ehci_usb_ofdata_to_platdata,
> +       .bind   = ehci_usb_bind,
>         .probe  = ehci_usb_probe,
>         .remove = ehci_deregister,
>         .ops    = &ehci_usb_ops,
> --
> 2.20.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

Thanks

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk