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

[Lukasz Majewski]

On Mon, 24 Jun 2019 12:06:28 +0200
Marek Vasut <marex at denx.de> wrote:

> On 6/24/19 8:33 AM, Peng Fan wrote:
> > Hi Marek,
> >   
> >> Subject: Re: [PATCH] usb: ehci-mx6: Fix bus enumeration for DM case
> >>
> >> On 6/21/19 3:45 AM, Peng Fan wrote:  
> >>> Hi Marek,
> >>>  
> >>>> -----Original Message-----
> >>>> From: Marek Vasut [mailto:marex at denx.de]
> >>>> Sent: 2019年6月21日 4:54
> >>>> To: u-boot at lists.denx.de
> >>>> Cc: Marek Vasut <marex at denx.de>; Abel Vesa <abel.vesa at nxp.com>;  
> >> Adam  
> >>>> Ford <aford173 at gmail.com>; Fabio Estevam <festevam at gmail.com>;  
> >> Ludwig  
> >>>> Zenz <lzenz at dh-electronics.com>; Peng Fan <peng.fan at nxp.com>;  
> >> Stefano  
> >>>> Babic <sbabic at denx.de>; Vagrant Cascadian <vagrant at debian.org>
> >>>> Subject: [PATCH] usb: ehci-mx6: Fix bus enumeration for DM case
> >>>>
> >>>> It is likely that the DM conversion of EHCI iMX6 driver was a
> >>>> derivative of EHCI VF, however the conversion is incomplete and
> >>>> is missing the bind workaround, which updates dev->seq number.
> >>>> Without this, all controllers have dev->seq number 0 . Add this
> >>>> bind workaround  
> >> into EHCI iMX6 driver as well.  
> >>>>
> >>>> 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: Peng Fan <peng.fan at nxp.com>
> >>>> Cc: Stefano Babic <sbabic at denx.de>
> >>>> Cc: Vagrant Cascadian <vagrant at debian.org>
> >>>> ---
> >>>>  drivers/usb/host/ehci-mx6.c | 17 +++++++++++++++++
> >>>>  1 file changed, 17 insertions(+)
> >>>>
> >>>> diff --git a/drivers/usb/host/ehci-mx6.c
> >>>> b/drivers/usb/host/ehci-mx6.c index 33abfeada0..109ed7ed4a 100644
> >>>> --- a/drivers/usb/host/ehci-mx6.c
> >>>> +++ b/drivers/usb/host/ehci-mx6.c
> >>>> @@ -503,6 +503,22 @@ static int
> >>>> ehci_usb_ofdata_to_platdata(struct udevice *dev)
> >>>>  	return 0;
> >>>>  }
> >>>>
> >>>> +static int ehci_usb_bind(struct udevice *dev) {
> >>>> +	static int num_controllers;
> >>>> +
> >>>> +	/*
> >>>> +	 * Without this hack, if we return ENODEV for USB
> >>>> Controller 0, on
> >>>> +	 * probe for the next controller, USB Controller 1 will
> >>>> be given a
> >>>> +	 * sequence number of 0. This conflicts with our
> >>>> requirement of
> >>>> +	 * sequence numbers while initialising the peripherals.
> >>>> +	 */
> >>>> +	dev->req_seq = num_controllers;  
> >>>
> >>> With alias in dts, no need this, right?  
> >>
> >> There are no aliases in the DTs for the USB controllers, so that
> >> doesn't help. I think for DM, the real solution would be to parse
> >> the PHY phandle and pass around the PHY address instead of some
> >> arbitrary index, but that's something to be done for next release.
> >> For this release, it's only fixes.  
> > 
> > I think the better method is use alias in dts, introducing
> > xx-u-boot.dtsi for such case.  
> 
> What good would that make if you can obtain the PHY address from the
> DT, without the need for arbitrary error-prone indexes? The indexes
> are remnants from when there was no DM or from the DM conversion, so
> we should remove them altogether.
> 
> >> Would you be interested in implementing the later for -next ? ;-)  
> > 
> > In NXP vendor tree, we have ehci_usb_get_phy to parse phy from dtb,
> > I could find some time send that to community.  
> What about implementing a proper USB PHY driver, which binds to a DT
> entry, and then hooking it up to the EHCI MX6 ? IIRC the
> infrastructure for this is already in place and that would be the
> right (TM) solution.
> 

Sorry, but I'm a bit confused.

The patch that you introduced above, in the commit message, states
clearly that it is a hack.

Why shall we allow introducing hacks instead of implementing it in the
"right way" from the outset ?

As you mentioned the infrastructure is already in place, so why not do
it in the correct way?

And yes, as Peng noted up, till now many boards use aliases to fix this
situation.



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/20190624/f1805e87/attachment.sig>