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

["Pandey, Radhey Shyam" <radheys@amd.com>]

On 5/14/2026 9:30 PM, Krzysztof Kozlowski wrote:
> 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?
> 


Yes, in next version will switch to a unified node representation for
Versal Gen2 MMI USB.

Based on your feedback:

-the fake wrapper hierarchy will be removed.
-parent node without reg will be dropped.
-syscon will not be used to model hierarchy.

Instead, I will introduce a separate unified-node binding for Versal
Gen2 MMI USB where: the USB controller register space is described
directly in the USB node, auxiliary USB configuration registers in
UDH SLCR are accessed through a syscon phandle only for sideband
configuration access.

New DT representation using single unified node.

usb@0xedec0000 {
     compatible = "xlnx,versal2-mmi-dwc3", "snps,dwc3";
     reg = <...>;

     xlnx,usb-syscon = <&udh_slcr>;
     <snip>
};

Thanks,
Radhey