Devicetree
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: "Pandey, Radhey Shyam" <radheys@amd.com>,
	Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Cc: gregkh@linuxfoundation.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, michal.simek@amd.com,
	Thinh.Nguyen@synopsys.com, p.zabel@pengutronix.de,
	linux-usb@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, git@amd.com
Subject: Re: [PATCH v3 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform
Date: Thu, 14 May 2026 18:00:38 +0200	[thread overview]
Message-ID: <a51d0e53-3134-475d-a19f-67d7d0695cfe@kernel.org> (raw)
In-Reply-To: <f9f25ef4-a541-45a2-b98c-4a411239993b@amd.com>

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

  reply	other threads:[~2026-05-14 16:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-29 17:30 [PATCH v3 0/4] usb: dwc3: xilinx: Add Versal2 MMI USB 3.2 controller support Radhey Shyam Pandey
2026-04-29 17:30 ` [PATCH v3 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform Radhey Shyam Pandey
2026-05-03 12:22   ` Krzysztof Kozlowski
2026-05-03 12:24     ` Krzysztof Kozlowski
2026-05-07 19:01     ` Pandey, Radhey Shyam
2026-05-14 16:00       ` Krzysztof Kozlowski [this message]
2026-04-29 17:30 ` [PATCH v3 2/4] usb: dwc3: xilinx: Introduce dwc3_xlnx_config for per-platform data Radhey Shyam Pandey
2026-04-29 17:30 ` [PATCH v3 3/4] usb: dwc3: xilinx: Add Versal2 MMI USB 3.2 controller support Radhey Shyam Pandey
2026-04-29 17:30 ` [PATCH v3 4/4] usb: dwc3: xilinx: Add support to program MMI USB TX deemphasis Radhey Shyam Pandey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a51d0e53-3134-475d-a19f-67d7d0695cfe@kernel.org \
    --to=krzk@kernel.org \
    --cc=Thinh.Nguyen@synopsys.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=git@amd.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michal.simek@amd.com \
    --cc=p.zabel@pengutronix.de \
    --cc=radhey.shyam.pandey@amd.com \
    --cc=radheys@amd.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox