All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support
@ 2021-06-23 15:02 Geert Uytterhoeven
  2021-06-23 15:02 ` [PATCH 1/2] dt-bindings: irqchip: renesas-irqc: Add R-Car V3U support Geert Uytterhoeven
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2021-06-23 15:02 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Kieran Bingham; +Cc: linux-renesas-soc, Geert Uytterhoeven

	Hi all,

This patch series adds support for the Interrupt Controller for External
Devices (INT-EC) in the Renesas R-Car V3U (r8a779a0) SoC.

As there are two known issues, I'm posting this to a limited audience:

  1. External interrupts have not been tested.

     Kieran: perhaps IRQ0 can be tested on Falcon with the MIPI DSI/eDP
     bridge, by changing

	 -    interrupt-parent = <&gpio1>;
	 -    interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
	 +    interrupt-parent = <&intc_ex>;
	 +    interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;

     ? The "ti,sn65dsi86" driver doesn't seem to use interrupts, though,
     so I don't know how feasible this is.

     Alternatively, with physical access, IRQ0 is available on test
     point CP47, and IRQ2 on the GPIO CN.

  2. As recent revisions of R-Car Gen3 Hardware User's Manuals stopped
     documenting module clocks for interrupt controllers, I don't know
     which Module Stop bits control it.  Hence I could not add the intc-ex
     module clock to the R-Car V3U clock driver, but have used the CP
     clock instead in DTS.  Alternatively, as the driver doesn't really
     use the clock (except implicitly through Runtime PM), we can drop
     it, but that would need an update to the DT bindings.

This series been boot-tested on a remote Falcon development board.

Thanks for your comments!

Geert Uytterhoeven (2):
  dt-bindings: irqchip: renesas-irqc: Add R-Car V3U support
  arm64: dts: renesas: r8a779a0: Add INTC-EX device node

 .../interrupt-controller/renesas,irqc.yaml        |  1 +
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi         | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

-- 
2.25.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

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

* [PATCH 1/2] dt-bindings: irqchip: renesas-irqc: Add R-Car V3U support
  2021-06-23 15:02 [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Geert Uytterhoeven
@ 2021-06-23 15:02 ` Geert Uytterhoeven
  2021-06-23 15:02 ` [PATCH 2/2] arm64: dts: renesas: r8a779a0: Add INTC-EX device node Geert Uytterhoeven
  2021-06-23 16:13 ` [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Kieran Bingham
  2 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2021-06-23 15:02 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Kieran Bingham; +Cc: linux-renesas-soc, Geert Uytterhoeven

Document support for the Interrupt Controller for External Devices
(INT-EC) in the Renesas R-Car V3U (r8a779a0) SoC.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 .../devicetree/bindings/interrupt-controller/renesas,irqc.yaml   | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,irqc.yaml
index 79d0358e2f612f24..620f01775e429c3e 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,irqc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,irqc.yaml
@@ -36,6 +36,7 @@ properties:
           - renesas,intc-ex-r8a77980    # R-Car V3H
           - renesas,intc-ex-r8a77990    # R-Car E3
           - renesas,intc-ex-r8a77995    # R-Car D3
+          - renesas,intc-ex-r8a779a0    # R-Car V3U
       - const: renesas,irqc
 
   '#interrupt-cells':
-- 
2.25.1


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

* [PATCH 2/2] arm64: dts: renesas: r8a779a0: Add INTC-EX device node
  2021-06-23 15:02 [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Geert Uytterhoeven
  2021-06-23 15:02 ` [PATCH 1/2] dt-bindings: irqchip: renesas-irqc: Add R-Car V3U support Geert Uytterhoeven
@ 2021-06-23 15:02 ` Geert Uytterhoeven
  2021-06-23 16:13 ` [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Kieran Bingham
  2 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2021-06-23 15:02 UTC (permalink / raw)
  To: Yoshihiro Shimoda, Kieran Bingham; +Cc: linux-renesas-soc, Geert Uytterhoeven

Populate the device node for the Interrupt Controller for External
Devices (INTC-EX) on R-Car V3U, which serves external IRQ pins IRQ[0-5].

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
index 3676b2bd3d646cdb..eeb49a3287bccb2a 100644
--- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
@@ -340,6 +340,21 @@ tsc: thermal@e6190000 {
 			#thermal-sensor-cells = <1>;
 		};
 
+		intc_ex: interrupt-controller@e61c0000 {
+			compatible = "renesas,intc-ex-r8a779a0", "renesas,irqc";
+			#interrupt-cells = <2>;
+			interrupt-controller;
+			reg = <0 0xe61c0000 0 0x200>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_CORE R8A779A0_CLK_CP>;
+			power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>;
+		};
+
 		tmu0: timer@e61e0000 {
 			compatible = "renesas,tmu-r8a779a0", "renesas,tmu";
 			reg = <0 0xe61e0000 0 0x30>;
-- 
2.25.1


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

* Re: [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support
  2021-06-23 15:02 [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Geert Uytterhoeven
  2021-06-23 15:02 ` [PATCH 1/2] dt-bindings: irqchip: renesas-irqc: Add R-Car V3U support Geert Uytterhoeven
  2021-06-23 15:02 ` [PATCH 2/2] arm64: dts: renesas: r8a779a0: Add INTC-EX device node Geert Uytterhoeven
@ 2021-06-23 16:13 ` Kieran Bingham
  2021-06-24  8:07   ` Geert Uytterhoeven
  2 siblings, 1 reply; 8+ messages in thread
From: Kieran Bingham @ 2021-06-23 16:13 UTC (permalink / raw)
  To: Geert Uytterhoeven, Yoshihiro Shimoda, Kieran Bingham; +Cc: linux-renesas-soc

Hi Geert,

On 23/06/2021 16:02, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This patch series adds support for the Interrupt Controller for External
> Devices (INT-EC) in the Renesas R-Car V3U (r8a779a0) SoC.
> 
> As there are two known issues, I'm posting this to a limited audience:
> 
>   1. External interrupts have not been tested.
> 
>      Kieran: perhaps IRQ0 can be tested on Falcon with the MIPI DSI/eDP
>      bridge, by changing
> 
> 	 -    interrupt-parent = <&gpio1>;
> 	 -    interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
> 	 +    interrupt-parent = <&intc_ex>;
> 	 +    interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> 
>      ? The "ti,sn65dsi86" driver doesn't seem to use interrupts, though,
>      so I don't know how feasible this is.

I can add an interrupt handler if that's what you need, but I suspect
that the change here simply 're-routes' the interrupt through the
intc_ex so that it still needs an interrupt to be generated by the
SN65DSI86? is that right?



>      Alternatively, with physical access, IRQ0 is available on test
>      point CP47, and IRQ2 on the GPIO CN.

I do have physical access, so I can trigger this - Is there a suitable
voltage or condition I can apply? (I.e. take a signal from a nearby pin
to short it?)

--
Kieran



>   2. As recent revisions of R-Car Gen3 Hardware User's Manuals stopped
>      documenting module clocks for interrupt controllers, I don't know
>      which Module Stop bits control it.  Hence I could not add the intc-ex
>      module clock to the R-Car V3U clock driver, but have used the CP
>      clock instead in DTS.  Alternatively, as the driver doesn't really
>      use the clock (except implicitly through Runtime PM), we can drop
>      it, but that would need an update to the DT bindings.
> 
> This series been boot-tested on a remote Falcon development board.
> 
> Thanks for your comments!
> 
> Geert Uytterhoeven (2):
>   dt-bindings: irqchip: renesas-irqc: Add R-Car V3U support
>   arm64: dts: renesas: r8a779a0: Add INTC-EX device node
> 
>  .../interrupt-controller/renesas,irqc.yaml        |  1 +
>  arch/arm64/boot/dts/renesas/r8a779a0.dtsi         | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 

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

* Re: [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support
  2021-06-23 16:13 ` [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Kieran Bingham
@ 2021-06-24  8:07   ` Geert Uytterhoeven
  2021-09-22 14:14     ` Kieran Bingham
  2022-01-05 22:27     ` Kieran Bingham
  0 siblings, 2 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2021-06-24  8:07 UTC (permalink / raw)
  To: Kieran Bingham; +Cc: Yoshihiro Shimoda, Kieran Bingham, Linux-Renesas

Hi Kieran,

On Wed, Jun 23, 2021 at 6:13 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
> On 23/06/2021 16:02, Geert Uytterhoeven wrote:
> > This patch series adds support for the Interrupt Controller for External
> > Devices (INT-EC) in the Renesas R-Car V3U (r8a779a0) SoC.
> >
> > As there are two known issues, I'm posting this to a limited audience:
> >
> >   1. External interrupts have not been tested.
> >
> >      Kieran: perhaps IRQ0 can be tested on Falcon with the MIPI DSI/eDP
> >      bridge, by changing
> >
> >        -    interrupt-parent = <&gpio1>;
> >        -    interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
> >        +    interrupt-parent = <&intc_ex>;
> >        +    interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> >
> >      ? The "ti,sn65dsi86" driver doesn't seem to use interrupts, though,
> >      so I don't know how feasible this is.
>
> I can add an interrupt handler if that's what you need, but I suspect
> that the change here simply 're-routes' the interrupt through the
> intc_ex so that it still needs an interrupt to be generated by the
> SN65DSI86? is that right?

Correct, you need to make the SN65DSI86 generate an interrupt, too.
No idea how to do that...

> >      Alternatively, with physical access, IRQ0 is available on test
> >      point CP47, and IRQ2 on the GPIO CN.
>
> I do have physical access, so I can trigger this - Is there a suitable
> voltage or condition I can apply? (I.e. take a signal from a nearby pin
> to short it?)

As IRQ0 is driven by the single gate U59, you better don't cause logic
conflicts, and play with IRQ2 instead.

Note that high level is SPI_D1.8V/3.3V, which is 1.8V by default!
The GPIO CN connector carries a.o. SPI_D1.8V/3.3V and GND.
Internal pull-up should be enabled for IRQ2 by reset state, but you
may want to measure the pin's voltage to be sure.

To configure pin control, you need to add the following, and hook it
up to the pfc node:

        irq2_pins: irq2 {
                groups = "intc_ex_irq2";
                function = "intc_ex";
        };

You should be able to test this using gpio-keys, with a key subnode that
has an interrupts instead of a gpios property.

This might be a good opportunity to wire up the slide and push switches
(SW46-49) as gpio-keys, too...

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

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

* Re: [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support
  2021-06-24  8:07   ` Geert Uytterhoeven
@ 2021-09-22 14:14     ` Kieran Bingham
  2021-09-22 17:33       ` Geert Uytterhoeven
  2022-01-05 22:27     ` Kieran Bingham
  1 sibling, 1 reply; 8+ messages in thread
From: Kieran Bingham @ 2021-09-22 14:14 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Yoshihiro Shimoda, Kieran Bingham, Linux-Renesas

Hi Geert,

I've been trying to test this on the Falcon-V3U


On 24/06/2021 09:07, Geert Uytterhoeven wrote:
> Hi Kieran,
> 
> On Wed, Jun 23, 2021 at 6:13 PM Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
>> On 23/06/2021 16:02, Geert Uytterhoeven wrote:
>>> This patch series adds support for the Interrupt Controller for External
>>> Devices (INT-EC) in the Renesas R-Car V3U (r8a779a0) SoC.
>>>
>>> As there are two known issues, I'm posting this to a limited audience:
>>>
>>>   1. External interrupts have not been tested.
>>>
>>>      Kieran: perhaps IRQ0 can be tested on Falcon with the MIPI DSI/eDP
>>>      bridge, by changing
>>>
>>>        -    interrupt-parent = <&gpio1>;
>>>        -    interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
>>>        +    interrupt-parent = <&intc_ex>;
>>>        +    interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
>>>
>>>      ? The "ti,sn65dsi86" driver doesn't seem to use interrupts, though,
>>>      so I don't know how feasible this is.
>>
>> I can add an interrupt handler if that's what you need, but I suspect
>> that the change here simply 're-routes' the interrupt through the
>> intc_ex so that it still needs an interrupt to be generated by the
>> SN65DSI86? is that right?
> 
> Correct, you need to make the SN65DSI86 generate an interrupt, too.
> No idea how to do that...

I'm going to ignore the SN65DSI86, and go with a route that I can
explicitly control ... so IRQ2 it is.



> 
>>>      Alternatively, with physical access, IRQ0 is available on test
>>>      point CP47, and IRQ2 on the GPIO CN.
>>
>> I do have physical access, so I can trigger this - Is there a suitable
>> voltage or condition I can apply? (I.e. take a signal from a nearby pin
>> to short it?)
> 
> As IRQ0 is driven by the single gate U59, you better don't cause logic
> conflicts, and play with IRQ2 instead.
> 
> Note that high level is SPI_D1.8V/3.3V, which is 1.8V by default!
> The GPIO CN connector carries a.o. SPI_D1.8V/3.3V and GND.
> Internal pull-up should be enabled for IRQ2 by reset state, but you
> may want to measure the pin's voltage to be sure.

Pin7 appears to be IRQ2. It is reading at 1.8v.
Pin 5 (SPI_D1.8v/3.3v) is reading at 1.8v
Pin 3 (D3.3v) no prizes for guessing here.

And of course pin 1 is ground.

So I have some wires I can play with.


> To configure pin control, you need to add the following, and hook it
> up to the pfc node:
> 
>         irq2_pins: irq2 {
>                 groups = "intc_ex_irq2";
>                 function = "intc_ex";
>         };
> 
> You should be able to test this using gpio-keys, with a key subnode that
> has an interrupts instead of a gpios property.


I'm afraid I haven't been able to successfully test this. I have this
series applied and have tried the following:



diff --git a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
index dc671ff57ec7..1286b553e370 100644
--- a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
+++ b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
@@ -10,6 +10,10 @@
 #include "r8a779a0-falcon-csi-dsi.dtsi"
 #include "r8a779a0-falcon-ethernet.dtsi"

+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/gpio-keys.h>
+#include <dt-bindings/input/input.h>
+
 / {
 	model = "Renesas Falcon CPU and Breakout boards based on r8a779a0";
 	compatible = "renesas,falcon-breakout", "renesas,falcon-cpu",
"renesas,r8a779a0";
@@ -17,6 +21,23 @@ / {
 	aliases {
 		ethernet0 = &avb0;
 	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+
+		btn1 {
+			pinctrl-0 = <&irq2_pins>;
+			pinctrl-names = "default";
+
+			debounce-interval = <50>;
+			label = "button1";
+			linux,code = <KEY_1>;
+			interrupt-parent = <&intc_ex>;
+			interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+
+			//gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
+		};
+	};
 };

 &avb0 {
@@ -45,6 +66,14 @@ eeprom@51 {
 };

 &pfc {
+	// Intc_ex testing
+	irq2_pins: irq2 {
+		groups = "intc_ex_irq2";
+		function = "intc_ex";
+
+		bias-pull-up;
+	};
+
 	avb0_pins: avb0 {
 		mux {
 			groups = "avb0_link", "avb0_mdio", "avb0_rgmii",
diff --git a/drivers/pinctrl/renesas/core.c b/drivers/pinctrl/renesas/core.c
index ef8ef05ba930..966883c6c64c 100644
--- a/drivers/pinctrl/renesas/core.c
+++ b/drivers/pinctrl/renesas/core.c
@@ -228,7 +228,7 @@ static void sh_pfc_write_config_reg(struct sh_pfc *pfc,

 	sh_pfc_config_reg_helper(pfc, crp, field, &mapped_reg, &mask, &pos);

-	dev_dbg(pfc->dev, "write_reg addr = %x, value = 0x%x, field = %u, "
+	dev_err(pfc->dev, "KB: write_reg addr = %x, value = 0x%x, field = %u, "
 		"r_width = %u, f_width = %u\n",
 		crp->reg, value, field, crp->reg_width, hweight32(mask));





And I have ribbon cable with the pitch for cn4 now which allows me to
connect pin 7 to pin 1 to ground it.


I use
 sudo evtest /dev/input/event0

to monitor the line for changes, and grounding pin7 has no effect in
this configuration.


However, to try to sanity check my test, I changed the gpio-keys to use
gpio1 26 directly - disabling the pinctrl, and using the gpios reference
directly instead :


diff --git a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
index 1286b553e370..af85881de2c4 100644
--- a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
+++ b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
@@ -26,16 +26,11 @@ gpio_keys {
                compatible = "gpio-keys";

                btn1 {
-                       pinctrl-0 = <&irq2_pins>;
-                       pinctrl-names = "default";
-
                        debounce-interval = <50>;
                        label = "button1";
                        linux,code = <KEY_1>;
-                       interrupt-parent = <&intc_ex>;
-                       interrupts = <2 IRQ_TYPE_LEVEL_LOW>;

-                       //gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
+                       gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
                };
        };
 };


This shows active key events when grounding pin 7 to pin 1...
> kbingham@falcon-v3u:~$ sudo evtest /dev/input/event0
> Input driver version is 1.0.1
> Input device ID: bus 0x19 vendor 0x1 product 0x1 version 0x100
> Input device name: "gpio_keys"
> Supported events:
>   Event type 0 (EV_SYN)
>   Event type 1 (EV_KEY)
>     Event code 2 (KEY_1)
> Properties:
> Testing ... (interrupt to exit)
> Event: time 1632319971.126234, type 1 (EV_KEY), code 2 (KEY_1), value 1
> Event: time 1632319971.126234, -------------- SYN_REPORT ------------
> Event: time 1632319971.579966, type 1 (EV_KEY), code 2 (KEY_1), value 0
> Event: time 1632319971.579966, -------------- SYN_REPORT ------------
> Event: time 1632319981.461018, type 1 (EV_KEY), code 2 (KEY_1), value 1
> Event: time 1632319981.461018, -------------- SYN_REPORT ------------
> Event: time 1632319981.835693, type 1 (EV_KEY), code 2 (KEY_1), value 0
> Event: time 1632319981.835693, -------------- SYN_REPORT ------------
> Event: time 1632319982.112104, type 1 (EV_KEY), code 2 (KEY_1), value 1
> Event: time 1632319982.112104, -------------- SYN_REPORT ------------



If there's anything else you'd like me to test or change let me know.

--
Kieran





> 
> This might be a good opportunity to wire up the slide and push switches
> (SW46-49) as gpio-keys, too...
> 
> 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
> 

-- 
--
Kieran

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

* Re: [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support
  2021-09-22 14:14     ` Kieran Bingham
@ 2021-09-22 17:33       ` Geert Uytterhoeven
  0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2021-09-22 17:33 UTC (permalink / raw)
  To: Kieran Bingham; +Cc: Yoshihiro Shimoda, Linux-Renesas

Hi Kieran,

On Wed, Sep 22, 2021 at 4:14 PM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> I've been trying to test this on the Falcon-V3U

Thanks for testing!

> On 24/06/2021 09:07, Geert Uytterhoeven wrote:
> > On Wed, Jun 23, 2021 at 6:13 PM Kieran Bingham
> > <kieran.bingham@ideasonboard.com> wrote:
> >> On 23/06/2021 16:02, Geert Uytterhoeven wrote:
> >>> This patch series adds support for the Interrupt Controller for External
> >>> Devices (INT-EC) in the Renesas R-Car V3U (r8a779a0) SoC.
> >>>
> >>> As there are two known issues, I'm posting this to a limited audience:
> >>>
> >>>   1. External interrupts have not been tested.

> >>>      Alternatively, with physical access, IRQ0 is available on test
> >>>      point CP47, and IRQ2 on the GPIO CN.
> >>
> >> I do have physical access, so I can trigger this - Is there a suitable
> >> voltage or condition I can apply? (I.e. take a signal from a nearby pin
> >> to short it?)
> >
> > As IRQ0 is driven by the single gate U59, you better don't cause logic
> > conflicts, and play with IRQ2 instead.
> >
> > Note that high level is SPI_D1.8V/3.3V, which is 1.8V by default!
> > The GPIO CN connector carries a.o. SPI_D1.8V/3.3V and GND.
> > Internal pull-up should be enabled for IRQ2 by reset state, but you
> > may want to measure the pin's voltage to be sure.
>
> Pin7 appears to be IRQ2. It is reading at 1.8v.
> Pin 5 (SPI_D1.8v/3.3v) is reading at 1.8v
> Pin 3 (D3.3v) no prizes for guessing here.
>
> And of course pin 1 is ground.
>
> So I have some wires I can play with.
>
> > To configure pin control, you need to add the following, and hook it
> > up to the pfc node:
> >
> >         irq2_pins: irq2 {
> >                 groups = "intc_ex_irq2";
> >                 function = "intc_ex";
> >         };
> >
> > You should be able to test this using gpio-keys, with a key subnode that
> > has an interrupts instead of a gpios property.
>
> I'm afraid I haven't been able to successfully test this. I have this
> series applied and have tried the following:

> --- a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
> @@ -10,6 +10,10 @@
>  #include "r8a779a0-falcon-csi-dsi.dtsi"
>  #include "r8a779a0-falcon-ethernet.dtsi"
>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/gpio-keys.h>
> +#include <dt-bindings/input/input.h>
> +
>  / {
>         model = "Renesas Falcon CPU and Breakout boards based on r8a779a0";
>         compatible = "renesas,falcon-breakout", "renesas,falcon-cpu",
> "renesas,r8a779a0";
> @@ -17,6 +21,23 @@ / {
>         aliases {
>                 ethernet0 = &avb0;
>         };
> +
> +       gpio_keys {
> +               compatible = "gpio-keys";
> +
> +               btn1 {
> +                       pinctrl-0 = <&irq2_pins>;
> +                       pinctrl-names = "default";
> +
> +                       debounce-interval = <50>;
> +                       label = "button1";
> +                       linux,code = <KEY_1>;
> +                       interrupt-parent = <&intc_ex>;
> +                       interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> +
> +                       //gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
> +               };
> +       };
>  };
>
>  &avb0 {
> @@ -45,6 +66,14 @@ eeprom@51 {
>  };
>
>  &pfc {
> +       // Intc_ex testing
> +       irq2_pins: irq2 {
> +               groups = "intc_ex_irq2";
> +               function = "intc_ex";
> +
> +               bias-pull-up;
> +       };
> +
>         avb0_pins: avb0 {
>                 mux {
>                         groups = "avb0_link", "avb0_mdio", "avb0_rgmii",

Looks good to me.

> diff --git a/drivers/pinctrl/renesas/core.c b/drivers/pinctrl/renesas/core.c
> index ef8ef05ba930..966883c6c64c 100644
> --- a/drivers/pinctrl/renesas/core.c
> +++ b/drivers/pinctrl/renesas/core.c
> @@ -228,7 +228,7 @@ static void sh_pfc_write_config_reg(struct sh_pfc *pfc,
>
>         sh_pfc_config_reg_helper(pfc, crp, field, &mapped_reg, &mask, &pos);
>
> -       dev_dbg(pfc->dev, "write_reg addr = %x, value = 0x%x, field = %u, "
> +       dev_err(pfc->dev, "KB: write_reg addr = %x, value = 0x%x, field = %u, "
>                 "r_width = %u, f_width = %u\n",
>                 crp->reg, value, field, crp->reg_width, hweight32(mask));
>
> And I have ribbon cable with the pitch for cn4 now which allows me to
> connect pin 7 to pin 1 to ground it.
>
>
> I use
>  sudo evtest /dev/input/event0
>
> to monitor the line for changes, and grounding pin7 has no effect in
> this configuration.
>
>
> However, to try to sanity check my test, I changed the gpio-keys to use
> gpio1 26 directly - disabling the pinctrl, and using the gpios reference
> directly instead :
>
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
> b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
> index 1286b553e370..af85881de2c4 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts
> @@ -26,16 +26,11 @@ gpio_keys {
>                 compatible = "gpio-keys";
>
>                 btn1 {
> -                       pinctrl-0 = <&irq2_pins>;
> -                       pinctrl-names = "default";
> -
>                         debounce-interval = <50>;
>                         label = "button1";
>                         linux,code = <KEY_1>;
> -                       interrupt-parent = <&intc_ex>;
> -                       interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
>
> -                       //gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
> +                       gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
>                 };
>         };
>  };

Looks almost good to me.
You probably still want to enable bias-pull-up for GP1_26, but as
it works without...

> This shows active key events when grounding pin 7 to pin 1...
> > kbingham@falcon-v3u:~$ sudo evtest /dev/input/event0
> > Input driver version is 1.0.1
> > Input device ID: bus 0x19 vendor 0x1 product 0x1 version 0x100
> > Input device name: "gpio_keys"
> > Supported events:
> >   Event type 0 (EV_SYN)
> >   Event type 1 (EV_KEY)
> >     Event code 2 (KEY_1)
> > Properties:
> > Testing ... (interrupt to exit)
> > Event: time 1632319971.126234, type 1 (EV_KEY), code 2 (KEY_1), value 1
> > Event: time 1632319971.126234, -------------- SYN_REPORT ------------
> > Event: time 1632319971.579966, type 1 (EV_KEY), code 2 (KEY_1), value 0
> > Event: time 1632319971.579966, -------------- SYN_REPORT ------------
> > Event: time 1632319981.461018, type 1 (EV_KEY), code 2 (KEY_1), value 1
> > Event: time 1632319981.461018, -------------- SYN_REPORT ------------
> > Event: time 1632319981.835693, type 1 (EV_KEY), code 2 (KEY_1), value 0
> > Event: time 1632319981.835693, -------------- SYN_REPORT ------------
> > Event: time 1632319982.112104, type 1 (EV_KEY), code 2 (KEY_1), value 1
> > Event: time 1632319982.112104, -------------- SYN_REPORT ------------

Good!

> If there's anything else you'd like me to test or change let me know.

There are still several things that could be wrong:
  - Bug in the pin control tables,
  - Wrong parent interrupt description,
  - Undocumented INTC-EX module clock is turned off
    (does it retain register values?),
  - Wrong initial values in INTC-EX registers the driver doesn't touch,
  - ...

Can you print the value of the INTC-EX MONITOR register?
It should match the state of the external pins.

> > This might be a good opportunity to wire up the slide and push switches
> > (SW46-49) as gpio-keys, too...

Any chance with these? ;-)

Thanks again!

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

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

* Re: [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support
  2021-06-24  8:07   ` Geert Uytterhoeven
  2021-09-22 14:14     ` Kieran Bingham
@ 2022-01-05 22:27     ` Kieran Bingham
  1 sibling, 0 replies; 8+ messages in thread
From: Kieran Bingham @ 2022-01-05 22:27 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Yoshihiro Shimoda, Linux-Renesas

Hi Geert,

Quoting Geert Uytterhoeven (2021-06-24 09:07:30)
> Hi Kieran,
> 
> On Wed, Jun 23, 2021 at 6:13 PM Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
> > On 23/06/2021 16:02, Geert Uytterhoeven wrote:
> > > This patch series adds support for the Interrupt Controller for External
> > > Devices (INT-EC) in the Renesas R-Car V3U (r8a779a0) SoC.
> > >
> > > As there are two known issues, I'm posting this to a limited audience:
> > >
> > >   1. External interrupts have not been tested.
> > >
> > >      Kieran: perhaps IRQ0 can be tested on Falcon with the MIPI DSI/eDP
> > >      bridge, by changing
> > >
> > >        -    interrupt-parent = <&gpio1>;
> > >        -    interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
> > >        +    interrupt-parent = <&intc_ex>;
> > >        +    interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > >
> > >      ? The "ti,sn65dsi86" driver doesn't seem to use interrupts, though,
> > >      so I don't know how feasible this is.
> >
> > I can add an interrupt handler if that's what you need, but I suspect
> > that the change here simply 're-routes' the interrupt through the
> > intc_ex so that it still needs an interrupt to be generated by the
> > SN65DSI86? is that right?
> 
> Correct, you need to make the SN65DSI86 generate an interrupt, too.
> No idea how to do that...

   o/ Oh oh I do I do ... (now).

I've added a handler, enabled all interrupts on the device and given
myself an interrupt storm:

 95:       3203      irqc   0 Level     sn65dsi86-irq

So I believe I can now (finally) add a 

Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

here.

And I'll continue working on the interrupt handling for the SN65DSI86 to
see if I can add hotplug detection over interrupts.



> 
> > >      Alternatively, with physical access, IRQ0 is available on test
> > >      point CP47, and IRQ2 on the GPIO CN.
> >
> > I do have physical access, so I can trigger this - Is there a suitable
> > voltage or condition I can apply? (I.e. take a signal from a nearby pin
> > to short it?)
> 
> As IRQ0 is driven by the single gate U59, you better don't cause logic
> conflicts, and play with IRQ2 instead.
> 
> Note that high level is SPI_D1.8V/3.3V, which is 1.8V by default!
> The GPIO CN connector carries a.o. SPI_D1.8V/3.3V and GND.
> Internal pull-up should be enabled for IRQ2 by reset state, but you
> may want to measure the pin's voltage to be sure.
> 
> To configure pin control, you need to add the following, and hook it
> up to the pfc node:
> 
>         irq2_pins: irq2 {
>                 groups = "intc_ex_irq2";
>                 function = "intc_ex";
>         };
> 
> You should be able to test this using gpio-keys, with a key subnode that
> has an interrupts instead of a gpios property.
> 
> This might be a good opportunity to wire up the slide and push switches
> (SW46-49) as gpio-keys, too...
> 
> 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

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

end of thread, other threads:[~2022-01-05 22:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23 15:02 [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Geert Uytterhoeven
2021-06-23 15:02 ` [PATCH 1/2] dt-bindings: irqchip: renesas-irqc: Add R-Car V3U support Geert Uytterhoeven
2021-06-23 15:02 ` [PATCH 2/2] arm64: dts: renesas: r8a779a0: Add INTC-EX device node Geert Uytterhoeven
2021-06-23 16:13 ` [PATCH 0/2] arm64: dts: renesas: r8a779a0: Add INTC-EX support Kieran Bingham
2021-06-24  8:07   ` Geert Uytterhoeven
2021-09-22 14:14     ` Kieran Bingham
2021-09-22 17:33       ` Geert Uytterhoeven
2022-01-05 22:27     ` Kieran Bingham

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.