[PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Geert Uytterhoeven]

	Hi Simon, Magnus, Chris,

This patch series adds support for devices on the I2C bus on the RSKRZA1
board, including the GPIO expander that drives 3 LEDs.

Thanks!

Geert Uytterhoeven (2):
  ARM: dts: rskrza1: Add I2C support
  ARM: dts: rskrza1: Add remaining LEDs

 arch/arm/boot/dts/r7s72100-rskrza1.dts | 46 +++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/2] ARM: dts: rskrza1: Add I2C support

[Geert Uytterhoeven]

Enable the I2C bus, and add the following devices:
  - Two CAT9554 port expanders (8 GPIOs, interrupt not wired by
    default),
  - R1EX24016ASAS0A EEPROM.

The bus also contains a MAX9856 Audio Codec, which is not yet supported.

All devices (incl. the audio codec) are documented to support an I2C bus
running at 400 kHz.

Pinctrl is based on the RZ/A BSP.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Pending acceptance of the DT binding patches documenting the
"onnn,cat9554" and "renesas,r1ex24016" compatible values.
---
 arch/arm/boot/dts/r7s72100-rskrza1.dts | 33 ++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts b/arch/arm/boot/dts/r7s72100-rskrza1.dts
index 8ee44a100e9a44c4..60e80ecbbf6826bc 100644
--- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
+++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
@@ -47,6 +47,34 @@
 	clock-frequency = <13330000>;
 };
 
+&i2c3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c3_pins>;
+	status = "okay";
+
+	clock-frequency = <400000>;
+
+	io_expander1: gpio@20 {
+		compatible = "onnn,cat9554";
+		reg = <0x20>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	io_expander2: gpio@21 {
+		compatible = "onnn,cat9554";
+		reg = <0x21>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	eeprom@50 {
+		compatible = "renesas,r1ex24016", "atmel,24c16";
+		reg = <0x50>;
+		pagesize = <16>;
+	};
+};
+
 &usb_x1_clk {
 	clock-frequency = <48000000>;
 };
@@ -56,6 +84,11 @@
 };
 
 &pinctrl {
+	/* RIIC ch3 (Port Expander, EEPROM (MAC Addr), Audio Codec) */
+	i2c3_pins: i2c3 {
+		pinmux = <RZA1_PINMUX(1, 6, 1)>,	/* RIIC3SCL */
+			 <RZA1_PINMUX(1, 7, 1)>;	/* RIIC3SDA */
+	};
 
 	/* Serial Console */
 	scif2_pins: serial2 {
-- 
2.17.1

Re: [PATCH 1/2] ARM: dts: rskrza1: Add I2C support

[Simon Horman]

On Wed, Mar 20, 2019 at 08:44:09PM +0100, Geert Uytterhoeven wrote:
> Enable the I2C bus, and add the following devices:
>   - Two CAT9554 port expanders (8 GPIOs, interrupt not wired by
>     default),
>   - R1EX24016ASAS0A EEPROM.
> 
> The bus also contains a MAX9856 Audio Codec, which is not yet supported.
> 
> All devices (incl. the audio codec) are documented to support an I2C bus
> running at 400 kHz.
> 
> Pinctrl is based on the RZ/A BSP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Pending acceptance of the DT binding patches documenting the
> "onnn,cat9554" and "renesas,r1ex24016" compatible values.

Thanks.

I'm marking this series as deferred pending acceptance of the bindings.
Please repost or ping me once that has happened.

> ---
>  arch/arm/boot/dts/r7s72100-rskrza1.dts | 33 ++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts b/arch/arm/boot/dts/r7s72100-rskrza1.dts
> index 8ee44a100e9a44c4..60e80ecbbf6826bc 100644
> --- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
> +++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
> @@ -47,6 +47,34 @@
>  	clock-frequency = <13330000>;
>  };
>  
> +&i2c3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c3_pins>;
> +	status = "okay";
> +
> +	clock-frequency = <400000>;
> +
> +	io_expander1: gpio@20 {
> +		compatible = "onnn,cat9554";
> +		reg = <0x20>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +	};
> +
> +	io_expander2: gpio@21 {
> +		compatible = "onnn,cat9554";
> +		reg = <0x21>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +	};
> +
> +	eeprom@50 {
> +		compatible = "renesas,r1ex24016", "atmel,24c16";
> +		reg = <0x50>;
> +		pagesize = <16>;
> +	};
> +};
> +
>  &usb_x1_clk {
>  	clock-frequency = <48000000>;
>  };
> @@ -56,6 +84,11 @@
>  };
>  
>  &pinctrl {
> +	/* RIIC ch3 (Port Expander, EEPROM (MAC Addr), Audio Codec) */
> +	i2c3_pins: i2c3 {
> +		pinmux = <RZA1_PINMUX(1, 6, 1)>,	/* RIIC3SCL */
> +			 <RZA1_PINMUX(1, 7, 1)>;	/* RIIC3SDA */
> +	};
>  
>  	/* Serial Console */
>  	scif2_pins: serial2 {
> -- 
> 2.17.1
> 

Re: [PATCH 1/2] ARM: dts: rskrza1: Add I2C support

[Geert Uytterhoeven]

Hi Simon,

On Fri, Mar 22, 2019 at 11:23 AM Simon Horman <horms@verge.net.au> wrote:
> On Wed, Mar 20, 2019 at 08:44:09PM +0100, Geert Uytterhoeven wrote:
> > Enable the I2C bus, and add the following devices:
> >   - Two CAT9554 port expanders (8 GPIOs, interrupt not wired by
> >     default),
> >   - R1EX24016ASAS0A EEPROM.
> >
> > The bus also contains a MAX9856 Audio Codec, which is not yet supported.
> >
> > All devices (incl. the audio codec) are documented to support an I2C bus
> > running at 400 kHz.
> >
> > Pinctrl is based on the RZ/A BSP.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Pending acceptance of the DT binding patches documenting the
> > "onnn,cat9554" and "renesas,r1ex24016" compatible values.
>
> Thanks.
>
> I'm marking this series as deferred pending acceptance of the bindings.
> Please repost or ping me once that has happened.

next-20190325 has:

    d333bad1f6f3e0df dt-bindings: at24: add Renesas R1EX24016
    5c9f8cfe3cb67dcb dt-bindings: gpio: pca953x: Document onnn,cat9554

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] ARM: dts: rskrza1: Add I2C support

[Geert Uytterhoeven]

Hi Simon,

On Fri, Mar 22, 2019 at 11:23 AM Simon Horman <horms@verge.net.au> wrote:
> On Wed, Mar 20, 2019 at 08:44:09PM +0100, Geert Uytterhoeven wrote:
> > Enable the I2C bus, and add the following devices:
> >   - Two CAT9554 port expanders (8 GPIOs, interrupt not wired by
> >     default),
> >   - R1EX24016ASAS0A EEPROM.
> >
> > The bus also contains a MAX9856 Audio Codec, which is not yet supported.
> >
> > All devices (incl. the audio codec) are documented to support an I2C bus
> > running at 400 kHz.
> >
> > Pinctrl is based on the RZ/A BSP.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Pending acceptance of the DT binding patches documenting the
> > "onnn,cat9554" and "renesas,r1ex24016" compatible values.
>
> Thanks.
>
> I'm marking this series as deferred pending acceptance of the bindings.
> Please repost or ping me once that has happened.

next-20190325 has:

    d333bad1f6f3e0df dt-bindings: at24: add Renesas R1EX24016
    5c9f8cfe3cb67dcb dt-bindings: gpio: pca953x: Document onnn,cat9554

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] ARM: dts: rskrza1: Add I2C support

[Simon Horman]

On Mon, Mar 25, 2019 at 11:55:50AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Mar 22, 2019 at 11:23 AM Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Mar 20, 2019 at 08:44:09PM +0100, Geert Uytterhoeven wrote:
> > > Enable the I2C bus, and add the following devices:
> > >   - Two CAT9554 port expanders (8 GPIOs, interrupt not wired by
> > >     default),
> > >   - R1EX24016ASAS0A EEPROM.
> > >
> > > The bus also contains a MAX9856 Audio Codec, which is not yet supported.
> > >
> > > All devices (incl. the audio codec) are documented to support an I2C bus
> > > running at 400 kHz.
> > >
> > > Pinctrl is based on the RZ/A BSP.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > Pending acceptance of the DT binding patches documenting the
> > > "onnn,cat9554" and "renesas,r1ex24016" compatible values.
> >
> > Thanks.
> >
> > I'm marking this series as deferred pending acceptance of the bindings.
> > Please repost or ping me once that has happened.
> 
> next-20190325 has:
> 
>     d333bad1f6f3e0df dt-bindings: at24: add Renesas R1EX24016
>     5c9f8cfe3cb67dcb dt-bindings: gpio: pca953x: Document onnn,cat9554

Thanks, applied for v5.2.

Re: [PATCH 1/2] ARM: dts: rskrza1: Add I2C support

[Simon Horman]

On Mon, Mar 25, 2019 at 11:55:50AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Mar 22, 2019 at 11:23 AM Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Mar 20, 2019 at 08:44:09PM +0100, Geert Uytterhoeven wrote:
> > > Enable the I2C bus, and add the following devices:
> > >   - Two CAT9554 port expanders (8 GPIOs, interrupt not wired by
> > >     default),
> > >   - R1EX24016ASAS0A EEPROM.
> > >
> > > The bus also contains a MAX9856 Audio Codec, which is not yet supported.
> > >
> > > All devices (incl. the audio codec) are documented to support an I2C bus
> > > running at 400 kHz.
> > >
> > > Pinctrl is based on the RZ/A BSP.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > Pending acceptance of the DT binding patches documenting the
> > > "onnn,cat9554" and "renesas,r1ex24016" compatible values.
> >
> > Thanks.
> >
> > I'm marking this series as deferred pending acceptance of the bindings.
> > Please repost or ping me once that has happened.
> 
> next-20190325 has:
> 
>     d333bad1f6f3e0df dt-bindings: at24: add Renesas R1EX24016
>     5c9f8cfe3cb67dcb dt-bindings: gpio: pca953x: Document onnn,cat9554

Thanks, applied for v5.2.

[PATCH 2/2] ARM: dts: rskrza1: Add remaining LEDs

[Geert Uytterhoeven]

Describe the remaining 3 LEDs, which are driven by the first CAT9554
port expander.

Drop the superfluous status property from the leds node while at it.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r7s72100-rskrza1.dts | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts b/arch/arm/boot/dts/r7s72100-rskrza1.dts
index 60e80ecbbf6826bc..ff24301dc1be54de 100644
--- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
+++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
@@ -34,12 +34,23 @@
 	};
 
 	leds {
-		status = "okay";
 		compatible = "gpio-leds";
 
 		led0 {
 			gpios = <&port7 1 GPIO_ACTIVE_LOW>;
 		};
+
+		led1 {
+			gpios = <&io_expander1 0 GPIO_ACTIVE_LOW>;
+		};
+
+		led2 {
+			gpios = <&io_expander1 1 GPIO_ACTIVE_LOW>;
+		};
+
+		led3 {
+			gpios = <&io_expander1 2 GPIO_ACTIVE_LOW>;
+		};
 	};
 };
 
-- 
2.17.1

Re: [PATCH 2/2] ARM: dts: rskrza1: Add remaining LEDs

[Simon Horman]

On Wed, Mar 20, 2019 at 08:44:10PM +0100, Geert Uytterhoeven wrote:
> Describe the remaining 3 LEDs, which are driven by the first CAT9554
> port expander.
> 
> Drop the superfluous status property from the leds node while at it.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, applied for v5.2.

> ---
>  arch/arm/boot/dts/r7s72100-rskrza1.dts | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts b/arch/arm/boot/dts/r7s72100-rskrza1.dts
> index 60e80ecbbf6826bc..ff24301dc1be54de 100644
> --- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
> +++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
> @@ -34,12 +34,23 @@
>  	};
>  
>  	leds {
> -		status = "okay";
>  		compatible = "gpio-leds";
>  
>  		led0 {
>  			gpios = <&port7 1 GPIO_ACTIVE_LOW>;
>  		};
> +
> +		led1 {
> +			gpios = <&io_expander1 0 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		led2 {
> +			gpios = <&io_expander1 1 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		led3 {
> +			gpios = <&io_expander1 2 GPIO_ACTIVE_LOW>;
> +		};
>  	};
>  };
>  
> -- 
> 2.17.1
> 

RE: [PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Chris Brandt]

Hi Geert,

On Wed, Mar 20, 2019, Geert Uytterhoeven wrote:
> 	Hi Simon, Magnus, Chris,
> 
> This patch series adds support for devices on the I2C bus on the RSKRZA1
> board, including the GPIO expander that drives 3 LEDs.
> 
> Thanks!

Very cool.

I figured there was a more graceful way of turning on/off those LEDs 
than what I was doing...which was just writing to the I2C bus in my LED 
heartbeat kernel thread.
https://github.com/renesas-rz/rza_linux-4.19/blob/master/arch/arm/mach-shmobile/board-rskrza1.c#L145


Chris

RE: [PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Chris Brandt]

Hi Geert,

On Wed, Mar 20, 2019, Geert Uytterhoeven wrote:
> 	Hi Simon, Magnus, Chris,
> 
> This patch series adds support for devices on the I2C bus on the RSKRZA1
> board, including the GPIO expander that drives 3 LEDs.
> 
> Thanks!

Very cool.

I figured there was a more graceful way of turning on/off those LEDs 
than what I was doing...which was just writing to the I2C bus in my LED 
heartbeat kernel thread.
https://github.com/renesas-rz/rza_linux-4.19/blob/master/arch/arm/mach-shmobile/board-rskrza1.c#L145


Chris

Re: [PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Geert Uytterhoeven]

Hi Chris,

On Wed, Mar 20, 2019 at 9:41 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wed, Mar 20, 2019, Geert Uytterhoeven wrote:
> > This patch series adds support for devices on the I2C bus on the RSKRZA1
> > board, including the GPIO expander that drives 3 LEDs.
>
> Very cool.
>
> I figured there was a more graceful way of turning on/off those LEDs
> than what I was doing...which was just writing to the I2C bus in my LED
> heartbeat kernel thread.
> https://github.com/renesas-rz/rza_linux-4.19/blob/master/arch/arm/mach-shmobile/board-rskrza1.c#L145

Yeah, I've seen that code. Not fit for upstream ;-)

I also wanted to add support for the switches, as I prefer to have a
remote wake-up source before I move a board to my board farm (sh_eth in
RZ/A1 does not seem to support wake-on-LAN). However, that doesn't
work yet.

The usual approach would be:

    #include <dt-bindings/input/input.h>

        keyboard {
                compatible = "gpio-keys";

                key-1 {
                        gpios = <&port1 9 GPIO_ACTIVE_LOW>;
                        linux,code = <KEY_1>;
                        label = "SW1";
                        wakeup-source;
                };
        };

But that doesn't work, as RZ/A1 GPIO does not support interrupts.

The GPIOs connected to the switches can also be used as IRQ lines
with (pseudo patch)

        keyboard {

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

                key-1 {

-                       gpios = <&port1 9 GPIO_ACTIVE_LOW>;
+                      interrupt-parent = <&gic>;
+                      interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_BOTH>;

                }
        }

+       keyboard_pins: keyboard {
+               pinmux = <RZA1_PINMUX(1, 9, 3)>;        /* IRQ3 */
+       };

However, that also doesn't work, as the PL390 GIC supports rising edge
and high-level interrupts only.  Falling edge and low-level interrupts
on the IRQ pins seem to be an RZ/A1-specific GIC extension, which
requires programming the non-standard registers (ICR0, ICR1, IRQRR).
Nevertheless, even with the plain supported interrupt modes I didn't
receive interrupts when using the switches.

I've just discovered commit 207c10b54ca81129 ("irq-gic: add support for
pins IRQ0-IRQ7") in rza_linux-4.14, but when using that, the system
locks up when pressing a switch :-(

Finally, I tried using the pin interrupts, which are the closest to GPIO
interrupts we seem to have.  As these support high-level and rising-edge
interrupts only, while the switches are active-low, this means only
key-up events are detected:

                interrupt-parent = <&gic>;
                interrupts = <GIC_SPI 401 IRQ_TYPE_EDGE_RISING>;

What am I doing wrong?
Do you have a better solution?
The IRQx support looks most promising, but needs some work (separate
compatible value, small driver with a custom .irq_set_type, deferring
the rest to the normal GIC driver).

Thanks!



Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Chris Brandt]

Hi Geert,

Look what fun you are having with an RZ/A1 board!
You might have even more fun with a RZ/A2 board ;)

> However, that also doesn't work, as the PL390 GIC supports rising edge
> and high-level interrupts only.  Falling edge and low-level interrupts
> on the IRQ pins seem to be an RZ/A1-specific GIC extension,

Yes. I get this question a couple times a year.

And no one wants rising or level high, so the GPIO interrupts would not 
work for them anyway.

So, as you found:

> I've just discovered commit 207c10b54ca81129 ("irq-gic: add support for
> pins IRQ0-IRQ7") in rza_linux-4.14, but when using that, the system
> locks up when pressing a switch :-(

It might be because the interrupt routine needs to go in and clear the 
interrupt by writing to the 'IRQ Interrupt Request Register' (IRQRR).
From the HW manual:

"IRQRR is a 16-bit register that indicates interrupt requests from 
external input pins IRQ7 to IRQ0. If edge detection is set for the IRQ7 to 
IRQ0 interrupts, writing 0 to the IRQ7F to IRQ0F bits after reading IRQ7F 
to IRQ0F = 1 cancels the retained interrupts."


The touchscreen driver for the LCD uses IRQ_TYPE_LEVEL_LOW. 
In the interrupt routine ft5x06_ts_isr, disable_irq_nosync(irq) is called immediately.
Then after the touch point is read out, enable_irq(ts->client->irq) is called in ft5x06_ts_poscheck.
https://github.com/renesas-rz/rza_linux-4.19/blob/master/drivers/input/touchscreen/ft5x06_ts.c#L232


> What am I doing wrong?
> Do you have a better solution?
> The IRQx support looks most promising, but needs some work (separate
> compatible value, small driver with a custom .irq_set_type, deferring
> the rest to the normal GIC driver).

I not sure about a 'better' solution...just the one I tell people 
(although I need to make some App Note someday).

Basically, I tell people they need to describe the IRQ in the Device 
Tree somewhere (so it can get allocated an interrupt ID).
Then, they can call request_irq() from inside a custom driver if they 
want to.
Or, they just make a very simple shim-driver in their board-xxx.c file 
that registers the interrupt and has a simple interrupt routine. Many 
times all people want to do is simply wire up an interrupt that the 
application can wait on. Or, use an IRQ pin to start an ADC conversion. Pretty 
simple stuff.

Of course they can also use a UIO driver to do these kinds of things as 
well.
As you know, the UIO subsystem will automatically turn of the interrupt 
each time, but users still need to write to the IRQRR register before 
turning the interrupt back on if using edge interrupts.

I always thought what I really need to do is instead just make a 
separate driver like you suggested above because technical, the "IRQ" block is 
not really part of the GIC, it is a separate block that feeds interrupts
into the GIC just like all the other peripherals, so it should be 
represented as a separate piece of hardware (ie, its own driver and 
capability string).


For what it's worth, the RZ/A2 has the exact same IRQs, so if I ever got
around for doing it for RZ/A1, it could also be used for RZ/A2.

Chris

Re: [PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Geert Uytterhoeven]

Hi Chris,

On Thu, Mar 21, 2019 at 1:16 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> Look what fun you are having with an RZ/A1 board!
> You might have even more fun with a RZ/A2 board ;)
>
> > However, that also doesn't work, as the PL390 GIC supports rising edge
> > and high-level interrupts only.  Falling edge and low-level interrupts
> > on the IRQ pins seem to be an RZ/A1-specific GIC extension,
>
> Yes. I get this question a couple times a year.
>
> And no one wants rising or level high, so the GPIO interrupts would not
> work for them anyway.
>
> So, as you found:
>
> > I've just discovered commit 207c10b54ca81129 ("irq-gic: add support for
> > pins IRQ0-IRQ7") in rza_linux-4.14, but when using that, the system
> > locks up when pressing a switch :-(
>
> It might be because the interrupt routine needs to go in and clear the
> interrupt by writing to the 'IRQ Interrupt Request Register' (IRQRR).
> From the HW manual:
>
> "IRQRR is a 16-bit register that indicates interrupt requests from
> external input pins IRQ7 to IRQ0. If edge detection is set for the IRQ7 to
> IRQ0 interrupts, writing 0 to the IRQ7F to IRQ0F bits after reading IRQ7F
> to IRQ0F = 1 cancels the retained interrupts."

OK.

> > What am I doing wrong?
> > Do you have a better solution?
> > The IRQx support looks most promising, but needs some work (separate
> > compatible value, small driver with a custom .irq_set_type, deferring
> > the rest to the normal GIC driver).
>
> I not sure about a 'better' solution...just the one I tell people
> (although I need to make some App Note someday).
>
> Basically, I tell people they need to describe the IRQ in the Device
> Tree somewhere (so it can get allocated an interrupt ID).
> Then, they can call request_irq() from inside a custom driver if they
> want to.
> Or, they just make a very simple shim-driver in their board-xxx.c file
> that registers the interrupt and has a simple interrupt routine. Many
> times all people want to do is simply wire up an interrupt that the
> application can wait on. Or, use an IRQ pin to start an ADC conversion. Pretty
> simple stuff.
>
> Of course they can also use a UIO driver to do these kinds of things as
> well.
> As you know, the UIO subsystem will automatically turn of the interrupt
> each time, but users still need to write to the IRQRR register before
> turning the interrupt back on if using edge interrupts.
>
> I always thought what I really need to do is instead just make a
> separate driver like you suggested above because technical, the "IRQ" block is
> not really part of the GIC, it is a separate block that feeds interrupts
> into the GIC just like all the other peripherals, so it should be
> represented as a separate piece of hardware (ie, its own driver and
> capability string).

Right, that makes sense: a small irqchip driver, with 8 inputs, and 8 outputs.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Geert Uytterhoeven]

Hi Chris,

On Thu, Mar 21, 2019 at 11:02 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Wed, Mar 20, 2019 at 9:41 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> > On Wed, Mar 20, 2019, Geert Uytterhoeven wrote:
> > > This patch series adds support for devices on the I2C bus on the RSKRZA1
> > > board, including the GPIO expander that drives 3 LEDs.
> >
> > Very cool.
> >
> > I figured there was a more graceful way of turning on/off those LEDs
> > than what I was doing...which was just writing to the I2C bus in my LED
> > heartbeat kernel thread.
> > https://github.com/renesas-rz/rza_linux-4.19/blob/master/arch/arm/mach-shmobile/board-rskrza1.c#L145
>
> Yeah, I've seen that code. Not fit for upstream ;-)

The proper way is to use that standard heartbeat LED trigger
CONFIG_LEDS_TRIGGER_HEARTBEAT=y, configurable from
sysfs.

For your sample DTS: you can even specify an LED trigger from DT, cfr.
Documentation/devicetree/bindings/leds/common.txt:

    linux,default-trigger = "heartbeat".

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 0/2] ARM: dts: rskrza1: Add I2C and remaining LEDs

[Chris Brandt]

Hi Geert,

On Fri, Mar 22, 2019, Geert Uytterhoeven wrote:
> > Yeah, I've seen that code. Not fit for upstream ;-)
> 
> The proper way is to use that standard heartbeat LED trigger
> CONFIG_LEDS_TRIGGER_HEARTBEAT=y, configurable from
> sysfs.
> 
> For your sample DTS: you can even specify an LED trigger from DT, cfr.
> Documentation/devicetree/bindings/leds/common.txt:
> 
>     linux,default-trigger = "heartbeat"


Wow, I am such a hack when it comes to Linux!

I'm going to add this to my TODO list along with your other suggestion 
of the irqchip driver for the interrupt pins.


Thanks,
Chris