| Message ID | bc794e2165244bd0cee81bc0106f1e2d1bef1613.1704788539.git.ysato@users.sourceforge.jp (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Device Tree support for SH7751 based board | expand |
Hi Yoshinori, thanks for your patch! On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > + renesas,icr-irlm: > + $ref: /schemas/types.yaml#/definitions/flag > + description: If true four independent interrupt requests mode (ICR.IRLM is 1). > + > + renesas,ipr-map: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: | > + IRQ to IPR mapping definition. > + 1st - INTEVT code > + 2nd - Register > + 3rd - bit index (...) > + renesas,ipr-map = <0x240 IPRD IPR_B12>, /* IRL0 */ > + <0x2a0 IPRD IPR_B8>, /* IRL1 */ > + <0x300 IPRD IPR_B4>, /* IRL2 */ > + <0x360 IPRD IPR_B0>, /* IRL3 */ (...) Is it really necessary to have all this in the device tree? You know from the compatible that this is "renesas,sh7751-intc" and I bet this table will be the same for any sh7751 right? Then just put it in a table in the driver instead and skip this from the device tree and bindings. If more interrupt controllers need to be supported by the driver, you can simply look up the table from the compatible string. Yours, Linus Walleij
On Tue, 09 Jan 2024 21:30:34 +0900, Linus Walleij wrote: > > Hi Yoshinori, > > thanks for your patch! > > On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato > <ysato@users.sourceforge.jp> wrote: > > > + renesas,icr-irlm: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: If true four independent interrupt requests mode (ICR.IRLM is 1). > > + > > + renesas,ipr-map: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + description: | > > + IRQ to IPR mapping definition. > > + 1st - INTEVT code > > + 2nd - Register > > + 3rd - bit index > > (...) > > > + renesas,ipr-map = <0x240 IPRD IPR_B12>, /* IRL0 */ > > + <0x2a0 IPRD IPR_B8>, /* IRL1 */ > > + <0x300 IPRD IPR_B4>, /* IRL2 */ > > + <0x360 IPRD IPR_B0>, /* IRL3 */ > (...) > > Is it really necessary to have all this in the device tree? > > You know from the compatible that this is "renesas,sh7751-intc" > and I bet this table will be the same for any sh7751 right? > > Then just put it in a table in the driver instead and skip this from > the device tree and bindings. If more interrupt controllers need > to be supported by the driver, you can simply look up the table from > the compatible string. The SH interrupt controller has the same structure, only this part is different for each SoC. Currently, we are targeting only the 7751, but in the future we plan to handle all SoCs. Is it better to differentiate SoC only by compatible? > Yours, > Linus Walleij >
Hi Sato-san, On Wed, Jan 17, 2024 at 10:46 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > On Tue, 09 Jan 2024 21:30:34 +0900, > Linus Walleij wrote: > > On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato > > <ysato@users.sourceforge.jp> wrote: > > > > > + renesas,icr-irlm: > > > + $ref: /schemas/types.yaml#/definitions/flag > > > + description: If true four independent interrupt requests mode (ICR.IRLM is 1). > > > + > > > + renesas,ipr-map: > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > + description: | > > > + IRQ to IPR mapping definition. > > > + 1st - INTEVT code > > > + 2nd - Register > > > + 3rd - bit index > > > > (...) > > > > > + renesas,ipr-map = <0x240 IPRD IPR_B12>, /* IRL0 */ > > > + <0x2a0 IPRD IPR_B8>, /* IRL1 */ > > > + <0x300 IPRD IPR_B4>, /* IRL2 */ > > > + <0x360 IPRD IPR_B0>, /* IRL3 */ > > (...) > > > > Is it really necessary to have all this in the device tree? > > > > You know from the compatible that this is "renesas,sh7751-intc" > > and I bet this table will be the same for any sh7751 right? > > > > Then just put it in a table in the driver instead and skip this from > > the device tree and bindings. If more interrupt controllers need > > to be supported by the driver, you can simply look up the table from > > the compatible string. > > The SH interrupt controller has the same structure, only this part is different for each SoC. > Currently, we are targeting only the 7751, but in the future we plan to handle all SoCs. > Is it better to differentiate SoC only by compatible? Yes, it is better to differentiate SoC only by compatible value. When you describe all differences explicitly using properties, you might discover later that you missed something important, causing backwards compatibility issues with old DTBs. DT is a stable ABI, while you can always update a driver when needed. Gr{oetje,eeting}s, Geert
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-intc.yaml new file mode 100644 index 000000000000..beb29b225ac2 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-intc.yaml @@ -0,0 +1,105 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/renesas,sh7751-intc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Renesas SH7751 Interrupt Controller + +maintainers: + - Yoshinori Sato <ysato@users.sourceforge.jp> + +properties: + compatible: + items: + - const: renesas,sh7751-intc + + '#interrupt-cells': + const: 1 + + interrupt-controller: true + + reg: + maxItems: 2 + + reg-names: + items: + - const: ICR + - const: INTPRI00 + + renesas,icr-irlm: + $ref: /schemas/types.yaml#/definitions/flag + description: If true four independent interrupt requests mode (ICR.IRLM is 1). + + renesas,ipr-map: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: | + IRQ to IPR mapping definition. + 1st - INTEVT code + 2nd - Register + 3rd - bit index + +required: + - compatible + - reg + - reg-names + - '#interrupt-cells' + - interrupt-controller + - renesas,ipr-map + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/renesas,sh7751-intc.h> + shintc: interrupt-controller@ffd00000 { + compatible = "renesas,sh7751-intc"; + reg = <0xffd00000 14>, <0xfe080000 128>; + reg-names = "ICR", "INTPRI00"; + #interrupt-cells = <1>; + interrupt-controller; + renesas,ipr-map = <0x240 IPRD IPR_B12>, /* IRL0 */ + <0x2a0 IPRD IPR_B8>, /* IRL1 */ + <0x300 IPRD IPR_B4>, /* IRL2 */ + <0x360 IPRD IPR_B0>, /* IRL3 */ + <0x400 IPRA IPR_B12>, /* TMU0 */ + <0x420 IPRA IPR_B8>, /* TMU1 */ + <0x440 IPRA IPR_B4>, /* TMU2 TNUI */ + <0x460 IPRA IPR_B4>, /* TMU2 TICPI */ + <0x480 IPRA IPR_B0>, /* RTC ATI */ + <0x4a0 IPRA IPR_B0>, /* RTC PRI */ + <0x4c0 IPRA IPR_B0>, /* RTC CUI */ + <0x4e0 IPRB IPR_B4>, /* SCI ERI */ + <0x500 IPRB IPR_B4>, /* SCI RXI */ + <0x520 IPRB IPR_B4>, /* SCI TXI */ + <0x540 IPRB IPR_B4>, /* SCI TEI */ + <0x560 IPRB IPR_B12>, /* WDT */ + <0x580 IPRB IPR_B8>, /* REF RCMI */ + <0x5a0 IPRB IPR_B4>, /* REF ROVI */ + <0x600 IPRC IPR_B0>, /* H-UDI */ + <0x620 IPRC IPR_B12>, /* GPIO */ + <0x640 IPRC IPR_B8>, /* DMAC DMTE0 */ + <0x660 IPRC IPR_B8>, /* DMAC DMTE1 */ + <0x680 IPRC IPR_B8>, /* DMAC DMTE2 */ + <0x6a0 IPRC IPR_B8>, /* DMAC DMTE3 */ + <0x6c0 IPRC IPR_B8>, /* DMAC DMAE */ + <0x700 IPRC IPR_B4>, /* SCIF ERI */ + <0x720 IPRC IPR_B4>, /* SCIF RXI */ + <0x740 IPRC IPR_B4>, /* SCIF BRI */ + <0x760 IPRC IPR_B4>, /* SCIF TXI */ + <0x780 IPRC IPR_B8>, /* DMAC DMTE4 */ + <0x7a0 IPRC IPR_B8>, /* DMAC DMTE5 */ + <0x7c0 IPRC IPR_B8>, /* DMAC DMTE6 */ + <0x7e0 IPRC IPR_B8>, /* DMAC DMTE7 */ + <0xa00 INTPRI00 IPR_B0>, /* PCIC PCISERR */ + <0xa20 INTPRI00 IPR_B4>, /* PCIC PCIDMA3 */ + <0xa40 INTPRI00 IPR_B4>, /* PCIC PCIDMA2 */ + <0xa60 INTPRI00 IPR_B4>, /* PCIC PCIDMA1 */ + <0xa80 INTPRI00 IPR_B4>, /* PCIC PCIDMA0 */ + <0xaa0 INTPRI00 IPR_B4>, /* PCIC PCIPWON */ + <0xac0 INTPRI00 IPR_B4>, /* PCIC PCIPWDWN */ + <0xae0 INTPRI00 IPR_B4>, /* PCIC PCIERR */ + <0xb00 INTPRI00 IPR_B8>, /* TMU3 */ + <0xb80 INTPRI00 IPR_B12>; /* TMU4 */ + }; +...
Renesas SH7751 INTC json-schema. Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> --- .../renesas,sh7751-intc.yaml | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-intc.yaml