Re: [PATCH v3 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform

Re: [PATCH v3 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform

[Pandey, Radhey Shyam]

> On Wed, Apr 29, 2026 at 11:00:47PM +0530, Radhey Shyam Pandey wrote:
>>   additionalProperties: false
>>   
>>   examples:
>> @@ -156,3 +193,30 @@ examples:
>>               };
>>           };
>>       };
>> +  - |
>> +    #include <dt-bindings/power/xlnx-zynqmp-power.h>
>> +    #include <dt-bindings/reset/xlnx-zynqmp-resets.h>
>> +    #include <dt-bindings/phy/phy.h>
>> +    usb {
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
> Please follow DTS coding style.
Thanks for the review. will fix it in next version.
>> +        compatible = "xlnx,versal2-mmi-dwc3";
> I really doubt that DWC3 block comes without addressing space
> (registers), so either you just misrepresented things, like created a
> fake block and syscon, or forgot to combine DWC3 with the wrapper.
>
> And if you built with W=1 your DTS you would see errors. How do you see
> it now? Where do you place it? Wrapper must be outside of soc, but DWC3
> child must be inside. Did you read submitting patches and writing
> bindings documents?
Apologies for missing the DTS sanity check earlier. I am summarizing the
problem statement and possible solution. Please review.

For MMI USB in current implementation it need a parent/child
representation. However, the parent IP is shared across DP, USB,
and HDCP, so it cannot have a USB-dedicated parent reg space.

1. Versal platform
   - Parent: USB wrapper IP → has its own I/O space
   - Child: USB DWC3

2. Versal Gen2 platform - MMI USB
   - Parent subsystem combines DP, USB, and HDCP in a single I/O space
   - Children:
     - USB DWC3
     - DP
     - HDCP

To model the Versal Gen2 MMI USB parent register space, I introduced
xlnx,usb-syscon, allowing the DWC3 driver to access parent registers
via a syscon handle, addressing the v1 review comment.

However, making reg optional satisfies schema validation but fails
DTB checks.

versal2.dtsi:1: Warning (simple_bus_reg):
/axi/mmi-usb: missing or empty reg/ranges property

To fix it i think we can switch from parent/child representation to
flat DT representation for the Versal Gen2 platform, similar to
existing implementations in qcom,snps-dwc3 and Google Tensor G5 DWC3
bindings[1].

The Google Tensor DWC3 binding uses a syscon phandle to access USB
configuration registers, which aligns well with the Versal Gen2 MMI
USB IP, where wrapper subsystem shares a common register space for
USB along with other IPs.

If this approach looks fine , will create binding for MMI USB using
this flat representation and send out next version.

usb@fe200000 {
compatible = "xlnx,versal2-mmi-dwc3";
reg = <0xfe200000 0x40000>;
xlnx,usb-syscon = <&udh_slcr 0x005c 0x0070 0x00c4 0x00f8>;
<snip>
};

[1]: 32bc790a8e49 dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
>> +        clocks = <&zynqmp_clk 32>, <&zynqmp_clk 34>;
>> +        clock-names = "bus_clk", "ref_clk";
>> +        power-domains = <&zynqmp_firmware PD_USB_0>;
>> +        resets = <&zynqmp_reset ZYNQMP_RESET_USB1_CORERESET>;
>> +        reset-names = "usb_crst";
>> +        phys = <&psgtr 2 PHY_TYPE_USB3 0 2>;
>> +        phy-names = "usb3-phy";
>> +        xlnx,usb-syscon = <&udh_slcr 0x005c 0x0070 0x00c4 0x00f8>;
>> +        ranges;
>> +
>> +        usb@fe200000 {
>> +            compatible = "snps,dwc3";
>> +            reg = <0xfe200000 0x40000>;
>> +            interrupt-names = "host", "otg";
>> +            interrupts = <0 65 4>, <0 69 4>;
> Why these are not proper defines?

will fix it in next version.

>> +            dr_mode = "host";
>> +            dma-coherent;
>> +        };
>> +    };
>> -- 
>> 2.43.0
>>

Re: [PATCH v3 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform

[Krzysztof Kozlowski]

On 07/05/2026 21:01, Pandey, Radhey Shyam wrote:
>> On Wed, Apr 29, 2026 at 11:00:47PM +0530, Radhey Shyam Pandey wrote:
>>>   additionalProperties: false
>>>   
>>>   examples:
>>> @@ -156,3 +193,30 @@ examples:
>>>               };
>>>           };
>>>       };
>>> +  - |
>>> +    #include <dt-bindings/power/xlnx-zynqmp-power.h>
>>> +    #include <dt-bindings/reset/xlnx-zynqmp-resets.h>
>>> +    #include <dt-bindings/phy/phy.h>
>>> +    usb {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>> Please follow DTS coding style.
> Thanks for the review. will fix it in next version.
>>> +        compatible = "xlnx,versal2-mmi-dwc3";
>> I really doubt that DWC3 block comes without addressing space
>> (registers), so either you just misrepresented things, like created a
>> fake block and syscon, or forgot to combine DWC3 with the wrapper.
>>
>> And if you built with W=1 your DTS you would see errors. How do you see
>> it now? Where do you place it? Wrapper must be outside of soc, but DWC3
>> child must be inside. Did you read submitting patches and writing
>> bindings documents?
> Apologies for missing the DTS sanity check earlier. I am summarizing the
> problem statement and possible solution. Please review.
> 
> For MMI USB in current implementation it need a parent/child
> representation. However, the parent IP is shared across DP, USB,
> and HDCP, so it cannot have a USB-dedicated parent reg space.
> 
> 1. Versal platform
>    - Parent: USB wrapper IP → has its own I/O space
>    - Child: USB DWC3
> 
> 2. Versal Gen2 platform - MMI USB
>    - Parent subsystem combines DP, USB, and HDCP in a single I/O space
>    - Children:
>      - USB DWC3
>      - DP
>      - HDCP
> 
> To model the Versal Gen2 MMI USB parent register space, I introduced
> xlnx,usb-syscon, allowing the DWC3 driver to access parent registers
> via a syscon handle, addressing the v1 review comment.

Syscon phandle is not to express such relationsship.

> 
> However, making reg optional satisfies schema validation but fails
> DTB checks.
> 
> versal2.dtsi:1: Warning (simple_bus_reg):
> /axi/mmi-usb: missing or empty reg/ranges property

Yep, exactly.

> 
> To fix it i think we can switch from parent/child representation to
> flat DT representation for the Versal Gen2 platform, similar to
> existing implementations in qcom,snps-dwc3 and Google Tensor G5 DWC3
> bindings[1].
> 
> The Google Tensor DWC3 binding uses a syscon phandle to access USB
> configuration registers, which aligns well with the Versal Gen2 MMI

Not true. Just read the binding. If you refer to Tensor, then to access
A FEW configuration registers. If you refer to LGA, then it has address
space.

> USB IP, where wrapper subsystem shares a common register space for
> USB along with other IPs.



> 
> If this approach looks fine , will create binding for MMI USB using
> this flat representation and send out next version.
> 
> usb@fe200000 {
> compatible = "xlnx,versal2-mmi-dwc3";
> reg = <0xfe200000 0x40000>;
> xlnx,usb-syscon = <&udh_slcr 0x005c 0x0070 0x00c4 0x00f8>;
> <snip>
> };

So I am confused. We ask, since long time, to have unified child.
Several platforms were already converted. What are you discussing with
in such case?

Are you going to have unified node or not?


Best regards,
Krzysztof