RE: [PATCH 2/5] irqchip: Add Renesas RZ/A1 Interrupt Controller driver

From: Chris Brandt <Chris.Brandt@renesas.com>
To: Marc Zyngier <marc.zyngier@arm.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Simon Horman <horms@verge.net.au>,
	Magnus Damm <magnus.damm@gmail.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 2/5] irqchip: Add Renesas RZ/A1 Interrupt Controller driver
Date: Mon, 29 Apr 2019 17:31:18 +0000	[thread overview]
Message-ID: <TY1PR01MB15620363F895ACD71CFE77878A390@TY1PR01MB1562.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <e6c6dfe7-f67d-90a9-eee5-d30b94c7c978@arm.com>

Hi Geert and Marc,

I tested Geert's patches on RZ/A1 and RZ/A2 boards (push buttons and LCD
touchscreen controller). Works great!

$ cat /proc/interrupts
 49:       5050  fcfef800.interrupt-controller   1 Level     ft5x06_ts
 50:         32  fcfef800.interrupt-controller   3 Edge      SW1
 51:         12  fcfef800.interrupt-controller   2 Edge      SW2
 52:         10  fcfef800.interrupt-controller   5 Edge      SW3


For RZ/A2, I patched the driver/Kconfig as follows:

----------------------------------------------------------------------

diff --git a/drivers/irqchip/irq-renesas-rza1.c b/drivers/irqchip/irq-renesas-rza1.c
index 65110019adda..89a721864ff9 100644
--- a/drivers/irqchip/irq-renesas-rza1.c
+++ b/drivers/irqchip/irq-renesas-rza1.c
@@ -206,7 +206,12 @@ struct rza1_irqc_info rza1_irqc_info = {
        .gic_spi_base = 0,
 };
 
+struct rza1_irqc_info rza2_irqc_info = {
+       .gic_spi_base = 4,
+};
+
 static const struct of_device_id rza1_irqc_dt_ids[] = {
+       { .compatible = "renesas,r7s9210-irqc", &rza2_irqc_info },
        { .compatible = "renesas,rza1-irqc", &rza1_irqc_info },
        {},
 };
diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 1448b6dbcdb2..4ab7dcfd6a2f 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -65,6 +65,7 @@ config ARCH_R7S9210
        bool "RZ/A2 (R7S9210)"
        select PM
        select PM_GENERIC_DOMAINS
+       select RENESAS_RZA1_IRQC
        select RENESAS_OSTM
 
 config ARCH_R8A73A4
----------------------------------------------------------------------


However, as per:


On Mon, Apr 29, 2019, Marc Zyngier wrote:
> >>> +struct rza1_irqc_info rza1_irqc_info = {
> >>> +     .gic_spi_base = 0,
> >>> +};
> >>
> >> To answer your question in the cover letter, I'd rather this came from
> >> DT. And otherwise, it should be be static.
> >
> > (Oops, forget the "static const")
> >
> > Using a custom property, or derived from 8 interrupt specifiers in the
> > interrupts property?
> 
> A custom property is fine by me (everybody does that anyway).


I'll re-test after v2 of the patch when gic_spi_base moves to DT.



If you want to add RZ/A2 into this series to get it all done at once, 
I'll test that as well.

Just note that on the RZ/A2M EVB, only SW3 is connected to an interrupt 
pin (IRQ0).

&pinctrl {
	/* SW3 = IRQ0 */
	keyboard_pins: keyboard {
		pinmux = <RZA2_PINMUX(PORTJ, 1, 6)>;	/* IRQ0 */
	};
};

/ {
	keyboard {
		compatible = "gpio-keys";

		pinctrl-names = "default";
		pinctrl-0 = <&keyboard_pins>;

		key-3 {
			interrupt-parent = <&irqc>;
			interrupts = <0 IRQ_TYPE_EDGE_BOTH>;
			linux,code = <KEY_3>;
			label = "SW3";
			wakeup-source;
		};
	};
};



Chris

