[U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver

[Vignesh Raghavendra]

On 30/08/19 3:36 PM, Sherry Sun wrote:
> Hi Jean,
> 
>>
>>
>> On 28/08/2019 13:50, Sherry Sun wrote:
>>> Hi Jean,
>>>
>>>> Hi Jean,
>>>>
>>>>> Hi Marek, Sherry,
>>>>>
>>>>>
>>>>>>>>> we keep the cdns3 node for usb gadget driver, then add a usb
>>>>>>>>> host node for
>>>>>>>>> xhci-imx8 driver in *-uboot.dtsi. so here is no need to change
>>>>>>>>> the host driver
>>>>>>>> compatible.
>>>>>>>>> But the compatible in gadget driver should be changed later.
>>>>>>>> We should try avoiding ABI breaks in DT.
>>>>>>> But the cdns3 usb gaget driver and host driver in different uclass
>>>>>>> need two
>>>>> dt nodes to bind with.
>>>>>>> And the compatible of the two node cannot be same.
>>>>>>> So for gadget driver, the compatible may use "cdns,usb3-1.0.0",
>>>>>>> for host
>>>>> driver, the compatible will use "cdns,usb3-1.0.0-host".
>>>>>>> What do you think about it?
>>>>>> CCing Jean, since I think he did solve similar topic for his platform.
>>>>> I've been OOO for a few weeks and didn't look at the whole series.
>>>>>
>>>>> For this particular issue, the solution I used is to let the wrapper
>>>>> do the binding job. The name of the driver to use is hard-coded in
>>>>> the
>>>> wrapper diver.
>>>>> This is done in dwc3_glue_bind().
>>>> Thanks for your suggestions.
>>>>
>>>> So if I want to use the cdns3 usb node as both usb gadget device and
>>>> usb host device, do you mean that I should make the cdns3 usb node as
>>>> a usb wrapper device, and create two subnodes in it.
>>
>> I think we should not change the binding to adapt to out driver but keep the
>> bindings that exist in linux and adapt the u-boot driver
>>
>> In the version used by our platform, there is a wrapper around the USB:
>>
>>      usbss0: cdns_usb at 4104000 {
>>          compatible = "ti,j721e-usb";
>>          [....]
>>          usb0: usb at 6000000 {
>>              compatible = "cdns,usb3-1.0.1";
>>
>> The driver selection (host or device) could be done by the wrapper when it
>> binds its children (same as the dwc3).
>>
>> OR
>>
>> The "cdns,usb3-1.0.1" could be a dumb driver the role of which would be
>> only to bind a new driver (host or device) based on "dr_mode". The binding
>> could be done using the same node, it doesn't have to be a subnode.
>>
>>
>> Maybe the second solution will be better, as it would work for platforms that
>> do not use a wrapper.
>>
> 
> I just communicated with Vignesh Raghavendra <vigneshr at ti.com>, and he suggest that I should keep this cdns3 driver under Linux kernel and U-Boot in sync. And show me your downstream code with v10  of Cadence USB3 kernel driver ported to U-Boot. So I decide to follow your way to deal with this issue.
> 
> But I want to ask another question: 
> The two solutions you gave before both make the usb node with compatible "cdns,usb3-1.0.1" as a definite device (host or gadget) by its dr_mode property. If I want use this usb device works as both host and gadget driver, which means I want to change its status runtime, such as I want to use this usb device to run both fastboot or usb start command, how can we deal with this?  .
> 

AFAIK, U-Boot does not support runtime switching of USB port to host
from device and vice versa. This is the case for existing driver like
DWC3/MUSB etc.

Ideally we would need a role switch driver that unbinds and rebinds host
vs device driver as when required based on U-Boot cmd or via an API (if
required to be done during SPL stage etc).

Regards
Vignesh

> Best regards
> Sherry sun
> 
>>
>> JJ
>>
>>>> Then when binding the wrapper node, it will hard-coded the two
>> subnodes
>>>> to different driver(gadge/host driver) according to the dr_mode property
>> in
>>>> nodes.
>>>>
>>> Do you think I understand it right?
>>
>>>
>>> Best regards
>>> Sherry sun
>>>
>>>> Best regards
>>>> Sherry sun
>>>>
>>>>> JJ
>>>>>
>>>>>
>>>>>
>>>>>>>>>>> +	{ }
>>>>>>>>>>> +};
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de
>>>> https://lists.d
>>>> enx.de%2Flistinfo%2Fu-
>>>>
>> boot&data=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
>>>>
>> 670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
>>>>
>> C637025710721487079&sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
>>>> GrNOxDH6HC0%3D&reserved=0

-- 
Regards
Vignesh