2020-09-11 12:43:12

by Adam Ford

[permalink] [raw]
Subject: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management

The bandgap sensor can be idled when the processor is too, but it
isn't currently being done, so the power consumption of OMAP3
boards can elevated if the bangap sensor is enabled.

This patch attempts to use some additional power management
to idle the clock to the bandgap when not needed.

Signed-off-by: Adam Ford <[email protected]>
Reported-by: kernel test robot <[email protected]>
Tested-by: Andreas Kemnade <[email protected]> # GTA04
---
V3: bandgap_omap_cpu_notifier is only defined when CONFIG_PM_SLEEP
is enabled, so make all references to it also depend on
CONFIG_PM_SLEEP as well

V2: Fix issue where variable stating the suspend mode isn't being
properly set and cleared.

diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index ab19ceff6e2a..5e596168ba73 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -25,10 +25,20 @@
#include <linux/of_platform.h>
#include <linux/of_irq.h>
#include <linux/io.h>
+#include <linux/cpu_pm.h>
+#include <linux/device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm.h>
+#include <linux/of.h>
+#include <linux/of_device.h>

#include "ti-bandgap.h"

static int ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id);
+#ifdef CONFIG_PM_SLEEP
+static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
+ unsigned long cmd, void *v);
+#endif

/*** Helper functions to access registers and their bitfields ***/

@@ -1008,6 +1018,11 @@ int ti_bandgap_probe(struct platform_device *pdev)
}
}

+#ifdef CONFIG_PM_SLEEP
+ bgp->nb.notifier_call = bandgap_omap_cpu_notifier;
+ cpu_pm_register_notifier(&bgp->nb);
+#endif
+
return 0;

remove_last_cooling:
@@ -1041,7 +1056,9 @@ int ti_bandgap_remove(struct platform_device *pdev)
struct ti_bandgap *bgp = platform_get_drvdata(pdev);
int i;

- /* First thing is to remove sensor interfaces */
+ cpu_pm_unregister_notifier(&bgp->nb);
+
+ /* Remove sensor interfaces */
for (i = 0; i < bgp->conf->sensor_count; i++) {
if (bgp->conf->sensors[i].unregister_cooling)
bgp->conf->sensors[i].unregister_cooling(bgp, i);
@@ -1150,9 +1167,43 @@ static int ti_bandgap_suspend(struct device *dev)
if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
clk_disable_unprepare(bgp->fclock);

+ bgp->is_suspended = true;
+
return err;
}

+static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
+ unsigned long cmd, void *v)
+{
+ struct ti_bandgap *bgp;
+
+ bgp = container_of(nb, struct ti_bandgap, nb);
+
+ spin_lock(&bgp->lock);
+ switch (cmd) {
+ case CPU_CLUSTER_PM_ENTER:
+ if (bgp->is_suspended)
+ break;
+ ti_bandgap_save_ctxt(bgp);
+ ti_bandgap_power(bgp, false);
+ if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
+ clk_disable(bgp->fclock);
+ break;
+ case CPU_CLUSTER_PM_ENTER_FAILED:
+ case CPU_CLUSTER_PM_EXIT:
+ if (bgp->is_suspended)
+ break;
+ if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
+ clk_enable(bgp->fclock);
+ ti_bandgap_power(bgp, true);
+ ti_bandgap_restore_ctxt(bgp);
+ break;
+ }
+ spin_unlock(&bgp->lock);
+
+ return NOTIFY_OK;
+}
+
static int ti_bandgap_resume(struct device *dev)
{
struct ti_bandgap *bgp = dev_get_drvdata(dev);
@@ -1161,6 +1212,7 @@ static int ti_bandgap_resume(struct device *dev)
clk_prepare_enable(bgp->fclock);

ti_bandgap_power(bgp, true);
+ bgp->is_suspended = false;

return ti_bandgap_restore_ctxt(bgp);
}
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
index fce4657e9486..ed0ea4b17b25 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
@@ -12,6 +12,10 @@
#include <linux/spinlock.h>
#include <linux/types.h>
#include <linux/err.h>
+#include <linux/cpu_pm.h>
+#include <linux/device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm.h>

struct gpio_desc;

@@ -203,6 +207,8 @@ struct ti_bandgap {
int irq;
struct gpio_desc *tshut_gpiod;
u32 clk_rate;
+ struct notifier_block nb;
+ unsigned int is_suspended:1;
};

/**
--
2.25.1


2020-09-11 12:43:32

by Adam Ford

[permalink] [raw]
Subject: [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL

With the additional power management options enabled,
this patch enables OMAP3_THERMAL by default.

Signed-off-by: Adam Ford <[email protected]>
---
V3: No change
V2: No change

diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index fe383f5a92fb..efcc46305a47 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -303,6 +303,7 @@ CONFIG_THERMAL_GOV_FAIR_SHARE=y
CONFIG_THERMAL_GOV_USER_SPACE=y
CONFIG_CPU_THERMAL=y
CONFIG_TI_THERMAL=y
+CONFIG_OMAP3_THERMAL=y
CONFIG_OMAP4_THERMAL=y
CONFIG_OMAP5_THERMAL=y
CONFIG_DRA752_THERMAL=y
--
2.25.1

2020-10-08 21:43:49

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management

On Fri, Sep 11, 2020 at 7:32 AM Adam Ford <[email protected]> wrote:
>
> The bandgap sensor can be idled when the processor is too, but it
> isn't currently being done, so the power consumption of OMAP3
> boards can elevated if the bangap sensor is enabled.
>
> This patch attempts to use some additional power management
> to idle the clock to the bandgap when not needed.
>
> Signed-off-by: Adam Ford <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Tested-by: Andreas Kemnade <[email protected]> # GTA04
> ---
> V3: bandgap_omap_cpu_notifier is only defined when CONFIG_PM_SLEEP
> is enabled, so make all references to it also depend on
> CONFIG_PM_SLEEP as well
>

Gentle nudge on this one. It was in the queue for a while, and got
lost, then resurrected again.

thanks

adam

> V2: Fix issue where variable stating the suspend mode isn't being
> properly set and cleared.
>
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> index ab19ceff6e2a..5e596168ba73 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> @@ -25,10 +25,20 @@
> #include <linux/of_platform.h>
> #include <linux/of_irq.h>
> #include <linux/io.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
> #include "ti-bandgap.h"
>
> static int ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id);
> +#ifdef CONFIG_PM_SLEEP
> +static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
> + unsigned long cmd, void *v);
> +#endif
>
> /*** Helper functions to access registers and their bitfields ***/
>
> @@ -1008,6 +1018,11 @@ int ti_bandgap_probe(struct platform_device *pdev)
> }
> }
>
> +#ifdef CONFIG_PM_SLEEP
> + bgp->nb.notifier_call = bandgap_omap_cpu_notifier;
> + cpu_pm_register_notifier(&bgp->nb);
> +#endif
> +
> return 0;
>
> remove_last_cooling:
> @@ -1041,7 +1056,9 @@ int ti_bandgap_remove(struct platform_device *pdev)
> struct ti_bandgap *bgp = platform_get_drvdata(pdev);
> int i;
>
> - /* First thing is to remove sensor interfaces */
> + cpu_pm_unregister_notifier(&bgp->nb);
> +
> + /* Remove sensor interfaces */
> for (i = 0; i < bgp->conf->sensor_count; i++) {
> if (bgp->conf->sensors[i].unregister_cooling)
> bgp->conf->sensors[i].unregister_cooling(bgp, i);
> @@ -1150,9 +1167,43 @@ static int ti_bandgap_suspend(struct device *dev)
> if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
> clk_disable_unprepare(bgp->fclock);
>
> + bgp->is_suspended = true;
> +
> return err;
> }
>
> +static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
> + unsigned long cmd, void *v)
> +{
> + struct ti_bandgap *bgp;
> +
> + bgp = container_of(nb, struct ti_bandgap, nb);
> +
> + spin_lock(&bgp->lock);
> + switch (cmd) {
> + case CPU_CLUSTER_PM_ENTER:
> + if (bgp->is_suspended)
> + break;
> + ti_bandgap_save_ctxt(bgp);
> + ti_bandgap_power(bgp, false);
> + if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
> + clk_disable(bgp->fclock);
> + break;
> + case CPU_CLUSTER_PM_ENTER_FAILED:
> + case CPU_CLUSTER_PM_EXIT:
> + if (bgp->is_suspended)
> + break;
> + if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
> + clk_enable(bgp->fclock);
> + ti_bandgap_power(bgp, true);
> + ti_bandgap_restore_ctxt(bgp);
> + break;
> + }
> + spin_unlock(&bgp->lock);
> +
> + return NOTIFY_OK;
> +}
> +
> static int ti_bandgap_resume(struct device *dev)
> {
> struct ti_bandgap *bgp = dev_get_drvdata(dev);
> @@ -1161,6 +1212,7 @@ static int ti_bandgap_resume(struct device *dev)
> clk_prepare_enable(bgp->fclock);
>
> ti_bandgap_power(bgp, true);
> + bgp->is_suspended = false;
>
> return ti_bandgap_restore_ctxt(bgp);
> }
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> index fce4657e9486..ed0ea4b17b25 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> @@ -12,6 +12,10 @@
> #include <linux/spinlock.h>
> #include <linux/types.h>
> #include <linux/err.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm.h>
>
> struct gpio_desc;
>
> @@ -203,6 +207,8 @@ struct ti_bandgap {
> int irq;
> struct gpio_desc *tshut_gpiod;
> u32 clk_rate;
> + struct notifier_block nb;
> + unsigned int is_suspended:1;
> };
>
> /**
> --
> 2.25.1
>

2020-10-09 00:43:09

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management

On 11/09/2020 14:31, Adam Ford wrote:
> The bandgap sensor can be idled when the processor is too, but it
> isn't currently being done, so the power consumption of OMAP3
> boards can elevated if the bangap sensor is enabled.
>
> This patch attempts to use some additional power management
> to idle the clock to the bandgap when not needed.
>
> Signed-off-by: Adam Ford <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Tested-by: Andreas Kemnade <[email protected]> # GTA04
> ---

[ ... ]

> - /* First thing is to remove sensor interfaces */
> + cpu_pm_unregister_notifier(&bgp->nb);
> +
> + /* Remove sensor interfaces */
> for (i = 0; i < bgp->conf->sensor_count; i++) {
> if (bgp->conf->sensors[i].unregister_cooling)
> bgp->conf->sensors[i].unregister_cooling(bgp, i);
> @@ -1150,9 +1167,43 @@ static int ti_bandgap_suspend(struct device *dev)
> if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
> clk_disable_unprepare(bgp->fclock);
>
> + bgp->is_suspended = true;

Is this flag really needed?




--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2020-10-16 18:46:26

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL

On Fri, Sep 11, 2020 at 7:32 AM Adam Ford <[email protected]> wrote:
>
> With the additional power management options enabled,
> this patch enables OMAP3_THERMAL by default.
>
> Signed-off-by: Adam Ford <[email protected]>
> ---
> V3: No change
> V2: No change

Tony,

Can you apply [2/2] to the OMAP branch?

It looks like 1/2 was applied to the linux-pm [1]

thanks,

adam
[1] - https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/commit/?h=thermal/linux-next&id=5093402e5b449b64f7bbaa09057ce40a8f3c1484



>
> diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
> index fe383f5a92fb..efcc46305a47 100644
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -303,6 +303,7 @@ CONFIG_THERMAL_GOV_FAIR_SHARE=y
> CONFIG_THERMAL_GOV_USER_SPACE=y
> CONFIG_CPU_THERMAL=y
> CONFIG_TI_THERMAL=y
> +CONFIG_OMAP3_THERMAL=y
> CONFIG_OMAP4_THERMAL=y
> CONFIG_OMAP5_THERMAL=y
> CONFIG_DRA752_THERMAL=y
> --
> 2.25.1
>

2020-11-12 11:39:19

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL

On Fri, Oct 16, 2020 at 11:19 AM Adam Ford <[email protected]> wrote:
>
> On Fri, Sep 11, 2020 at 7:32 AM Adam Ford <[email protected]> wrote:
> >
> > With the additional power management options enabled,
> > this patch enables OMAP3_THERMAL by default.
> >
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > V3: No change
> > V2: No change
>
> Tony,
>
> Can you apply [2/2] to the OMAP branch?
>
> It looks like 1/2 was applied to the linux-pm [1]

Tony,

The defconfig update doesn't show in your branch. Is there someone
else I should ping about this?

thanks

>
> thanks,
>
> adam
> [1] - https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/commit/?h=thermal/linux-next&id=5093402e5b449b64f7bbaa09057ce40a8f3c1484
>
>
>
> >
> > diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
> > index fe383f5a92fb..efcc46305a47 100644
> > --- a/arch/arm/configs/omap2plus_defconfig
> > +++ b/arch/arm/configs/omap2plus_defconfig
> > @@ -303,6 +303,7 @@ CONFIG_THERMAL_GOV_FAIR_SHARE=y
> > CONFIG_THERMAL_GOV_USER_SPACE=y
> > CONFIG_CPU_THERMAL=y
> > CONFIG_TI_THERMAL=y
> > +CONFIG_OMAP3_THERMAL=y
> > CONFIG_OMAP4_THERMAL=y
> > CONFIG_OMAP5_THERMAL=y
> > CONFIG_DRA752_THERMAL=y
> > --
> > 2.25.1
> >

2020-11-16 09:02:56

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL

* Adam Ford <[email protected]> [201016 15:19]:
> On Fri, Sep 11, 2020 at 7:32 AM Adam Ford <[email protected]> wrote:
> >
> > With the additional power management options enabled,
> > this patch enables OMAP3_THERMAL by default.
> >
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > V3: No change
> > V2: No change
>
> Tony,
>
> Can you apply [2/2] to the OMAP branch?

Sorry for the delay, applying into omap-for-v5.11/defconfig thanks.

Regards,

Tony