| Message ID | 20250308200921.1089980-2-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add GBETH glue layer driver for Renesas RZ/V2H(P) SoC | expand |
On Sat, Mar 08, 2025 at 08:09:19PM +0000, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Increase the `maxItems` value for the `interrupts` and `interrupt-names` > properties to accommodate the Renesas RZ/V2H(P) SoC, which features the > `snps,dwmac-5.20` IP with 11 interrupts. > > Also add `additionalItems: true` to allow specifying extra interrupts > beyond the predefined ones. Update the `interrupt-names` property to > allow specifying extra `interrupt-names`. > > Also refactor the optional `interrupt-names` property by consolidating > repeated enums into a single enum list. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > Note, for this change I will be sending a sperate patch for vendor > bindings to add constraints. > > v1->v2 > - No change > --- > Documentation/devicetree/bindings/net/snps,dwmac.yaml | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > index 3f0aa46d798e..fad0d611a75c 100644 > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > @@ -114,6 +114,8 @@ properties: > > interrupts: > minItems: 1 > + maxItems: 11 > + additionalItems: true > items: > - description: Combined signal for various interrupt events > - description: The interrupt to manage the remote wake-up packet detection > @@ -122,11 +124,11 @@ properties: > > interrupt-names: > minItems: 1 > + maxItems: 11 > + additionalItems: true > items: > - const: macirq > - enum: [eth_wake_irq, eth_lpi, sfty] > - - enum: [eth_wake_irq, eth_lpi, sfty] > - - enum: [eth_wake_irq, eth_lpi, sfty] I think this should be structured similar to the DWC PCIe binding where we define all possible names, but not the order: minItems: 1 maxItems: 11 items: oneOf: - const: macirq description: ... - const: eth_wake_irq description: ... - pattern: '^rx-queue-[0-3]$' description: ... - pattern: '^tx-queue-[0-3]$' description: ... And so on. Move the descriptions from 'interrupts' and drop 'items' and 'additionalItems' from it. Rob
Hi Rob, Thank you for the review. On Mon, Mar 10, 2025 at 9:30 PM Rob Herring <robh@kernel.org> wrote: > > On Sat, Mar 08, 2025 at 08:09:19PM +0000, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Increase the `maxItems` value for the `interrupts` and `interrupt-names` > > properties to accommodate the Renesas RZ/V2H(P) SoC, which features the > > `snps,dwmac-5.20` IP with 11 interrupts. > > > > Also add `additionalItems: true` to allow specifying extra interrupts > > beyond the predefined ones. Update the `interrupt-names` property to > > allow specifying extra `interrupt-names`. > > > > Also refactor the optional `interrupt-names` property by consolidating > > repeated enums into a single enum list. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > Note, for this change I will be sending a sperate patch for vendor > > bindings to add constraints. > > > > v1->v2 > > - No change > > --- > > Documentation/devicetree/bindings/net/snps,dwmac.yaml | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > index 3f0aa46d798e..fad0d611a75c 100644 > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > @@ -114,6 +114,8 @@ properties: > > > > interrupts: > > minItems: 1 > > + maxItems: 11 > > + additionalItems: true > > items: > > - description: Combined signal for various interrupt events > > - description: The interrupt to manage the remote wake-up packet detection > > @@ -122,11 +124,11 @@ properties: > > > > interrupt-names: > > minItems: 1 > > + maxItems: 11 > > + additionalItems: true > > items: > > - const: macirq > > - enum: [eth_wake_irq, eth_lpi, sfty] > > - - enum: [eth_wake_irq, eth_lpi, sfty] > > - - enum: [eth_wake_irq, eth_lpi, sfty] > > I think this should be structured similar to the DWC PCIe binding where > we define all possible names, but not the order: > > minItems: 1 > maxItems: 11 > items: > oneOf: > - const: macirq > description: ... > - const: eth_wake_irq > description: ... > - pattern: '^rx-queue-[0-3]$' > description: ... > - pattern: '^tx-queue-[0-3]$' > description: ... > > And so on. Move the descriptions from 'interrupts' and drop 'items' and > 'additionalItems' from it. > Thanks for the pointer, I'll do as suggested above. @Russel, are you OK from me to add rx-queue/tx-queue in this binding file or would you suggest different names for it. Please share your thoughts. Cheers, Prabhakar
Hi Russell, On Tue, Mar 11, 2025 at 7:16 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > Hi Rob, > > Thank you for the review. > > On Mon, Mar 10, 2025 at 9:30 PM Rob Herring <robh@kernel.org> wrote: > > > > On Sat, Mar 08, 2025 at 08:09:19PM +0000, Prabhakar wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > Increase the `maxItems` value for the `interrupts` and `interrupt-names` > > > properties to accommodate the Renesas RZ/V2H(P) SoC, which features the > > > `snps,dwmac-5.20` IP with 11 interrupts. > > > > > > Also add `additionalItems: true` to allow specifying extra interrupts > > > beyond the predefined ones. Update the `interrupt-names` property to > > > allow specifying extra `interrupt-names`. > > > > > > Also refactor the optional `interrupt-names` property by consolidating > > > repeated enums into a single enum list. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > Note, for this change I will be sending a sperate patch for vendor > > > bindings to add constraints. > > > > > > v1->v2 > > > - No change > > > --- > > > Documentation/devicetree/bindings/net/snps,dwmac.yaml | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > index 3f0aa46d798e..fad0d611a75c 100644 > > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > > > @@ -114,6 +114,8 @@ properties: > > > > > > interrupts: > > > minItems: 1 > > > + maxItems: 11 > > > + additionalItems: true > > > items: > > > - description: Combined signal for various interrupt events > > > - description: The interrupt to manage the remote wake-up packet detection > > > @@ -122,11 +124,11 @@ properties: > > > > > > interrupt-names: > > > minItems: 1 > > > + maxItems: 11 > > > + additionalItems: true > > > items: > > > - const: macirq > > > - enum: [eth_wake_irq, eth_lpi, sfty] > > > - - enum: [eth_wake_irq, eth_lpi, sfty] > > > - - enum: [eth_wake_irq, eth_lpi, sfty] > > > > I think this should be structured similar to the DWC PCIe binding where > > we define all possible names, but not the order: > > > > minItems: 1 > > maxItems: 11 > > items: > > oneOf: > > - const: macirq > > description: ... > > - const: eth_wake_irq > > description: ... > > - pattern: '^rx-queue-[0-3]$' > > description: ... > > - pattern: '^tx-queue-[0-3]$' > > description: ... > > > > And so on. Move the descriptions from 'interrupts' and drop 'items' and > > 'additionalItems' from it. > > > Thanks for the pointer, I'll do as suggested above. > > @Russel, are you OK from me to add rx-queue/tx-queue in this binding Apologies for the typo in your name. Cheers, Prabhakar
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index 3f0aa46d798e..fad0d611a75c 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml @@ -114,6 +114,8 @@ properties: interrupts: minItems: 1 + maxItems: 11 + additionalItems: true items: - description: Combined signal for various interrupt events - description: The interrupt to manage the remote wake-up packet detection @@ -122,11 +124,11 @@ properties: interrupt-names: minItems: 1 + maxItems: 11 + additionalItems: true items: - const: macirq - enum: [eth_wake_irq, eth_lpi, sfty] - - enum: [eth_wake_irq, eth_lpi, sfty] - - enum: [eth_wake_irq, eth_lpi, sfty] clocks: minItems: 1