On Tue, Dec 05, 2023 at 06:45:38PM +0900, Yoshinori Sato wrote: > Renesas SH7751 external interrupt encoder json-schema. > > Signed-off-by: Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx> > --- > .../renesas,sh7751-irl-ext.yaml | 83 +++++++++++++++++++ > 1 file changed, 83 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..ba4fe2e4d749 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > @@ -0,0 +1,83 @@ > +# 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 IRL external encoder with enable regs. IRL? In Real Life? > + > +maintainers: > + - Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx> > + > +description: | Don't need '|' if no formatting to preserve. > + This is the generally used external interrupt encoder on SH7751 based boards. > + > +properties: > + compatible: > + items: > + - const: renesas,sh7751-irl-ext > + > + reg: > + minItems: 1 maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 1 > + > + '#address-cells': > + const: 0 > + > + '#size-cells': > + const: 0 Don't need #size-cells. > + > + renesas,width: > + description: Enable register width > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [8, 16, 32] reg-io-width is the standard property for this purpose. > + > + renesas,set-to-disable: > + $ref: /schemas/types.yaml#/definitions/flag > + description: Setting this flag to 1 disables it. You can't set a boolean to 1. What is 'it' here? > + > + renesas,enable-bit: > + $ref: /schemas/types.yaml#/definitions/uint32-array You've described a 2 entry matrix, not an array. > + description: | > + IRL enable register bit mapping > + 1st word IRL > + 2nd word bit index of enable register Needs a better description of what this is for. If it is per SoC then it should be implied from the compatible string. > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - renesas,width > + - renesas,enable-bit > + > +additionalProperties: false > + > +examples: > + - | > + r2dintc: sh7751irl_encoder@a4000000 { interrupt-controller@... Drop unused labels. > + compatible = "renesas,sh7751-irl-ext"; > + reg = <0xa4000000 0x02>; > + interrupt-controller; > + #address-cells = <0>; > + #size-cells = <0>; > + #interrupt-cells = <1>; > + renesas,width = <16>; > + 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 the first value is just the index of the entry, so drop it and use the index. But better yet, these are all per interrupt values. Put them into the interrupt cell values instead. For example the RTC would have something like: interrupts = <6 5>, <7 4>; Though I do wonder if you really need the first value, or that was just interrupt numbers you made up and then created this mapping? Rob
