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

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

[Marek Vasut]

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@denx.de>
Cc: Abel Vesa <abel.vesa@nxp.com>
Cc: Adam Ford <aford173@gmail.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Ludwig Zenz <lzenz@dh-electronics.com>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Vagrant Cascadian <vagrant@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,
-- 
2.20.1

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

[Adam Ford]

On Mon, Jun 24, 2019 at 12:07 PM Marek Vasut <marex@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@denx.de>

I can test this, but I am curious to know what I am supposed to see
and/if what's observable since I have been on vacation, I'm still
catching up on e-mails.

adam
> Cc: Abel Vesa <abel.vesa@nxp.com>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Ludwig Zenz <lzenz@dh-electronics.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Vagrant Cascadian <vagrant@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,
> --
> 2.20.1
>

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

[Marek Vasut]

On 6/24/19 7:47 PM, Adam Ford wrote:
> On Mon, Jun 24, 2019 at 12:07 PM Marek Vasut <marex@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@denx.de>
> 
> I can test this, but I am curious to know what I am supposed to see
> and/if what's observable since I have been on vacation, I'm still
> catching up on e-mails.

Try "usb reset", on some platforms with DM_USB enabled, every controller
except for the first will either fail to init or the system will hang.

-- 
Best regards,
Marek Vasut

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

[Lukasz Majewski]

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@denx.de>
> Cc: Abel Vesa <abel.vesa@nxp.com>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Ludwig Zenz <lzenz@dh-electronics.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Vagrant Cascadian <vagrant@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@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190626/fee856d8/attachment.sig>

[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@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@denx.de>
> > Cc: Abel Vesa <abel.vesa@nxp.com>
> > Cc: Adam Ford <aford173@gmail.com>

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

> > Cc: Fabio Estevam <festevam@gmail.com>
> > Cc: Ludwig Zenz <lzenz@dh-electronics.com>
> > Cc: Lukasz Majewski <lukma@denx.de>
> > Cc: Peng Fan <peng.fan@nxp.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Vagrant Cascadian <vagrant@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@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

[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@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@denx.de>
> Cc: Abel Vesa <abel.vesa@nxp.com>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Ludwig Zenz <lzenz@dh-electronics.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Vagrant Cascadian <vagrant@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

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

[Marek Vasut]

On 10/8/19 7:25 PM, Igor Opaniuk wrote:
> Hi Marek,

Hi,

[...]

>>  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:

I don't have an iMX7 board, but I can still test on the iMX6 ones. Can
you prepare a patch ?

[...]