From: Rob Herring <robh@kernel.org>
To: Conor Dooley <conor@kernel.org>
Cc: Pawel Laszczak <pawell@cadence.com>,
Peter Chen <peter.chen@kernel.org>,
Roger Quadros <rogerq@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string
Date: Mon, 1 Jun 2026 16:55:25 -0500 [thread overview]
Message-ID: <20260601215525.GA4114052-robh@kernel.org> (raw)
In-Reply-To: <20260519-defog-spiritual-2eeebb390ba7@spud>
On Tue, May 19, 2026 at 06:49:36PM +0100, Conor Dooley wrote:
> On Tue, May 19, 2026 at 08:47:30AM +0000, Pawel Laszczak wrote:
> > >
> > >On Fri, May 15, 2026 at 01:24:38PM +0200, Pawel Laszczak via B4 Relay wrote:
> > >> From: Pawel Laszczak <pawell@cadence.com>
> > >>
> > >> Introduce a new compatible string 'cdns,cdnsp-no-drd' for Cadence
> > >> USBSS/USBSSP controllers to support hardware configurations where the
> > >> Dual-Role Device (DRD) register block is missing or inaccessible.
> > >>
> > >> This change follows the recommendation to imply hardware limitations
> > >> directly from the compatible string rather than using a dedicated
> > >> boolean property.
> > >>
> > >> When 'cdns,cdnsp-no-drd' is used:
> > >> - The 'otg' register and interrupt resources are not required.
> > >> - The 'reg' and 'interrupts' properties are restricted to 2 items
> > >> (host and device).
> > >> - 'dr_mode' must be explicitly set to either 'host' or 'peripheral'.
> > >>
> > >> The standard 'cdns,usb3' compatible remains unchanged, maintaining
> > >> backward compatibility by requiring all 3 resource sets (otg, host, dev).
> > >>
> > >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> > >> ---
> > >> v8:
> > >> - Update commit message to reflect schema changes.
> > >> - Removed 'cdns,no-drd' boolean property as per Rob Herring's suggestion.
> > >> - Introduced a new compatible string 'cdns,cdnsp-no-drd' for controller
> > >> variants that lack the DRD/OTG register block.
> > >>
> > >> v7:
> > >> - Rename 'no_drd' to 'cdns,no-drd'.
> > >> - Update commit message to reflect property renaming and schema
> > >changes.
> > >> - Simplify 'reg-names' using a single enum.
> > >> - Revert 'interrupt-names' to a list of constants.
> > >> - Move 'reg' item descriptions to if/else blocks for accuracy.
> > >> - Clean up 'if/then' logic (remove redundant checks).
> > >> - Add explicit 'items' list for 'interrupt-names' in the 'else' block.
> > >>
> > >> v6:
> > >> - Fixed validation error for 'interrupt-names' by correcting
> > >> the items definition.
> > >> - Adjusted 'minItems'/'maxItems' to properly support the optional
> > >> 'wakeup' interrupt.
> > >> - Fixed 'too long' schema error in examples.
> > >>
> > >> v5:
> > >> - Implemented strict conditional validation using if-then-else logic.
> > >> - Enforced 2 register/interrupt items and required 'dr_mode'
> > >> (host or peripheral) when 'no_drd' is present.
> > >> - Enforced the standard 3 register/interrupt items (otg, host, dev)
> > >> when 'no_drd' is absent to ensure backward compatibility.
> > >> - Updated 'reg-names' and 'interrupt-names' to use enums in the main
> > >> properties section to support flexible resource ordering during
> > >> validation.
> > >> ---
> > >> ---
> > >> .../devicetree/bindings/usb/cdns,usb3.yaml | 60
> > >++++++++++++++++++----
> > >> 1 file changed, 50 insertions(+), 10 deletions(-)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > >b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > >> index 2d95fb7321af..7b0aa9c4a2bd 100644
> > >> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > >> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > >> @@ -17,22 +17,22 @@ description:
> > >>
> > >> properties:
> > >> compatible:
> > >> - const: cdns,usb3
> > >> + enum:
> > >> + - cdns,usb3
> > >> + - cdns,cdnsp-no-drd
> > >
> > >I doubt this is what Rob meant, he asked for soc-specific compatibles
> > >and this is generic. A soc-specific compatible would be something like
> > >microchip,newsoc-usb3
> >
> > Hi Conor, Rob
> >
> > To clarify the hardware situation: I am developing and testing this on
> > an FPGA development board connected via pure PCI, without any Device Tree.
> > I do not have a specific SoC platform, nor do I know what silicon target
> > the end customers will use in the future.
> > Introducing a fake SoC-specific compatible string just to pass the DT
> > validation feels misleading and unnecessary.
> >
> > Since cdns,usb3 is already an established generic compatible, and we
> > cannot use a generic configuration string like cdns,cdnsp-no-drd,
> > the cleanest way forward is to make this resource-driven instead of
> > compatible-driven.
> >
> > In v9, I propose to:
> > - Keep only the existing cdns,usb3 compatible.
> > - Update cdns,usb3.yaml to allow minItems: 2 for reg and reg-names
> > if dr_mode is explicitly set to "host" or "peripheral" (indicating
> > that the OTG register block is absent).
> > - In the driver code, determine the lack of DRD dynamically if the "otg"
> > resource/register block is missing.
> >
> > This elegantly solves the issue for my PCI/FPGA platform (where I just
> > won't pass the "otg" resource), complies with DT practices, and leaves
> > the door open for any future customer SoC to use cdns,usb3 with
> > standard dr_mode constraints.
> >
> > Does this approach look acceptable to you?
>
> Not really. I want enforcement of the correct properties in the binding,
> not permission of multiple different combinations.
>
> Perhaps we could go back to the original cdns,usbssp compatible and
> leave an empty slot for the soc-specific compatibles, like:
> - items:
> - {}
> - const: sifive,clint2 # SiFive CLINT v2 IP block
>
> That way you can proceed with this effort without a soc-compatible
> but we don't end up permitting a generic compatible.
That sounds good to me.
Rob
next prev parent reply other threads:[~2026-06-01 21:55 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-15 11:24 [PATCH v8 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay
2026-05-15 11:24 ` Pawel Laszczak
2026-05-15 11:24 ` [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string Pawel Laszczak via B4 Relay
2026-05-15 11:24 ` Pawel Laszczak
2026-05-15 11:21 ` sashiko-bot
2026-05-15 17:12 ` Conor Dooley
2026-05-19 8:47 ` Pawel Laszczak
2026-05-19 17:49 ` Conor Dooley
2026-06-01 21:55 ` Rob Herring [this message]
2026-05-15 11:24 ` [PATCH v8 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
2026-05-15 11:24 ` Pawel Laszczak
2026-05-15 11:54 ` sashiko-bot
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=20260601215525.GA4114052-robh@kernel.org \
--to=robh@kernel.org \
--cc=conor+dt@kernel.org \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=krzk+dt@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=pawell@cadence.com \
--cc=peter.chen@kernel.org \
--cc=rogerq@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.