diff mbox series

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

Message ID 1578899321-1365-2-git-send-email-qiangqing.zhang@nxp.com
State Changes Requested, archived
Headers show
Series irqchip: add NXP INTMUX interrupt controller | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema fail build log

Commit Message

Joakim Zhang Jan. 13, 2020, 7:08 a.m. UTC
This patch adds the DT bindings for the NXP INTMUX interrupt multiplexer
for i.MX8 family SoCs.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 .../interrupt-controller/fsl,intmux.yaml      | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml

Comments

Marc Zyngier Jan. 13, 2020, 9:16 a.m. UTC | #1
On 2020-01-13 07:08, 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@nxp.com>
> ---
>  .../interrupt-controller/fsl,intmux.yaml      | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> 
> diff --git
> a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> new file mode 100644
> index 000000000000..4dba532fe0bd
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale INTMUX interrupt multiplexer
> +
> +maintainers:
> +  - Marc Zyngier <maz@kernel.org>

Err... No. I have absolutely no desire to maintain this binding on its 
own.
Feel free to add yourself as the maintainer for this file, as I'm merely
the conduit for updates to irqchip DT bindings.

Thanks,

         M.
Joakim Zhang Jan. 13, 2020, 9:59 a.m. UTC | #2
> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: 2020年1月13日 17:16
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: jason@lakedaemon.net; tglx@linutronix.de; robh+dt@kernel.org;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
> 
> On 2020-01-13 07:08, 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@nxp.com>
> > ---
> >  .../interrupt-controller/fsl,intmux.yaml      | 77 +++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> >
> > 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,intmux.ya
> > ml
> > @@ -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%2Fdevic
> >
> etree.org%2Fschemas%2Finterrupt-controller%2Ffsl%2Cintmux.yaml%23&am
> p;
> >
> data=02%7C01%7Cqiangqing.zhang%40nxp.com%7C3f74ea5d4ee84dfcd49908
> d7980
> >
> 941f2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637145037851
> 545924&
> >
> amp;sdata=4wYFFndzX9ecexaAkGEE3hQL2wuHL17LUrWp84wHyY4%3D&amp;
> reserved=
> > 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%7C3f74ea5d4ee84dfcd49908d7980941f2%7C686ea1d3b
> c2b4c6
> >
> +fa92cd99c5c301635%7C0%7C0%7C637145037851545924&amp;sdata=2NbrC
> AgjDBrp
> > +lzYPulVsY%2FmHgVupsonwwK1gn%2BsdB7o%3D&amp;reserved=0
> > +
> > +title: Freescale INTMUX interrupt multiplexer
> > +
> > +maintainers:
> > +  - Marc Zyngier <maz@kernel.org>
> 
> Err... No. I have absolutely no desire to maintain this binding on its own.
> Feel free to add yourself as the maintainer for this file, as I'm merely the
> conduit for updates to irqchip DT bindings.

Got it! Thanks:-)

Best Regards,
Joakim Zhang
> Thanks,
> 
>          M.
> --
> Jazz is not dead. It just smells funny...
Rob Herring (Arm) Jan. 13, 2020, 9:03 p.m. UTC | #3
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@nxp.com>
> ---
>  .../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

> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> new file mode 100644
> index 000000000000..4dba532fe0bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale INTMUX interrupt multiplexer
> +
> +maintainers:
> +  - Marc Zyngier <maz@kernel.org>
> +
> +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.

Is this any different from the length of 'interrupts' which you can 
count?

> +    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
>
Joakim Zhang Jan. 14, 2020, 2:43 a.m. UTC | #4
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2020年1月14日 5:04
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> 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@nxp.com>
> > ---
> >  .../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@kernel.org>
> > +
> > +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
> >
Joakim Zhang Jan. 14, 2020, 8:22 a.m. UTC | #5
> -----Original Message-----
> From: Joakim Zhang <qiangqing.zhang@nxp.com>
> Sent: 2020年1月14日 10:44
> To: Rob Herring <robh@kernel.org>
> Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
> 
> 
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: 2020年1月14日 5:04
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com;
> > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > 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@nxp.com>
> > > ---
> > >  .../interrupt-controller/fsl,intmux.yaml      | 77
> +++++++++++++++++++
> > >  1 file changed, 77 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > > ml
> >
> > 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,int
> > > +++ mu
> > > +++ 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%2Fde
> > > +vi
> > >
> >
> +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%2Fde
> > > +vi
> > >
> >
> +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@kernel.org>
> > > +
> > > +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>;

One more add, the number of channels is fixed to 8. It will make more clear to users that it supports 8 channels with 8 items for 'interrupts', and users can decide how many
channels they use with 'fsl,intmux_chans' property.

Best Regards,
Joakim Zhang
> 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
> > >
Rob Herring (Arm) Jan. 14, 2020, 2:11 p.m. UTC | #6
On Tue, Jan 14, 2020 at 2:22 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
>
> > -----Original Message-----
> > From: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Sent: 2020年1月14日 10:44
> > To: Rob Herring <robh@kernel.org>
> > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> > INTMUX interrupt multiplexer
> >
> >
> > > -----Original Message-----
> > > From: Rob Herring <robh@kernel.org>
> > > Sent: 2020年1月14日 5:04
> > > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com;
> > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > > 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@nxp.com>
> > > > ---
> > > >  .../interrupt-controller/fsl,intmux.yaml      | 77
> > +++++++++++++++++++
> > > >  1 file changed, 77 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.ya
> > > > ml
> > >
> > > 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,int
> > > > +++ mu
> > > > +++ 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%2Fde
> > > > +vi
> > > >
> > >
> > +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%2Fde
> > > > +vi
> > > >
> > >
> > +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@kernel.org>
> > > > +
> > > > +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>;
>
> One more add, the number of channels is fixed to 8. It will make more clear to users that it supports 8 channels with 8 items for 'interrupts', and users can decide how many
> channels they use with 'fsl,intmux_chans' property.

How does one decide how many? Why would you not use as many channels
as possible (other than muxing interrupts or not doesn't really make a
bit difference in servicing overhead)?

If you wanted to configure how many parent interrupts, wouldn't you
also want to configure the routing of child interrupts to specific
parent interrupts?

So I would drop this property. You can define both how many parents
and the routing with interrupt-map property, though I would not do
that until you have a real need.

Rob
Joakim Zhang Jan. 15, 2020, 2:27 a.m. UTC | #7
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2020年1月14日 22:12
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; dl-linux-imx
> <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> Subject: Re: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for NXP
> INTMUX interrupt multiplexer
> 
> On Tue, Jan 14, 2020 at 2:22 AM Joakim Zhang <qiangqing.zhang@nxp.com>
> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > Sent: 2020年1月14日 10:44
> > > To: Rob Herring <robh@kernel.org>
> > > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > > mark.rutland@arm.com; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com;
> > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > > Subject: RE: [PATCH V4 RESEND 1/2] dt-bindings/irq: add binding for
> > > NXP INTMUX interrupt multiplexer
> > >
> > >
> > > > -----Original Message-----
> > > > From: Rob Herring <robh@kernel.org>
> > > > Sent: 2020年1月14日 5:04
> > > > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > > Cc: maz@kernel.org; jason@lakedaemon.net; tglx@linutronix.de;
> > > > mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de;
> > > > kernel@pengutronix.de; festevam@gmail.com;
> > > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > > > linux-arm-kernel@lists.infradead.org; dl-linux-imx
> > > > <linux-imx@nxp.com>; Andy Duan <fugang.duan@nxp.com>
> > > > 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@nxp.com>
> > > > > ---
> > > > >  .../interrupt-controller/fsl,intmux.yaml      | 77
> > > +++++++++++++++++++
> > > > >  1 file changed, 77 insertions(+)  create mode 100644
> > > > > Documentation/devicetree/bindings/interrupt-controller/fsl,intmu
> > > > > x.ya
> > > > > ml
> > > >
> > > > 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
> > > > > +++ ,int
> > > > > +++ mu
> > > > > +++ 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%
> > > > > +2Fde
> > > > > +vi
> > > > >
> > > >
> > >
> +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%
> > > > > +2Fde
> > > > > +vi
> > > > >
> > > >
> > >
> +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@kernel.org>
> > > > > +
> > > > > +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>;
> >
> > One more add, the number of channels is fixed to 8. It will make more
> > clear to users that it supports 8 channels with 8 items for 'interrupts', and
> users can decide how many channels they use with 'fsl,intmux_chans' property.
> 
> How does one decide how many? Why would you not use as many channels as
> possible (other than muxing interrupts or not doesn't really make a bit
> difference in servicing overhead)?
> 
> If you wanted to configure how many parent interrupts, wouldn't you also want
> to configure the routing of child interrupts to specific parent interrupts?
> 
> So I would drop this property. You can define both how many parents and the
> routing with interrupt-map property, though I would not do that until you have
> a real need.

Thanks Rob. Agree. I will update both bindings and driver.

Best Regards,
Joakim Zhang
> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
new file mode 100644
index 000000000000..4dba532fe0bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,intmux.yaml
@@ -0,0 +1,77 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale INTMUX interrupt multiplexer
+
+maintainers:
+  - Marc Zyngier <maz@kernel.org>
+
+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:
+    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 {
+		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>;
+	};
+