RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP INTMUX interrupt multiplexer

[Joakim Zhang]

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: 2020年1月14日 5:04
> To: Joakim Zhang <qiangqing.zhang@xxxxxxx>
> Cc: maz@xxxxxxxxxx; jason@xxxxxxxxxxxxxx; tglx@xxxxxxxxxxxxx;
> mark.rutland@xxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
> kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>; Andy Duan <fugang.duan@xxxxxxx>
> Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
> 
> On Mon, Jan 13, 2020 at 03:08:40PM +0800, Joakim Zhang wrote:
> > This patch adds the DT bindings for the NXP INTMUX interrupt
> > multiplexer for i.MX8 family SoCs.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@xxxxxxx>
> > ---
> >  .../interrupt-controller/fsl,intmux.yaml      | 77 +++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> 
> Please run 'make dt_binding_check' and fix the errors:
> 
> Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml:
> while scanning for the next token found character that cannot start any token
>   in "<unicode string>", line 60, column 1
Got it. Will keep in mind. Thanks.

> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > ml
> > b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > ml
> > new file mode 100644
> > index 000000000000..4dba532fe0bd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmu
> > +++ x.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cintmux.yaml%23&a
> m
> >
> +p;data=02%7C01%7Cqiangqing.zhang%40nxp.com%7Cdc2443dc111149805c7
> 208d7
> >
> +986c157f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63714546
> 2291934
> >
> +492&amp;sdata=Ao4iuj2D48KAeC%2FvQvJqUUxGJEjSY0HyL5ZlT2XrSrg%3D&
> amp;re
> > +served=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cqia
> ngqing
> >
> +.zhang%40nxp.com%7Cdc2443dc111149805c7208d7986c157f%7C686ea1d3b
> c2b4c6
> >
> +fa92cd99c5c301635%7C0%7C0%7C637145462291934492&amp;sdata=YoHb
> TO5C8Nlq
> > +YYoWTNufaIxnvdtPUZaKzvwK49I9Zdc%3D&amp;reserved=0
> > +
> > +title: Freescale INTMUX interrupt multiplexer
> > +
> > +maintainers:
> > +  - Marc Zyngier <maz@xxxxxxxxxx>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      const: fsl,imx-intmux
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 8
> > +    description: |
> > +      Should contain the parent interrupt lines (up to 8) used to multiplex
> > +      the input interrupts.
> > +
> > +  interrupt-controller: true
> > +
> > +  '#interrupt-cells':
> > +    const: 2
> > +
> > +  clocks:
> > +    maxItems: 1
> > +    description: ipg clock.
> > +
> > +  clock-names:
> > +    items:
> > +      const: ipg
> > +
> > +  fsl,intmux_chans:
> 
> Don't use '_' in property names.
Got it.

> Is this any different from the length of 'interrupts' which you can count?
A bit different. Such as, the length of 'interrupts' is 8, but we can set fsl,intmux_chans value is 4. That means there are 8 channels, but actually we only use 4 channels.
If you think this make no sense, due to we can assign 4 items for 'interrupts' to get the same result. So we can count the length of 'interrupts' to get the channels configured, then this property is no need.
Which one do you think is better? 
		interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
		fsl,intmux_chans = <4>;

Best Regards,
Joakim Zhang
> > +    maxItems: 1
> > +    description: |
> > +      The number of channels used for interrupt source. The Maximum
> value is 8.
> > +      If this property is not set in DT then driver uses 1 channel by default.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-controller
> > +  - '#interrupt-cells'
> > +  - clocks
> > +  - clock-name
> > +  - fsl,intmux_chans
> > +
> > +additionalProperties: false
> > +
> > +Example:
> > +
> > +	intmux@37400000 {
> 
> interrupt-controller@...
> 
> > +		compatible = "fsl,imx-intmux";
> > +		reg = <0x37400000 0x1000>;
> > +		interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> > +			     <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <2>;
> > +		clocks = <&clk IMX8QM_CM40_IPG_CLK>;
> > +		clock-names = "ipg";
> > +		fsl,intmux_chans = <8>;
> > +	};
> > +
> > --
> > 2.17.1
> >