Linux-OMAP Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
@ 2020-09-11 12:31 Adam Ford
  2020-09-11 12:31 ` [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL Adam Ford
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Adam Ford @ 2020-09-11 12:31 UTC (permalink / raw)
  To: linux-pm, linux-omap
  Cc: aford, Adam Ford, kernel test robot, Andreas Kemnade,
	Tony Lindgren, Russell King, Eduardo Valentin, Keerthy,
	Zhang Rui, Daniel Lezcano, Amit Kucheria, linux-arm-kernel,
	linux-kernel

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 <aford173@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
Tested-by: Andreas Kemnade <andreas@kemnade.info> # 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


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

* [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL
  2020-09-11 12:31 [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management Adam Ford
@ 2020-09-11 12:31 ` Adam Ford
  2020-10-16 15:19   ` Adam Ford
  2020-10-08 20:59 ` [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management Adam Ford
  2020-10-08 23:23 ` Daniel Lezcano
  2 siblings, 1 reply; 11+ messages in thread
From: Adam Ford @ 2020-09-11 12:31 UTC (permalink / raw)
  To: linux-pm, linux-omap
  Cc: aford, Adam Ford, Tony Lindgren, Russell King, Eduardo Valentin,
	Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria,
	linux-arm-kernel, linux-kernel

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

Signed-off-by: Adam Ford <aford173@gmail.com>
---
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


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

* Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
  2020-09-11 12:31 [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management Adam Ford
  2020-09-11 12:31 ` [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL Adam Ford
@ 2020-10-08 20:59 ` Adam Ford
  2020-10-08 23:23 ` Daniel Lezcano
  2 siblings, 0 replies; 11+ messages in thread
From: Adam Ford @ 2020-10-08 20:59 UTC (permalink / raw)
  To: linux-pm, Linux-OMAP
  Cc: Adam Ford-BE, kernel test robot, Andreas Kemnade, Tony Lindgren,
	Russell King, Eduardo Valentin, Keerthy, Zhang Rui,
	Daniel Lezcano, Amit Kucheria, arm-soc,
	Linux Kernel Mailing List

On Fri, Sep 11, 2020 at 7:32 AM Adam Ford <aford173@gmail.com> 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 <aford173@gmail.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Tested-by: Andreas Kemnade <andreas@kemnade.info> # 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
>

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

* Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
  2020-09-11 12:31 [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management Adam Ford
  2020-09-11 12:31 ` [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL Adam Ford
  2020-10-08 20:59 ` [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management Adam Ford
@ 2020-10-08 23:23 ` Daniel Lezcano
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Lezcano @ 2020-10-08 23:23 UTC (permalink / raw)
  To: Adam Ford, linux-pm, linux-omap
  Cc: aford, kernel test robot, Andreas Kemnade, Tony Lindgren,
	Russell King, Eduardo Valentin, Keerthy, Zhang Rui,
	Amit Kucheria, linux-arm-kernel, linux-kernel

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 <aford173@gmail.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Tested-by: Andreas Kemnade <andreas@kemnade.info> # 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

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

* Re: [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL
  2020-09-11 12:31 ` [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL Adam Ford
@ 2020-10-16 15:19   ` Adam Ford
  0 siblings, 0 replies; 11+ messages in thread
From: Adam Ford @ 2020-10-16 15:19 UTC (permalink / raw)
  To: linux-pm, Linux-OMAP
  Cc: Adam Ford-BE, Tony Lindgren, Russell King, Eduardo Valentin,
	Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria, arm-soc,
	Linux Kernel Mailing List

On Fri, Sep 11, 2020 at 7:32 AM Adam Ford <aford173@gmail.com> wrote:
>
> With the additional power management options enabled,
> this patch enables OMAP3_THERMAL by default.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
> 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
>

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

* Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
  2020-08-19 12:21   ` Adam Ford
@ 2020-08-19 12:54     ` Tony Lindgren
  0 siblings, 0 replies; 11+ messages in thread
From: Tony Lindgren @ 2020-08-19 12:54 UTC (permalink / raw)
  To: Adam Ford
  Cc: Linux-OMAP, Adam Ford-BE, Russell King, Eduardo Valentin,
	Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria, arm-soc,
	Linux Kernel Mailing List, linux-pm

* Adam Ford <aford173@gmail.com> [200819 12:21]:
> On Tue, Aug 18, 2020 at 11:58 PM Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Adam Ford <aford173@gmail.com> [200818 15:46]:
> > > @@ -1153,6 +1166,38 @@ static int ti_bandgap_suspend(struct device *dev)
> > >       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;
> > > +}
> >
> > Hmm to me it looks like is_suspended is not used right now?
> > I guess you want to set it in ti_bandgap_suspend() and clear
> > it in ti_bandgap_resume()?
> >
> > Otherwise looks good to me, I can't test the power consumption
> > right now though so you may want to check it to make sure
> > device still hits off mode during idle.
> 
> I have a V2.  Do you want me to re-post 2/2 with V2 as no change, or
> should I just submit this patch alone?

Up to you, might make it easier for other folks to follow if the
whole series is reposted.

Regards,

Tony

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

* Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
  2020-08-19  4:59 ` Tony Lindgren
@ 2020-08-19 12:21   ` Adam Ford
  2020-08-19 12:54     ` Tony Lindgren
  0 siblings, 1 reply; 11+ messages in thread
From: Adam Ford @ 2020-08-19 12:21 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linux-OMAP, Adam Ford-BE, Russell King, Eduardo Valentin,
	Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria, arm-soc,
	Linux Kernel Mailing List, linux-pm

On Tue, Aug 18, 2020 at 11:58 PM Tony Lindgren <tony@atomide.com> wrote:
>
> * Adam Ford <aford173@gmail.com> [200818 15:46]:
> > @@ -1153,6 +1166,38 @@ static int ti_bandgap_suspend(struct device *dev)
> >       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;
> > +}
>
> Hmm to me it looks like is_suspended is not used right now?
> I guess you want to set it in ti_bandgap_suspend() and clear
> it in ti_bandgap_resume()?
>
> Otherwise looks good to me, I can't test the power consumption
> right now though so you may want to check it to make sure
> device still hits off mode during idle.

I have a V2.  Do you want me to re-post 2/2 with V2 as no change, or
should I just submit this patch alone?

adam
>
> Regards,
>
> Tony

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

* Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
  2020-08-18 15:46 Adam Ford
  2020-08-18 19:49 ` kernel test robot
  2020-08-18 20:01 ` kernel test robot
@ 2020-08-19  4:59 ` Tony Lindgren
  2020-08-19 12:21   ` Adam Ford
  2 siblings, 1 reply; 11+ messages in thread
From: Tony Lindgren @ 2020-08-19  4:59 UTC (permalink / raw)
  To: Adam Ford
  Cc: linux-omap, aford, Russell King, Eduardo Valentin, Keerthy,
	Zhang Rui, Daniel Lezcano, Amit Kucheria, linux-arm-kernel,
	linux-kernel, linux-pm

* Adam Ford <aford173@gmail.com> [200818 15:46]:
> @@ -1153,6 +1166,38 @@ static int ti_bandgap_suspend(struct device *dev)
>  	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;
> +}

Hmm to me it looks like is_suspended is not used right now?
I guess you want to set it in ti_bandgap_suspend() and clear
it in ti_bandgap_resume()?

Otherwise looks good to me, I can't test the power consumption
right now though so you may want to check it to make sure
device still hits off mode during idle.

Regards,

Tony

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

* Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
  2020-08-18 15:46 Adam Ford
  2020-08-18 19:49 ` kernel test robot
@ 2020-08-18 20:01 ` kernel test robot
  2020-08-19  4:59 ` Tony Lindgren
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-08-18 20:01 UTC (permalink / raw)
  To: Adam Ford, linux-omap
  Cc: kbuild-all, clang-built-linux, aford, Adam Ford, Tony Lindgren,
	Russell King, Eduardo Valentin, Keerthy, Zhang Rui,
	Daniel Lezcano, Amit Kucheria


[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]

Hi Adam,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on omap/for-next]
[also build test WARNING on balbi-usb/testing/next linus/master v5.9-rc1 next-20200818]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Adam-Ford/thermal-ti-soc-thermal-Enable-addition-power-management/20200819-000006
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git for-next
config: riscv-randconfig-r033-20200818 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 790878f291fa5dc58a1c560cb6cc76fd1bfd1c5a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/thermal/ti-soc-thermal/ti-bandgap.c:38:12: warning: function 'bandgap_omap_cpu_notifier' has internal linkage but is not defined [-Wundefined-internal]
   static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
              ^
   drivers/thermal/ti-soc-thermal/ti-bandgap.c:1019:26: note: used here
           bgp->nb.notifier_call = bandgap_omap_cpu_notifier;
                                   ^
   1 warning generated.

# https://github.com/0day-ci/linux/commit/ad9a82ecd12253bef4dfda9192916ae04b09ea6e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Adam-Ford/thermal-ti-soc-thermal-Enable-addition-power-management/20200819-000006
git checkout ad9a82ecd12253bef4dfda9192916ae04b09ea6e
vim +/bandgap_omap_cpu_notifier +38 drivers/thermal/ti-soc-thermal/ti-bandgap.c

    36	
    37	static int ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id);
  > 38	static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
    39					  unsigned long cmd, void *v);
    40	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38279 bytes --]

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

* Re: [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
  2020-08-18 15:46 Adam Ford
@ 2020-08-18 19:49 ` kernel test robot
  2020-08-18 20:01 ` kernel test robot
  2020-08-19  4:59 ` Tony Lindgren
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-08-18 19:49 UTC (permalink / raw)
  To: Adam Ford, linux-omap
  Cc: kbuild-all, aford, Adam Ford, Tony Lindgren, Russell King,
	Eduardo Valentin, Keerthy, Zhang Rui, Daniel Lezcano,
	Amit Kucheria


[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

Hi Adam,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on omap/for-next]
[also build test WARNING on balbi-usb/testing/next linus/master v5.9-rc1 next-20200818]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Adam-Ford/thermal-ti-soc-thermal-Enable-addition-power-management/20200819-000006
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git for-next
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/thermal/ti-soc-thermal/ti-bandgap.c:38:12: warning: 'bandgap_omap_cpu_notifier' used but never defined
      38 | static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~

# https://github.com/0day-ci/linux/commit/ad9a82ecd12253bef4dfda9192916ae04b09ea6e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Adam-Ford/thermal-ti-soc-thermal-Enable-addition-power-management/20200819-000006
git checkout ad9a82ecd12253bef4dfda9192916ae04b09ea6e
vim +/bandgap_omap_cpu_notifier +38 drivers/thermal/ti-soc-thermal/ti-bandgap.c

    36	
    37	static int ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id);
  > 38	static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
    39					  unsigned long cmd, void *v);
    40	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57199 bytes --]

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

* [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management
@ 2020-08-18 15:46 Adam Ford
  2020-08-18 19:49 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Adam Ford @ 2020-08-18 15:46 UTC (permalink / raw)
  To: linux-omap
  Cc: aford, Adam Ford, Tony Lindgren, Russell King, Eduardo Valentin,
	Keerthy, Zhang Rui, Daniel Lezcano, Amit Kucheria,
	linux-arm-kernel, linux-kernel, linux-pm

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 <aford173@gmail.com>

diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index ab19ceff6e2a..923ef3abb867 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -25,10 +25,18 @@
 #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);
+static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
+				  unsigned long cmd, void *v);
 
 /***   Helper functions to access registers and their bitfields   ***/
 
@@ -1008,6 +1016,9 @@ int ti_bandgap_probe(struct platform_device *pdev)
 		}
 	}
 
+	bgp->nb.notifier_call = bandgap_omap_cpu_notifier;
+	cpu_pm_register_notifier(&bgp->nb);
+
 	return 0;
 
 remove_last_cooling:
@@ -1041,7 +1052,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);
@@ -1153,6 +1166,38 @@ static int ti_bandgap_suspend(struct device *dev)
 	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);
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.17.1


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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11 12:31 [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management Adam Ford
2020-09-11 12:31 ` [PATCH 2/2] ARM: omap2plus_defconfig: Enable OMAP3_THERMAL Adam Ford
2020-10-16 15:19   ` Adam Ford
2020-10-08 20:59 ` [PATCH 1/2] thermal: ti-soc-thermal: Enable addition power management Adam Ford
2020-10-08 23:23 ` Daniel Lezcano
  -- strict thread matches above, loose matches on Subject: below --
2020-08-18 15:46 Adam Ford
2020-08-18 19:49 ` kernel test robot
2020-08-18 20:01 ` kernel test robot
2020-08-19  4:59 ` Tony Lindgren
2020-08-19 12:21   ` Adam Ford
2020-08-19 12:54     ` Tony Lindgren

Linux-OMAP Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-omap/0 linux-omap/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-omap linux-omap/ https://lore.kernel.org/linux-omap \
		linux-omap@vger.kernel.org
	public-inbox-index linux-omap

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-omap


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git