[V4,3/3] cpufreq: add suspend/resume support in Armada 37xx DVFS driver

Message ID d6be605205f6af01968a99582b05026aec2eca42.1524737432.git.viresh.kumar@linaro.org
State New
Headers show
Series
  • Untitled series #11000
Related show

Commit Message

Viresh Kumar April 26, 2018, 10:12 a.m.
From: Miquel Raynal <miquel.raynal@bootlin.com>


Add suspend/resume hooks in Armada 37xx DVFS driver to handle S2RAM
operations. As there is currently no 'driver' structure, create one
to store both the regmap and the register values during suspend
operation.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
V3->V4:
- Fix the "unused put_clk label" build error that happened between V2
  and V3.

 drivers/cpufreq/armada-37xx-cpufreq.c | 67 +++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 2 deletions(-)

-- 
2.15.0.194.g9af6a3dea062

Comments

Gregory CLEMENT May 2, 2018, 3:52 p.m. | #1
Hi Viresh,
 
 On jeu., avril 26 2018, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> From: Miquel Raynal <miquel.raynal@bootlin.com>

>

> Add suspend/resume hooks in Armada 37xx DVFS driver to handle S2RAM

> operations. As there is currently no 'driver' structure, create one

> to store both the regmap and the register values during suspend

> operation.


I tested this patch on a Armada 3720 based board (EspressoBin), and it
worked well. I even made some power consumption measurement and after
the resume, the power consumption had the same value that before the
suspend, for low CPU frequency and high CPU frequency.

Tested-by: Gregory CLEMENT <gregory.clement@bootlin.com>


Thanks,

Gregory



>

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> ---

> V3->V4:

> - Fix the "unused put_clk label" build error that happened between V2

>   and V3.

>

>  drivers/cpufreq/armada-37xx-cpufreq.c | 67 +++++++++++++++++++++++++++++++++--

>  1 file changed, 65 insertions(+), 2 deletions(-)

>

> diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c

> index 1d5db6f6ace9..739da90ff3f6 100644

> --- a/drivers/cpufreq/armada-37xx-cpufreq.c

> +++ b/drivers/cpufreq/armada-37xx-cpufreq.c

> @@ -23,6 +23,8 @@

>  #include <linux/regmap.h>

>  #include <linux/slab.h>

>  

> +#include "cpufreq-dt.h"

> +

>  /* Power management in North Bridge register set */

>  #define ARMADA_37XX_NB_L0L1	0x18

>  #define ARMADA_37XX_NB_L2L3	0x1C

> @@ -56,6 +58,16 @@

>   */

>  #define LOAD_LEVEL_NR	4

>  

> +struct armada37xx_cpufreq_state {

> +	struct regmap *regmap;

> +	u32 nb_l0l1;

> +	u32 nb_l2l3;

> +	u32 nb_dyn_mod;

> +	u32 nb_cpu_load;

> +};

> +

> +static struct armada37xx_cpufreq_state *armada37xx_cpufreq_state;

> +

>  struct armada_37xx_dvfs {

>  	u32 cpu_freq_max;

>  	u8 divider[LOAD_LEVEL_NR];

> @@ -136,7 +148,7 @@ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base,

>  	clk_set_parent(clk, parent);

>  }

>  

> -static void __init armada37xx_cpufreq_disable_dvfs(struct regmap *base)

> +static void armada37xx_cpufreq_disable_dvfs(struct regmap *base)

>  {

>  	unsigned int reg = ARMADA_37XX_NB_DYN_MOD,

>  		mask = ARMADA_37XX_NB_DFS_EN;

> @@ -162,8 +174,44 @@ static void __init armada37xx_cpufreq_enable_dvfs(struct regmap *base)

>  	regmap_update_bits(base, reg, mask, mask);

>  }

>  

> +static int armada37xx_cpufreq_suspend(struct cpufreq_policy *policy)

> +{

> +	struct armada37xx_cpufreq_state *state = armada37xx_cpufreq_state;

> +

> +	regmap_read(state->regmap, ARMADA_37XX_NB_L0L1, &state->nb_l0l1);

> +	regmap_read(state->regmap, ARMADA_37XX_NB_L2L3, &state->nb_l2l3);

> +	regmap_read(state->regmap, ARMADA_37XX_NB_CPU_LOAD,

> +		    &state->nb_cpu_load);

> +	regmap_read(state->regmap, ARMADA_37XX_NB_DYN_MOD, &state->nb_dyn_mod);

> +

> +	return 0;

> +}

> +

> +static int armada37xx_cpufreq_resume(struct cpufreq_policy *policy)

> +{

> +	struct armada37xx_cpufreq_state *state = armada37xx_cpufreq_state;

> +

> +	/* Ensure DVFS is disabled otherwise the following registers are RO */

> +	armada37xx_cpufreq_disable_dvfs(state->regmap);

> +

> +	regmap_write(state->regmap, ARMADA_37XX_NB_L0L1, state->nb_l0l1);

> +	regmap_write(state->regmap, ARMADA_37XX_NB_L2L3, state->nb_l2l3);

> +	regmap_write(state->regmap, ARMADA_37XX_NB_CPU_LOAD,

> +		     state->nb_cpu_load);

> +

> +	/*

> +	 * NB_DYN_MOD register is the one that actually enable back DVFS if it

> +	 * was enabled before the suspend operation. This must be done last

> +	 * otherwise other registers are not writable.

> +	 */

> +	regmap_write(state->regmap, ARMADA_37XX_NB_DYN_MOD, state->nb_dyn_mod);

> +

> +	return 0;

> +}

> +

>  static int __init armada37xx_cpufreq_driver_init(void)

>  {

> +	struct cpufreq_dt_platform_data pdata;

>  	struct armada_37xx_dvfs *dvfs;

>  	struct platform_device *pdev;

>  	unsigned long freq;

> @@ -213,6 +261,15 @@ static int __init armada37xx_cpufreq_driver_init(void)

>  		return -EINVAL;

>  	}

>  

> +	armada37xx_cpufreq_state = kmalloc(sizeof(*armada37xx_cpufreq_state),

> +					   GFP_KERNEL);

> +	if (!armada37xx_cpufreq_state) {

> +		clk_put(clk);

> +		return -ENOMEM;

> +	}

> +

> +	armada37xx_cpufreq_state->regmap = nb_pm_base;

> +

>  	armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider);

>  	clk_put(clk);

>  

> @@ -228,7 +285,11 @@ static int __init armada37xx_cpufreq_driver_init(void)

>  	/* Now that everything is setup, enable the DVFS at hardware level */

>  	armada37xx_cpufreq_enable_dvfs(nb_pm_base);

>  

> -	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);

> +	pdata.suspend = armada37xx_cpufreq_suspend;

> +	pdata.resume = armada37xx_cpufreq_resume;

> +

> +	pdev = platform_device_register_data(NULL, "cpufreq-dt", -1, &pdata,

> +					     sizeof(pdata));

>  	ret = PTR_ERR_OR_ZERO(pdev);

>  	if (ret)

>  		goto disable_dvfs;

> @@ -244,6 +305,8 @@ static int __init armada37xx_cpufreq_driver_init(void)

>  		dev_pm_opp_remove(cpu_dev, freq);

>  	}

>  

> +	kfree(armada37xx_cpufreq_state);

> +

>  	return ret;

>  }

>  /* late_initcall, to guarantee the driver is loaded after A37xx clock driver */

> -- 

> 2.15.0.194.g9af6a3dea062

>


-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Viresh Kumar May 3, 2018, 4:36 a.m. | #2
On 02-05-18, 17:52, Gregory CLEMENT wrote:
> Hi Viresh,

>  

>  On jeu., avril 26 2018, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> 

> > From: Miquel Raynal <miquel.raynal@bootlin.com>

> >

> > Add suspend/resume hooks in Armada 37xx DVFS driver to handle S2RAM

> > operations. As there is currently no 'driver' structure, create one

> > to store both the regmap and the register values during suspend

> > operation.

> 

> I tested this patch on a Armada 3720 based board (EspressoBin), and it

> worked well. I even made some power consumption measurement and after

> the resume, the power consumption had the same value that before the

> suspend, for low CPU frequency and high CPU frequency.

> 

> Tested-by: Gregory CLEMENT <gregory.clement@bootlin.com>


Thanks Gregory.

-- 
viresh

Patch

diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c
index 1d5db6f6ace9..739da90ff3f6 100644
--- a/drivers/cpufreq/armada-37xx-cpufreq.c
+++ b/drivers/cpufreq/armada-37xx-cpufreq.c
@@ -23,6 +23,8 @@ 
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
+#include "cpufreq-dt.h"
+
 /* Power management in North Bridge register set */
 #define ARMADA_37XX_NB_L0L1	0x18
 #define ARMADA_37XX_NB_L2L3	0x1C
@@ -56,6 +58,16 @@ 
  */
 #define LOAD_LEVEL_NR	4
 
+struct armada37xx_cpufreq_state {
+	struct regmap *regmap;
+	u32 nb_l0l1;
+	u32 nb_l2l3;
+	u32 nb_dyn_mod;
+	u32 nb_cpu_load;
+};
+
+static struct armada37xx_cpufreq_state *armada37xx_cpufreq_state;
+
 struct armada_37xx_dvfs {
 	u32 cpu_freq_max;
 	u8 divider[LOAD_LEVEL_NR];
@@ -136,7 +148,7 @@  static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base,
 	clk_set_parent(clk, parent);
 }
 
-static void __init armada37xx_cpufreq_disable_dvfs(struct regmap *base)
+static void armada37xx_cpufreq_disable_dvfs(struct regmap *base)
 {
 	unsigned int reg = ARMADA_37XX_NB_DYN_MOD,
 		mask = ARMADA_37XX_NB_DFS_EN;
@@ -162,8 +174,44 @@  static void __init armada37xx_cpufreq_enable_dvfs(struct regmap *base)
 	regmap_update_bits(base, reg, mask, mask);
 }
 
+static int armada37xx_cpufreq_suspend(struct cpufreq_policy *policy)
+{
+	struct armada37xx_cpufreq_state *state = armada37xx_cpufreq_state;
+
+	regmap_read(state->regmap, ARMADA_37XX_NB_L0L1, &state->nb_l0l1);
+	regmap_read(state->regmap, ARMADA_37XX_NB_L2L3, &state->nb_l2l3);
+	regmap_read(state->regmap, ARMADA_37XX_NB_CPU_LOAD,
+		    &state->nb_cpu_load);
+	regmap_read(state->regmap, ARMADA_37XX_NB_DYN_MOD, &state->nb_dyn_mod);
+
+	return 0;
+}
+
+static int armada37xx_cpufreq_resume(struct cpufreq_policy *policy)
+{
+	struct armada37xx_cpufreq_state *state = armada37xx_cpufreq_state;
+
+	/* Ensure DVFS is disabled otherwise the following registers are RO */
+	armada37xx_cpufreq_disable_dvfs(state->regmap);
+
+	regmap_write(state->regmap, ARMADA_37XX_NB_L0L1, state->nb_l0l1);
+	regmap_write(state->regmap, ARMADA_37XX_NB_L2L3, state->nb_l2l3);
+	regmap_write(state->regmap, ARMADA_37XX_NB_CPU_LOAD,
+		     state->nb_cpu_load);
+
+	/*
+	 * NB_DYN_MOD register is the one that actually enable back DVFS if it
+	 * was enabled before the suspend operation. This must be done last
+	 * otherwise other registers are not writable.
+	 */
+	regmap_write(state->regmap, ARMADA_37XX_NB_DYN_MOD, state->nb_dyn_mod);
+
+	return 0;
+}
+
 static int __init armada37xx_cpufreq_driver_init(void)
 {
+	struct cpufreq_dt_platform_data pdata;
 	struct armada_37xx_dvfs *dvfs;
 	struct platform_device *pdev;
 	unsigned long freq;
@@ -213,6 +261,15 @@  static int __init armada37xx_cpufreq_driver_init(void)
 		return -EINVAL;
 	}
 
+	armada37xx_cpufreq_state = kmalloc(sizeof(*armada37xx_cpufreq_state),
+					   GFP_KERNEL);
+	if (!armada37xx_cpufreq_state) {
+		clk_put(clk);
+		return -ENOMEM;
+	}
+
+	armada37xx_cpufreq_state->regmap = nb_pm_base;
+
 	armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider);
 	clk_put(clk);
 
@@ -228,7 +285,11 @@  static int __init armada37xx_cpufreq_driver_init(void)
 	/* Now that everything is setup, enable the DVFS at hardware level */
 	armada37xx_cpufreq_enable_dvfs(nb_pm_base);
 
-	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+	pdata.suspend = armada37xx_cpufreq_suspend;
+	pdata.resume = armada37xx_cpufreq_resume;
+
+	pdev = platform_device_register_data(NULL, "cpufreq-dt", -1, &pdata,
+					     sizeof(pdata));
 	ret = PTR_ERR_OR_ZERO(pdev);
 	if (ret)
 		goto disable_dvfs;
@@ -244,6 +305,8 @@  static int __init armada37xx_cpufreq_driver_init(void)
 		dev_pm_opp_remove(cpu_dev, freq);
 	}
 
+	kfree(armada37xx_cpufreq_state);
+
 	return ret;
 }
 /* late_initcall, to guarantee the driver is loaded after A37xx clock driver */