[PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

Hi Marek,

On 2023/01/24 0:49, Marek Vasut wrote:
> On 1/23/23 06:01, Kunihiko Hayashi wrote:
>> Hi Marek,
> 
> Hello Hayashi-san,
> 
> [...]
> 
>>> On the other hand, the PXS2 controller for example is not a bus:
>>>
>>> arch/arm/dts/uniphier-pxs2.dtsi:
>>> 596 _usb0: usb at 65a00000 {
>>> 597     compatible = "socionext,uniphier-dwc3", "snps,dwc3";
>>> 598     status = "disabled";
>>> 599     reg = <0x65a00000 0xcd00>;
>>> ...
>>> 610 };
>>
>> I understand. However, this node isn't used in u-boot.
>> (see below for details)
>>
>>>>> Is this needed for socionext dwc3 variant to handle the simple-mfd in
>>>>> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
>>>>>
>>>>> 614 usb-glue at 65b00000 {
>>>>> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
>>>>> 616              "simple-mfd";
>>>>>
>>>>> ?
>>>>
>>>> In case of U-Boot, the glue driver is probed by:
>>>>
>>>>        /* FIXME: U-Boot own node */
>>>>        usb at 65b00000 {
>>>>                compatible = "socionext,uniphier-pxs3-dwc3";
>>>>
>>>> And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
>>>> Even if using "simple-mfd", this is included in
>>>> drivers/core/simple-bus.c
>>>> which is declared as UCLASS_SIMPLE_BUS.
>>>
>>> If I understand this correctly, node compatible with
>>> "socionext,uniphier-pxs3-dwc3-glue" is not used at all , right ?
>>
>> Yes.
>> Original uniphier devicetree has the following usb nodes.
>>
>>       usb at 65a00000 {
>>           compatible = "snps,dwc3";
>>       };
>>       usb-glue at 65b00000 {
>>           compatible = "socionext,uniphier-pxs3-dwc3-glue", "simple-mfd";
>>       };
>>
>> However, U-Boot dwc3-generic needs to put dwc3 node under the glue node.
> 
> Have a look at arch/arm/dts/imx8mq.dtsi which does not use any glue
> node, the snps,dwc3 compatible node is directly placed on the soc at 0 bus:
> 
> 1417
> 1418         usb_dwc3_0: usb at 38100000 {
> 1419             compatible = "fsl,imx8mq-dwc3", "snps,dwc3";
> 1420             reg = <0x38100000 0x10000>;
> 1421             clocks = <&clk IMX8MQ_CLK_USB1_CTRL_ROOT>,
> 
>> Due to this restriction, there is another usb node dedicated to u-boot.
>>
>>       /* FIXME: U-Boot own node */
>>       usb at 65b00000 { /* glue */
>>           compatible = "socionext,uniphier-pxs3-dwc3";
>>
>>           dwc3 at 65a00000 {
>>               compatible = "snps,dwc3";
>>           };
>>       };
>>
>> So instead of "socionext,uniphier-pxs3-dwc3-glue", the glue driver
>> uses "socionext,uniphier-pxs3-dwc3" in U-Boot.
> 
> Can we use the same method as imx8mq.dtsi uses, to avoid this special
> node workaround ?

Umm, it's curious. It seems imx8mq doesn't have any glue registers and defines
dwc3 core registers directly.

On the other hand, "fsl,imx8mq-dwc3" is included in dwc3-generic.c, though,
dwc3_glue_bind() calls the driver that the child node shows.

     ofnode_for_each_subnode(node, dev_ofnode(parent)) {
         ...
         device_bind_driver_to_node(parent, driver, name,
                                    node, &dev);
         ...
     }

Maybe the child node expects the driver that "snps,dwc3" indicates.

If we use the same method as imx8mq.dtsi, I think there is no way to probe
the driver for "snps,dwc3" in dwc3-generic.c.

BTW, xchi-dwc3.c can handle "snps,dwc3", but it seems this doesn't use
usb/dwc3/ functions (so I'm confusing it).

>>> The generic driver binds to node compatible with
>>> "socionext,uniphier-dwc3" , right ?
>>
>> No, the generic driver binds "socionext,uniphier-pxs3-dwc3".
>>
>>> That means, there is nothing which would be a bus, and so the
>>> dwc3-uniphier.c can be switched from UCLASS_SIMPLE_BUS to UCLASS_NOP ,
>>> is that correct ?
>>
>> There is still the issue of different usb node between Original and
>> u-boot,
>> however, the glue driver can be switched to UCLASS_NOP.
> 
> Understood, thank you for the clarification.

Thank you,

---
Best Regards
Kunihiko Hayashi