[PATCH/RFC v2 0/2] R-Car V3U GPIO support

[PATCH/RFC v2 0/2] R-Car V3U GPIO support

[Geert Uytterhoeven]

	Hi all,

This RFC patch series adds support for the GPIO blocks on the R-Car V3U
(r8a77990) SoC.  As this is an RFC, I didn't include patches from v1[1]
that have not changed.  This series depends on [2].

Compared to v1 and the BSP, v2 takes a different approach to handle the
new General Input Register (INDT): instead of programming this register
during GPIO line configuration and depending on GPIO line direction, v2
enables input unconditionally in probe and resume, so the driver can
read pin state at all times.

Due to lack of hardware, the second patch is compile-tested only.

Thanks for your comments!

[1] "[PATCH/RFC 0/6] R-Car V3U GPIO support"
    https://lore.kernel.org/linux-renesas-soc/20201019120614.22149-1-geert+renesas@glider.be/
[2] "[PATCH 0/4] gpio: rcar: Cleanups and improvements"
    https://lore.kernel.org/linux-renesas-soc/20201028141504.1729093-1-geert+renesas@glider.be/

Geert Uytterhoeven (2):
  gpio: rcar: Optimize GPIO pin state read on R-Car Gen3
  gpio: rcar: Add R-Car V3U (R8A7799A) support

 drivers/gpio/gpio-rcar.c | 64 ++++++++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 9 deletions(-)

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

[PATCH/RFC v2 1/2] gpio: rcar: Optimize GPIO pin state read on R-Car Gen3

[Geert Uytterhoeven]

Currently, the R-Car GPIO driver treats R-Car Gen2 and R-Car Gen3 GPIO
controllers the same.  However, there exist small differences, like the
behavior of the General Input Register (INDT):
  - On R-Car Gen1, R-Car Gen2, and RZ/G1, INDT only reflects the state
    of an input pin if the GPIO is configured for input,
  - On R-Car Gen3 and RZ/G2, INDT always reflects the state of the input
    pins.
Hence to accommodate all variants, the driver does not use the INDT
register to read the status of a GPIO line when configured for output,
at the expense of doing 2 or 3 register reads instead of 1.

Given register accesses are slow, change the .get() and .get_multiple()
callbacks to always use INDT to read pin state on SoCs where this is
supported.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpio/gpio-rcar.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 0b572dbc4a36801d..cbe9257fff9fc50f 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -35,6 +35,7 @@ struct gpio_rcar_bank_info {
 struct gpio_rcar_info {
 	bool has_outdtsel;
 	bool has_both_edge_trigger;
+	bool has_always_in;
 };
 
 struct gpio_rcar_priv {
@@ -302,9 +303,9 @@ static int gpio_rcar_get(struct gpio_chip *chip, unsigned offset)
 	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
 	u32 bit = BIT(offset);
 
-	/* testing on r8a7790 shows that INDT does not show correct pin state
-	 * when configured as output, so use OUTDT in case of output pins */
-	if (gpio_rcar_read(p, INOUTSEL) & bit)
+	/* Before R-Car Gen3, INDT does not show correct pin state when
+	 * configured as output, so use OUTDT in case of output pins */
+	if (!p->info.has_always_in && (gpio_rcar_read(p, INOUTSEL) & bit))
 		return !!(gpio_rcar_read(p, OUTDT) & bit);
 	else
 		return !!(gpio_rcar_read(p, INDT) & bit);
@@ -324,6 +325,11 @@ static int gpio_rcar_get_multiple(struct gpio_chip *chip, unsigned long *mask,
 	if (!bankmask)
 		return 0;
 
+	if (p->info.has_always_in) {
+		bits[0] = gpio_rcar_read(p, INDT) & bankmask;
+		return 0;
+	}
+
 	spin_lock_irqsave(&p->lock, flags);
 	outputs = gpio_rcar_read(p, INOUTSEL);
 	m = outputs & bankmask;
@@ -383,11 +389,19 @@ static int gpio_rcar_direction_output(struct gpio_chip *chip, unsigned offset,
 static const struct gpio_rcar_info gpio_rcar_info_gen1 = {
 	.has_outdtsel = false,
 	.has_both_edge_trigger = false,
+	.has_always_in = false,
 };
 
 static const struct gpio_rcar_info gpio_rcar_info_gen2 = {
 	.has_outdtsel = true,
 	.has_both_edge_trigger = true,
+	.has_always_in = false,
+};
+
+static const struct gpio_rcar_info gpio_rcar_info_gen3 = {
+	.has_outdtsel = true,
+	.has_both_edge_trigger = true,
+	.has_always_in = true,
 };
 
 static const struct of_device_id gpio_rcar_of_table[] = {
@@ -412,12 +426,10 @@ static const struct of_device_id gpio_rcar_of_table[] = {
 		.data = &gpio_rcar_info_gen2,
 	}, {
 		.compatible = "renesas,gpio-r8a7795",
-		/* Gen3 GPIO is identical to Gen2. */
-		.data = &gpio_rcar_info_gen2,
+		.data = &gpio_rcar_info_gen3,
 	}, {
 		.compatible = "renesas,gpio-r8a7796",
-		/* Gen3 GPIO is identical to Gen2. */
-		.data = &gpio_rcar_info_gen2,
+		.data = &gpio_rcar_info_gen3,
 	}, {
 		.compatible = "renesas,rcar-gen1-gpio",
 		.data = &gpio_rcar_info_gen1,
@@ -426,8 +438,7 @@ static const struct of_device_id gpio_rcar_of_table[] = {
 		.data = &gpio_rcar_info_gen2,
 	}, {
 		.compatible = "renesas,rcar-gen3-gpio",
-		/* Gen3 GPIO is identical to Gen2. */
-		.data = &gpio_rcar_info_gen2,
+		.data = &gpio_rcar_info_gen3,
 	}, {
 		.compatible = "renesas,gpio-rcar",
 		.data = &gpio_rcar_info_gen1,
-- 
2.25.1

RE: [PATCH/RFC v2 1/2] gpio: rcar: Optimize GPIO pin state read on R-Car Gen3

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for the patch!

> From: Geert Uytterhoeven, Sent: Friday, October 30, 2020 5:15 PM
> 
> Currently, the R-Car GPIO driver treats R-Car Gen2 and R-Car Gen3 GPIO
> controllers the same.  However, there exist small differences, like the
> behavior of the General Input Register (INDT):
>   - On R-Car Gen1, R-Car Gen2, and RZ/G1, INDT only reflects the state
>     of an input pin if the GPIO is configured for input,
>   - On R-Car Gen3 and RZ/G2, INDT always reflects the state of the input
>     pins.
> Hence to accommodate all variants, the driver does not use the INDT
> register to read the status of a GPIO line when configured for output,
> at the expense of doing 2 or 3 register reads instead of 1.
> 
> Given register accesses are slow, change the .get() and .get_multiple()
> callbacks to always use INDT to read pin state on SoCs where this is
> supported.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
<snip>
> @@ -302,9 +303,9 @@ static int gpio_rcar_get(struct gpio_chip *chip, unsigned offset)
>  	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
>  	u32 bit = BIT(offset);
> 
> -	/* testing on r8a7790 shows that INDT does not show correct pin state
> -	 * when configured as output, so use OUTDT in case of output pins */
> -	if (gpio_rcar_read(p, INOUTSEL) & bit)
> +	/* Before R-Car Gen3, INDT does not show correct pin state when
> +	 * configured as output, so use OUTDT in case of output pins */

nit: checkpatch.pl said warning as the following.
---
WARNING: Block comments use a trailing */ on a separate line
#46: FILE: drivers/gpio/gpio-rcar.c:307:
+        * configured as output, so use OUTDT in case of output pins */

total: 0 errors, 1 warnings, 72 lines checked
---

Regardless:

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda

Re: [PATCH/RFC v2 1/2] gpio: rcar: Optimize GPIO pin state read on R-Car Gen3

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, Nov 4, 2020 at 6:23 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Friday, October 30, 2020 5:15 PM
> > Currently, the R-Car GPIO driver treats R-Car Gen2 and R-Car Gen3 GPIO
> > controllers the same.  However, there exist small differences, like the
> > behavior of the General Input Register (INDT):
> >   - On R-Car Gen1, R-Car Gen2, and RZ/G1, INDT only reflects the state
> >     of an input pin if the GPIO is configured for input,
> >   - On R-Car Gen3 and RZ/G2, INDT always reflects the state of the input
> >     pins.
> > Hence to accommodate all variants, the driver does not use the INDT
> > register to read the status of a GPIO line when configured for output,
> > at the expense of doing 2 or 3 register reads instead of 1.
> >
> > Given register accesses are slow, change the .get() and .get_multiple()
> > callbacks to always use INDT to read pin state on SoCs where this is
> > supported.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> <snip>
> > @@ -302,9 +303,9 @@ static int gpio_rcar_get(struct gpio_chip *chip, unsigned offset)
> >       struct gpio_rcar_priv *p = gpiochip_get_data(chip);
> >       u32 bit = BIT(offset);
> >
> > -     /* testing on r8a7790 shows that INDT does not show correct pin state
> > -      * when configured as output, so use OUTDT in case of output pins */
> > -     if (gpio_rcar_read(p, INOUTSEL) & bit)
> > +     /* Before R-Car Gen3, INDT does not show correct pin state when
> > +      * configured as output, so use OUTDT in case of output pins */
>
> nit: checkpatch.pl said warning as the following.
> ---
> WARNING: Block comments use a trailing */ on a separate line
> #46: FILE: drivers/gpio/gpio-rcar.c:307:
> +        * configured as output, so use OUTDT in case of output pins */
>
> total: 0 errors, 1 warnings, 72 lines checked

That's a pre-existing issue. But it might be the right time to fix that ;-)
Will do.

> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

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/RFC v2 1/2] gpio: rcar: Optimize GPIO pin state read on R-Car Gen3

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

On Fri, Oct 30, 2020 at 09:15:21AM +0100, Geert Uytterhoeven wrote:
> Currently, the R-Car GPIO driver treats R-Car Gen2 and R-Car Gen3 GPIO
> controllers the same.  However, there exist small differences, like the
> behavior of the General Input Register (INDT):
>   - On R-Car Gen1, R-Car Gen2, and RZ/G1, INDT only reflects the state
>     of an input pin if the GPIO is configured for input,
>   - On R-Car Gen3 and RZ/G2, INDT always reflects the state of the input
>     pins.
> Hence to accommodate all variants, the driver does not use the INDT
> register to read the status of a GPIO line when configured for output,
> at the expense of doing 2 or 3 register reads instead of 1.
> 
> Given register accesses are slow, change the .get() and .get_multiple()
> callbacks to always use INDT to read pin state on SoCs where this is
> supported.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH/RFC v2 2/2] gpio: rcar: Add R-Car V3U (R8A7799A) support

[Geert Uytterhoeven]

Add support for the GPIO controller block in the R-Car V3U (R8A779A0)
SoC, which is very similar to the block found on other R-Car Gen3 SoCs.
However, this block has a new General Input Enable Register (INEN),
whose reset state is to have all inputs disabled.

Enable input for all available pins in probe and resume, to support the
use of the General Input Register (INDT) for reading pin state at all
times.  This preserves backwards compatibility with other R-Car Gen3
SoCs, as recommended by the Hardware Manual.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Untested on actual hardware.

v2:
  - Enable input unconditionally in probe and resume, instead of during
    GPIO line configuration and depending on GPIO line direction,
  - Assumed authorship, as this patch is very different from v1, written
    by Phong Hoang.
---
 drivers/gpio/gpio-rcar.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index cbe9257fff9fc50f..edf67fc9c54bd648 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -36,6 +36,7 @@ struct gpio_rcar_info {
 	bool has_outdtsel;
 	bool has_both_edge_trigger;
 	bool has_always_in;
+	bool has_inen;
 };
 
 struct gpio_rcar_priv {
@@ -63,6 +64,7 @@ struct gpio_rcar_priv {
 #define FILONOFF	0x28	/* Chattering Prevention On/Off Register */
 #define OUTDTSEL	0x40	/* Output Data Select Register */
 #define BOTHEDGE	0x4c	/* One Edge/Both Edge Select Register */
+#define INEN		0x50	/* General Input Enable Register */
 
 #define RCAR_MAX_GPIO_PER_BANK		32
 
@@ -390,18 +392,28 @@ static const struct gpio_rcar_info gpio_rcar_info_gen1 = {
 	.has_outdtsel = false,
 	.has_both_edge_trigger = false,
 	.has_always_in = false,
+	.has_inen = false,
 };
 
 static const struct gpio_rcar_info gpio_rcar_info_gen2 = {
 	.has_outdtsel = true,
 	.has_both_edge_trigger = true,
 	.has_always_in = false,
+	.has_inen = false,
 };
 
 static const struct gpio_rcar_info gpio_rcar_info_gen3 = {
 	.has_outdtsel = true,
 	.has_both_edge_trigger = true,
 	.has_always_in = true,
+	.has_inen = false,
+};
+
+static const struct gpio_rcar_info gpio_rcar_info_v3u = {
+	.has_outdtsel = true,
+	.has_both_edge_trigger = true,
+	.has_always_in = true,
+	.has_inen = true,
 };
 
 static const struct of_device_id gpio_rcar_of_table[] = {
@@ -430,6 +442,9 @@ static const struct of_device_id gpio_rcar_of_table[] = {
 	}, {
 		.compatible = "renesas,gpio-r8a7796",
 		.data = &gpio_rcar_info_gen3,
+	}, {
+		.compatible = "renesas,gpio-r8a779a0",
+		.data = &gpio_rcar_info_v3u,
 	}, {
 		.compatible = "renesas,rcar-gen1-gpio",
 		.data = &gpio_rcar_info_gen1,
@@ -471,6 +486,17 @@ static int gpio_rcar_parse_dt(struct gpio_rcar_priv *p, unsigned int *npins)
 	return 0;
 }
 
+static void gpio_rcar_enable_inputs(struct gpio_rcar_priv *p)
+{
+	u32 mask = GENMASK(p->gpio_chip.ngpio - 1, 0);
+
+	/* Select "Input Enable" in INEN */
+	if (p->gpio_chip.valid_mask)
+		mask &= p->gpio_chip.valid_mask[0];
+	if (mask)
+		gpio_rcar_write(p, INEN, gpio_rcar_read(p, INEN) | mask);
+}
+
 static int gpio_rcar_probe(struct platform_device *pdev)
 {
 	struct gpio_rcar_priv *p;
@@ -560,6 +586,12 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 		goto err1;
 	}
 
+	if (p->info.has_inen) {
+		pm_runtime_get_sync(p->dev);
+		gpio_rcar_enable_inputs(p);
+		pm_runtime_put(p->dev);
+	}
+
 	dev_info(dev, "driving %d GPIOs\n", npins);
 
 	return 0;
@@ -635,6 +667,9 @@ static int gpio_rcar_resume(struct device *dev)
 		}
 	}
 
+	if (p->info.has_inen)
+		gpio_rcar_enable_inputs(p);
+
 	return 0;
 }
 #endif /* CONFIG_PM_SLEEP*/
-- 
2.25.1

RE: [PATCH/RFC v2 2/2] gpio: rcar: Add R-Car V3U (R8A7799A) support

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for the patch!

> From: Geert Uytterhoeven, Sent: Friday, October 30, 2020 5:15 PM
> Subject: [PATCH/RFC v2 2/2] gpio: rcar: Add R-Car V3U (R8A7799A) support

This subject should be "R8A779A0", not "R8A7799A".

After fixed it,

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda

Re: [PATCH/RFC v2 2/2] gpio: rcar: Add R-Car V3U (R8A7799A) support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On Fri, Oct 30, 2020 at 09:15:22AM +0100, Geert Uytterhoeven wrote:
> Add support for the GPIO controller block in the R-Car V3U (R8A779A0)
> SoC, which is very similar to the block found on other R-Car Gen3 SoCs.
> However, this block has a new General Input Enable Register (INEN),
> whose reset state is to have all inputs disabled.
> 
> Enable input for all available pins in probe and resume, to support the
> use of the General Input Register (INDT) for reading pin state at all
> times.  This preserves backwards compatibility with other R-Car Gen3
> SoCs, as recommended by the Hardware Manual.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]