| Message ID | 1569527977-21213-1-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v2] dt-bindings: irqchip: renesas: intc-irqpin: convert bindings to json-schema | expand |
On Fri, Sep 27, 2019 at 04:59:37AM +0900, Yoshihiro Kaneko wrote: > Convert R-/SH-Mobile IRQPin Controller bindings documentation to json-schema. > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > --- > > v2 > - correct Geert-san's E-mail address. > - delete Guennadi-san from the maintainer of this binding. > - give 'sense-bitfield-width' the uint32 type. > - describe 'control-parent' property as a boolean. > > .../interrupt-controller/renesas,intc-irqpin.txt | 62 ------------- > .../interrupt-controller/renesas,intc-irqpin.yaml | 102 +++++++++++++++++++++ > 2 files changed, 102 insertions(+), 62 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml > diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml > new file mode 100644 > index 0000000..5925890 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml > @@ -0,0 +1,102 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,intc-irqpin.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: DT bindings for the R-/SH-Mobile irqpin controller > + > +maintainers: > + - Geert Uytterhoeven <geert+renesas@glider.be> > + > +properties: > + compatible: > + items: > + - enum: > + - renesas,intc-irqpin-r8a7740 # R-Mobile A1 > + - renesas,intc-irqpin-r8a7778 # R-Car M1A > + - renesas,intc-irqpin-r8a7779 # R-Car H1 > + - renesas,intc-irqpin-sh73a0 # SH-Mobile AG5 > + - const: renesas,intc-irqpin > + > + reg: > + # Base address and length of each register bank used by the external > + # IRQ pins driven by the interrupt controller hardware module. The base > + # addresses, length and number of required register banks varies with > + # soctype. > + minItems: 1 > + maxItems: 6 Every entry is the same thing? > + > + interrupt-controller: true > + # Identifies the node as an interrupt controller. No need to define standard properties. > + > + '#interrupt-cells': > + # an interrupt index and flags, as defined in interrupts.txt in this > + # directory. Same here. > + const: 2 > + > + interrupts: > + # Must contain a list of interrupt specifiers. For each interrupt > + # provided by this irqpin controller instance, there must be one entry, > + # referring to the corresponding parent interrupt. And here. > + maxItems: 1 > + > + sense-bitfield-width: > + # width of a single sense bitfield in the SENSE register, if different > + # from the default 4 bits Use 'description' > + $ref: /schemas/types.yaml#/definitions/uint32 > + maxItems: 1 Update dtschema and run 'make dt_binding_check' and I think you'll find this fails now. The problem is $ref needs to be under an 'allOf' if there's additional schema. maxItems is also wrong here. 'uint32' type implies that. What you should have is a definition of possible values using enum or minimum/maximum. > + > + control-parent: > + # disable and enable interrupts on the parent interrupt controller, > + # needed for some broken implementations Use 'description' > + type: boolean > + > + clocks: > + # Must contain a reference to the functional clock. This property is > + # mandatory if the hardware implements a controllable functional clock for > + # the irqpin controller instance. Drop this. A single entry doesn't need more description. > + maxItems: 1 > + > + power-domains: > + # Must contain a reference to the power domain. This property is > + # mandatory if the irqpin controller instance is part of a controllable > + # power domain. Same here. > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/r8a7740-clock.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + irqpin1: interrupt-controller@e6900004 { > + compatible = "renesas,intc-irqpin-r8a7740", > + "renesas,intc-irqpin"; > + #interrupt-cells = <2>; > + interrupt-controller; > + reg = <0xe6900004 4>, > + <0xe6900014 4>, > + <0xe6900024 1>, > + <0xe6900044 1>, > + <0xe6900064 1>; Really only 1 byte? > + interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>; <> each interrupt specifier. Above you said there is only 1 interrupt... > + clocks = <&mstp2_clks R8A7740_CLK_INTCA>; > + power-domains = <&pd_a4s>; > + }; > -- > 1.9.1 >
Hi Rob, Kaneko-san, On Fri, Oct 11, 2019 at 12:07 AM Rob Herring <robh@kernel.org> wrote: > On Fri, Sep 27, 2019 at 04:59:37AM +0900, Yoshihiro Kaneko wrote: > > Convert R-/SH-Mobile IRQPin Controller bindings documentation to json-schema. > > > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml > > @@ -0,0 +1,102 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,intc-irqpin.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: DT bindings for the R-/SH-Mobile irqpin controller > > + > > +maintainers: > > + - Geert Uytterhoeven <geert+renesas@glider.be> Looks like I received many maintainerships recently ;-) > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - renesas,intc-irqpin-r8a7740 # R-Mobile A1 > > + - renesas,intc-irqpin-r8a7778 # R-Car M1A > > + - renesas,intc-irqpin-r8a7779 # R-Car H1 > > + - renesas,intc-irqpin-sh73a0 # SH-Mobile AG5 > > + - const: renesas,intc-irqpin > > + > > + reg: > > + # Base address and length of each register bank used by the external > > + # IRQ pins driven by the interrupt controller hardware module. The base > > + # addresses, length and number of required register banks varies with > > + # soctype. > > + minItems: 1 minItems: 5 > > + maxItems: 6 > > Every entry is the same thing? No they're not. First entry is the Interrupt control register. Second entry is the Interrupt priority register. Third entry is the Interrupt source register. Fourth entry is the Interrupt mask register. Fifth entry is the Interrupt mask clear register. Sixth entry is the optional Interrupt control register for ICR0 with IRLM bit. The fact that SH/R-Mobile SoCs have 4 instances of this block, with interleaved registers, and different register widths, doesn't help much, and I understand that was the reason for the individual register descriptions. This is a very old binding, which tried to describe everything in DT, using a generic compatible value. Of course this lead to a mess when having different register layouts, optional registers, and a sense-bitfield-width property... The modern way would be to describe all differences in the driver, based on SoC-specific compatible values. Given this is for rather old SoCs, I see no point in upgrading the bindings. > > + irqpin1: interrupt-controller@e6900004 { > > + compatible = "renesas,intc-irqpin-r8a7740", > > + "renesas,intc-irqpin"; > > + #interrupt-cells = <2>; > > + interrupt-controller; > > + reg = <0xe6900004 4>, > > + <0xe6900014 4>, > > + <0xe6900024 1>, > > + <0xe6900044 1>, > > + <0xe6900064 1>; > > Really only 1 byte? Yep. Some registers are 8-bit on some SoCs... > > + interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>; > > <> each interrupt specifier. > > Above you said there is only 1 interrupt... Which is wrong. But the description was correct. Gr{oetje,eeting}s, Geert
On Fri, Oct 11, 2019 at 1:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Rob, Kaneko-san, > > On Fri, Oct 11, 2019 at 12:07 AM Rob Herring <robh@kernel.org> wrote: > > On Fri, Sep 27, 2019 at 04:59:37AM +0900, Yoshihiro Kaneko wrote: > > > Convert R-/SH-Mobile IRQPin Controller bindings documentation to json-schema. > > > > > > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml > > > @@ -0,0 +1,102 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,intc-irqpin.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: DT bindings for the R-/SH-Mobile irqpin controller > > > + > > > +maintainers: > > > + - Geert Uytterhoeven <geert+renesas@glider.be> > > Looks like I received many maintainerships recently ;-) Maybe I should have called it something else, but I view this as who's the owner not who applies it. There are lots of binding files with no maintainer, so it's me by default. I wanted to avoid that and not by having 4K MAINTAINERS entries. > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - enum: > > > + - renesas,intc-irqpin-r8a7740 # R-Mobile A1 > > > + - renesas,intc-irqpin-r8a7778 # R-Car M1A > > > + - renesas,intc-irqpin-r8a7779 # R-Car H1 > > > + - renesas,intc-irqpin-sh73a0 # SH-Mobile AG5 > > > + - const: renesas,intc-irqpin > > > + > > > + reg: > > > + # Base address and length of each register bank used by the external > > > + # IRQ pins driven by the interrupt controller hardware module. The base > > > + # addresses, length and number of required register banks varies with > > > + # soctype. > > > + minItems: 1 > > minItems: 5 > > > > + maxItems: 6 > > > > Every entry is the same thing? > > No they're not. > > First entry is the Interrupt control register. > Second entry is the Interrupt priority register. > Third entry is the Interrupt source register. > Fourth entry is the Interrupt mask register. > Fifth entry is the Interrupt mask clear register. > Sixth entry is the optional Interrupt control register for ICR0 with IRLM bit. So this should be an 'items' list with these descriptions. Rob
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt deleted file mode 100644 index 772c550..0000000 --- a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt +++ /dev/null @@ -1,62 +0,0 @@ -DT bindings for the R-/SH-Mobile irqpin controller - -Required properties: - -- compatible: has to be "renesas,intc-irqpin-<soctype>", "renesas,intc-irqpin" - as fallback. - Examples with soctypes are: - - "renesas,intc-irqpin-r8a7740" (R-Mobile A1) - - "renesas,intc-irqpin-r8a7778" (R-Car M1A) - - "renesas,intc-irqpin-r8a7779" (R-Car H1) - - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5) - -- reg: Base address and length of each register bank used by the external - IRQ pins driven by the interrupt controller hardware module. The base - addresses, length and number of required register banks varies with soctype. -- interrupt-controller: Identifies the node as an interrupt controller. -- #interrupt-cells: has to be <2>: an interrupt index and flags, as defined in - interrupts.txt in this directory. -- interrupts: Must contain a list of interrupt specifiers. For each interrupt - provided by this irqpin controller instance, there must be one entry, - referring to the corresponding parent interrupt. - -Optional properties: - -- any properties, listed in interrupts.txt, and any standard resource allocation - properties -- sense-bitfield-width: width of a single sense bitfield in the SENSE register, - if different from the default 4 bits -- control-parent: disable and enable interrupts on the parent interrupt - controller, needed for some broken implementations -- clocks: Must contain a reference to the functional clock. This property is - mandatory if the hardware implements a controllable functional clock for - the irqpin controller instance. -- power-domains: Must contain a reference to the power domain. This property is - mandatory if the irqpin controller instance is part of a controllable power - domain. - - -Example -------- - - irqpin1: interrupt-controller@e6900004 { - compatible = "renesas,intc-irqpin-r8a7740", - "renesas,intc-irqpin"; - #interrupt-cells = <2>; - interrupt-controller; - reg = <0xe6900004 4>, - <0xe6900014 4>, - <0xe6900024 1>, - <0xe6900044 1>, - <0xe6900064 1>; - interrupts = <0 149 IRQ_TYPE_LEVEL_HIGH - 0 149 IRQ_TYPE_LEVEL_HIGH - 0 149 IRQ_TYPE_LEVEL_HIGH - 0 149 IRQ_TYPE_LEVEL_HIGH - 0 149 IRQ_TYPE_LEVEL_HIGH - 0 149 IRQ_TYPE_LEVEL_HIGH - 0 149 IRQ_TYPE_LEVEL_HIGH - 0 149 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&mstp2_clks R8A7740_CLK_INTCA>; - power-domains = <&pd_a4s>; - }; diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml new file mode 100644 index 0000000..5925890 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml @@ -0,0 +1,102 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/renesas,intc-irqpin.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: DT bindings for the R-/SH-Mobile irqpin controller + +maintainers: + - Geert Uytterhoeven <geert+renesas@glider.be> + +properties: + compatible: + items: + - enum: + - renesas,intc-irqpin-r8a7740 # R-Mobile A1 + - renesas,intc-irqpin-r8a7778 # R-Car M1A + - renesas,intc-irqpin-r8a7779 # R-Car H1 + - renesas,intc-irqpin-sh73a0 # SH-Mobile AG5 + - const: renesas,intc-irqpin + + reg: + # Base address and length of each register bank used by the external + # IRQ pins driven by the interrupt controller hardware module. The base + # addresses, length and number of required register banks varies with + # soctype. + minItems: 1 + maxItems: 6 + + interrupt-controller: true + # Identifies the node as an interrupt controller. + + '#interrupt-cells': + # an interrupt index and flags, as defined in interrupts.txt in this + # directory. + const: 2 + + interrupts: + # Must contain a list of interrupt specifiers. For each interrupt + # provided by this irqpin controller instance, there must be one entry, + # referring to the corresponding parent interrupt. + maxItems: 1 + + sense-bitfield-width: + # width of a single sense bitfield in the SENSE register, if different + # from the default 4 bits + $ref: /schemas/types.yaml#/definitions/uint32 + maxItems: 1 + + control-parent: + # disable and enable interrupts on the parent interrupt controller, + # needed for some broken implementations + type: boolean + + clocks: + # Must contain a reference to the functional clock. This property is + # mandatory if the hardware implements a controllable functional clock for + # the irqpin controller instance. + maxItems: 1 + + power-domains: + # Must contain a reference to the power domain. This property is + # mandatory if the irqpin controller instance is part of a controllable + # power domain. + maxItems: 1 + +required: + - compatible + - reg + - interrupt-controller + - '#interrupt-cells' + - interrupts + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/r8a7740-clock.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + irqpin1: interrupt-controller@e6900004 { + compatible = "renesas,intc-irqpin-r8a7740", + "renesas,intc-irqpin"; + #interrupt-cells = <2>; + interrupt-controller; + reg = <0xe6900004 4>, + <0xe6900014 4>, + <0xe6900024 1>, + <0xe6900044 1>, + <0xe6900064 1>; + interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&mstp2_clks R8A7740_CLK_INTCA>; + power-domains = <&pd_a4s>; + };
Convert R-/SH-Mobile IRQPin Controller bindings documentation to json-schema. Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> --- v2 - correct Geert-san's E-mail address. - delete Guennadi-san from the maintainer of this binding. - give 'sense-bitfield-width' the uint32 type. - describe 'control-parent' property as a boolean. .../interrupt-controller/renesas,intc-irqpin.txt | 62 ------------- .../interrupt-controller/renesas,intc-irqpin.yaml | 102 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 62 deletions(-) delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.yaml