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

[Pandey, Radhey Shyam]

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