Add bindings to indicate properties required to support multiport
on Synopsys DWC3 controller.
Suggested-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/usb/snps,dwc3.yaml | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index a696f23730d3..5bc941355b43 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -85,15 +85,16 @@ properties:
phys:
minItems: 1
- maxItems: 2
+ maxItems: 8
phy-names:
minItems: 1
- maxItems: 2
- items:
- enum:
- - usb2-phy
- - usb3-phy
+ maxItems: 8
+ oneOf:
+ - items:
+ enum: [ usb2-phy, usb3-phy ]
+ - items:
+ pattern: "^usb[23]-port[0-3]$"
power-domains:
description:
--
2.40.0On Mon, Aug 28, 2023 at 07:00:22PM +0530, Krishna Kurapati wrote: > Add bindings to indicate properties required to support multiport > on Synopsys DWC3 controller. > > Suggested-by: Bjorn Andersson <quic_bjorande@quicinc.com> > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > index a696f23730d3..5bc941355b43 100644 > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > @@ -85,15 +85,16 @@ properties: > > phys: > minItems: 1 > - maxItems: 2 > + maxItems: 8 > > phy-names: > minItems: 1 > - maxItems: 2 > - items: > - enum: > - - usb2-phy > - - usb3-phy > + maxItems: 8 > + oneOf: > + - items: > + enum: [ usb2-phy, usb3-phy ] > + - items: > + pattern: "^usb[23]-port[0-3]$" Shouldn't this just be pattern: "^usb[23]-[0-3]$" so that it matches the names that are used by the nvidia bindings? We already have some inconsistency in that Amlogic uses a variant based on the legacy names that needlessly includes "phy" in the names: const: usb2-phy0 const: usb2-phy1 const: usb3-phy0 ... I don't think we should be introducing a third naming scheme here so I suggest just following the nvidia bindings. Johan
On 11/10/2023 6:58 PM, Johan Hovold wrote: > On Mon, Aug 28, 2023 at 07:00:22PM +0530, Krishna Kurapati wrote: >> Add bindings to indicate properties required to support multiport >> on Synopsys DWC3 controller. >> >> Suggested-by: Bjorn Andersson <quic_bjorande@quicinc.com> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> Reviewed-by: Rob Herring <robh@kernel.org> >> --- >> .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +++++++------ >> 1 file changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >> index a696f23730d3..5bc941355b43 100644 >> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >> @@ -85,15 +85,16 @@ properties: >> >> phys: >> minItems: 1 >> - maxItems: 2 >> + maxItems: 8 >> >> phy-names: >> minItems: 1 >> - maxItems: 2 >> - items: >> - enum: >> - - usb2-phy >> - - usb3-phy >> + maxItems: 8 >> + oneOf: >> + - items: >> + enum: [ usb2-phy, usb3-phy ] >> + - items: >> + pattern: "^usb[23]-port[0-3]$" > > Shouldn't this just be > > pattern: "^usb[23]-[0-3]$" > > so that it matches the names that are used by the nvidia bindings? > > We already have some inconsistency in that Amlogic uses a variant based > on the legacy names that needlessly includes "phy" in the names: > > const: usb2-phy0 > const: usb2-phy1 > const: usb3-phy0 > ... > > I don't think we should be introducing a third naming scheme here so I > suggest just following the nvidia bindings. > In that case, why don't we use "^usb[23]-phy[0-3]$". I think its close to what we have on dwc3 core already today (usb2-phy/usb3-phy). Regards, Krishna,
On 11/11/2023 2:00 PM, Krishna Kurapati PSSNV wrote: > > > On 11/10/2023 6:58 PM, Johan Hovold wrote: >> On Mon, Aug 28, 2023 at 07:00:22PM +0530, Krishna Kurapati wrote: >>> Add bindings to indicate properties required to support multiport >>> on Synopsys DWC3 controller. >>> >>> Suggested-by: Bjorn Andersson <quic_bjorande@quicinc.com> >>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >>> Reviewed-by: Rob Herring <robh@kernel.org> >>> --- >>> .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +++++++------ >>> 1 file changed, 7 insertions(+), 6 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> index a696f23730d3..5bc941355b43 100644 >>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>> @@ -85,15 +85,16 @@ properties: >>> phys: >>> minItems: 1 >>> - maxItems: 2 >>> + maxItems: 8 >>> phy-names: >>> minItems: 1 >>> - maxItems: 2 >>> - items: >>> - enum: >>> - - usb2-phy >>> - - usb3-phy >>> + maxItems: 8 >>> + oneOf: >>> + - items: >>> + enum: [ usb2-phy, usb3-phy ] >>> + - items: >>> + pattern: "^usb[23]-port[0-3]$" >> >> Shouldn't this just be >> >> pattern: "^usb[23]-[0-3]$" >> >> so that it matches the names that are used by the nvidia bindings? >> >> We already have some inconsistency in that Amlogic uses a variant based >> on the legacy names that needlessly includes "phy" in the names: >> >> const: usb2-phy0 >> const: usb2-phy1 >> const: usb3-phy0 >> ... >> >> I don't think we should be introducing a third naming scheme here so I >> suggest just following the nvidia bindings. >> > In that case, why don't we use "^usb[23]-phy[0-3]$". I think its close > to what we have on dwc3 core already today (usb2-phy/usb3-phy). > I mean, it isn't needless. It is a phy and shouldn't the binding suggest that and include "-phy" in the name ? Regards, Krishna,
On Sat, Nov 11, 2023 at 03:17:40PM +0530, Krishna Kurapati PSSNV wrote:
> On 11/11/2023 2:00 PM, Krishna Kurapati PSSNV wrote:
> > On 11/10/2023 6:58 PM, Johan Hovold wrote:
> >>> phy-names:
> >>> minItems: 1
> >>> - maxItems: 2
> >>> - items:
> >>> - enum:
> >>> - - usb2-phy
> >>> - - usb3-phy
> >>> + maxItems: 8
> >>> + oneOf:
> >>> + - items:
> >>> + enum: [ usb2-phy, usb3-phy ]
> >>> + - items:
> >>> + pattern: "^usb[23]-port[0-3]$"
> >>
> >> Shouldn't this just be
> >>
> >> pattern: "^usb[23]-[0-3]$"
> >>
> >> so that it matches the names that are used by the nvidia bindings?
> >>
> >> We already have some inconsistency in that Amlogic uses a variant based
> >> on the legacy names that needlessly includes "phy" in the names:
> >>
> >> const: usb2-phy0
> >> const: usb2-phy1
> >> const: usb3-phy0
> >> ...
> >>
> >> I don't think we should be introducing a third naming scheme here so I
> >> suggest just following the nvidia bindings.
> >> > In that case, why don't we use "^usb[23]-phy[0-3]$". I think its close
> > to what we have on dwc3 core already today (usb2-phy/usb3-phy).
>
> I mean, it isn't needless. It is a phy and shouldn't the binding suggest
> that and include "-phy" in the name ?
No, adding a '-phy' suffix to each name is unnecessary since the
property is called 'phy-names'.
This is also documented:
For names used in {clock,dma,interrupt,reset}-names, do not add
any suffix, e.g.: "tx" instead of "txirq" (for interrupt).
https://docs.kernel.org/devicetree/bindings/writing-bindings.html
and we've already discussed this when I asked you to drop the likewise
redundant '_irq' suffix from the interrupt names.
Johan
On 11/11/2023 4:25 PM, Johan Hovold wrote:
> On Sat, Nov 11, 2023 at 03:17:40PM +0530, Krishna Kurapati PSSNV wrote:
>> On 11/11/2023 2:00 PM, Krishna Kurapati PSSNV wrote:
>>> On 11/10/2023 6:58 PM, Johan Hovold wrote:
>
>>>>> phy-names:
>>>>> minItems: 1
>>>>> - maxItems: 2
>>>>> - items:
>>>>> - enum:
>>>>> - - usb2-phy
>>>>> - - usb3-phy
>>>>> + maxItems: 8
>>>>> + oneOf:
>>>>> + - items:
>>>>> + enum: [ usb2-phy, usb3-phy ]
>>>>> + - items:
>>>>> + pattern: "^usb[23]-port[0-3]$"
>>>>
>>>> Shouldn't this just be
>>>>
>>>> pattern: "^usb[23]-[0-3]$"
>>>>
>>>> so that it matches the names that are used by the nvidia bindings?
>>>>
>>>> We already have some inconsistency in that Amlogic uses a variant based
>>>> on the legacy names that needlessly includes "phy" in the names:
>>>>
>>>> const: usb2-phy0
>>>> const: usb2-phy1
>>>> const: usb3-phy0
>>>> ...
>>>>
>>>> I don't think we should be introducing a third naming scheme here so I
>>>> suggest just following the nvidia bindings.
>
>>>>> In that case, why don't we use "^usb[23]-phy[0-3]$". I think its close
>>> to what we have on dwc3 core already today (usb2-phy/usb3-phy).
>>
>> I mean, it isn't needless. It is a phy and shouldn't the binding suggest
>> that and include "-phy" in the name ?
>
> No, adding a '-phy' suffix to each name is unnecessary since the
> property is called 'phy-names'.
>
> This is also documented:
>
> For names used in {clock,dma,interrupt,reset}-names, do not add
> any suffix, e.g.: "tx" instead of "txirq" (for interrupt).
>
> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
>
Thanks for the explanation.
> and we've already discussed this when I asked you to drop the likewise
> redundant '_irq' suffix from the interrupt names.
Yes, we did discuss this in irq context. But in this case I thought it
was fine because we already have usb(2/3)-"phy" already present.
When pushing v14, will make this change to usb(2/3)-(0-3) and skip port/phy.
Regards,
Krishna,
© 2016 - 2025 Red Hat, Inc.