Re: [PATCH] irqchip: Add support for Renesas RZ/N1 GPIO interrupt multiplexer

[Rob Herring <robh@kernel.org>]

On Mon, Apr 23, 2018 at 02:33:06PM +0100, Phil Edworthy wrote:
> On RZ/N1 devices, there are 3 Synopsys DesignWare GPIO blocks each
> configured to have 32 interrupt outputs, so we have a total of 96 GPIO
> interrupts. All of these are passed to the GPIO IRQ Muxer, which selects
> 8 of the GPIO interrupts to pass onto the GIC. The interrupt signals
> aren't latched, so there is nothing to do in this driver when an interrupt
> is received, other than tell the corresponding GPIO block.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  .../interrupt-controller/renesas,rzn1-mux.txt      |  85 ++++++++++

Please split bindings to a separate patch.

>  drivers/irqchip/Kconfig                            |  10 ++
>  drivers/irqchip/Makefile                           |   1 +
>  drivers/irqchip/irq-rzn1-irq-mux.c                 | 178 +++++++++++++++++++++
>  4 files changed, 274 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt
>  create mode 100644 drivers/irqchip/irq-rzn1-irq-mux.c
> 
> 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 0000000..f28a365
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzn1-mux.txt
> @@ -0,0 +1,85 @@
> +* Renesas RZ/N1 GPIO Interrupt Multiplexer
> +
> +On Renesas RZ/N1 devices, there are 3 Synopsys DesignWare GPIO blocks each
> +configured to have 32 interrupt outputs, so we have a total of 96 GPIO
> +interrupts. All of these are passed to the GPIO IRQ Muxer, which selects
> +8 of the GPIO interrupts to pass onto the GIC.
> +
> +A single node in the device tree is used to describe the GPIO IRQ Muxer.
> +
> +Required properties:
> +- compatible: the SoC specific name, i.e. "renesas,r9a06g032-gpioirq"
> +   or "renesas,r9a06g033-gpioirq" followed by the SoC family name, i.e.
> +   "renesas,rzn1-gpioirq".
> +- interrupt-controller: Identifies the node as an interrupt controller.
> +- #interrupt-cells: should be <1>. The meaning of the cells is the input
> +   interrupt index, 0 to 95.
> +- reg: Base address and size of GPIO IRQ Muxer registers.
> +- interrupts: The list of interrupts generated by the muxer which are then
> +   connected to a parent interrupt controller. The format of the interrupt
> +   specifier depends in the interrupt parent controller.
> +- gpioirq-#N: One property for each interrupt output from the GPIO IRQ Muxer
> +   that specifies the input interrupt to use, #N is from 0 to 7.

Why do you need this in DT? Can't the driver handle this dynamically? 
When you request an interrupt on a GPIO line, then connect that GPIO 
line to a free IRQ line. 

If you really need this in DT, then interrupt-map can be used here.

> +
> +Optional properties:
> +- interrupt-parent: pHandle of the parent interrupt controller, if not
> +   inherited from the parent node.
> +
> +
> +Example:
> +
> +	The following is an example for the RZ/N1D SoC.
> +
> +	gpioirq: gpioirq@51000480 {
> +		compatible = "renesas,r9a06g032-gpioirq",
> +				"renesas,rzn1-gpioirq";
> +		reg = <0x51000480 0x20>;
> +		interrupts =
> +			<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
> +			<GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		status = "disabled";

Don't show status in examples.

> +	};
> +
> +	gpio0: gpio@5000b000 {
> +		compatible = "snps,dw-apb-gpio";
> +		reg = <0x5000b000 0x80>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		clock-names = "bus";
> +		clocks = <&hclk_gpio0>;
> +		status = "disabled";
> +
> +		gpio0a: gpio-controller@0 {
> +			compatible = "snps,dw-apb-gpio-port";
> +			bank-name = "gpio0a";
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			snps,nr-gpios = <32>;
> +			reg = <0>;
> +
> +			interrupt-controller;
> +			interrupt-parent = <&gpioirq>;
> +			interrupts =   < 0  1  2  3  4  5  6  7
> +					 8  9 10 11 12 13 14 15
> +					16 17 18 19 20 21 22 23
> +					24 25 26 27 28 29 30 31 >;
> +			#interrupt-cells = <2>;
> +		};
> +	};
> +
> +
> +	The following is an example for a board using this.

Don't show the soc/board split. This is convention, but not part of the 
binding.

> +
> +	&gpioirq {
> +		status = "okay";
> +		gpioirq-0 = <24>;	/* gpio0a 24 */
> +		gpioirq-4 = <3>;	/* gpio0a 3 */
> +	};