Message ID | e98965068836e673ea826e8a03fd01e168fce3ed.1524561482.git.viresh.kumar@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | cpufreq: dt: Allow platforms to provide suspend/resume hooks | expand |
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
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 --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 */