linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] gpio: rcar: (re)add Runtime PM support
@ 2016-12-08 17:32 Niklas Söderlund
  2016-12-08 17:32 ` [PATCH 1/2] gpio: rcar: set IRQ chip parent_device Niklas Söderlund
  2016-12-08 17:32 ` [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support Niklas Söderlund
  0 siblings, 2 replies; 12+ messages in thread
From: Niklas Söderlund @ 2016-12-08 17:32 UTC (permalink / raw)
  To: Geert Uytterhoeven, Linus Walleij, linux-gpio, linux-renesas-soc
  Cc: Jon Hunter, Laurent Pinchart, Niklas Söderlund

Hi all,

This series (re)adds Runtime PM support which was reverted by:

commit 42c25013ca95ce79 ('Revert "gpio: rcar: Add Runtime PM handling for interrupts"')
commit ce0e2c60e69e5f87 ('Revert "gpio: rcar: Fine-grained Runtime PM support"')

The reason it was reverted was that 'Runtime PM handling for interrupts'
could triggering a BUG() in linux/drivers/base/power/runtime.c 'BUG: 
sleeping function called from invalid context', see [1]. Since then Jon 
Hunter have solved this in a generic way in:

commit be45beb2df6909d4 ("genirq: Add runtime power management support for IRQ chips")

Patch 1/2 make use of Jons work to make sure proper PM handling when an 
GPIO is requested as an interrupt and patch 2/2 is just a resurrection 
of Geerts original patch with small fixups to catch up with changes to 
the driver.

I have tested the series on Koelsch and Salvator-X H3 and can in in 
sysfs observe that only GPIO blocks which are used are powered on.  
Requesting a GPIO from a powered down block will power it on and 
releasing it will power it off.

To test that patch 1/2 works as expected I have done tests on Salvator-X 
where the EtherAVB will not work with patch 2/2 applied, which moves 
pm_runtime_get_sync() from probe time to gpio_rcar_request(). Whit that 
move the GPIO block used by EtherAVB will not be powered on unless done 
so when the interrupt is requested, which we add support for in 1/2.

I'm not sure on how to best resurrect a patch which have been reverted, 
I dropped all Signed-off-by lines except Geerts as he is the author and 
added my own Signed-off-by since I'm resending it. Hope this is OK, if 
not please let me know.

1. https://www.spinics.net/lists/linux-renesas-soc/msg02710.html

Geert Uytterhoeven (1):
  gpio: rcar: Fine-grained Runtime PM support

Niklas Söderlund (1):
  gpio: rcar: set IRQ chip parent_device

 drivers/gpio/gpio-rcar.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

-- 
2.10.2


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

* [PATCH 1/2] gpio: rcar: set IRQ chip parent_device
  2016-12-08 17:32 [PATCH 0/2] gpio: rcar: (re)add Runtime PM support Niklas Söderlund
@ 2016-12-08 17:32 ` Niklas Söderlund
  2016-12-08 21:36   ` Laurent Pinchart
                     ` (2 more replies)
  2016-12-08 17:32 ` [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support Niklas Söderlund
  1 sibling, 3 replies; 12+ messages in thread
From: Niklas Söderlund @ 2016-12-08 17:32 UTC (permalink / raw)
  To: Geert Uytterhoeven, Linus Walleij, linux-gpio, linux-renesas-soc
  Cc: Jon Hunter, Laurent Pinchart, Niklas Söderlund

This enables Runtime PM handling for interrupts.

By setting the parent_device in struct irq_chip genirq will call the
pm_runtime_get/put APIs when an IRQ is requested/freed.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/gpio/gpio-rcar.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 2be48f5..3b77c10 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -460,6 +460,7 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 
 	irq_chip = &p->irq_chip;
 	irq_chip->name = name;
+	irq_chip->parent_device = dev;
 	irq_chip->irq_mask = gpio_rcar_irq_disable;
 	irq_chip->irq_unmask = gpio_rcar_irq_enable;
 	irq_chip->irq_set_type = gpio_rcar_irq_set_type;
-- 
2.10.2


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

* [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
  2016-12-08 17:32 [PATCH 0/2] gpio: rcar: (re)add Runtime PM support Niklas Söderlund
  2016-12-08 17:32 ` [PATCH 1/2] gpio: rcar: set IRQ chip parent_device Niklas Söderlund
@ 2016-12-08 17:32 ` Niklas Söderlund
  2016-12-08 21:40   ` Laurent Pinchart
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Niklas Söderlund @ 2016-12-08 17:32 UTC (permalink / raw)
  To: Geert Uytterhoeven, Linus Walleij, linux-gpio, linux-renesas-soc
  Cc: Jon Hunter, Laurent Pinchart, Geert Uytterhoeven,
	Niklas Söderlund

From: Geert Uytterhoeven <geert+renesas@glider.be>

Currently gpio modules are runtime-resumed at probe time. This means the
gpio module will be active all the time (except during system suspend,
if not configured as a wake-up source).

While an R-Car Gen2 gpio module retains pins configured for output at
the requested level while put in standby mode, gpio register cannot be
accessed while suspended.  Unfortunately pm_runtime_get_sync() cannot be
called from all contexts where gpio register access is needed. Hence
move the Runtime PM handling from probe/remove time to gpio request/free
time, which is probably the best we can do.

On r8a7791/koelsch, gpio modules 0, 1, 3, and 4 are now suspended during
normal use (gpio2 is used for LEDs and regulators, gpio5 for keys, gpio6
for SD-Card CD & WP, gpio7 for keys and regulators).

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
[Niklas: s/gpio_to_priv(chip)/gpiochip_get_data(chip)/]
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/gpio/gpio-rcar.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 3b77c10..31ad288 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -242,11 +242,24 @@ static void gpio_rcar_config_general_input_output_mode(struct gpio_chip *chip,
 
 static int gpio_rcar_request(struct gpio_chip *chip, unsigned offset)
 {
-	return pinctrl_request_gpio(chip->base + offset);
+	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
+	int error;
+
+	error = pm_runtime_get_sync(&p->pdev->dev);
+	if (error < 0)
+		return error;
+
+	error = pinctrl_request_gpio(chip->base + offset);
+	if (error)
+		pm_runtime_put(&p->pdev->dev);
+
+	return error;
 }
 
 static void gpio_rcar_free(struct gpio_chip *chip, unsigned offset)
 {
+	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
+
 	pinctrl_free_gpio(chip->base + offset);
 
 	/*
@@ -254,6 +267,8 @@ static void gpio_rcar_free(struct gpio_chip *chip, unsigned offset)
 	 * drive the GPIO pin as an output.
 	 */
 	gpio_rcar_config_general_input_output_mode(chip, offset, false);
+
+	pm_runtime_put(&p->pdev->dev);
 }
 
 static int gpio_rcar_direction_input(struct gpio_chip *chip, unsigned offset)
@@ -426,7 +441,6 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(dev);
-	pm_runtime_get_sync(dev);
 
 	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -495,7 +509,6 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 err1:
 	gpiochip_remove(gpio_chip);
 err0:
-	pm_runtime_put(dev);
 	pm_runtime_disable(dev);
 	return ret;
 }
@@ -506,7 +519,6 @@ static int gpio_rcar_remove(struct platform_device *pdev)
 
 	gpiochip_remove(&p->gpio_chip);
 
-	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return 0;
 }
-- 
2.10.2


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

* Re: [PATCH 1/2] gpio: rcar: set IRQ chip parent_device
  2016-12-08 17:32 ` [PATCH 1/2] gpio: rcar: set IRQ chip parent_device Niklas Söderlund
@ 2016-12-08 21:36   ` Laurent Pinchart
  2016-12-09 10:43   ` Geert Uytterhoeven
  2016-12-28  0:31   ` Linus Walleij
  2 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2016-12-08 21:36 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linus Walleij, linux-gpio, linux-renesas-soc,
	Jon Hunter

Hi Niklas,

Thank you for the patch.

On Thursday 08 Dec 2016 18:32:27 Niklas Söderlund wrote:
> This enables Runtime PM handling for interrupts.
> 
> By setting the parent_device in struct irq_chip genirq will call the
> pm_runtime_get/put APIs when an IRQ is requested/freed.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpio/gpio-rcar.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
> index 2be48f5..3b77c10 100644
> --- a/drivers/gpio/gpio-rcar.c
> +++ b/drivers/gpio/gpio-rcar.c
> @@ -460,6 +460,7 @@ static int gpio_rcar_probe(struct platform_device *pdev)
> 
>  	irq_chip = &p->irq_chip;
>  	irq_chip->name = name;
> +	irq_chip->parent_device = dev;
>  	irq_chip->irq_mask = gpio_rcar_irq_disable;
>  	irq_chip->irq_unmask = gpio_rcar_irq_enable;
>  	irq_chip->irq_set_type = gpio_rcar_irq_set_type;

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
  2016-12-08 17:32 ` [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support Niklas Söderlund
@ 2016-12-08 21:40   ` Laurent Pinchart
  2016-12-08 23:15     ` Niklas Söderlund
  2016-12-09 10:26   ` Geert Uytterhoeven
  2016-12-28  0:33   ` Linus Walleij
  2 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2016-12-08 21:40 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linus Walleij, linux-gpio, linux-renesas-soc,
	Jon Hunter, Geert Uytterhoeven

Hi Niklas,

Thank you for the patch.

On Thursday 08 Dec 2016 18:32:28 Niklas Söderlund wrote:
> From: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Currently gpio modules are runtime-resumed at probe time. This means the
> gpio module will be active all the time (except during system suspend,
> if not configured as a wake-up source).
> 
> While an R-Car Gen2 gpio module retains pins configured for output at
> the requested level while put in standby mode, gpio register cannot be
> accessed while suspended.  Unfortunately pm_runtime_get_sync() cannot be
> called from all contexts where gpio register access is needed. Hence
> move the Runtime PM handling from probe/remove time to gpio request/free
> time, which is probably the best we can do.
> 
> On r8a7791/koelsch, gpio modules 0, 1, 3, and 4 are now suspended during
> normal use (gpio2 is used for LEDs and regulators, gpio5 for keys, gpio6
> for SD-Card CD & WP, gpio7 for keys and regulators).
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> [Niklas: s/gpio_to_priv(chip)/gpiochip_get_data(chip)/]

Just curious, what's the rationale for this ?

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpio/gpio-rcar.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
> index 3b77c10..31ad288 100644
> --- a/drivers/gpio/gpio-rcar.c
> +++ b/drivers/gpio/gpio-rcar.c
> @@ -242,11 +242,24 @@ static void
> gpio_rcar_config_general_input_output_mode(struct gpio_chip *chip,
> 
>  static int gpio_rcar_request(struct gpio_chip *chip, unsigned offset)
>  {
> -	return pinctrl_request_gpio(chip->base + offset);
> +	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
> +	int error;
> +
> +	error = pm_runtime_get_sync(&p->pdev->dev);
> +	if (error < 0)
> +		return error;
> +
> +	error = pinctrl_request_gpio(chip->base + offset);
> +	if (error)
> +		pm_runtime_put(&p->pdev->dev);
> +
> +	return error;
>  }
> 
>  static void gpio_rcar_free(struct gpio_chip *chip, unsigned offset)
>  {
> +	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
> +
>  	pinctrl_free_gpio(chip->base + offset);
> 
>  	/*
> @@ -254,6 +267,8 @@ static void gpio_rcar_free(struct gpio_chip *chip,
> unsigned offset) * drive the GPIO pin as an output.
>  	 */
>  	gpio_rcar_config_general_input_output_mode(chip, offset, false);
> +
> +	pm_runtime_put(&p->pdev->dev);
>  }
> 
>  static int gpio_rcar_direction_input(struct gpio_chip *chip, unsigned
> offset) @@ -426,7 +441,6 @@ static int gpio_rcar_probe(struct
> platform_device *pdev) }
> 
>  	pm_runtime_enable(dev);
> -	pm_runtime_get_sync(dev);
> 
>  	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> @@ -495,7 +509,6 @@ static int gpio_rcar_probe(struct platform_device *pdev)
> err1:
>  	gpiochip_remove(gpio_chip);
>  err0:
> -	pm_runtime_put(dev);
>  	pm_runtime_disable(dev);
>  	return ret;
>  }
> @@ -506,7 +519,6 @@ static int gpio_rcar_remove(struct platform_device
> *pdev)
> 
>  	gpiochip_remove(&p->gpio_chip);
> 
> -	pm_runtime_put(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
  2016-12-08 21:40   ` Laurent Pinchart
@ 2016-12-08 23:15     ` Niklas Söderlund
  2016-12-08 23:21       ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Niklas Söderlund @ 2016-12-08 23:15 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Linus Walleij, linux-gpio, linux-renesas-soc,
	Jon Hunter, Geert Uytterhoeven

Hi Laurent,

On 2016-12-08 23:40:42 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thursday 08 Dec 2016 18:32:28 Niklas Söderlund wrote:
> > From: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > Currently gpio modules are runtime-resumed at probe time. This means the
> > gpio module will be active all the time (except during system suspend,
> > if not configured as a wake-up source).
> > 
> > While an R-Car Gen2 gpio module retains pins configured for output at
> > the requested level while put in standby mode, gpio register cannot be
> > accessed while suspended.  Unfortunately pm_runtime_get_sync() cannot be
> > called from all contexts where gpio register access is needed. Hence
> > move the Runtime PM handling from probe/remove time to gpio request/free
> > time, which is probably the best we can do.
> > 
> > On r8a7791/koelsch, gpio modules 0, 1, 3, and 4 are now suspended during
> > normal use (gpio2 is used for LEDs and regulators, gpio5 for keys, gpio6
> > for SD-Card CD & WP, gpio7 for keys and regulators).
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > [Niklas: s/gpio_to_priv(chip)/gpiochip_get_data(chip)/]
> 
> Just curious, what's the rationale for this ?

This was changed for the whole driver after the original patch was 
applied (at the time of change the patch was not yet reverted), see [1].  
I needed to update this when I resurrected the patch, maybe I could have 
been more clever and reverted the revert patch but this felt cleaner, if 
it's better to do it the other way around and revert a revert please let 
me know so I can do so in the future.

[1] c7b6f457cb53bcee ("gpio: rcar: use gpiochip data pointer").

> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > ---
> >  drivers/gpio/gpio-rcar.c | 20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
> > index 3b77c10..31ad288 100644
> > --- a/drivers/gpio/gpio-rcar.c
> > +++ b/drivers/gpio/gpio-rcar.c
> > @@ -242,11 +242,24 @@ static void
> > gpio_rcar_config_general_input_output_mode(struct gpio_chip *chip,
> > 
> >  static int gpio_rcar_request(struct gpio_chip *chip, unsigned offset)
> >  {
> > -	return pinctrl_request_gpio(chip->base + offset);
> > +	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
> > +	int error;
> > +
> > +	error = pm_runtime_get_sync(&p->pdev->dev);
> > +	if (error < 0)
> > +		return error;
> > +
> > +	error = pinctrl_request_gpio(chip->base + offset);
> > +	if (error)
> > +		pm_runtime_put(&p->pdev->dev);
> > +
> > +	return error;
> >  }
> > 
> >  static void gpio_rcar_free(struct gpio_chip *chip, unsigned offset)
> >  {
> > +	struct gpio_rcar_priv *p = gpiochip_get_data(chip);
> > +
> >  	pinctrl_free_gpio(chip->base + offset);
> > 
> >  	/*
> > @@ -254,6 +267,8 @@ static void gpio_rcar_free(struct gpio_chip *chip,
> > unsigned offset) * drive the GPIO pin as an output.
> >  	 */
> >  	gpio_rcar_config_general_input_output_mode(chip, offset, false);
> > +
> > +	pm_runtime_put(&p->pdev->dev);
> >  }
> > 
> >  static int gpio_rcar_direction_input(struct gpio_chip *chip, unsigned
> > offset) @@ -426,7 +441,6 @@ static int gpio_rcar_probe(struct
> > platform_device *pdev) }
> > 
> >  	pm_runtime_enable(dev);
> > -	pm_runtime_get_sync(dev);
> > 
> >  	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > @@ -495,7 +509,6 @@ static int gpio_rcar_probe(struct platform_device *pdev)
> > err1:
> >  	gpiochip_remove(gpio_chip);
> >  err0:
> > -	pm_runtime_put(dev);
> >  	pm_runtime_disable(dev);
> >  	return ret;
> >  }
> > @@ -506,7 +519,6 @@ static int gpio_rcar_remove(struct platform_device
> > *pdev)
> > 
> >  	gpiochip_remove(&p->gpio_chip);
> > 
> > -	pm_runtime_put(&pdev->dev);
> >  	pm_runtime_disable(&pdev->dev);
> >  	return 0;
> >  }
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
  2016-12-08 23:15     ` Niklas Söderlund
@ 2016-12-08 23:21       ` Laurent Pinchart
  2016-12-15 23:12         ` Linus Walleij
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2016-12-08 23:21 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, Linus Walleij, linux-gpio, linux-renesas-soc,
	Jon Hunter, Geert Uytterhoeven

Hi Niklas,

On Friday 09 Dec 2016 00:15:51 Niklas Söderlund wrote:
> On 2016-12-08 23:40:42 +0200, Laurent Pinchart wrote:
> > On Thursday 08 Dec 2016 18:32:28 Niklas Söderlund wrote:
> >> From: Geert Uytterhoeven <geert+renesas@glider.be>
> >> 
> >> Currently gpio modules are runtime-resumed at probe time. This means the
> >> gpio module will be active all the time (except during system suspend,
> >> if not configured as a wake-up source).
> >> 
> >> While an R-Car Gen2 gpio module retains pins configured for output at
> >> the requested level while put in standby mode, gpio register cannot be
> >> accessed while suspended.  Unfortunately pm_runtime_get_sync() cannot be
> >> called from all contexts where gpio register access is needed. Hence
> >> move the Runtime PM handling from probe/remove time to gpio request/free
> >> time, which is probably the best we can do.
> >> 
> >> On r8a7791/koelsch, gpio modules 0, 1, 3, and 4 are now suspended during
> >> normal use (gpio2 is used for LEDs and regulators, gpio5 for keys, gpio6
> >> for SD-Card CD & WP, gpio7 for keys and regulators).
> >> 
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> [Niklas: s/gpio_to_priv(chip)/gpiochip_get_data(chip)/]
> > 
> > Just curious, what's the rationale for this ?
> 
> This was changed for the whole driver after the original patch was
> applied (at the time of change the patch was not yet reverted), see [1].
> I needed to update this when I resurrected the patch, maybe I could have
> been more clever and reverted the revert patch but this felt cleaner, if
> it's better to do it the other way around and revert a revert please let
> me know so I can do so in the future.

No, this is fine. I'm a bit dubious about [1] given that it consumes more CPU 
cycles without any benefit as far as I can see. Maybe I can't see far enough 
though, Linus Walleij could prove me wrong :-)

> [1] c7b6f457cb53bcee ("gpio: rcar: use gpiochip data pointer").

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
  2016-12-08 17:32 ` [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support Niklas Söderlund
  2016-12-08 21:40   ` Laurent Pinchart
@ 2016-12-09 10:26   ` Geert Uytterhoeven
  2016-12-28  0:33   ` Linus Walleij
  2 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2016-12-09 10:26 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Linus Walleij, linux-gpio@vger.kernel.org, Linux-Renesas,
	Jon Hunter, Laurent Pinchart, Geert Uytterhoeven

Hi Niklas,

On Thu, Dec 8, 2016 at 6:32 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> From: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Currently gpio modules are runtime-resumed at probe time. This means the
> gpio module will be active all the time (except during system suspend,
> if not configured as a wake-up source).
>
> While an R-Car Gen2 gpio module retains pins configured for output at
> the requested level while put in standby mode, gpio register cannot be
> accessed while suspended.  Unfortunately pm_runtime_get_sync() cannot be
> called from all contexts where gpio register access is needed. Hence
> move the Runtime PM handling from probe/remove time to gpio request/free
> time, which is probably the best we can do.
>
> On r8a7791/koelsch, gpio modules 0, 1, 3, and 4 are now suspended during
> normal use (gpio2 is used for LEDs and regulators, gpio5 for keys, gpio6
> for SD-Card CD & WP, gpio7 for keys and regulators).

FWIW, gpio module 3 is no longer suspended, as gpio3 is used by HDMI since
commit 83a0731b39f3bc24 ("ARM: shmobile: koelsch: Add DU HDMI output support").

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> [Niklas: s/gpio_to_priv(chip)/gpiochip_get_data(chip)/]
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Thanks for fixing and resurrecting!

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] 12+ messages in thread

* Re: [PATCH 1/2] gpio: rcar: set IRQ chip parent_device
  2016-12-08 17:32 ` [PATCH 1/2] gpio: rcar: set IRQ chip parent_device Niklas Söderlund
  2016-12-08 21:36   ` Laurent Pinchart
@ 2016-12-09 10:43   ` Geert Uytterhoeven
  2016-12-28  0:31   ` Linus Walleij
  2 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2016-12-09 10:43 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Linus Walleij, linux-gpio@vger.kernel.org, Linux-Renesas,
	Jon Hunter, Laurent Pinchart

On Thu, Dec 8, 2016 at 6:32 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> This enables Runtime PM handling for interrupts.
>
> By setting the parent_device in struct irq_chip genirq will call the
> pm_runtime_get/put APIs when an IRQ is requested/freed.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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] 12+ messages in thread

* Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
  2016-12-08 23:21       ` Laurent Pinchart
@ 2016-12-15 23:12         ` Linus Walleij
  0 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2016-12-15 23:12 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Niklas Söderlund, Geert Uytterhoeven,
	linux-gpio@vger.kernel.org, Linux-Renesas, Jon Hunter,
	Geert Uytterhoeven

On Fri, Dec 9, 2016 at 12:21 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Friday 09 Dec 2016 00:15:51 Niklas Söderlund wrote:

>> This was changed for the whole driver after the original patch was
>> applied (at the time of change the patch was not yet reverted), see [1].
>> I needed to update this when I resurrected the patch, maybe I could have
>> been more clever and reverted the revert patch but this felt cleaner, if
>> it's better to do it the other way around and revert a revert please let
>> me know so I can do so in the future.
>
> No, this is fine. I'm a bit dubious about [1] given that it consumes more CPU
> cycles without any benefit as far as I can see. Maybe I can't see far enough
> though, Linus Walleij could prove me wrong :-)

This was changed mainly for maintainability and code readability.
Lots of subsystems have the option to carry around a data poiner
and/or allocate an extra memory chunk for state containers.

I am relaxed on it, if someone very much want to use container_of()
because of $REASON I am not really bothered.

Yours,
Linus Walleij

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

* Re: [PATCH 1/2] gpio: rcar: set IRQ chip parent_device
  2016-12-08 17:32 ` [PATCH 1/2] gpio: rcar: set IRQ chip parent_device Niklas Söderlund
  2016-12-08 21:36   ` Laurent Pinchart
  2016-12-09 10:43   ` Geert Uytterhoeven
@ 2016-12-28  0:31   ` Linus Walleij
  2 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2016-12-28  0:31 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, linux-gpio@vger.kernel.org, Linux-Renesas,
	Jon Hunter, Laurent Pinchart

On Thu, Dec 8, 2016 at 6:32 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:

> This enables Runtime PM handling for interrupts.
>
> By setting the parent_device in struct irq_chip genirq will call the
> pm_runtime_get/put APIs when an IRQ is requested/freed.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Patch applied with the ACKs.

Yours,
Linus Walleij

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

* Re: [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support
  2016-12-08 17:32 ` [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support Niklas Söderlund
  2016-12-08 21:40   ` Laurent Pinchart
  2016-12-09 10:26   ` Geert Uytterhoeven
@ 2016-12-28  0:33   ` Linus Walleij
  2 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2016-12-28  0:33 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Geert Uytterhoeven, linux-gpio@vger.kernel.org, Linux-Renesas,
	Jon Hunter, Laurent Pinchart, Geert Uytterhoeven

On Thu, Dec 8, 2016 at 6:32 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:

> From: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Currently gpio modules are runtime-resumed at probe time. This means the
> gpio module will be active all the time (except during system suspend,
> if not configured as a wake-up source).
>
> While an R-Car Gen2 gpio module retains pins configured for output at
> the requested level while put in standby mode, gpio register cannot be
> accessed while suspended.  Unfortunately pm_runtime_get_sync() cannot be
> called from all contexts where gpio register access is needed. Hence
> move the Runtime PM handling from probe/remove time to gpio request/free
> time, which is probably the best we can do.
>
> On r8a7791/koelsch, gpio modules 0, 1, 3, and 4 are now suspended during
> normal use (gpio2 is used for LEDs and regulators, gpio5 for keys, gpio6
> for SD-Card CD & WP, gpio7 for keys and regulators).
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> [Niklas: s/gpio_to_priv(chip)/gpiochip_get_data(chip)/]
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Patch applied.

Yours,
Linus Walleij

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

end of thread, other threads:[~2016-12-28  0:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-08 17:32 [PATCH 0/2] gpio: rcar: (re)add Runtime PM support Niklas Söderlund
2016-12-08 17:32 ` [PATCH 1/2] gpio: rcar: set IRQ chip parent_device Niklas Söderlund
2016-12-08 21:36   ` Laurent Pinchart
2016-12-09 10:43   ` Geert Uytterhoeven
2016-12-28  0:31   ` Linus Walleij
2016-12-08 17:32 ` [PATCH 2/2] gpio: rcar: Fine-grained Runtime PM support Niklas Söderlund
2016-12-08 21:40   ` Laurent Pinchart
2016-12-08 23:15     ` Niklas Söderlund
2016-12-08 23:21       ` Laurent Pinchart
2016-12-15 23:12         ` Linus Walleij
2016-12-09 10:26   ` Geert Uytterhoeven
2016-12-28  0:33   ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).