diff mbox series

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

Message ID e98965068836e673ea826e8a03fd01e168fce3ed.1524561482.git.viresh.kumar@linaro.org
State Superseded
Headers show
Series cpufreq: dt: Allow platforms to provide suspend/resume hooks | expand

Commit Message

Viresh Kumar April 24, 2018, 9:39 a.m. UTC
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>

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

-- 
2.15.0.194.g9af6a3dea062

Comments

kernel test robot April 26, 2018, 9:32 a.m. UTC | #1
Hi Miquel,

I love your patch! Yet something to improve:

[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.17-rc2 next-20180424]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Viresh-Kumar/cpufreq-dt-Allow-platforms-to-provide-suspend-resume-hooks/20180426-101030
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        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
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/cpufreq/armada-37xx-cpufreq.c: In function 'armada37xx_cpufreq_driver_init':
>> drivers/cpufreq/armada-37xx-cpufreq.c:268:3: error: label 'put_clk' used but not defined

      goto put_clk;
      ^~~~

vim +/put_clk +268 drivers/cpufreq/armada-37xx-cpufreq.c

   211	
   212	static int __init armada37xx_cpufreq_driver_init(void)
   213	{
   214		struct cpufreq_dt_platform_data pdata;
   215		struct armada_37xx_dvfs *dvfs;
   216		struct platform_device *pdev;
   217		unsigned long freq;
   218		unsigned int cur_frequency;
   219		struct regmap *nb_pm_base;
   220		struct device *cpu_dev;
   221		int load_lvl, ret;
   222		struct clk *clk;
   223	
   224		nb_pm_base =
   225			syscon_regmap_lookup_by_compatible("marvell,armada-3700-nb-pm");
   226	
   227		if (IS_ERR(nb_pm_base))
   228			return -ENODEV;
   229	
   230		/* Before doing any configuration on the DVFS first, disable it */
   231		armada37xx_cpufreq_disable_dvfs(nb_pm_base);
   232	
   233		/*
   234		 * On CPU 0 register the operating points supported (which are
   235		 * the nominal CPU frequency and full integer divisions of
   236		 * it).
   237		 */
   238		cpu_dev = get_cpu_device(0);
   239		if (!cpu_dev) {
   240			dev_err(cpu_dev, "Cannot get CPU\n");
   241			return -ENODEV;
   242		}
   243	
   244		clk = clk_get(cpu_dev, 0);
   245		if (IS_ERR(clk)) {
   246			dev_err(cpu_dev, "Cannot get clock for CPU0\n");
   247			return PTR_ERR(clk);
   248		}
   249	
   250		/* Get nominal (current) CPU frequency */
   251		cur_frequency = clk_get_rate(clk);
   252		if (!cur_frequency) {
   253			dev_err(cpu_dev, "Failed to get clock rate for CPU\n");
   254			clk_put(clk);
   255			return -EINVAL;
   256		}
   257	
   258		dvfs = armada_37xx_cpu_freq_info_get(cur_frequency);
   259		if (!dvfs) {
   260			clk_put(clk);
   261			return -EINVAL;
   262		}
   263	
   264		armada37xx_cpufreq_state = kmalloc(sizeof(*armada37xx_cpufreq_state),
   265						   GFP_KERNEL);
   266		if (!armada37xx_cpufreq_state) {
   267			ret = -ENOMEM;
 > 268			goto put_clk;

   269		}
   270	
   271		armada37xx_cpufreq_state->regmap = nb_pm_base;
   272	
   273		armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider);
   274		clk_put(clk);
   275	
   276		for (load_lvl = ARMADA_37XX_DVFS_LOAD_0; load_lvl < LOAD_LEVEL_NR;
   277		     load_lvl++) {
   278			freq = cur_frequency / dvfs->divider[load_lvl];
   279	
   280			ret = dev_pm_opp_add(cpu_dev, freq, 0);
   281			if (ret)
   282				goto remove_opp;
   283		}
   284	
   285		/* Now that everything is setup, enable the DVFS at hardware level */
   286		armada37xx_cpufreq_enable_dvfs(nb_pm_base);
   287	
   288		pdata.suspend = armada37xx_cpufreq_suspend;
   289		pdata.resume = armada37xx_cpufreq_resume;
   290	
   291		pdev = platform_device_register_data(NULL, "cpufreq-dt", -1, &pdata,
   292						     sizeof(pdata));
   293		ret = PTR_ERR_OR_ZERO(pdev);
   294		if (ret)
   295			goto disable_dvfs;
   296	
   297		return 0;
   298	
   299	disable_dvfs:
   300		armada37xx_cpufreq_disable_dvfs(nb_pm_base);
   301	remove_opp:
   302		/* clean-up the already added opp before leaving */
   303		while (load_lvl-- > ARMADA_37XX_DVFS_LOAD_0) {
   304			freq = cur_frequency / dvfs->divider[load_lvl];
   305			dev_pm_opp_remove(cpu_dev, freq);
   306		}
   307	
   308		kfree(armada37xx_cpufreq_state);
   309	
   310		return ret;
   311	}
   312	/* late_initcall, to guarantee the driver is loaded after A37xx clock driver */
   313	late_initcall(armada37xx_cpufreq_driver_init);
   314	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Viresh Kumar April 26, 2018, 10:12 a.m. UTC | #2
On 26-04-18, 17:32, kbuild test robot wrote:
> Hi Miquel,

> 

> I love your patch! Yet something to improve:

> 

> [auto build test ERROR on pm/linux-next]

> [also build test ERROR on v4.17-rc2 next-20180424]

> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

> 

> url:    https://github.com/0day-ci/linux/commits/Viresh-Kumar/cpufreq-dt-Allow-platforms-to-provide-suspend-resume-hooks/20180426-101030

> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next

> config: arm-allmodconfig (attached as .config)

> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0

> reproduce:

>         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

>         make.cross ARCH=arm 

> 

> All errors (new ones prefixed by >>):

> 

>    drivers/cpufreq/armada-37xx-cpufreq.c: In function 'armada37xx_cpufreq_driver_init':

> >> drivers/cpufreq/armada-37xx-cpufreq.c:268:3: error: label 'put_clk' used but not defined

>       goto put_clk;

>       ^~~~


Fixed, thanks.

-- 
viresh
diff mbox series

Patch

diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c
index 1d5db6f6ace9..5b808882ac8b 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) {
+		ret = -ENOMEM;
+		goto put_clk;
+	}
+
+	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 */