* [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).