| Message ID | 20220722151155.21100-3-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add PHY interrupt support for ETH{0,1} on RZ/G2L and RZ/V2L SMARC EVK | expand |
On Fri, 22 Jul 2022 16:11:54 +0100, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Update description for '#interrupt-cells' property to utilize the > RZG2L_{NMI,IRQX} for the first cell defined in the > include/dt-bindings/interrupt-controller/irqc-rzg2l.h file. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > v3: > * New patch > --- > .../bindings/interrupt-controller/renesas,rzg2l-irqc.yaml | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > index 33b90e975e33..ea7db3618b23 100644 > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > @@ -31,8 +31,9 @@ properties: > - const: renesas,rzg2l-irqc > > '#interrupt-cells': > - description: The first cell should contain external interrupt number (IRQ0-7) and the > - second cell is used to specify the flag. > + description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the > + include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second > + cell is used to specify the flag. I think a binding should be self describing, and not rely on an opaque macro. Mentioning that there is a macro that encodes it is fine, but the values are what matter, specially when considering that other OSs could (and should be able to) write their own DTs from scratch without depending on something that is very much Linux-specific. Thanks, M.
Hi Marc, On Sun, Jul 24, 2022 at 1:01 PM Marc Zyngier <maz@kernel.org> wrote: > On Fri, 22 Jul 2022 16:11:54 +0100, > Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > Update description for '#interrupt-cells' property to utilize the > > RZG2L_{NMI,IRQX} for the first cell defined in the > > include/dt-bindings/interrupt-controller/irqc-rzg2l.h file. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > v3: > > * New patch > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > > @@ -31,8 +31,9 @@ properties: > > - const: renesas,rzg2l-irqc > > > > '#interrupt-cells': > > - description: The first cell should contain external interrupt number (IRQ0-7) and the > > - second cell is used to specify the flag. > > + description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the > > + include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second > > + cell is used to specify the flag. > > I think a binding should be self describing, and not rely on an opaque > macro. Mentioning that there is a macro that encodes it is fine, but > the values are what matter, specially when considering that other OSs > could (and should be able to) write their own DTs from scratch without > depending on something that is very much Linux-specific. The macros are not Linux-specific, and are part of the bindings. But the only hard dependency on <dt-bindings/interrupt-controller/irqc-rzg2l.h> is the DT source file describing the board. Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Marc, On Thu, Aug 11, 2022 at 4:50 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Sun, Jul 24, 2022 at 1:01 PM Marc Zyngier <maz@kernel.org> wrote: > > On Fri, 22 Jul 2022 16:11:54 +0100, > > Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > > > Update description for '#interrupt-cells' property to utilize the > > > RZG2L_{NMI,IRQX} for the first cell defined in the > > > include/dt-bindings/interrupt-controller/irqc-rzg2l.h file. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > v3: > > > * New patch > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > > > @@ -31,8 +31,9 @@ properties: > > > - const: renesas,rzg2l-irqc > > > > > > '#interrupt-cells': > > > - description: The first cell should contain external interrupt number (IRQ0-7) and the > > > - second cell is used to specify the flag. > > > + description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the > > > + include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second > > > + cell is used to specify the flag. > > > > I think a binding should be self describing, and not rely on an opaque > > macro. Mentioning that there is a macro that encodes it is fine, but > > the values are what matter, specially when considering that other OSs > > could (and should be able to) write their own DTs from scratch without > > depending on something that is very much Linux-specific. > > The macros are not Linux-specific, and are part of the bindings. > But the only hard dependency on <dt-bindings/interrupt-controller/irqc-rzg2l.h> > is the DT source file describing the board. > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Looks like this fell through the cracks? The two other patches from this series were applied in v6.1. Note that the current DT bindings are incorrect, as they do not take into account that the value of zero is used to represent the NMI. Fixes: 96fed779d3d4cb3c ("dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller") Should we resend instead? Thanks! Gr{oetje,eeting}s, Geert
On Fri, 08 Sep 2023 10:31:35 +0100, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Marc, > > On Thu, Aug 11, 2022 at 4:50 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Sun, Jul 24, 2022 at 1:01 PM Marc Zyngier <maz@kernel.org> wrote: > > > On Fri, 22 Jul 2022 16:11:54 +0100, > > > Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > > > > > Update description for '#interrupt-cells' property to utilize the > > > > RZG2L_{NMI,IRQX} for the first cell defined in the > > > > include/dt-bindings/interrupt-controller/irqc-rzg2l.h file. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > --- > > > > v3: > > > > * New patch > > > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml > > > > @@ -31,8 +31,9 @@ properties: > > > > - const: renesas,rzg2l-irqc > > > > > > > > '#interrupt-cells': > > > > - description: The first cell should contain external interrupt number (IRQ0-7) and the > > > > - second cell is used to specify the flag. > > > > + description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the > > > > + include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second > > > > + cell is used to specify the flag. > > > > > > I think a binding should be self describing, and not rely on an opaque > > > macro. Mentioning that there is a macro that encodes it is fine, but > > > the values are what matter, specially when considering that other OSs > > > could (and should be able to) write their own DTs from scratch without > > > depending on something that is very much Linux-specific. > > > > The macros are not Linux-specific, and are part of the bindings. > > But the only hard dependency on <dt-bindings/interrupt-controller/irqc-rzg2l.h> > > is the DT source file describing the board. > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Looks like this fell through the cracks? > The two other patches from this series were applied in v6.1. > > Note that the current DT bindings are incorrect, as they do not take into > account that the value of zero is used to represent the NMI. > > Fixes: 96fed779d3d4cb3c ("dt-bindings: interrupt-controller: Add > Renesas RZ/G2L Interrupt Controller") > > Should we resend instead? It applied cleanly to v6.6-rc1, so there should be no need. Thanks, M.
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml index 33b90e975e33..ea7db3618b23 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml @@ -31,8 +31,9 @@ properties: - const: renesas,rzg2l-irqc '#interrupt-cells': - description: The first cell should contain external interrupt number (IRQ0-7) and the - second cell is used to specify the flag. + description: The first cell should contain a macro RZG2L_{NMI,IRQX} included in the + include/dt-bindings/interrupt-controller/irqc-rzg2l.h and the second + cell is used to specify the flag. const: 2 '#address-cells':
Update description for '#interrupt-cells' property to utilize the RZG2L_{NMI,IRQX} for the first cell defined in the include/dt-bindings/interrupt-controller/irqc-rzg2l.h file. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- v3: * New patch --- .../bindings/interrupt-controller/renesas,rzg2l-irqc.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)