RE: [PATCH V2] dt-bindings: interrupt-controller: Convert imx irqsteer to json-schema

[Aisheng Dong]

> From: Anson Huang <Anson.Huang@xxxxxxx>
> Sent: Monday, May 18, 2020 2:12 PM
> 
> Convert the i.MX IRQSTEER binding to DT schema format using json-schema.
> 
> Signed-off-by: Anson Huang <Anson.Huang@xxxxxxx>

A few minor comments, otherwise:
Reviewed-by: Dong Aisheng <aisheng.dong@xxxxxxx>

BTW, we probably better add ref to the common interrupt-controller.yaml to inherit
the common constraints. E.g. nodename.
But I noted some exist binding also did not include it, so maybe we can wait for Rob's
comments.

Other minor comments inline.

> ---
> Changes since V1:
> 	- Add "fsl,imx8m-irqsteer" compatible back.
> 	- Use "Multiplexer" instead of "multiplexer" for title.
> ---
>  .../bindings/interrupt-controller/fsl,irqsteer.txt | 35 ---------
>  .../interrupt-controller/fsl,irqsteer.yaml         | 89
> ++++++++++++++++++++++
>  2 files changed, 89 insertions(+), 35 deletions(-)  delete mode 100644
> Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt
>  create mode 100644
> Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml
> 
> diff --git
> a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt
> b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt
> deleted file mode 100644
> index 582991c..0000000
> --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.txt
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -Freescale IRQSTEER Interrupt multiplexer
> -
> -Required properties:
> -
> -- compatible: should be:
> -	- "fsl,imx8m-irqsteer"
> -	- "fsl,imx-irqsteer"
> -- reg: Physical base address and size of registers.
> -- interrupts: Should contain the up to 8 parent interrupt lines used to
> -  multiplex the input interrupts. They should be specified sequentially
> -  from output 0 to 7.
> -- clocks: Should contain one clock for entry in clock-names
> -  see Documentation/devicetree/bindings/clock/clock-bindings.txt
> -- clock-names:
> -   - "ipg": main logic clock
> -- 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 1.
> -- fsl,channel: The output channel that all input IRQs should be steered into.
> -- fsl,num-irqs: Number of input interrupts of this channel.
> -  Should be multiple of 32 input interrupts and up to 512 interrupts.
> -
> -Example:
> -
> -	interrupt-controller@32e2d000 {
> -		compatible = "fsl,imx8m-irqsteer", "fsl,imx-irqsteer";
> -		reg = <0x32e2d000 0x1000>;
> -		interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> -		clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>;
> -		clock-names = "ipg";
> -		fsl,channel = <0>;
> -		fsl,num-irqs = <64>;
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
> -	};
> diff --git
> a/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml
> b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqsteer.yaml
> new file mode 100644
> index 0000000..5242c97
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,irqstee
> +++ r.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---

[...]

> +title: Freescale IRQSTEER Interrupt Multiplexer
> +
> +maintainers:
> +  - Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx8m-irqsteer
> +      - fsl,imx-irqsteer
> +

Original compatible string definition seems not accurate.
It does not cover the case of using both together.
Anyway, that's exist issue, you can submit an extra patch to fix it later.

> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description: |
> +      should contain the up to 8 parent interrupt lines used to multiplex
> +      the input interrupts. They should be specified sequentially from
> +      output 0 to 7.
> +    items:
> +      - description: irqsteer channel 0
> +      - description: irqsteer channel 1
> +      - description: irqsteer channel 2
> +      - description: irqsteer channel 3
> +      - description: irqsteer channel 4
> +      - description: irqsteer channel 5
> +      - description: irqsteer channel 6
> +      - description: irqsteer channel 7

'channel' is confusing here, it's actually output interrupt 0-7 for one channel.

s/irqsteer channel/output interrupt x

Regards
Aisheng

> +    minItems: 1
> +    maxItems: 8
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: ipg
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  fsl,channel:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'
> +    description: |
> +      u32 value representing the output channel that all input IRQs should be
> +      steered into.
> +
> +  fsl,num-irqs:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'
> +    description: |
> +      u32 value representing the number of input interrupts of this channel,
> +      should be multiple of 32 input interrupts and up to 512 interrupts.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - fsl,channel
> +  - fsl,num-irqs
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8mq-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    interrupt-controller@32e2d000 {
> +        compatible = "fsl,imx-irqsteer";
> +        reg = <0x32e2d000 0x1000>;
> +        interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>;
> +        clock-names = "ipg";
> +        fsl,channel = <0>;
> +        fsl,num-irqs = <64>;
> +        interrupt-controller;
> +        #interrupt-cells = <1>;
> +    };
> --
> 2.7.4