On Tue, Jan 09, 2024 at 05:23:16PM +0900, Yoshinori Sato wrote: > Renesas SH7751 external interrupt encoder json-schema. > > Signed-off-by: Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx> > --- > .../renesas,sh7751-irl-ext.yaml | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > new file mode 100644 > index 000000000000..541b582b94ce > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,sh7751-irl-ext.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas SH7751 external interrupt encoder with enable regs. > + > +maintainers: > + - Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx> > + > +description: > + This is the generally used external interrupt encoder on SH7751 based boards. > + > +properties: > + compatible: > + items: > + - const: renesas,sh7751-irl-ext > + > + reg: true Must define how many entries and what they are if more than one. > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 1 > + > + '#address-cells': > + const: 0 > + > + renesas,set-to-disable: > + $ref: /schemas/types.yaml#/definitions/flag > + description: Invert enable registers. Setting the bit to 0 enables interrupts. Why is this a property? Does it change per board or instance? If not, it should be implied by compatible. > + > + renesas,enable-bit: > + $ref: /schemas/types.yaml#/definitions/uint32-matrix > + description: | > + IRQ enable register bit mapping > + 1st word interrupt level > + 2nd word bit index of enable register Same question here. If it remains, then you need: items: items: - description: interrupt level (does that mean high/low?) - description: bit index of enable register Plus any constraints on the values if possible. > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - renesas,enable-bit > + > +additionalProperties: false > + > +examples: > + - | > + r2dintc: sh7751irl_encoder@a4000000 { interrupt-controller@a4000000 { > + compatible = "renesas,sh7751-irl-ext"; > + reg = <0xa4000000 0x02>; > + interrupt-controller; > + #address-cells = <0>; > + #size-cells = <0>; > + #interrupt-cells = <1>; > + renesas,enable-bit = <0 11>, /* PCI INTD */ > + <1 9>, /* CF IDE */ > + <2 8>, /* CF CD */ > + <3 12>, /* PCI INTC */ > + <4 10>, /* SM501 */ > + <5 6>, /* KEY */ > + <6 5>, /* RTC ALARM */ > + <7 4>, /* RTC T */ > + <8 7>, /* SDCARD */ > + <9 14>, /* PCI INTA */ > + <10 13>, /* PCI INTB */ > + <11 0>, /* EXT */ > + <12 15>; /* TP */ Looks like 'interrupt level' is just the index of the values? Why not make this an array? Rob
