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

[Adam Ford]

On Wed, Jun 26, 2019 at 4:39 AM Lukasz Majewski <lukma at denx.de> wrote:
>
> Hi Marek,
>
> > 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.
>
> Indexes and offsets two biggest problems with DT/DTS conversion.
>
> >
> > 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>

Tested-by: Adam Ford <aford173 at gmail.com> #im6q_logic

> > 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.
> > +      *
> > +      * 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,
>
> Thank you for the patch (and hope that it will not be long int the
> tree).
>
> It doesn't break the TPC70 anymore, hence
>
> Tested-by: Lukasz Majewski <lukma at denx.de>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de