[PATCH v3 0/3] renesas: irqchip: Use wakeup_path i.s.o. explicit clock handling

[PATCH v3 0/3] renesas: irqchip: Use wakeup_path i.s.o. explicit clock handling

[Geert Uytterhoeven]

	Hi all,

If an interrupt controller in a Renesas ARM SoC is part of a Clock
Domain, and it is part of the wakeup path, it must be kept active during
system suspend.

Currently this is handled in all interrupt controller drivers by
explicitly increasing the use count of the module clock when the device
is part of the wakeup path.  However, this explicit clock handling is
merely a workaround for a failure to properly communicate wakeup
information to the device core.

Hence this series fixes the affected drivers by setting the devices'
power.wakeup_path fields instead, to indicate they are part of the
wakeup path.  Depending on the PM Domain's active_wakeup configuration,
the genpd core code will keep the device enabled (and the clock running)
during system suspend when needed.

Target trees:
  - Patches 1 and 2 are meant for the irqchip tree,
  - Patch 3 is meant for the GPIO tree.

Changes compared to v2 (sent by Ulf Hanson):
  - Revert to v1, as WAKEUP_PATH didn't make it,
  - Use an atomic_t instead of a boolean flag, as the boolean tracked
    only the status of the last configured GPIO, while multiple interrupt
    sources on the same irqchip may be involved with wake-up,
  - Use new helper device_set_wakeup_path(),
  - Drop checks for CONFIG_PM_SLEEP, as device_set_wakeup_path() is
    always available, unlike dev_pm_info.wakeup_path,
  - Mark suspend functions __maybe_unused.

Changes compared to v1:
  - Convert to use the WAKEUP_PATH driver PM flag (Ulf Hanson).

This has been tested on r8a73a4/ape6evm, r8a7740/armadillo,
r8a7791/koelsch, r8a7795/salvator-x and -xs, r8a7796/salvator-x, and
sh73a0/kzm9g.

Thanks for applying!

Geert Uytterhoeven (3):
  irqchip/renesas-intc-irqpin: Use wakeup_path i.s.o. explicit clock
    handling
  irqchip/renesas-irqc: Use wakeup_path i.s.o. explicit clock handling
  gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

 drivers/gpio/gpio-rcar.c                  | 38 +++++++++++++----------------
 drivers/irqchip/irq-renesas-intc-irqpin.c | 40 +++++++++++++------------------
 drivers/irqchip/irq-renesas-irqc.c        | 30 ++++++++++++-----------
 3 files changed, 48 insertions(+), 60 deletions(-)

-- 
2.7.4

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 v3 1/3] irqchip/renesas-intc-irqpin: Use wakeup_path i.s.o. explicit clock handling

[Geert Uytterhoeven]

Since commit 705bc96c2c15313c ("irqchip: renesas-intc-irqpin: Add
minimal runtime PM support"), when an IRQ is used for wakeup, the INTC
block's module clock (if exists) is manually kept running during system
suspend, to make sure the device stays active.

However, this explicit clock handling is merely a workaround for a
failure to properly communicate wakeup information to the device core.

Instead, set the device's power.wakeup_path field, to indicate this
device is part of the wakeup path.  Depending on the PM Domain's
active_wakeup configuration, the genpd core code will keep the device
enabled (and the clock running) during system suspend when needed.
This allows for the removal of all explicit clock handling code from the
driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v3:
  - Revert to v1, as WAKEUP_PATH didn't make it,
  - Use an atomic_t instead of a boolean flag, as the boolean tracked
    only the status of the last configured GPIO, while multiple
    interrupt pins on the same interrupt controller may be involved with
    wake-up,
  - Use new helper device_set_wakeup_path(),
  - Drop checks for CONFIG_PM_SLEEP, as device_set_wakeup_path() is
    always available, unlike dev_pm_info.wakeup_path,
  - Mark intc_irqpin_suspend() __maybe_unused,

v2 (sent by Ulf Hanson):
  - Convert to use the WAKEUP_PATH driver PM flag (Ulf).
---
 drivers/irqchip/irq-renesas-intc-irqpin.c | 40 +++++++++++++------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
index cee59fe1321c44f9..c6e6c9e9137ad1d2 100644
--- a/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
@@ -17,7 +17,6 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include <linux/clk.h>
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -78,16 +77,14 @@ struct intc_irqpin_priv {
 	struct platform_device *pdev;
 	struct irq_chip irq_chip;
 	struct irq_domain *irq_domain;
-	struct clk *clk;
+	atomic_t wakeup_path;
 	unsigned shared_irqs:1;
-	unsigned needs_clk:1;
 	u8 shared_irq_mask;
 };
 
 struct intc_irqpin_config {
 	unsigned int irlm_bit;
 	unsigned needs_irlm:1;
-	unsigned needs_clk:1;
 };
 
 static unsigned long intc_irqpin_read32(void __iomem *iomem)
@@ -287,14 +284,10 @@ static int intc_irqpin_irq_set_wake(struct irq_data *d, unsigned int on)
 	int hw_irq = irqd_to_hwirq(d);
 
 	irq_set_irq_wake(p->irq[hw_irq].requested_irq, on);
-
-	if (!p->clk)
-		return 0;
-
 	if (on)
-		clk_enable(p->clk);
+		atomic_inc(&p->wakeup_path);
 	else
-		clk_disable(p->clk);
+		atomic_dec(&p->wakeup_path);
 
 	return 0;
 }
@@ -369,12 +362,10 @@ static const struct irq_domain_ops intc_irqpin_irq_domain_ops = {
 static const struct intc_irqpin_config intc_irqpin_irlm_r8a777x = {
 	.irlm_bit = 23, /* ICR0.IRLM0 */
 	.needs_irlm = 1,
-	.needs_clk = 0,
 };
 
 static const struct intc_irqpin_config intc_irqpin_rmobile = {
 	.needs_irlm = 0,
-	.needs_clk = 1,
 };
 
 static const struct of_device_id intc_irqpin_dt_ids[] = {
@@ -426,18 +417,6 @@ static int intc_irqpin_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, p);
 
 	config = of_device_get_match_data(dev);
-	if (config)
-		p->needs_clk = config->needs_clk;
-
-	p->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(p->clk)) {
-		if (p->needs_clk) {
-			dev_err(dev, "unable to get clock\n");
-			ret = PTR_ERR(p->clk);
-			goto err0;
-		}
-		p->clk = NULL;
-	}
 
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
@@ -606,12 +585,25 @@ static int intc_irqpin_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused intc_irqpin_suspend(struct device *dev)
+{
+	struct intc_irqpin_priv *p = dev_get_drvdata(dev);
+
+	if (atomic_read(&p->wakeup_path))
+		device_set_wakeup_path(dev);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(intc_irqpin_pm_ops, intc_irqpin_suspend, NULL);
+
 static struct platform_driver intc_irqpin_device_driver = {
 	.probe		= intc_irqpin_probe,
 	.remove		= intc_irqpin_remove,
 	.driver		= {
 		.name	= "renesas_intc_irqpin",
 		.of_match_table = intc_irqpin_dt_ids,
+		.pm	= &intc_irqpin_pm_ops,
 	}
 };
 
-- 
2.7.4

[PATCH v3 2/3] irqchip/renesas-irqc: Use wakeup_path i.s.o. explicit clock handling

[Geert Uytterhoeven]

Since commit 6f46aedb9c85873b ("irqchip: renesas-irqc: Add wake-up
support"), when an IRQ is used for wakeup, the INTC
block's module clock is manually kept running during system suspend, to
make sure the device stays active.

However, this explicit clock handling is merely a workaround for a
failure to properly communicate wakeup information to the device core.

Instead, set the device's power.wakeup_path field, to indicate this
device is part of the wakeup path.  Depending on the PM Domain's
active_wakeup configuration, the genpd core code will keep the device
enabled (and the clock running) during system suspend when needed.
This allows for the removal of all explicit clock handling code from the
driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v3:
  - Revert to v1, as WAKEUP_PATH didn't make it,
  - Use an atomic_t instead of a boolean flag, as the boolean tracked
    only the status of the last configured GPIO, while multiple
    interrupt pins on the same interrupt controller may be involved with
    wake-up,
  - Use new helper device_set_wakeup_path(),
  - Drop checks for CONFIG_PM_SLEEP, as device_set_wakeup_path() is
    always available, unlike dev_pm_info.wakeup_path,
  - Mark irqc_suspend() __maybe_unused,
  - Drop RFC state, now the dependencies are upstream.

v2 (sent by Ulf Hanson):
  - Convert to use the WAKEUP_PATH driver PM flag (Ulf).
---
 drivers/irqchip/irq-renesas-irqc.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-irqc.c b/drivers/irqchip/irq-renesas-irqc.c
index 52304b139aa46a60..a4f11124024dcf32 100644
--- a/drivers/irqchip/irq-renesas-irqc.c
+++ b/drivers/irqchip/irq-renesas-irqc.c
@@ -17,7 +17,6 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include <linux/clk.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
@@ -64,7 +63,7 @@ struct irqc_priv {
 	struct platform_device *pdev;
 	struct irq_chip_generic *gc;
 	struct irq_domain *irq_domain;
-	struct clk *clk;
+	atomic_t wakeup_path;
 };
 
 static struct irqc_priv *irq_data_to_priv(struct irq_data *data)
@@ -111,14 +110,10 @@ static int irqc_irq_set_wake(struct irq_data *d, unsigned int on)
 	int hw_irq = irqd_to_hwirq(d);
 
 	irq_set_irq_wake(p->irq[hw_irq].requested_irq, on);
-
-	if (!p->clk)
-		return 0;
-
 	if (on)
-		clk_enable(p->clk);
+		atomic_inc(&p->wakeup_path);
 	else
-		clk_disable(p->clk);
+		atomic_dec(&p->wakeup_path);
 
 	return 0;
 }
@@ -159,12 +154,6 @@ static int irqc_probe(struct platform_device *pdev)
 	p->pdev = pdev;
 	platform_set_drvdata(pdev, p);
 
-	p->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(p->clk)) {
-		dev_warn(&pdev->dev, "unable to get clock\n");
-		p->clk = NULL;
-	}
-
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
@@ -276,6 +265,18 @@ static int irqc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused irqc_suspend(struct device *dev)
+{
+	struct irqc_priv *p = dev_get_drvdata(dev);
+
+	if (atomic_read(&p->wakeup_path))
+		device_set_wakeup_path(dev);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(irqc_pm_ops, irqc_suspend, NULL);
+
 static const struct of_device_id irqc_dt_ids[] = {
 	{ .compatible = "renesas,irqc", },
 	{},
@@ -288,6 +289,7 @@ static struct platform_driver irqc_device_driver = {
 	.driver		= {
 		.name	= "renesas_irqc",
 		.of_match_table	= irqc_dt_ids,
+		.pm	= &irqc_pm_ops,
 	}
 };
 
-- 
2.7.4

[PATCH v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Geert Uytterhoeven]

Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
block's module clock (if exists) is manually kept running during system
suspend, to make sure the device stays active.

However, this explicit clock handling is merely a workaround for a
failure to properly communicate wakeup information to the device core.

Instead, set the device's power.wakeup_path field, to indicate this
device is part of the wakeup path.  Depending on the PM Domain's
active_wakeup configuration, the genpd core code will keep the device
enabled (and the clock running) during system suspend when needed.
This allows for the removal of all explicit clock handling code from the
driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v3:
  - Revert to v1, as WAKEUP_PATH didn't make it,
  - Use an atomic_t instead of a boolean flag, as the latter tracked
    only the status of the last configured GPIO, while multiple GPIOs on
    the same GPIO controller may be involved with wake-up,
  - Use new helper device_set_wakeup_path(),
  - Drop checks for CONFIG_PM_SLEEP, as device_set_wakeup_path() is
    always available, unlike dev_pm_info.wakeup_path,
  - Mark gpio_rcar_suspend() __maybe_unused,
  - Drop RFC state, now the dependencies are upstream.

v2 (sent by Ulf Hanson):
  - Convert to use the WAKEUP_PATH driver PM flag (Ulf).
---
 drivers/gpio/gpio-rcar.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index e76de57dd617d7e2..ebaea8b1594b7fc4 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -14,7 +14,6 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/init.h>
@@ -37,10 +36,9 @@ struct gpio_rcar_priv {
 	struct platform_device *pdev;
 	struct gpio_chip gpio_chip;
 	struct irq_chip irq_chip;
-	struct clk *clk;
 	unsigned int irq_parent;
+	atomic_t wakeup_path;
 	bool has_both_edge_trigger;
-	bool needs_clk;
 };
 
 #define IOINTSEL 0x00	/* General IO/Interrupt Switching Register */
@@ -186,13 +184,10 @@ static int gpio_rcar_irq_set_wake(struct irq_data *d, unsigned int on)
 		}
 	}
 
-	if (!p->clk)
-		return 0;
-
 	if (on)
-		clk_enable(p->clk);
+		atomic_inc(&p->wakeup_path);
 	else
-		clk_disable(p->clk);
+		atomic_dec(&p->wakeup_path);
 
 	return 0;
 }
@@ -330,17 +325,14 @@ static int gpio_rcar_direction_output(struct gpio_chip *chip, unsigned offset,
 
 struct gpio_rcar_info {
 	bool has_both_edge_trigger;
-	bool needs_clk;
 };
 
 static const struct gpio_rcar_info gpio_rcar_info_gen1 = {
 	.has_both_edge_trigger = false,
-	.needs_clk = false,
 };
 
 static const struct gpio_rcar_info gpio_rcar_info_gen2 = {
 	.has_both_edge_trigger = true,
-	.needs_clk = true,
 };
 
 static const struct of_device_id gpio_rcar_of_table[] = {
@@ -403,7 +395,6 @@ static int gpio_rcar_parse_dt(struct gpio_rcar_priv *p, unsigned int *npins)
 	ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, &args);
 	*npins = ret == 0 ? args.args[2] : RCAR_MAX_GPIO_PER_BANK;
 	p->has_both_edge_trigger = info->has_both_edge_trigger;
-	p->needs_clk = info->needs_clk;
 
 	if (*npins == 0 || *npins > RCAR_MAX_GPIO_PER_BANK) {
 		dev_warn(&p->pdev->dev,
@@ -440,16 +431,6 @@ static int gpio_rcar_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, p);
 
-	p->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(p->clk)) {
-		if (p->needs_clk) {
-			dev_err(dev, "unable to get clock\n");
-			ret = PTR_ERR(p->clk);
-			goto err0;
-		}
-		p->clk = NULL;
-	}
-
 	pm_runtime_enable(dev);
 
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -531,11 +512,24 @@ static int gpio_rcar_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused gpio_rcar_suspend(struct device *dev)
+{
+	struct gpio_rcar_priv *p = dev_get_drvdata(dev);
+
+	if (atomic_read(&p->wakeup_path))
+		device_set_wakeup_path(dev);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(gpio_rcar_pm_ops, gpio_rcar_suspend, NULL);
+
 static struct platform_driver gpio_rcar_device_driver = {
 	.probe		= gpio_rcar_probe,
 	.remove		= gpio_rcar_remove,
 	.driver		= {
 		.name	= "gpio_rcar",
+		.pm     = &gpio_rcar_pm_ops,
 		.of_match_table = of_match_ptr(gpio_rcar_of_table),
 	}
 };
-- 
2.7.4

Re: [PATCH v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Linus Walleij]

On Mon, Feb 12, 2018 at 2:55 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
> when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
> block's module clock (if exists) is manually kept running during system
> suspend, to make sure the device stays active.
>
> However, this explicit clock handling is merely a workaround for a
> failure to properly communicate wakeup information to the device core.
>
> Instead, set the device's power.wakeup_path field, to indicate this
> device is part of the wakeup path.  Depending on the PM Domain's
> active_wakeup configuration, the genpd core code will keep the device
> enabled (and the clock running) during system suspend when needed.
> This allows for the removal of all explicit clock handling code from the
> driver.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Can I apply this and only this patch to GPIO?

Should it be applied for fixes, next?

Else please funnel this through the irqchip tree with my
ACK.

Yours,
Linus Walleij

Re: [PATCH v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Geert Uytterhoeven]

Hi Linus,

On Thu, Feb 22, 2018 at 3:23 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Feb 12, 2018 at 2:55 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>
>> Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
>> when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
>> block's module clock (if exists) is manually kept running during system
>> suspend, to make sure the device stays active.
>>
>> However, this explicit clock handling is merely a workaround for a
>> failure to properly communicate wakeup information to the device core.
>>
>> Instead, set the device's power.wakeup_path field, to indicate this
>> device is part of the wakeup path.  Depending on the PM Domain's
>> active_wakeup configuration, the genpd core code will keep the device
>> enabled (and the clock running) during system suspend when needed.
>> This allows for the removal of all explicit clock handling code from the
>> driver.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Can I apply this and only this patch to GPIO?

Yes please.

> Should it be applied for fixes, next?

No, as it depends on infrastructure (device_set_wakeup_path()) and fixes
(various "Keep wakeup sources active during system suspend") in v4.16-rc1.

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 v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Linus Walleij]

On Mon, Feb 12, 2018 at 2:55 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
> when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
> block's module clock (if exists) is manually kept running during system
> suspend, to make sure the device stays active.
>
> However, this explicit clock handling is merely a workaround for a
> failure to properly communicate wakeup information to the device core.
>
> Instead, set the device's power.wakeup_path field, to indicate this
> device is part of the wakeup path.  Depending on the PM Domain's
> active_wakeup configuration, the genpd core code will keep the device
> enabled (and the clock running) during system suspend when needed.
> This allows for the removal of all explicit clock handling code from the
> driver.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v3:

Patch applied for v4.16 fixes.

Yours,
Linus Walleij

Re: [PATCH v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Geert Uytterhoeven]

Hi Linus,

On Thu, Mar 1, 2018 at 5:06 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Feb 12, 2018 at 2:55 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>
>> Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
>> when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
>> block's module clock (if exists) is manually kept running during system
>> suspend, to make sure the device stays active.
>>
>> However, this explicit clock handling is merely a workaround for a
>> failure to properly communicate wakeup information to the device core.
>>
>> Instead, set the device's power.wakeup_path field, to indicate this
>> device is part of the wakeup path.  Depending on the PM Domain's
>> active_wakeup configuration, the genpd core code will keep the device
>> enabled (and the clock running) during system suspend when needed.
>> This allows for the removal of all explicit clock handling code from the
>> driver.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> v3:
>
> Patch applied for v4.16 fixes.

Are you aware this conflicts with commit 51750fb167a05468 ("gpio: gpio-rcar:
Support S2RAM") in gpio/for-next?

You can find the resolution in
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/drivers/gpio/gpio-rcar.c?h=renesas-drivers-2018-02-28-v4.16-rc3

If there's anything else I can do, just ask!

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 v3 3/3] gpio: rcar: Use wakeup_path i.s.o. explicit clock handling

[Linus Walleij]

On Fri, Mar 2, 2018 at 9:20 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Thu, Mar 1, 2018 at 5:06 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Mon, Feb 12, 2018 at 2:55 PM, Geert Uytterhoeven
>> <geert+renesas@glider.be> wrote:
>>
>>> Since commit ab82fa7da4dce5c7 ("gpio: rcar: Prevent module clock disable
>>> when wake-up is enabled"), when a GPIO is used for wakeup, the GPIO
>>> block's module clock (if exists) is manually kept running during system
>>> suspend, to make sure the device stays active.
>>>
>>> However, this explicit clock handling is merely a workaround for a
>>> failure to properly communicate wakeup information to the device core.
>>>
>>> Instead, set the device's power.wakeup_path field, to indicate this
>>> device is part of the wakeup path.  Depending on the PM Domain's
>>> active_wakeup configuration, the genpd core code will keep the device
>>> enabled (and the clock running) during system suspend when needed.
>>> This allows for the removal of all explicit clock handling code from the
>>> driver.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> v3:
>>
>> Patch applied for v4.16 fixes.
>
> Are you aware this conflicts with commit 51750fb167a05468 ("gpio: gpio-rcar:
> Support S2RAM") in gpio/for-next?

No but I guess I will become aware.

I have not yet mixed the fixes and devel branches for next.

> You can find the resolution in
> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/drivers/gpio/gpio-rcar.c?h=renesas-drivers-2018-02-28-v4.16-rc3

Thanks!

Yours,
Linus Walleij

Re: [PATCH v3 0/3] renesas: irqchip: Use wakeup_path i.s.o. explicit clock handling

[Ulf Hansson]

On 12 February 2018 at 14:55, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>         Hi all,
>
> If an interrupt controller in a Renesas ARM SoC is part of a Clock
> Domain, and it is part of the wakeup path, it must be kept active during
> system suspend.
>
> Currently this is handled in all interrupt controller drivers by
> explicitly increasing the use count of the module clock when the device
> is part of the wakeup path.  However, this explicit clock handling is
> merely a workaround for a failure to properly communicate wakeup
> information to the device core.
>
> Hence this series fixes the affected drivers by setting the devices'
> power.wakeup_path fields instead, to indicate they are part of the
> wakeup path.  Depending on the PM Domain's active_wakeup configuration,
> the genpd core code will keep the device enabled (and the clock running)
> during system suspend when needed.
>
> Target trees:
>   - Patches 1 and 2 are meant for the irqchip tree,
>   - Patch 3 is meant for the GPIO tree.
>
> Changes compared to v2 (sent by Ulf Hanson):
>   - Revert to v1, as WAKEUP_PATH didn't make it,
>   - Use an atomic_t instead of a boolean flag, as the boolean tracked
>     only the status of the last configured GPIO, while multiple interrupt
>     sources on the same irqchip may be involved with wake-up,
>   - Use new helper device_set_wakeup_path(),
>   - Drop checks for CONFIG_PM_SLEEP, as device_set_wakeup_path() is
>     always available, unlike dev_pm_info.wakeup_path,
>   - Mark suspend functions __maybe_unused.
>
> Changes compared to v1:
>   - Convert to use the WAKEUP_PATH driver PM flag (Ulf Hanson).
>
> This has been tested on r8a73a4/ape6evm, r8a7740/armadillo,
> r8a7791/koelsch, r8a7795/salvator-x and -xs, r8a7796/salvator-x, and
> sh73a0/kzm9g.
>
> Thanks for applying!
>
> Geert Uytterhoeven (3):
>   irqchip/renesas-intc-irqpin: Use wakeup_path i.s.o. explicit clock
>     handling
>   irqchip/renesas-irqc: Use wakeup_path i.s.o. explicit clock handling
>   gpio: rcar: Use wakeup_path i.s.o. explicit clock handling
>
>  drivers/gpio/gpio-rcar.c                  | 38 +++++++++++++----------------
>  drivers/irqchip/irq-renesas-intc-irqpin.c | 40 +++++++++++++------------------
>  drivers/irqchip/irq-renesas-irqc.c        | 30 ++++++++++++-----------
>  3 files changed, 48 insertions(+), 60 deletions(-)
>

For the series:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

Re: [PATCH v3 0/3] renesas: irqchip: Use wakeup_path i.s.o. explicit clock handling

[Marc Zyngier]

On 12/02/18 13:55, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> If an interrupt controller in a Renesas ARM SoC is part of a Clock
> Domain, and it is part of the wakeup path, it must be kept active during
> system suspend.
> 
> Currently this is handled in all interrupt controller drivers by
> explicitly increasing the use count of the module clock when the device
> is part of the wakeup path.  However, this explicit clock handling is
> merely a workaround for a failure to properly communicate wakeup
> information to the device core.
> 
> Hence this series fixes the affected drivers by setting the devices'
> power.wakeup_path fields instead, to indicate they are part of the
> wakeup path.  Depending on the PM Domain's active_wakeup configuration,
> the genpd core code will keep the device enabled (and the clock running)
> during system suspend when needed.
> 
> Target trees:
>   - Patches 1 and 2 are meant for the irqchip tree,
Patches queued for 4.17.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...