Re: [PATCH v3] dt-bindings: interrupt-controller: Convert Amlogic Meson GPIO interrupt controller binding

[Rob Herring <robh@kernel.org>]

On Mon, Jan 30, 2023 at 09:53:21PM +0100, Heiner Kallweit wrote:
> Convert Amlogic Meson GPIO interrupt controller binding to yaml.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - consider that more than one compatible may be set
> - remove bus part from example
> v3:
> - remove minItem/maxItem properties for compatible
> ---
>  .../amlogic,meson-gpio-intc.txt               | 38 ---------
>  .../amlogic,meson-gpio-intc.yaml              | 84 +++++++++++++++++++
>  2 files changed, 84 insertions(+), 38 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt
> deleted file mode 100644
> index bde63f8f0..000000000
> --- a/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.txt
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -Amlogic meson GPIO interrupt controller
> -
> -Meson SoCs contains an interrupt controller which is able to watch the SoC
> -pads and generate an interrupt on edge or level. The controller is essentially
> -a 256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge
> -or level and polarity. It does not expose all 256 mux inputs because the
> -documentation shows that the upper part is not mapped to any pad. The actual
> -number of interrupt exposed depends on the SoC.
> -
> -Required properties:
> -
> -- compatible : must have "amlogic,meson8-gpio-intc" and either
> -    "amlogic,meson8-gpio-intc" for meson8 SoCs (S802) or
> -    "amlogic,meson8b-gpio-intc" for meson8b SoCs (S805) or
> -    "amlogic,meson-gxbb-gpio-intc" for GXBB SoCs (S905) or
> -    "amlogic,meson-gxl-gpio-intc" for GXL SoCs (S905X, S912)
> -    "amlogic,meson-axg-gpio-intc" for AXG SoCs (A113D, A113X)
> -    "amlogic,meson-g12a-gpio-intc" for G12A SoCs (S905D2, S905X2, S905Y2)
> -    "amlogic,meson-sm1-gpio-intc" for SM1 SoCs (S905D3, S905X3, S905Y3)
> -    "amlogic,meson-a1-gpio-intc" for A1 SoCs (A113L)
> -    "amlogic,meson-s4-gpio-intc" for S4 SoCs (S802X2, S905Y4, S805X2G, S905W2)
> -- reg : Specifies base physical address and size of the registers.
> -- interrupt-controller : Identifies the node as an interrupt controller.
> -- #interrupt-cells : Specifies the number of cells needed to encode an
> -   interrupt source. The value must be 2.
> -- meson,channel-interrupts: Array with the 8 upstream hwirq numbers. These
> -   are the hwirqs used on the parent interrupt controller.
> -
> -Example:
> -
> -gpio_interrupt: interrupt-controller@9880 {
> -	compatible = "amlogic,meson-gxbb-gpio-intc",
> -		     "amlogic,meson-gpio-intc";
> -	reg = <0x0 0x9880 0x0 0x10>;
> -	interrupt-controller;
> -	#interrupt-cells = <2>;
> -	meson,channel-interrupts = <64 65 66 67 68 69 70 71>;
> -};
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.yaml
> new file mode 100644
> index 000000000..da55a2d25
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/amlogic,meson-gpio-intc.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/amlogic,meson-gpio-intc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic Meson GPIO interrupt controller
> +
> +maintainers:
> +  - Heiner Kallweit <hkallweit1@gmail.com>
> +
> +description: |
> +  Meson SoCs contains an interrupt controller which is able to watch the SoC
> +  pads and generate an interrupt on edge or level. The controller is essentially
> +  a 256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge
> +  or level and polarity. It does not expose all 256 mux inputs because the
> +  documentation shows that the upper part is not mapped to any pad. The actual
> +  number of interrupt exposed depends on the SoC.
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - amlogic,meson-gpio-intc
> +              - amlogic,meson8-gpio-intc
> +              - amlogic,meson8b-gpio-intc
> +              - amlogic,meson-gxbb-gpio-intc
> +              - amlogic,meson-gxl-gpio-intc
> +              - amlogic,meson-axg-gpio-intc
> +              - amlogic,meson-g12a-gpio-intc
> +              - amlogic,meson-sm1-gpio-intc
> +              - amlogic,meson-a1-gpio-intc
> +              - amlogic,meson-s4-gpio-intc
> +      - items:
> +          - enum:
> +              - amlogic,meson8-gpio-intc

We allow this with or without a fallback? Again, that should not be 
allowed and any dts files fixed.

> +              - amlogic,meson-axg-gpio-intc
> +              - amlogic,meson-g12a-gpio-intc
> +              - amlogic,meson-sm1-gpio-intc
> +              - amlogic,meson-s4-gpio-intc
> +          - const: amlogic,meson-gpio-intc
> +      - items:
> +          - const: amlogic,meson-gpio-intc
> +          - enum:
> +              - amlogic,meson8b-gpio-intc
> +              - amlogic,meson-gxbb-gpio-intc
> +              - amlogic,meson-gxl-gpio-intc

This looks wrong. The order is most specific to least specific 
compatible. amlogic,meson-gpio-intc is more specific (or a superset of) 
than amlogic,meson8b-gpio-intc? 

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  amlogic,channel-interrupts:

This was 'meson', not 'amlogic'. You can't just change things.

Really, 'interrupts' should have been used here, but oh well.

> +    description: Array with the upstream hwirq numbers
> +    minItems: 8
> +    maxItems: 12
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - amlogic,channel-interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    interrupt-controller@9880 {
> +      compatible = "amlogic,meson-gxbb-gpio-intc";
> +      reg = <0x9880 0x10>;
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +      amlogic,channel-interrupts = <64 65 66 67 68 69 70 71>;
> +    };
> -- 
> 2.39.1
>