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

[Marek Vasut]

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.

-- 
Best regards,
Marek Vasut