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