[PATCH v2 0/5] cpufreq: merge arm big.LITTLE and vexpress-spc drivers

[PATCH v2 0/5] cpufreq: merge arm big.LITTLE and vexpress-spc drivers

[Sudeep Holla]

Hi,

Since vexpress-spc is the sole user of arm_big_little cpufreq driver,
there's no point in keeping it separate anymore. I wanted to post these
patches for ages but kept postponing for no reason.

Regards,
Sudeep

v1->v2:
	- generated the patch using -B that helps to keep delta short
	  for review
	- Split the last patch into 3 different patches to deal with
	  removing bL_ops, debug messages and other code formatting
	  separately

Sudeep Holla (5):
  cpufreq: scpi: remove stale/outdated comment about the driver
  cpufreq: merge arm_big_little and vexpress-spc
  cpufreq: vexpress-spc: drop unnessary cpufreq_arm_bL_ops abstraction
  cpufreq: vexpress-spc: remove lots of debug messages
  cpufreq: vexpress-spc: fix some coding style issues

 MAINTAINERS                                   |   5 +-
 drivers/cpufreq/Kconfig.arm                   |  12 +-
 drivers/cpufreq/Makefile                      |   2 -
 drivers/cpufreq/arm_big_little.h              |  43 ---
 drivers/cpufreq/scpi-cpufreq.c                |   2 -
 ...rm_big_little.c => vexpress-spc-cpufreq.c} | 254 ++++++------------
 6 files changed, 93 insertions(+), 225 deletions(-)
 delete mode 100644 drivers/cpufreq/arm_big_little.h
 rename drivers/cpufreq/{arm_big_little.c => vexpress-spc-cpufreq.c} (68%)

--
2.17.1

[PATCH v2 1/5] cpufreq: scpi: remove stale/outdated comment about the driver

[Sudeep Holla]

Commit 343a8d17fa8d ("cpufreq: scpi: remove arm_big_little dependency")
removed the arm_big_little dependency from scpi driver and doesn't
provide any ops to arm_big_little cpufreq driver. Lets remove that
stale comment.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpufreq/scpi-cpufreq.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
index 2b51e0718c9f..20d1f85d5f5a 100644
--- a/drivers/cpufreq/scpi-cpufreq.c
+++ b/drivers/cpufreq/scpi-cpufreq.c
@@ -1,8 +1,6 @@
 /*
  * System Control and Power Interface (SCPI) based CPUFreq Interface driver
  *
- * It provides necessary ops to arm_big_little cpufreq driver.
- *
  * Copyright (C) 2015 ARM Ltd.
  * Sudeep Holla <sudeep.holla@arm.com>
  *
-- 
2.17.1

[PATCH v2 2/5] cpufreq: merge arm_big_little and vexpress-spc

[Sudeep Holla]

arm_big_little cpufreq driver was designed as a generic big little
driver that could be used by any platform and make use of bL switcher.
Over years alternate solutions have be designed and merged to deal with
bL/HMP systems like EAS.

Also since no other driver made use of generic arm_big_little cpufreq
driver except Vexpress SPC, we can merge them together as vexpress-spc
driver used only on Vexpress TC2(CA15_CA7) platform.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 MAINTAINERS                                   |  5 +-
 drivers/cpufreq/Kconfig.arm                   | 12 +--
 drivers/cpufreq/Makefile                      |  2 -
 drivers/cpufreq/arm_big_little.h              | 43 ----------
 ...rm_big_little.c => vexpress-spc-cpufreq.c} | 79 +++++++++++++++----
 5 files changed, 67 insertions(+), 74 deletions(-)
 delete mode 100644 drivers/cpufreq/arm_big_little.h
 rename drivers/cpufreq/{arm_big_little.c => vexpress-spc-cpufreq.c} (90%)

diff --git a/MAINTAINERS b/MAINTAINERS
index a69e6db80c79..78a4adff7892 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4269,14 +4269,13 @@ F:	include/linux/cpufreq.h
 F:	include/linux/sched/cpufreq.h
 F:	tools/testing/selftests/cpufreq/
 
-CPU FREQUENCY DRIVERS - ARM BIG LITTLE
+CPU FREQUENCY DRIVERS - VEXPRESS SPC ARM BIG LITTLE
 M:	Viresh Kumar <viresh.kumar@linaro.org>
 M:	Sudeep Holla <sudeep.holla@arm.com>
 L:	linux-pm@vger.kernel.org
 W:	http://www.arm.com/products/processors/technologies/biglittleprocessing.php
 S:	Maintained
-F:	drivers/cpufreq/arm_big_little.h
-F:	drivers/cpufreq/arm_big_little.c
+F:	drivers/cpufreq/vexpress-spc-cpufreq.c
 
 CPU POWER MONITORING SUBSYSTEM
 M:	Thomas Renninger <trenn@suse.com>
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index a905796f7f85..3858d86cf409 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -49,14 +49,6 @@ config ARM_ARMADA_8K_CPUFREQ
 
 	  If in doubt, say N.
 
-# big LITTLE core layer and glue drivers
-config ARM_BIG_LITTLE_CPUFREQ
-	tristate "Generic ARM big LITTLE CPUfreq driver"
-	depends on ARM_CPU_TOPOLOGY && HAVE_CLK
-	select PM_OPP
-	help
-	  This enables the Generic CPUfreq driver for ARM big.LITTLE platforms.
-
 config ARM_SCPI_CPUFREQ
 	tristate "SCPI based CPUfreq driver"
 	depends on ARM_SCPI_PROTOCOL && COMMON_CLK_SCPI
@@ -69,7 +61,9 @@ config ARM_SCPI_CPUFREQ
 
 config ARM_VEXPRESS_SPC_CPUFREQ
 	tristate "Versatile Express SPC based CPUfreq driver"
-	depends on ARM_BIG_LITTLE_CPUFREQ && ARCH_VEXPRESS_SPC
+	depends on ARM_CPU_TOPOLOGY && HAVE_CLK
+	depends on ARCH_VEXPRESS_SPC
+	select PM_OPP
 	help
 	  This add the CPUfreq driver support for Versatile Express
 	  big.LITTLE platforms using SPC for power management.
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 9a9f5ccd13d9..f6670c4abbb0 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -47,8 +47,6 @@ obj-$(CONFIG_X86_SFI_CPUFREQ)		+= sfi-cpufreq.o
 
 ##################################################################################
 # ARM SoC drivers
-obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ)	+= arm_big_little.o
-
 obj-$(CONFIG_ARM_ARMADA_37XX_CPUFREQ)	+= armada-37xx-cpufreq.o
 obj-$(CONFIG_ARM_ARMADA_8K_CPUFREQ)	+= armada-8k-cpufreq.o
 obj-$(CONFIG_ARM_BRCMSTB_AVS_CPUFREQ)	+= brcmstb-avs-cpufreq.o
diff --git a/drivers/cpufreq/arm_big_little.h b/drivers/cpufreq/arm_big_little.h
deleted file mode 100644
index 88a176e466c8..000000000000
--- a/drivers/cpufreq/arm_big_little.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * ARM big.LITTLE platform's CPUFreq header file
- *
- * Copyright (C) 2013 ARM Ltd.
- * Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
- *
- * Copyright (C) 2013 Linaro.
- * Viresh Kumar <viresh.kumar@linaro.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-#ifndef CPUFREQ_ARM_BIG_LITTLE_H
-#define CPUFREQ_ARM_BIG_LITTLE_H
-
-#include <linux/cpufreq.h>
-#include <linux/device.h>
-#include <linux/types.h>
-
-struct cpufreq_arm_bL_ops {
-	char name[CPUFREQ_NAME_LEN];
-
-	/*
-	 * This must set opp table for cpu_dev in a similar way as done by
-	 * dev_pm_opp_of_add_table().
-	 */
-	int (*init_opp_table)(const struct cpumask *cpumask);
-
-	/* Optional */
-	int (*get_transition_latency)(struct device *cpu_dev);
-	void (*free_opp_table)(const struct cpumask *cpumask);
-};
-
-int bL_cpufreq_register(const struct cpufreq_arm_bL_ops *ops);
-void bL_cpufreq_unregister(const struct cpufreq_arm_bL_ops *ops);
-
-#endif /* CPUFREQ_ARM_BIG_LITTLE_H */
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
similarity index 90%
rename from drivers/cpufreq/arm_big_little.c
rename to drivers/cpufreq/vexpress-spc-cpufreq.c
index 7fe52fcddcf1..b7e1aa000c80 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
@@ -1,20 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- * ARM big.LITTLE Platforms CPUFreq support
+ * Versatile Express SPC CPUFreq Interface driver
  *
- * Copyright (C) 2013 ARM Ltd.
- * Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
+ * Copyright (C) 2019 ARM Ltd.
+ * Sudeep Holla <sudeep.holla@arm.com>
  *
  * Copyright (C) 2013 Linaro.
  * Viresh Kumar <viresh.kumar@linaro.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -24,16 +16,29 @@
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
 #include <linux/cpu_cooling.h>
-#include <linux/export.h>
+#include <linux/device.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of_platform.h>
+#include <linux/platform_device.h>
 #include <linux/pm_opp.h>
 #include <linux/slab.h>
 #include <linux/topology.h>
 #include <linux/types.h>
 
-#include "arm_big_little.h"
+struct cpufreq_arm_bL_ops {
+	char name[CPUFREQ_NAME_LEN];
+
+	/*
+	 * This must set opp table for cpu_dev in a similar way as done by
+	 * dev_pm_opp_of_add_table().
+	 */
+	int (*init_opp_table)(const struct cpumask *cpumask);
+
+	/* Optional */
+	int (*get_transition_latency)(struct device *cpu_dev);
+	void (*free_opp_table)(const struct cpumask *cpumask);
+};
 
 /* Currently we support only two clusters */
 #define A15_CLUSTER	0
@@ -633,7 +638,6 @@ int bL_cpufreq_register(const struct cpufreq_arm_bL_ops *ops)
 	bL_switcher_put_enabled();
 	return ret;
 }
-EXPORT_SYMBOL_GPL(bL_cpufreq_register);
 
 void bL_cpufreq_unregister(const struct cpufreq_arm_bL_ops *ops)
 {
@@ -651,8 +655,49 @@ void bL_cpufreq_unregister(const struct cpufreq_arm_bL_ops *ops)
 			arm_bL_ops->name);
 	arm_bL_ops = NULL;
 }
-EXPORT_SYMBOL_GPL(bL_cpufreq_unregister);
+
+static int ve_spc_init_opp_table(const struct cpumask *cpumask)
+{
+	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
+	/*
+	 * platform specific SPC code must initialise the opp table
+	 * so just check if the OPP count is non-zero
+	 */
+	return dev_pm_opp_get_opp_count(cpu_dev) <= 0;
+}
+
+static int ve_spc_get_transition_latency(struct device *cpu_dev)
+{
+	return 1000000; /* 1 ms */
+}
+
+static const struct cpufreq_arm_bL_ops ve_spc_cpufreq_ops = {
+	.name	= "vexpress-spc",
+	.get_transition_latency = ve_spc_get_transition_latency,
+	.init_opp_table = ve_spc_init_opp_table,
+};
+
+static int ve_spc_cpufreq_probe(struct platform_device *pdev)
+{
+	return bL_cpufreq_register(&ve_spc_cpufreq_ops);
+}
+
+static int ve_spc_cpufreq_remove(struct platform_device *pdev)
+{
+	bL_cpufreq_unregister(&ve_spc_cpufreq_ops);
+	return 0;
+}
+
+static struct platform_driver ve_spc_cpufreq_platdrv = {
+	.driver = {
+		.name	= "vexpress-spc-cpufreq",
+	},
+	.probe		= ve_spc_cpufreq_probe,
+	.remove		= ve_spc_cpufreq_remove,
+};
+module_platform_driver(ve_spc_cpufreq_platdrv);
 
 MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>");
-MODULE_DESCRIPTION("Generic ARM big LITTLE cpufreq driver");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("Vexpress SPC ARM big LITTLE cpufreq driver");
 MODULE_LICENSE("GPL v2");
-- 
2.17.1

[PATCH v2 3/5] cpufreq: vexpress-spc: drop unnessary cpufreq_arm_bL_ops abstraction

[Sudeep Holla]

cpufreq_arm_bL_ops is no longer needed after merging the generic
arm_big_little and vexpress-spc driver. Remove it along with the
unused bL_cpufreq_{,un}register routines and rename some bL_*
functions to ve_spc_*.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpufreq/vexpress-spc-cpufreq.c | 148 +++++++------------------
 1 file changed, 37 insertions(+), 111 deletions(-)

diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
index b7e1aa000c80..9689f50a8973 100644
--- a/drivers/cpufreq/vexpress-spc-cpufreq.c
+++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
@@ -26,20 +26,6 @@
 #include <linux/topology.h>
 #include <linux/types.h>
 
-struct cpufreq_arm_bL_ops {
-	char name[CPUFREQ_NAME_LEN];
-
-	/*
-	 * This must set opp table for cpu_dev in a similar way as done by
-	 * dev_pm_opp_of_add_table().
-	 */
-	int (*init_opp_table)(const struct cpumask *cpumask);
-
-	/* Optional */
-	int (*get_transition_latency)(struct device *cpu_dev);
-	void (*free_opp_table)(const struct cpumask *cpumask);
-};
-
 /* Currently we support only two clusters */
 #define A15_CLUSTER	0
 #define A7_CLUSTER	1
@@ -62,7 +48,6 @@ static bool bL_switching_enabled;
 #define VIRT_FREQ(cluster, freq)    ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
 
 static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
-static const struct cpufreq_arm_bL_ops *arm_bL_ops;
 static struct clk *clk[MAX_CLUSTERS];
 static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
 static atomic_t cluster_usage[MAX_CLUSTERS + 1];
@@ -120,7 +105,7 @@ static unsigned int clk_get_cpu_rate(unsigned int cpu)
 	return rate;
 }
 
-static unsigned int bL_cpufreq_get_rate(unsigned int cpu)
+static unsigned int ve_spc_cpufreq_get_rate(unsigned int cpu)
 {
 	if (is_bL_switching_enabled()) {
 		pr_debug("%s: freq: %d\n", __func__, per_cpu(cpu_last_req_freq,
@@ -133,7 +118,7 @@ static unsigned int bL_cpufreq_get_rate(unsigned int cpu)
 }
 
 static unsigned int
-bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
+ve_spc_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 {
 	u32 new_rate, prev_rate;
 	int ret;
@@ -213,8 +198,8 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 }
 
 /* Set clock frequency */
-static int bL_cpufreq_set_target(struct cpufreq_policy *policy,
-		unsigned int index)
+static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,
+				     unsigned int index)
 {
 	u32 cpu = policy->cpu, cur_cluster, new_cluster, actual_cluster;
 	unsigned int freqs_new;
@@ -235,7 +220,8 @@ static int bL_cpufreq_set_target(struct cpufreq_policy *policy,
 		}
 	}
 
-	ret = bL_cpufreq_set_rate(cpu, actual_cluster, new_cluster, freqs_new);
+	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,
+				      freqs_new);
 
 	if (!ret) {
 		arch_set_freq_scale(policy->related_cpus, freqs_new,
@@ -321,8 +307,6 @@ static void _put_cluster_clk_and_freq_table(struct device *cpu_dev,
 
 	clk_put(clk[cluster]);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
-	if (arm_bL_ops->free_opp_table)
-		arm_bL_ops->free_opp_table(cpumask);
 	dev_dbg(cpu_dev, "%s: cluster: %d\n", __func__, cluster);
 }
 
@@ -361,18 +345,19 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
 	if (freq_table[cluster])
 		return 0;
 
-	ret = arm_bL_ops->init_opp_table(cpumask);
-	if (ret) {
-		dev_err(cpu_dev, "%s: init_opp_table failed, cpu: %d, err: %d\n",
-				__func__, cpu_dev->id, ret);
+	/*
+	 * platform specific SPC code must initialise the opp table
+	 * so just check if the OPP count is non-zero
+	 */
+	ret = dev_pm_opp_get_opp_count(cpu_dev) <= 0;
+	if (ret)
 		goto out;
-	}
 
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table[cluster]);
 	if (ret) {
 		dev_err(cpu_dev, "%s: failed to init cpufreq table, cpu: %d, err: %d\n",
 				__func__, cpu_dev->id, ret);
-		goto free_opp_table;
+		goto out;
 	}
 
 	clk[cluster] = clk_get(cpu_dev, NULL);
@@ -388,9 +373,6 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
 	ret = PTR_ERR(clk[cluster]);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
 
-free_opp_table:
-	if (arm_bL_ops->free_opp_table)
-		arm_bL_ops->free_opp_table(cpumask);
 out:
 	dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,
 			cluster);
@@ -459,7 +441,7 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev,
 }
 
 /* Per-CPU initialization */
-static int bL_cpufreq_init(struct cpufreq_policy *policy)
+static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)
 {
 	u32 cur_cluster = cpu_to_cluster(policy->cpu);
 	struct device *cpu_dev;
@@ -489,8 +471,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 		return ret;
 
 	policy->freq_table = freq_table[cur_cluster];
-	policy->cpuinfo.transition_latency =
-				arm_bL_ops->get_transition_latency(cpu_dev);
+	policy->cpuinfo.transition_latency = 1000000; /* 1 ms */
 
 	dev_pm_opp_of_register_em(policy->cpus);
 
@@ -501,7 +482,7 @@ static int bL_cpufreq_init(struct cpufreq_policy *policy)
 	return 0;
 }
 
-static int bL_cpufreq_exit(struct cpufreq_policy *policy)
+static int ve_spc_cpufreq_exit(struct cpufreq_policy *policy)
 {
 	struct device *cpu_dev;
 	int cur_cluster = cpu_to_cluster(policy->cpu);
@@ -524,7 +505,7 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
 	return 0;
 }
 
-static void bL_cpufreq_ready(struct cpufreq_policy *policy)
+static void ve_spc_cpufreq_ready(struct cpufreq_policy *policy)
 {
 	int cur_cluster = cpu_to_cluster(policy->cpu);
 
@@ -535,17 +516,17 @@ static void bL_cpufreq_ready(struct cpufreq_policy *policy)
 	cdev[cur_cluster] = of_cpufreq_cooling_register(policy);
 }
 
-static struct cpufreq_driver bL_cpufreq_driver = {
-	.name			= "arm-big-little",
+static struct cpufreq_driver ve_spc_cpufreq_driver = {
+	.name			= "vexpress-spc",
 	.flags			= CPUFREQ_STICKY |
 					CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
 					CPUFREQ_NEED_INITIAL_FREQ_CHECK,
 	.verify			= cpufreq_generic_frequency_table_verify,
-	.target_index		= bL_cpufreq_set_target,
-	.get			= bL_cpufreq_get_rate,
-	.init			= bL_cpufreq_init,
-	.exit			= bL_cpufreq_exit,
-	.ready			= bL_cpufreq_ready,
+	.target_index		= ve_spc_cpufreq_set_target,
+	.get			= ve_spc_cpufreq_get_rate,
+	.init			= ve_spc_cpufreq_init,
+	.exit			= ve_spc_cpufreq_exit,
+	.ready			= ve_spc_cpufreq_ready,
 	.attr			= cpufreq_generic_attr,
 };
 
@@ -558,17 +539,17 @@ static int bL_cpufreq_switcher_notifier(struct notifier_block *nfb,
 	switch (action) {
 	case BL_NOTIFY_PRE_ENABLE:
 	case BL_NOTIFY_PRE_DISABLE:
-		cpufreq_unregister_driver(&bL_cpufreq_driver);
+		cpufreq_unregister_driver(&ve_spc_cpufreq_driver);
 		break;
 
 	case BL_NOTIFY_POST_ENABLE:
 		set_switching_enabled(true);
-		cpufreq_register_driver(&bL_cpufreq_driver);
+		cpufreq_register_driver(&ve_spc_cpufreq_driver);
 		break;
 
 	case BL_NOTIFY_POST_DISABLE:
 		set_switching_enabled(false);
-		cpufreq_register_driver(&bL_cpufreq_driver);
+		cpufreq_register_driver(&ve_spc_cpufreq_driver);
 		break;
 
 	default:
@@ -596,95 +577,40 @@ static int __bLs_register_notifier(void) { return 0; }
 static int __bLs_unregister_notifier(void) { return 0; }
 #endif
 
-int bL_cpufreq_register(const struct cpufreq_arm_bL_ops *ops)
+static int ve_spc_cpufreq_probe(struct platform_device *pdev)
 {
 	int ret, i;
 
-	if (arm_bL_ops) {
-		pr_debug("%s: Already registered: %s, exiting\n", __func__,
-				arm_bL_ops->name);
-		return -EBUSY;
-	}
-
-	if (!ops || !strlen(ops->name) || !ops->init_opp_table ||
-	    !ops->get_transition_latency) {
-		pr_err("%s: Invalid arm_bL_ops, exiting\n", __func__);
-		return -ENODEV;
-	}
-
-	arm_bL_ops = ops;
-
 	set_switching_enabled(bL_switcher_get_enabled());
 
 	for (i = 0; i < MAX_CLUSTERS; i++)
 		mutex_init(&cluster_lock[i]);
 
-	ret = cpufreq_register_driver(&bL_cpufreq_driver);
+	ret = cpufreq_register_driver(&ve_spc_cpufreq_driver);
 	if (ret) {
 		pr_info("%s: Failed registering platform driver: %s, err: %d\n",
-				__func__, ops->name, ret);
-		arm_bL_ops = NULL;
+			__func__, ve_spc_cpufreq_driver.name, ret);
 	} else {
 		ret = __bLs_register_notifier();
-		if (ret) {
-			cpufreq_unregister_driver(&bL_cpufreq_driver);
-			arm_bL_ops = NULL;
-		} else {
+		if (ret)
+			cpufreq_unregister_driver(&ve_spc_cpufreq_driver);
+		else
 			pr_info("%s: Registered platform driver: %s\n",
-					__func__, ops->name);
-		}
+				__func__, ve_spc_cpufreq_driver.name);
 	}
 
 	bL_switcher_put_enabled();
 	return ret;
 }
 
-void bL_cpufreq_unregister(const struct cpufreq_arm_bL_ops *ops)
+static int ve_spc_cpufreq_remove(struct platform_device *pdev)
 {
-	if (arm_bL_ops != ops) {
-		pr_err("%s: Registered with: %s, can't unregister, exiting\n",
-				__func__, arm_bL_ops->name);
-		return;
-	}
-
 	bL_switcher_get_enabled();
 	__bLs_unregister_notifier();
-	cpufreq_unregister_driver(&bL_cpufreq_driver);
+	cpufreq_unregister_driver(&ve_spc_cpufreq_driver);
 	bL_switcher_put_enabled();
 	pr_info("%s: Un-registered platform driver: %s\n", __func__,
-			arm_bL_ops->name);
-	arm_bL_ops = NULL;
-}
-
-static int ve_spc_init_opp_table(const struct cpumask *cpumask)
-{
-	struct device *cpu_dev = get_cpu_device(cpumask_first(cpumask));
-	/*
-	 * platform specific SPC code must initialise the opp table
-	 * so just check if the OPP count is non-zero
-	 */
-	return dev_pm_opp_get_opp_count(cpu_dev) <= 0;
-}
-
-static int ve_spc_get_transition_latency(struct device *cpu_dev)
-{
-	return 1000000; /* 1 ms */
-}
-
-static const struct cpufreq_arm_bL_ops ve_spc_cpufreq_ops = {
-	.name	= "vexpress-spc",
-	.get_transition_latency = ve_spc_get_transition_latency,
-	.init_opp_table = ve_spc_init_opp_table,
-};
-
-static int ve_spc_cpufreq_probe(struct platform_device *pdev)
-{
-	return bL_cpufreq_register(&ve_spc_cpufreq_ops);
-}
-
-static int ve_spc_cpufreq_remove(struct platform_device *pdev)
-{
-	bL_cpufreq_unregister(&ve_spc_cpufreq_ops);
+		ve_spc_cpufreq_driver.name);
 	return 0;
 }
 
-- 
2.17.1

[PATCH v2 4/5] cpufreq: vexpress-spc: remove lots of debug messages

[Sudeep Holla]

This driver have been used and tested for year now and the extensive
debug/log messages in the driver are not really required anymore.
Get rid of those unnecessary log messages.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpufreq/vexpress-spc-cpufreq.c | 72 +++++---------------------
 1 file changed, 13 insertions(+), 59 deletions(-)

diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
index 9689f50a8973..81064430317f 100644
--- a/drivers/cpufreq/vexpress-spc-cpufreq.c
+++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
@@ -84,9 +84,6 @@ static unsigned int find_cluster_maxfreq(int cluster)
 			max_freq = cpu_freq;
 	}
 
-	pr_debug("%s: cluster: %d, max freq: %d\n", __func__, cluster,
-			max_freq);
-
 	return max_freq;
 }
 
@@ -99,22 +96,15 @@ static unsigned int clk_get_cpu_rate(unsigned int cpu)
 	if (is_bL_switching_enabled())
 		rate = VIRT_FREQ(cur_cluster, rate);
 
-	pr_debug("%s: cpu: %d, cluster: %d, freq: %u\n", __func__, cpu,
-			cur_cluster, rate);
-
 	return rate;
 }
 
 static unsigned int ve_spc_cpufreq_get_rate(unsigned int cpu)
 {
-	if (is_bL_switching_enabled()) {
-		pr_debug("%s: freq: %d\n", __func__, per_cpu(cpu_last_req_freq,
-					cpu));
-
+	if (is_bL_switching_enabled())
 		return per_cpu(cpu_last_req_freq, cpu);
-	} else {
+	else
 		return clk_get_cpu_rate(cpu);
-	}
 }
 
 static unsigned int
@@ -137,9 +127,6 @@ ve_spc_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 		new_rate = rate;
 	}
 
-	pr_debug("%s: cpu: %d, old cluster: %d, new cluster: %d, freq: %d\n",
-			__func__, cpu, old_cluster, new_cluster, new_rate);
-
 	ret = clk_set_rate(clk[new_cluster], new_rate * 1000);
 	if (!ret) {
 		/*
@@ -155,8 +142,6 @@ ve_spc_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 	}
 
 	if (WARN_ON(ret)) {
-		pr_err("clk_set_rate failed: %d, new cluster: %d\n", ret,
-				new_cluster);
 		if (bLs) {
 			per_cpu(cpu_last_req_freq, cpu) = prev_rate;
 			per_cpu(physical_cluster, cpu) = old_cluster;
@@ -171,9 +156,6 @@ ve_spc_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 
 	/* Recalc freq for old cluster when switching clusters */
 	if (old_cluster != new_cluster) {
-		pr_debug("%s: cpu: %d, old cluster: %d, new cluster: %d\n",
-				__func__, cpu, old_cluster, new_cluster);
-
 		/* Switch cluster */
 		bL_switch_request(cpu, new_cluster);
 
@@ -183,14 +165,9 @@ ve_spc_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 		new_rate = find_cluster_maxfreq(old_cluster);
 		new_rate = ACTUAL_FREQ(old_cluster, new_rate);
 
-		if (new_rate) {
-			pr_debug("%s: Updating rate of old cluster: %d, to freq: %d\n",
-					__func__, old_cluster, new_rate);
-
-			if (clk_set_rate(clk[old_cluster], new_rate * 1000))
-				pr_err("%s: clk_set_rate failed: %d, old cluster: %d\n",
-						__func__, ret, old_cluster);
-		}
+		if (new_rate && clk_set_rate(clk[old_cluster], new_rate * 1000))
+			pr_err("%s: clk_set_rate failed: %d, old cluster: %d\n",
+			       __func__, ret, old_cluster);
 		mutex_unlock(&cluster_lock[old_cluster]);
 	}
 
@@ -283,8 +260,6 @@ static int merge_cluster_tables(void)
 				j++) {
 			table[k].frequency = VIRT_FREQ(i,
 					freq_table[i][j].frequency);
-			pr_debug("%s: index: %d, freq: %d\n", __func__, k,
-					table[k].frequency);
 			k++;
 		}
 	}
@@ -292,8 +267,6 @@ static int merge_cluster_tables(void)
 	table[k].driver_data = k;
 	table[k].frequency = CPUFREQ_TABLE_END;
 
-	pr_debug("%s: End, table: %p, count: %d\n", __func__, table, k);
-
 	return 0;
 }
 
@@ -307,7 +280,6 @@ static void _put_cluster_clk_and_freq_table(struct device *cpu_dev,
 
 	clk_put(clk[cluster]);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
-	dev_dbg(cpu_dev, "%s: cluster: %d\n", __func__, cluster);
 }
 
 static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
@@ -324,11 +296,9 @@ static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
 
 	for_each_present_cpu(i) {
 		struct device *cdev = get_cpu_device(i);
-		if (!cdev) {
-			pr_err("%s: failed to get cpu%d device\n", __func__, i);
-			return;
-		}
 
+		if (!cdev)
+			return;
 		_put_cluster_clk_and_freq_table(cdev, cpumask);
 	}
 
@@ -354,19 +324,12 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
 		goto out;
 
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table[cluster]);
-	if (ret) {
-		dev_err(cpu_dev, "%s: failed to init cpufreq table, cpu: %d, err: %d\n",
-				__func__, cpu_dev->id, ret);
+	if (ret)
 		goto out;
-	}
 
 	clk[cluster] = clk_get(cpu_dev, NULL);
-	if (!IS_ERR(clk[cluster])) {
-		dev_dbg(cpu_dev, "%s: clk: %p & freq table: %p, cluster: %d\n",
-				__func__, clk[cluster], freq_table[cluster],
-				cluster);
+	if (!IS_ERR(clk[cluster]))
 		return 0;
-	}
 
 	dev_err(cpu_dev, "%s: Failed to get clk for cpu: %d, cluster: %d\n",
 			__func__, cpu_dev->id, cluster);
@@ -401,11 +364,9 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev,
 	 */
 	for_each_present_cpu(i) {
 		struct device *cdev = get_cpu_device(i);
-		if (!cdev) {
-			pr_err("%s: failed to get cpu%d device\n", __func__, i);
-			return -ENODEV;
-		}
 
+		if (!cdev)
+			return -ENODEV;
 		ret = _get_cluster_clk_and_freq_table(cdev, cpumask);
 		if (ret)
 			goto put_clusters;
@@ -419,19 +380,14 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev,
 	clk_big_min = get_table_min(freq_table[0]);
 	clk_little_max = VIRT_FREQ(1, get_table_max(freq_table[1]));
 
-	pr_debug("%s: cluster: %d, clk_big_min: %d, clk_little_max: %d\n",
-			__func__, cluster, clk_big_min, clk_little_max);
-
 	return 0;
 
 put_clusters:
 	for_each_present_cpu(i) {
 		struct device *cdev = get_cpu_device(i);
-		if (!cdev) {
-			pr_err("%s: failed to get cpu%d device\n", __func__, i);
-			return -ENODEV;
-		}
 
+		if (!cdev)
+			return -ENODEV;
 		_put_cluster_clk_and_freq_table(cdev, cpumask);
 	}
 
@@ -500,8 +456,6 @@ static int ve_spc_cpufreq_exit(struct cpufreq_policy *policy)
 	}
 
 	put_cluster_clk_and_freq_table(cpu_dev, policy->related_cpus);
-	dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
-
 	return 0;
 }
 
-- 
2.17.1

[PATCH v2 5/5] cpufreq: vexpress-spc: fix some coding style issues

[Sudeep Holla]

Fix the following checkpatch checks/warnings:

CHECK: Unnecessary parentheses around the code
CHECK: Alignment should match open parenthesis
CHECK: Prefer kernel type 'u32' over 'uint32_t'
WARNING: Missing a blank line after declarations

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
index 81064430317f..8ecb2961be86 100644
--- a/drivers/cpufreq/vexpress-spc-cpufreq.c
+++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
@@ -79,8 +79,8 @@ static unsigned int find_cluster_maxfreq(int cluster)
 	for_each_online_cpu(j) {
 		cpu_freq = per_cpu(cpu_last_req_freq, j);
 
-		if ((cluster == per_cpu(physical_cluster, j)) &&
-				(max_freq < cpu_freq))
+		if (cluster == per_cpu(physical_cluster, j) &&
+		    max_freq < cpu_freq)
 			max_freq = cpu_freq;
 	}
 
@@ -188,22 +188,19 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,
 	freqs_new = freq_table[cur_cluster][index].frequency;
 
 	if (is_bL_switching_enabled()) {
-		if ((actual_cluster == A15_CLUSTER) &&
-				(freqs_new < clk_big_min)) {
+		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)
 			new_cluster = A7_CLUSTER;
-		} else if ((actual_cluster == A7_CLUSTER) &&
-				(freqs_new > clk_little_max)) {
+		else if (actual_cluster == A7_CLUSTER &&
+			 freqs_new > clk_little_max)
 			new_cluster = A15_CLUSTER;
-		}
 	}
 
 	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,
 				      freqs_new);
 
-	if (!ret) {
+	if (!ret)
 		arch_set_freq_scale(policy->related_cpus, freqs_new,
 				    policy->cpuinfo.max_freq);
-	}
 
 	return ret;
 }
@@ -222,7 +219,8 @@ static inline u32 get_table_count(struct cpufreq_frequency_table *table)
 static inline u32 get_table_min(struct cpufreq_frequency_table *table)
 {
 	struct cpufreq_frequency_table *pos;
-	uint32_t min_freq = ~0;
+	u32 min_freq = ~0;
+
 	cpufreq_for_each_entry(pos, table)
 		if (pos->frequency < min_freq)
 			min_freq = pos->frequency;
@@ -233,7 +231,8 @@ static inline u32 get_table_min(struct cpufreq_frequency_table *table)
 static inline u32 get_table_max(struct cpufreq_frequency_table *table)
 {
 	struct cpufreq_frequency_table *pos;
-	uint32_t max_freq = 0;
+	u32 max_freq = 0;
+
 	cpufreq_for_each_entry(pos, table)
 		if (pos->frequency > max_freq)
 			max_freq = pos->frequency;
@@ -255,14 +254,11 @@ static int merge_cluster_tables(void)
 	freq_table[MAX_CLUSTERS] = table;
 
 	/* Add in reverse order to get freqs in increasing order */
-	for (i = MAX_CLUSTERS - 1; i >= 0; i--) {
+	for (i = MAX_CLUSTERS - 1; i >= 0; i--)
 		for (j = 0; freq_table[i][j].frequency != CPUFREQ_TABLE_END;
-				j++) {
-			table[k].frequency = VIRT_FREQ(i,
-					freq_table[i][j].frequency);
-			k++;
-		}
-	}
+		     j++, k++)
+			table[k].frequency =
+				VIRT_FREQ(i, freq_table[i][j].frequency);
 
 	table[k].driver_data = k;
 	table[k].frequency = CPUFREQ_TABLE_END;
@@ -332,13 +328,13 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
 		return 0;
 
 	dev_err(cpu_dev, "%s: Failed to get clk for cpu: %d, cluster: %d\n",
-			__func__, cpu_dev->id, cluster);
+		__func__, cpu_dev->id, cluster);
 	ret = PTR_ERR(clk[cluster]);
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
 
 out:
 	dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,
-			cluster);
+		cluster);
 	return ret;
 }
 
@@ -406,7 +402,7 @@ static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
 		pr_err("%s: failed to get cpu%d device\n", __func__,
-				policy->cpu);
+		       policy->cpu);
 		return -ENODEV;
 	}
 
@@ -432,7 +428,8 @@ static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)
 	dev_pm_opp_of_register_em(policy->cpus);
 
 	if (is_bL_switching_enabled())
-		per_cpu(cpu_last_req_freq, policy->cpu) = clk_get_cpu_rate(policy->cpu);
+		per_cpu(cpu_last_req_freq, policy->cpu) =
+						clk_get_cpu_rate(policy->cpu);
 
 	dev_info(cpu_dev, "%s: CPU %d initialized\n", __func__, policy->cpu);
 	return 0;
@@ -451,7 +448,7 @@ static int ve_spc_cpufreq_exit(struct cpufreq_policy *policy)
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
 		pr_err("%s: failed to get cpu%d device\n", __func__,
-				policy->cpu);
+		       policy->cpu);
 		return -ENODEV;
 	}
 
-- 
2.17.1

Re: [PATCH v2 0/5] cpufreq: merge arm big.LITTLE and vexpress-spc drivers

[Nicolas Pitre]

On Thu, 17 Oct 2019, Sudeep Holla wrote:

> Hi,
> 
> Since vexpress-spc is the sole user of arm_big_little cpufreq driver,
> there's no point in keeping it separate anymore. I wanted to post these
> patches for ages but kept postponing for no reason.

For these patches:

Acked-by: Nicolas Pitre <nico@fluxnic.net>


Nicolas

Re: [PATCH v2 2/5] cpufreq: merge arm_big_little and vexpress-spc

[Viresh Kumar]

On 17-10-19, 13:35, Sudeep Holla wrote:
> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
> similarity index 90%
> rename from drivers/cpufreq/arm_big_little.c
> rename to drivers/cpufreq/vexpress-spc-cpufreq.c
> index 7fe52fcddcf1..b7e1aa000c80 100644
> --- a/drivers/cpufreq/arm_big_little.c
> +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
> @@ -1,20 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
> - * ARM big.LITTLE Platforms CPUFreq support
> + * Versatile Express SPC CPUFreq Interface driver
>   *
> - * Copyright (C) 2013 ARM Ltd.
> - * Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> + * Copyright (C) 2019 ARM Ltd.

Should this be 2013-2019 instead ?

> + * Sudeep Holla <sudeep.holla@arm.com>
>   *
>   * Copyright (C) 2013 Linaro.
>   * Viresh Kumar <viresh.kumar@linaro.org>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> - * kind, whether express or implied; without even the implied warranty
> - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
>   */

-- 
viresh

Re: [PATCH v2 2/5] cpufreq: merge arm_big_little and vexpress-spc

[Sudeep Holla]

On Fri, Oct 18, 2019 at 11:17:40AM +0530, Viresh Kumar wrote:
> On 17-10-19, 13:35, Sudeep Holla wrote:
> > diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
> > similarity index 90%
> > rename from drivers/cpufreq/arm_big_little.c
> > rename to drivers/cpufreq/vexpress-spc-cpufreq.c
> > index 7fe52fcddcf1..b7e1aa000c80 100644
> > --- a/drivers/cpufreq/arm_big_little.c
> > +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
> > @@ -1,20 +1,12 @@
> > +// SPDX-License-Identifier: GPL-2.0
> >  /*
> > - * ARM big.LITTLE Platforms CPUFreq support
> > + * Versatile Express SPC CPUFreq Interface driver
> >   *
> > - * Copyright (C) 2013 ARM Ltd.
> > - * Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> > + * Copyright (C) 2019 ARM Ltd.
>
> Should this be 2013-2019 instead ?
>

Sure.

--
Regards,
Sudeep

Re: [PATCH v2 5/5] cpufreq: vexpress-spc: fix some coding style issues

[Viresh Kumar]

On 17-10-19, 13:35, Sudeep Holla wrote:
> Fix the following checkpatch checks/warnings:
> 
> CHECK: Unnecessary parentheses around the code
> CHECK: Alignment should match open parenthesis
> CHECK: Prefer kernel type 'u32' over 'uint32_t'
> WARNING: Missing a blank line after declarations
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------
>  1 file changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
> index 81064430317f..8ecb2961be86 100644
> --- a/drivers/cpufreq/vexpress-spc-cpufreq.c
> +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
> @@ -79,8 +79,8 @@ static unsigned int find_cluster_maxfreq(int cluster)
>  	for_each_online_cpu(j) {
>  		cpu_freq = per_cpu(cpu_last_req_freq, j);
>  
> -		if ((cluster == per_cpu(physical_cluster, j)) &&
> -				(max_freq < cpu_freq))
> +		if (cluster == per_cpu(physical_cluster, j) &&
> +		    max_freq < cpu_freq)
>  			max_freq = cpu_freq;
>  	}
>  
> @@ -188,22 +188,19 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,
>  	freqs_new = freq_table[cur_cluster][index].frequency;
>  
>  	if (is_bL_switching_enabled()) {
> -		if ((actual_cluster == A15_CLUSTER) &&
> -				(freqs_new < clk_big_min)) {
> +		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)
>  			new_cluster = A7_CLUSTER;
> -		} else if ((actual_cluster == A7_CLUSTER) &&
> -				(freqs_new > clk_little_max)) {
> +		else if (actual_cluster == A7_CLUSTER &&
> +			 freqs_new > clk_little_max)
>  			new_cluster = A15_CLUSTER;
> -		}
>  	}
>  
>  	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,
>  				      freqs_new);
>  
> -	if (!ret) {
> +	if (!ret)

That's not the standard way in Linux I believe. We do use {} even when
the body is single line but broken into two, like below.

>  		arch_set_freq_scale(policy->related_cpus, freqs_new,
>  				    policy->cpuinfo.max_freq);
> -	}
>  
>  	return ret;
>  }
> @@ -222,7 +219,8 @@ static inline u32 get_table_count(struct cpufreq_frequency_table *table)
>  static inline u32 get_table_min(struct cpufreq_frequency_table *table)
>  {
>  	struct cpufreq_frequency_table *pos;
> -	uint32_t min_freq = ~0;
> +	u32 min_freq = ~0;
> +
>  	cpufreq_for_each_entry(pos, table)
>  		if (pos->frequency < min_freq)
>  			min_freq = pos->frequency;
> @@ -233,7 +231,8 @@ static inline u32 get_table_min(struct cpufreq_frequency_table *table)
>  static inline u32 get_table_max(struct cpufreq_frequency_table *table)
>  {
>  	struct cpufreq_frequency_table *pos;
> -	uint32_t max_freq = 0;
> +	u32 max_freq = 0;
> +
>  	cpufreq_for_each_entry(pos, table)
>  		if (pos->frequency > max_freq)
>  			max_freq = pos->frequency;
> @@ -255,14 +254,11 @@ static int merge_cluster_tables(void)
>  	freq_table[MAX_CLUSTERS] = table;
>  
>  	/* Add in reverse order to get freqs in increasing order */
> -	for (i = MAX_CLUSTERS - 1; i >= 0; i--) {
> +	for (i = MAX_CLUSTERS - 1; i >= 0; i--)
>  		for (j = 0; freq_table[i][j].frequency != CPUFREQ_TABLE_END;
> -				j++) {
> -			table[k].frequency = VIRT_FREQ(i,
> -					freq_table[i][j].frequency);
> -			k++;
> -		}
> -	}
> +		     j++, k++)

same here, please keep {}.

> +			table[k].frequency =
> +				VIRT_FREQ(i, freq_table[i][j].frequency);
>  
>  	table[k].driver_data = k;
>  	table[k].frequency = CPUFREQ_TABLE_END;
> @@ -332,13 +328,13 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
>  		return 0;
>  
>  	dev_err(cpu_dev, "%s: Failed to get clk for cpu: %d, cluster: %d\n",
> -			__func__, cpu_dev->id, cluster);
> +		__func__, cpu_dev->id, cluster);
>  	ret = PTR_ERR(clk[cluster]);
>  	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
>  
>  out:
>  	dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,
> -			cluster);
> +		cluster);
>  	return ret;
>  }
>  
> @@ -406,7 +402,7 @@ static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)
>  	cpu_dev = get_cpu_device(policy->cpu);
>  	if (!cpu_dev) {
>  		pr_err("%s: failed to get cpu%d device\n", __func__,
> -				policy->cpu);
> +		       policy->cpu);
>  		return -ENODEV;
>  	}
>  
> @@ -432,7 +428,8 @@ static int ve_spc_cpufreq_init(struct cpufreq_policy *policy)
>  	dev_pm_opp_of_register_em(policy->cpus);
>  
>  	if (is_bL_switching_enabled())
> -		per_cpu(cpu_last_req_freq, policy->cpu) = clk_get_cpu_rate(policy->cpu);
> +		per_cpu(cpu_last_req_freq, policy->cpu) =
> +						clk_get_cpu_rate(policy->cpu);
>  
>  	dev_info(cpu_dev, "%s: CPU %d initialized\n", __func__, policy->cpu);
>  	return 0;
> @@ -451,7 +448,7 @@ static int ve_spc_cpufreq_exit(struct cpufreq_policy *policy)
>  	cpu_dev = get_cpu_device(policy->cpu);
>  	if (!cpu_dev) {
>  		pr_err("%s: failed to get cpu%d device\n", __func__,
> -				policy->cpu);
> +		       policy->cpu);
>  		return -ENODEV;
>  	}
>  
> -- 
> 2.17.1

-- 
viresh

Re: [PATCH v2 4/5] cpufreq: vexpress-spc: remove lots of debug messages

[Viresh Kumar]

On 17-10-19, 13:35, Sudeep Holla wrote:
> This driver have been used and tested for year now and the extensive
> debug/log messages in the driver are not really required anymore.
> Get rid of those unnecessary log messages.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/cpufreq/vexpress-spc-cpufreq.c | 72 +++++---------------------
>  1 file changed, 13 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
>  static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
> @@ -324,11 +296,9 @@ static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
>  
>  	for_each_present_cpu(i) {
>  		struct device *cdev = get_cpu_device(i);
> -		if (!cdev) {
> -			pr_err("%s: failed to get cpu%d device\n", __func__, i);
> -			return;
> -		}
>  
> +		if (!cdev)
> +			return;

We had a blank line after this, which isn't there in your version
anymore. Please keep that here and few more places below.

>  		_put_cluster_clk_and_freq_table(cdev, cpumask);
>  	}
>  
> @@ -354,19 +324,12 @@ static int _get_cluster_clk_and_freq_table(struct device *cpu_dev,
>  		goto out;
>  
>  	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table[cluster]);
> -	if (ret) {
> -		dev_err(cpu_dev, "%s: failed to init cpufreq table, cpu: %d, err: %d\n",
> -				__func__, cpu_dev->id, ret);
> +	if (ret)
>  		goto out;
> -	}
>  
>  	clk[cluster] = clk_get(cpu_dev, NULL);
> -	if (!IS_ERR(clk[cluster])) {
> -		dev_dbg(cpu_dev, "%s: clk: %p & freq table: %p, cluster: %d\n",
> -				__func__, clk[cluster], freq_table[cluster],
> -				cluster);
> +	if (!IS_ERR(clk[cluster]))
>  		return 0;
> -	}
>  
>  	dev_err(cpu_dev, "%s: Failed to get clk for cpu: %d, cluster: %d\n",
>  			__func__, cpu_dev->id, cluster);
> @@ -401,11 +364,9 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev,
>  	 */
>  	for_each_present_cpu(i) {
>  		struct device *cdev = get_cpu_device(i);
> -		if (!cdev) {
> -			pr_err("%s: failed to get cpu%d device\n", __func__, i);
> -			return -ENODEV;
> -		}
>  
> +		if (!cdev)
> +			return -ENODEV;
>  		ret = _get_cluster_clk_and_freq_table(cdev, cpumask);
>  		if (ret)
>  			goto put_clusters;
> @@ -419,19 +380,14 @@ static int get_cluster_clk_and_freq_table(struct device *cpu_dev,
>  	clk_big_min = get_table_min(freq_table[0]);
>  	clk_little_max = VIRT_FREQ(1, get_table_max(freq_table[1]));
>  
> -	pr_debug("%s: cluster: %d, clk_big_min: %d, clk_little_max: %d\n",
> -			__func__, cluster, clk_big_min, clk_little_max);
> -
>  	return 0;
>  
>  put_clusters:
>  	for_each_present_cpu(i) {
>  		struct device *cdev = get_cpu_device(i);
> -		if (!cdev) {
> -			pr_err("%s: failed to get cpu%d device\n", __func__, i);
> -			return -ENODEV;
> -		}
>  
> +		if (!cdev)
> +			return -ENODEV;
>  		_put_cluster_clk_and_freq_table(cdev, cpumask);
>  	}
>  
> @@ -500,8 +456,6 @@ static int ve_spc_cpufreq_exit(struct cpufreq_policy *policy)
>  	}
>  
>  	put_cluster_clk_and_freq_table(cpu_dev, policy->related_cpus);
> -	dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
> -
>  	return 0;
>  }
>  
> -- 
> 2.17.1

-- 
viresh

Re: [PATCH v2 5/5] cpufreq: vexpress-spc: fix some coding style issues

[Sudeep Holla]

On Fri, Oct 18, 2019 at 11:25:17AM +0530, Viresh Kumar wrote:
> On 17-10-19, 13:35, Sudeep Holla wrote:
> > Fix the following checkpatch checks/warnings:
> > 
> > CHECK: Unnecessary parentheses around the code
> > CHECK: Alignment should match open parenthesis
> > CHECK: Prefer kernel type 'u32' over 'uint32_t'
> > WARNING: Missing a blank line after declarations
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------
> >  1 file changed, 20 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
> > index 81064430317f..8ecb2961be86 100644
> > --- a/drivers/cpufreq/vexpress-spc-cpufreq.c
> > +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
> > @@ -79,8 +79,8 @@ static unsigned int find_cluster_maxfreq(int cluster)
> >  	for_each_online_cpu(j) {
> >  		cpu_freq = per_cpu(cpu_last_req_freq, j);
> >  
> > -		if ((cluster == per_cpu(physical_cluster, j)) &&
> > -				(max_freq < cpu_freq))
> > +		if (cluster == per_cpu(physical_cluster, j) &&
> > +		    max_freq < cpu_freq)
> >  			max_freq = cpu_freq;
> >  	}
> >  
> > @@ -188,22 +188,19 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,
> >  	freqs_new = freq_table[cur_cluster][index].frequency;
> >  
> >  	if (is_bL_switching_enabled()) {
> > -		if ((actual_cluster == A15_CLUSTER) &&
> > -				(freqs_new < clk_big_min)) {
> > +		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)
> >  			new_cluster = A7_CLUSTER;
> > -		} else if ((actual_cluster == A7_CLUSTER) &&
> > -				(freqs_new > clk_little_max)) {
> > +		else if (actual_cluster == A7_CLUSTER &&
> > +			 freqs_new > clk_little_max)
> >  			new_cluster = A15_CLUSTER;
> > -		}
> >  	}
> >  
> >  	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,
> >  				      freqs_new);
> >  
> > -	if (!ret) {
> > +	if (!ret)
> 
> That's not the standard way in Linux I believe. We do use {} even when
> the body is single line but broken into two, like below.
>

OK, wasn't aware of that. I will update. Generally I ignore checkpatch
warnings, but the list was big and fixed a bunch of them :)

--
Regards,
Sudeep

Re: [PATCH v2 4/5] cpufreq: vexpress-spc: remove lots of debug messages

[Sudeep Holla]

On Fri, Oct 18, 2019 at 11:27:20AM +0530, Viresh Kumar wrote:
> On 17-10-19, 13:35, Sudeep Holla wrote:
> > This driver have been used and tested for year now and the extensive
> > debug/log messages in the driver are not really required anymore.
> > Get rid of those unnecessary log messages.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  drivers/cpufreq/vexpress-spc-cpufreq.c | 72 +++++---------------------
> >  1 file changed, 13 insertions(+), 59 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
> >  static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
> > @@ -324,11 +296,9 @@ static void put_cluster_clk_and_freq_table(struct device *cpu_dev,
> >  
> >  	for_each_present_cpu(i) {
> >  		struct device *cdev = get_cpu_device(i);
> > -		if (!cdev) {
> > -			pr_err("%s: failed to get cpu%d device\n", __func__, i);
> > -			return;
> > -		}
> >  
> > +		if (!cdev)
> > +			return;
> 
> We had a blank line after this, which isn't there in your version
> anymore. Please keep that here and few more places below.
>

Ah, this one is spurious change when doing in bulk not intended. I will
add back the blank line.

--
Regards,
Sudeep

Re: [PATCH v2 5/5] cpufreq: vexpress-spc: fix some coding style issues

[Nicolas Pitre]

On Fri, 18 Oct 2019, Sudeep Holla wrote:

> On Fri, Oct 18, 2019 at 11:25:17AM +0530, Viresh Kumar wrote:
> > On 17-10-19, 13:35, Sudeep Holla wrote:
> > > Fix the following checkpatch checks/warnings:
> > > 
> > > CHECK: Unnecessary parentheses around the code
> > > CHECK: Alignment should match open parenthesis
> > > CHECK: Prefer kernel type 'u32' over 'uint32_t'
> > > WARNING: Missing a blank line after declarations
> > > 
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > ---
> > >  drivers/cpufreq/vexpress-spc-cpufreq.c | 43 ++++++++++++--------------
> > >  1 file changed, 20 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
> > > index 81064430317f..8ecb2961be86 100644
> > > --- a/drivers/cpufreq/vexpress-spc-cpufreq.c
> > > +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
> > > @@ -79,8 +79,8 @@ static unsigned int find_cluster_maxfreq(int cluster)
> > >  	for_each_online_cpu(j) {
> > >  		cpu_freq = per_cpu(cpu_last_req_freq, j);
> > >  
> > > -		if ((cluster == per_cpu(physical_cluster, j)) &&
> > > -				(max_freq < cpu_freq))
> > > +		if (cluster == per_cpu(physical_cluster, j) &&
> > > +		    max_freq < cpu_freq)
> > >  			max_freq = cpu_freq;
> > >  	}
> > >  
> > > @@ -188,22 +188,19 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,
> > >  	freqs_new = freq_table[cur_cluster][index].frequency;
> > >  
> > >  	if (is_bL_switching_enabled()) {
> > > -		if ((actual_cluster == A15_CLUSTER) &&
> > > -				(freqs_new < clk_big_min)) {
> > > +		if (actual_cluster == A15_CLUSTER && freqs_new < clk_big_min)
> > >  			new_cluster = A7_CLUSTER;
> > > -		} else if ((actual_cluster == A7_CLUSTER) &&
> > > -				(freqs_new > clk_little_max)) {
> > > +		else if (actual_cluster == A7_CLUSTER &&
> > > +			 freqs_new > clk_little_max)
> > >  			new_cluster = A15_CLUSTER;
> > > -		}
> > >  	}
> > >  
> > >  	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,
> > >  				      freqs_new);
> > >  
> > > -	if (!ret) {
> > > +	if (!ret)
> > 
> > That's not the standard way in Linux I believe. We do use {} even when
> > the body is single line but broken into two, like below.
> >
> 
> OK, wasn't aware of that. I will update. Generally I ignore checkpatch
> warnings, but the list was big and fixed a bunch of them :)

In cases like this one, the best is to go with whatever makes checkpatch 
happy.


Nicolas