Re: [PATCH net-next v2 1/3] dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and 'interrupt-names'

[Lad, Prabhakar]

Hi Russell,

On Tue, Mar 11, 2025 at 7:16 AM Lad, Prabhakar
<prabhakar.csengg@xxxxxxxxx> wrote:
>
> Hi Rob,
>
> Thank you for the review.
>
> On Mon, Mar 10, 2025 at 9:30 PM Rob Herring <robh@xxxxxxxxxx> wrote:
> >
> > On Sat, Mar 08, 2025 at 08:09:19PM +0000, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxx>
> > > ---
> > > 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