devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] irqchip: Add support for Renesas RZ/N1 GPIO interrupt multiplexer
@ 2018-11-13 13:09 Phil Edworthy
  2018-11-13 13:09 ` [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding Phil Edworthy
  0 siblings, 1 reply; 6+ messages in thread
From: Phil Edworthy @ 2018-11-13 13:09 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Jason Cooper, Rob Herring,
	Mark Rutland
  Cc: Geert Uytterhoeven, linux-renesas-soc, linux-kernel,
	Phil Edworthy, devicetree

On RZ/N1 devices, there are lots of GPIO interrupts that are multiplexed before
getting to the GIC interrupt controller. Other than the multiplexing, there is
no other logic applied to the signals.

The multiplexing cannot be handled dynamically because there is another CPU that
runs firmware. It's likely that the firmware will use some of these GPIO
interrupts and so we don't want them to move around.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v3:
 - Use 'interrupt-map' DT property to map the interrupts, this is very similar
   to PCIe MSI. The only difference is that we need to get hold of the interrupt
   specifier for the interupts coming into the irqmux.
   I had completely messed up the use of 'interrupt-map' in v2... oops.
 - Do not use a chained interrupt controller.
v2:
 - Split DT bindings into separate patch.
 - Use interrupt-map to allow the GPIO controller info to be specified
   as part of the irq.
 - Don't show status in binding examples.
 - Don't show the soc/board split in binding doc.
 - Renamed struct and funcs from 'girq' to a more comprehenisble 'irqmux'.

Phil Edworthy (2):
  dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding
  irqchip: Add support for Renesas RZ/N1 GPIO interrupt multiplexer

 .../interrupt-controller/renesas,rzn1-mux.txt |  73 +++++++
 drivers/irqchip/Kconfig                       |   9 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/rzn1-irq-mux.c                | 205 ++++++++++++++++++
 4 files changed, 288 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt
 create mode 100644 drivers/irqchip/rzn1-irq-mux.c

-- 
2.17.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding
  2018-11-13 13:09 [PATCH v3 0/2] irqchip: Add support for Renesas RZ/N1 GPIO interrupt multiplexer Phil Edworthy
@ 2018-11-13 13:09 ` Phil Edworthy
  2018-11-17 14:32   ` Rob Herring
  0 siblings, 1 reply; 6+ messages in thread
From: Phil Edworthy @ 2018-11-13 13:09 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Jason Cooper, Rob Herring,
	Mark Rutland
  Cc: Geert Uytterhoeven, linux-renesas-soc, linux-kernel,
	Phil Edworthy, devicetree

Add device binding documentation for the Renesas RZ/N1 GPIO interrupt
multiplexer.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v3:
 - Use 'interrupt-map' DT property correctly.
v2:
 - Use interrupt-map to allow the GPIO controller info to be specified
   as part of the irq.
 - Don't show status in binding examples.
 - Don't show the soc/board split in binding doc.
---
 .../interrupt-controller/renesas,rzn1-mux.txt | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt
new file mode 100644
index 000000000000..6515880e25cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt
@@ -0,0 +1,73 @@
+* Renesas RZ/N1 GPIO Interrupt Multiplexer
+
+On Renesas RZ/N1 devices, there are several GPIO Controllers each with a number
+of interrupt outputs. All of the interrupts from the GPIO Controllers are passed
+to the GPIO Interrupt Multiplexer, which selects a sub-set of the interrupts to
+pass onto the system interrupt controller.
+
+A single node in the device tree is used to describe the GPIO IRQ Muxer.
+
+Required properties:
+- compatible: SoC-specific compatible string "renesas,<soc-specific>-gpioirqmux"
+  followed by "renesas,rzn1-gpioirqmux" as fallback. The SoC-specific compatible
+  strings must be one of:
+	"renesas,r9a06g032-gpioirqmux" for RZ/N1D
+	"renesas,r9a06g033-gpioirqmux" for RZ/N1S
+- reg: Base address and size of GPIO IRQ Muxer registers.
+- interrupts: List of output interrupts.
+- #interrupt-cells: Numder of cells in the input interrupt specifier, must be 1.
+- #address-cells: Must be 0.
+- interrupt-map-mask: must be 127.
+- interrupt-map: Standard property detailing the maps between input irqs and the
+  corresponding output irq. This consist of a list of:
+	<input-irq-spec phandle-to-interrupt-controller output-irq-spec>
+  The input-irq-spec is from 0 to 95, corresponding to the outputs of the GPIO
+  Controllers.
+
+Example:
+
+	The following is an example for the RZ/N1D SoC.
+
+	gpioirqmux: gpioirqmux@51000480 {
+		compatible = "renesas,r9a06g032-gpioirqmux",
+				"renesas,rzn1-gpioirqmux";
+		reg = <0x51000480 0x20>;
+		interrupts =
+			<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+			<GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+
+		#interrupt-cells = <1>;
+		#address-cells = <0>;
+		interrupt-map-mask = <127>;
+		interrupt-map =
+			/* gpio2a 24, pin 146: ETH Port 1 IRQ */
+			<88 &gic GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+			/* gpio2a 26, pin 148: TouchSCRN_IRQ */
+			<90 &gic GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	gpio2: gpio@5000d000 {
+		compatible = "snps,dw-apb-gpio";
+		reg = <0x5000d000 0x80>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clock-names = "bus";
+		clocks = <&sysctrl R9A06G032_HCLK_GPIO2>;
+
+		gpio2a: gpio-controller@0 {
+			compatible = "snps,dw-apb-gpio-port";
+			bank-name = "gpio2a";
+			gpio-controller;
+			#gpio-cells = <2>;
+			snps,nr-gpios = <32>;
+			reg = <0>;
+
+			interrupt-controller;
+			interrupt-parent = <&gpioirqmux>;
+			interrupts =  < 64 65 66 67 68 69 70 71
+					72 73 74 75 76 77 78 79
+					80 81 82 83 84 85 86 87
+					88 89 90 91 92 93 94 95 >;
+			#interrupt-cells = <2>;
+		};
+	};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding
  2018-11-13 13:09 ` [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding Phil Edworthy
@ 2018-11-17 14:32   ` Rob Herring
  2018-11-19  9:39     ` Phil Edworthy
  0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2018-11-17 14:32 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Marc Zyngier, Thomas Gleixner, Jason Cooper, Mark Rutland,
	Geert Uytterhoeven, linux-renesas-soc, linux-kernel, devicetree

On Tue, Nov 13, 2018 at 01:09:09PM +0000, Phil Edworthy wrote:
> Add device binding documentation for the Renesas RZ/N1 GPIO interrupt
> multiplexer.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> v3:
>  - Use 'interrupt-map' DT property correctly.
> v2:
>  - Use interrupt-map to allow the GPIO controller info to be specified
>    as part of the irq.
>  - Don't show status in binding examples.
>  - Don't show the soc/board split in binding doc.
> ---
>  .../interrupt-controller/renesas,rzn1-mux.txt | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt

A few nits, otherwise:

Reviewed-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt
> new file mode 100644
> index 000000000000..6515880e25cc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt
> @@ -0,0 +1,73 @@
> +* Renesas RZ/N1 GPIO Interrupt Multiplexer
> +
> +On Renesas RZ/N1 devices, there are several GPIO Controllers each with a number
> +of interrupt outputs. All of the interrupts from the GPIO Controllers are passed
> +to the GPIO Interrupt Multiplexer, which selects a sub-set of the interrupts to
> +pass onto the system interrupt controller.
> +
> +A single node in the device tree is used to describe the GPIO IRQ Muxer.
> +
> +Required properties:
> +- compatible: SoC-specific compatible string "renesas,<soc-specific>-gpioirqmux"
> +  followed by "renesas,rzn1-gpioirqmux" as fallback. The SoC-specific compatible
> +  strings must be one of:
> +	"renesas,r9a06g032-gpioirqmux" for RZ/N1D
> +	"renesas,r9a06g033-gpioirqmux" for RZ/N1S
> +- reg: Base address and size of GPIO IRQ Muxer registers.
> +- interrupts: List of output interrupts.
> +- #interrupt-cells: Numder of cells in the input interrupt specifier, must be 1.
> +- #address-cells: Must be 0.
> +- interrupt-map-mask: must be 127.
> +- interrupt-map: Standard property detailing the maps between input irqs and the
> +  corresponding output irq. This consist of a list of:
> +	<input-irq-spec phandle-to-interrupt-controller output-irq-spec>
> +  The input-irq-spec is from 0 to 95, corresponding to the outputs of the GPIO
> +  Controllers.
> +
> +Example:
> +
> +	The following is an example for the RZ/N1D SoC.
> +
> +	gpioirqmux: gpioirqmux@51000480 {

interrupt-controller@...

> +		compatible = "renesas,r9a06g032-gpioirqmux",
> +				"renesas,rzn1-gpioirqmux";
> +		reg = <0x51000480 0x20>;
> +		interrupts =
> +			<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;

This is a bit redundant as the same information is in interrupt-map, but 
I guess you need it to get the irq resources.

> +
> +		#interrupt-cells = <1>;
> +		#address-cells = <0>;
> +		interrupt-map-mask = <127>;

Use hex for masks.

> +		interrupt-map =
> +			/* gpio2a 24, pin 146: ETH Port 1 IRQ */
> +			<88 &gic GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> +			/* gpio2a 26, pin 148: TouchSCRN_IRQ */
> +			<90 &gic GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> +	};
> +
> +	gpio2: gpio@5000d000 {
> +		compatible = "snps,dw-apb-gpio";
> +		reg = <0x5000d000 0x80>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clock-names = "bus";
> +		clocks = <&sysctrl R9A06G032_HCLK_GPIO2>;
> +
> +		gpio2a: gpio-controller@0 {

gpio@0

> +			compatible = "snps,dw-apb-gpio-port";
> +			bank-name = "gpio2a";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			snps,nr-gpios = <32>;
> +			reg = <0>;
> +
> +			interrupt-controller;
> +			interrupt-parent = <&gpioirqmux>;
> +			interrupts =  < 64 65 66 67 68 69 70 71
> +					72 73 74 75 76 77 78 79
> +					80 81 82 83 84 85 86 87
> +					88 89 90 91 92 93 94 95 >;
> +			#interrupt-cells = <2>;
> +		};
> +	};
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding
  2018-11-17 14:32   ` Rob Herring
@ 2018-11-19  9:39     ` Phil Edworthy
  2018-11-19 21:50       ` Rob Herring
  0 siblings, 1 reply; 6+ messages in thread
From: Phil Edworthy @ 2018-11-19  9:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marc Zyngier, Thomas Gleixner, Jason Cooper, Mark Rutland,
	Geert Uytterhoeven, linux-renesas-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org

Hi Rob,

On 17 November 2018 14:33 Rob Herring wrote:
> On Tue, Nov 13, 2018 at 01:09:09PM +0000, Phil Edworthy wrote:
> > Add device binding documentation for the Renesas RZ/N1 GPIO interrupt
> > multiplexer.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> > v3:
> >  - Use 'interrupt-map' DT property correctly.
> > v2:
> >  - Use interrupt-map to allow the GPIO controller info to be specified
> >    as part of the irq.
> >  - Don't show status in binding examples.
> >  - Don't show the soc/board split in binding doc.
> > ---
> >  .../interrupt-controller/renesas,rzn1-mux.txt | 73
> > +++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mu
> > x.txt
> 
> A few nits, otherwise:
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
Thanks for the review!

> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-
> > mux.txt
> > b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-
> > mux.txt
> > new file mode 100644
> > index 000000000000..6515880e25cc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,r
> > +++ zn1-mux.txt
> > @@ -0,0 +1,73 @@
> > +* Renesas RZ/N1 GPIO Interrupt Multiplexer
> > +
> > +On Renesas RZ/N1 devices, there are several GPIO Controllers each
> > +with a number of interrupt outputs. All of the interrupts from the
> > +GPIO Controllers are passed to the GPIO Interrupt Multiplexer, which
> > +selects a sub-set of the interrupts to pass onto the system interrupt
> controller.
> > +
> > +A single node in the device tree is used to describe the GPIO IRQ Muxer.
> > +
> > +Required properties:
> > +- compatible: SoC-specific compatible string "renesas,<soc-specific>-
> gpioirqmux"
> > +  followed by "renesas,rzn1-gpioirqmux" as fallback. The SoC-specific
> > +compatible
> > +  strings must be one of:
> > +	"renesas,r9a06g032-gpioirqmux" for RZ/N1D
> > +	"renesas,r9a06g033-gpioirqmux" for RZ/N1S
> > +- reg: Base address and size of GPIO IRQ Muxer registers.
> > +- interrupts: List of output interrupts.
> > +- #interrupt-cells: Numder of cells in the input interrupt specifier, must be
> 1.
> > +- #address-cells: Must be 0.
> > +- interrupt-map-mask: must be 127.
> > +- interrupt-map: Standard property detailing the maps between input
> > +irqs and the
> > +  corresponding output irq. This consist of a list of:
> > +	<input-irq-spec phandle-to-interrupt-controller output-irq-spec>
> > +  The input-irq-spec is from 0 to 95, corresponding to the outputs of
> > +the GPIO
> > +  Controllers.
> > +
> > +Example:
> > +
> > +	The following is an example for the RZ/N1D SoC.
> > +
> > +	gpioirqmux: gpioirqmux@51000480 {
> 
> interrupt-controller@...
Sure

> > +		compatible = "renesas,r9a06g032-gpioirqmux",
> > +				"renesas,rzn1-gpioirqmux";
> > +		reg = <0x51000480 0x20>;
> > +		interrupts =
> > +			<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> > +			<GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> 
> This is a bit redundant as the same information is in interrupt-map, but I
> guess you need it to get the irq resources.
That's right.

> > +
> > +		#interrupt-cells = <1>;
> > +		#address-cells = <0>;
> > +		interrupt-map-mask = <127>;
> 
> Use hex for masks.
Ok.

> > +		interrupt-map =
> > +			/* gpio2a 24, pin 146: ETH Port 1 IRQ */
> > +			<88 &gic GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> > +			/* gpio2a 26, pin 148: TouchSCRN_IRQ */
> > +			<90 &gic GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > +	};
> > +
> > +	gpio2: gpio@5000d000 {
> > +		compatible = "snps,dw-apb-gpio";
> > +		reg = <0x5000d000 0x80>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		clock-names = "bus";
> > +		clocks = <&sysctrl R9A06G032_HCLK_GPIO2>;
> > +
> > +		gpio2a: gpio-controller@0 {
> 
> gpio@0
Are you sure about this?
The bindings in Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
show an example where the sub-nodes for gpio banks are gpio-controller@.
This is also in Documentation/devicetree/bindings/gpio/gpio.txt.

Thanks
Phil

> > +			compatible = "snps,dw-apb-gpio-port";
> > +			bank-name = "gpio2a";
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +			snps,nr-gpios = <32>;
> > +			reg = <0>;
> > +
> > +			interrupt-controller;
> > +			interrupt-parent = <&gpioirqmux>;
> > +			interrupts =  < 64 65 66 67 68 69 70 71
> > +					72 73 74 75 76 77 78 79
> > +					80 81 82 83 84 85 86 87
> > +					88 89 90 91 92 93 94 95 >;
> > +			#interrupt-cells = <2>;
> > +		};
> > +	};
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding
  2018-11-19  9:39     ` Phil Edworthy
@ 2018-11-19 21:50       ` Rob Herring
  2018-11-20 10:30         ` Phil Edworthy
  0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2018-11-19 21:50 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Marc Zyngier, Thomas Gleixner, Jason Cooper, Mark Rutland,
	Geert Uytterhoeven, open list:MEDIA DRIVERS FOR RENESAS - FCP,
	linux-kernel@vger.kernel.org, devicetree

On Mon, Nov 19, 2018 at 3:39 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
>
> Hi Rob,
>
> On 17 November 2018 14:33 Rob Herring wrote:
> > On Tue, Nov 13, 2018 at 01:09:09PM +0000, Phil Edworthy wrote:
> > > Add device binding documentation for the Renesas RZ/N1 GPIO interrupt
> > > multiplexer.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > ---
> > > v3:
> > >  - Use 'interrupt-map' DT property correctly.
> > > v2:
> > >  - Use interrupt-map to allow the GPIO controller info to be specified
> > >    as part of the irq.
> > >  - Don't show status in binding examples.
> > >  - Don't show the soc/board split in binding doc.
> > > ---
> > >  .../interrupt-controller/renesas,rzn1-mux.txt | 73
> > > +++++++++++++++++++
> > >  1 file changed, 73 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mu
> > > x.txt
> >
> > A few nits, otherwise:
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> Thanks for the review!
>
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-
> > > mux.txt
> > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-
> > > mux.txt
> > > new file mode 100644
> > > index 000000000000..6515880e25cc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,r
> > > +++ zn1-mux.txt
> > > @@ -0,0 +1,73 @@
> > > +* Renesas RZ/N1 GPIO Interrupt Multiplexer
> > > +
> > > +On Renesas RZ/N1 devices, there are several GPIO Controllers each
> > > +with a number of interrupt outputs. All of the interrupts from the
> > > +GPIO Controllers are passed to the GPIO Interrupt Multiplexer, which
> > > +selects a sub-set of the interrupts to pass onto the system interrupt
> > controller.
> > > +
> > > +A single node in the device tree is used to describe the GPIO IRQ Muxer.
> > > +
> > > +Required properties:
> > > +- compatible: SoC-specific compatible string "renesas,<soc-specific>-
> > gpioirqmux"
> > > +  followed by "renesas,rzn1-gpioirqmux" as fallback. The SoC-specific
> > > +compatible
> > > +  strings must be one of:
> > > +   "renesas,r9a06g032-gpioirqmux" for RZ/N1D
> > > +   "renesas,r9a06g033-gpioirqmux" for RZ/N1S
> > > +- reg: Base address and size of GPIO IRQ Muxer registers.
> > > +- interrupts: List of output interrupts.
> > > +- #interrupt-cells: Numder of cells in the input interrupt specifier, must be
> > 1.
> > > +- #address-cells: Must be 0.
> > > +- interrupt-map-mask: must be 127.
> > > +- interrupt-map: Standard property detailing the maps between input
> > > +irqs and the
> > > +  corresponding output irq. This consist of a list of:
> > > +   <input-irq-spec phandle-to-interrupt-controller output-irq-spec>
> > > +  The input-irq-spec is from 0 to 95, corresponding to the outputs of
> > > +the GPIO
> > > +  Controllers.
> > > +
> > > +Example:
> > > +
> > > +   The following is an example for the RZ/N1D SoC.
> > > +
> > > +   gpioirqmux: gpioirqmux@51000480 {
> >
> > interrupt-controller@...
> Sure
>
> > > +           compatible = "renesas,r9a06g032-gpioirqmux",
> > > +                           "renesas,rzn1-gpioirqmux";
> > > +           reg = <0x51000480 0x20>;
> > > +           interrupts =
> > > +                   <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> > > +                   <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> >
> > This is a bit redundant as the same information is in interrupt-map, but I
> > guess you need it to get the irq resources.
> That's right.
>
> > > +
> > > +           #interrupt-cells = <1>;
> > > +           #address-cells = <0>;
> > > +           interrupt-map-mask = <127>;
> >
> > Use hex for masks.
> Ok.
>
> > > +           interrupt-map =
> > > +                   /* gpio2a 24, pin 146: ETH Port 1 IRQ */
> > > +                   <88 &gic GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> > > +                   /* gpio2a 26, pin 148: TouchSCRN_IRQ */
> > > +                   <90 &gic GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > > +   };
> > > +
> > > +   gpio2: gpio@5000d000 {
> > > +           compatible = "snps,dw-apb-gpio";
> > > +           reg = <0x5000d000 0x80>;
> > > +           #address-cells = <1>;
> > > +           #size-cells = <0>;
> > > +           clock-names = "bus";
> > > +           clocks = <&sysctrl R9A06G032_HCLK_GPIO2>;
> > > +
> > > +           gpio2a: gpio-controller@0 {
> >
> > gpio@0
> Are you sure about this?

Yes, 'gpio' is what the DT spec says. It is the oddball though and I
always have to think about it.

> The bindings in Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> show an example where the sub-nodes for gpio banks are gpio-controller@.
> This is also in Documentation/devicetree/bindings/gpio/gpio.txt.

Indeed. Those should be fixed. I would be easily persuaded to just
change the spec, but 'gpio@' is much more widely used:

$ git grep gpio@ -- arch/ | wc
   1110    4378   67766
$ git grep gpio-controller@ -- arch/ | wc
     60     232    4270

Rob

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding
  2018-11-19 21:50       ` Rob Herring
@ 2018-11-20 10:30         ` Phil Edworthy
  0 siblings, 0 replies; 6+ messages in thread
From: Phil Edworthy @ 2018-11-20 10:30 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marc Zyngier, Thomas Gleixner, Jason Cooper, Mark Rutland,
	Geert Uytterhoeven, open list:MEDIA DRIVERS FOR RENESAS - FCP,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org

Hi Rob,

On 19 November 2018 21:51, Rob Herring wrote:
> On Mon, Nov 19, 2018 at 3:39 AM Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> >
> > Hi Rob,
> >
> > On 17 November 2018 14:33 Rob Herring wrote:
> > > On Tue, Nov 13, 2018 at 01:09:09PM +0000, Phil Edworthy wrote:
> > > > Add device binding documentation for the Renesas RZ/N1 GPIO
> > > > interrupt multiplexer.
> > > >
> > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > ---
> > > > v3:
> > > >  - Use 'interrupt-map' DT property correctly.
> > > > v2:
> > > >  - Use interrupt-map to allow the GPIO controller info to be specified
> > > >    as part of the irq.
> > > >  - Don't show status in binding examples.
> > > >  - Don't show the soc/board split in binding doc.
> > > > ---
> > > >  .../interrupt-controller/renesas,rzn1-mux.txt | 73
> > > > +++++++++++++++++++
> > > >  1 file changed, 73 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/interrupt-controller/renesas,rzn
> > > > 1-mu
> > > > x.txt
> > >
> > > A few nits, otherwise:
> > >
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > Thanks for the review!
> >
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/interrupt-controller/renesas,r
> > > > zn1-
> > > > mux.txt
> > > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,r
> > > > zn1-
> > > > mux.txt
> > > > new file mode 100644
> > > > index 000000000000..6515880e25cc
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/renes
> > > > +++ as,r
> > > > +++ zn1-mux.txt
> > > > @@ -0,0 +1,73 @@
> > > > +* Renesas RZ/N1 GPIO Interrupt Multiplexer
> > > > +
> > > > +On Renesas RZ/N1 devices, there are several GPIO Controllers each
> > > > +with a number of interrupt outputs. All of the interrupts from
> > > > +the GPIO Controllers are passed to the GPIO Interrupt
> > > > +Multiplexer, which selects a sub-set of the interrupts to pass
> > > > +onto the system interrupt
> > > controller.
> > > > +
> > > > +A single node in the device tree is used to describe the GPIO IRQ
> Muxer.
> > > > +
> > > > +Required properties:
> > > > +- compatible: SoC-specific compatible string
> > > > +"renesas,<soc-specific>-
> > > gpioirqmux"
> > > > +  followed by "renesas,rzn1-gpioirqmux" as fallback. The
> > > > +SoC-specific compatible
> > > > +  strings must be one of:
> > > > +   "renesas,r9a06g032-gpioirqmux" for RZ/N1D
> > > > +   "renesas,r9a06g033-gpioirqmux" for RZ/N1S
> > > > +- reg: Base address and size of GPIO IRQ Muxer registers.
> > > > +- interrupts: List of output interrupts.
> > > > +- #interrupt-cells: Numder of cells in the input interrupt
> > > > +specifier, must be
> > > 1.
> > > > +- #address-cells: Must be 0.
> > > > +- interrupt-map-mask: must be 127.
> > > > +- interrupt-map: Standard property detailing the maps between
> > > > +input irqs and the
> > > > +  corresponding output irq. This consist of a list of:
> > > > +   <input-irq-spec phandle-to-interrupt-controller
> > > > +output-irq-spec>
> > > > +  The input-irq-spec is from 0 to 95, corresponding to the
> > > > +outputs of the GPIO
> > > > +  Controllers.
> > > > +
> > > > +Example:
> > > > +
> > > > +   The following is an example for the RZ/N1D SoC.
> > > > +
> > > > +   gpioirqmux: gpioirqmux@51000480 {
> > >
> > > interrupt-controller@...
> > Sure
> >
> > > > +           compatible = "renesas,r9a06g032-gpioirqmux",
> > > > +                           "renesas,rzn1-gpioirqmux";
> > > > +           reg = <0x51000480 0x20>;
> > > > +           interrupts =
> > > > +                   <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                   <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > This is a bit redundant as the same information is in interrupt-map,
> > > but I guess you need it to get the irq resources.
> > That's right.
> >
> > > > +
> > > > +           #interrupt-cells = <1>;
> > > > +           #address-cells = <0>;
> > > > +           interrupt-map-mask = <127>;
> > >
> > > Use hex for masks.
> > Ok.
> >
> > > > +           interrupt-map =
> > > > +                   /* gpio2a 24, pin 146: ETH Port 1 IRQ */
> > > > +                   <88 &gic GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                   /* gpio2a 26, pin 148: TouchSCRN_IRQ */
> > > > +                   <90 &gic GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > > > +   };
> > > > +
> > > > +   gpio2: gpio@5000d000 {
> > > > +           compatible = "snps,dw-apb-gpio";
> > > > +           reg = <0x5000d000 0x80>;
> > > > +           #address-cells = <1>;
> > > > +           #size-cells = <0>;
> > > > +           clock-names = "bus";
> > > > +           clocks = <&sysctrl R9A06G032_HCLK_GPIO2>;
> > > > +
> > > > +           gpio2a: gpio-controller@0 {
> > >
> > > gpio@0
> > Are you sure about this?
> 
> Yes, 'gpio' is what the DT spec says. It is the oddball though and I always have
> to think about it.
OK, got it.

> > The bindings in
> > Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> > show an example where the sub-nodes for gpio banks are gpio-
> controller@.
> > This is also in Documentation/devicetree/bindings/gpio/gpio.txt.
> 
> Indeed. Those should be fixed. I would be easily persuaded to just change
> the spec, but 'gpio@' is much more widely used:
> 
> $ git grep gpio@ -- arch/ | wc
>    1110    4378   67766
> $ git grep gpio-controller@ -- arch/ | wc
>      60     232    4270
> 
Yes, indeed.

Thanks
Phil

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-11-20 10:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-13 13:09 [PATCH v3 0/2] irqchip: Add support for Renesas RZ/N1 GPIO interrupt multiplexer Phil Edworthy
2018-11-13 13:09 ` [PATCH v3 1/2] dt-bindings/interrupt-controller: rzn1: Add RZ/N1 gpio irq mux binding Phil Edworthy
2018-11-17 14:32   ` Rob Herring
2018-11-19  9:39     ` Phil Edworthy
2018-11-19 21:50       ` Rob Herring
2018-11-20 10:30         ` Phil Edworthy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).