[PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Sudeep Holla]

On some ARM based systems, a separate Cortex-M based System Control
Processor(SCP) provides the overall power, clock, reset and system
control including CPU DVFS. SCPI Message Protocol is used to
communicate with the SCPI.

This patch adds a interface driver for adding OPPs and registering
the arm_big_little cpufreq driver for such systems.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org
---
 drivers/cpufreq/Kconfig.arm    |   9 ++++
 drivers/cpufreq/Makefile       |   1 +
 drivers/cpufreq/scpi-cpufreq.c | 103 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+)
 create mode 100644 drivers/cpufreq/scpi-cpufreq.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 4f3dbc8cf729..9e678bf1687c 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
           This add the CPUfreq driver support for Versatile Express
 	  big.LITTLE platforms using SPC for power management.
 
+config ARM_SCPI_CPUFREQ
+        tristate "SCPI based CPUfreq driver"
+	depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL
+        help
+          This add the CPUfreq driver support for ARM big.LITTLE platforms
+	  using SCPI interface for CPU power management.
+
+	  This driver works only if firmware the supporting CPU DVFS adhere
+	  to SCPI protocol.
 
 config ARM_EXYNOS_CPUFREQ
 	tristate "SAMSUNG EXYNOS CPUfreq Driver"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index cdce92ae2e8b..02fc9f849d4b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_ARM_SA1110_CPUFREQ)	+= sa1110-cpufreq.o
 obj-$(CONFIG_ARM_SPEAR_CPUFREQ)		+= spear-cpufreq.o
 obj-$(CONFIG_ARM_TEGRA_CPUFREQ)		+= tegra-cpufreq.o
 obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ)	+= vexpress-spc-cpufreq.o
+obj-$(CONFIG_ARM_SCPI_CPUFREQ)		+= scpi-cpufreq.o
 
 ##################################################################################
 # PowerPC platform drivers
diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
new file mode 100644
index 000000000000..4c2c11a9dfc6
--- /dev/null
+++ b/drivers/cpufreq/scpi-cpufreq.c
@@ -0,0 +1,103 @@
+/*
+ * SCPI CPUFreq Interface driver
+ *
+ * It provides necessary ops to arm_big_little cpufreq driver.
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ * Sudeep Holla <sudeep.holla@arm.com>
+ *
+ * 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
+
+#include <linux/cpufreq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/scpi_protocol.h>
+#include <linux/types.h>
+
+#include "arm_big_little.h"
+
+static struct scpi_ops *scpi_ops;
+
+static int scpi_init_opp_table(struct device *cpu_dev)
+{
+	u8 domain = topology_physical_package_id(cpu_dev->id);
+	struct scpi_dvfs_info *info;
+	struct scpi_opp *opp;
+	int idx, ret = 0;
+
+	if ((dev_pm_opp_get_opp_count(cpu_dev)) > 0)
+		return 0;
+	info = scpi_ops->dvfs_get_info(domain);
+	if (IS_ERR(info))
+		return PTR_ERR(info);
+
+	opp = info->opps;
+	if (!opp)
+		return -EIO;
+
+	for (idx = 0; idx < info->count; idx++, opp++) {
+		ret = dev_pm_opp_add(cpu_dev, opp->freq, opp->m_volt * 1000);
+		if (ret) {
+			dev_warn(cpu_dev, "failed to add opp %uHz %umV\n",
+				 opp->freq, opp->m_volt);
+			return ret;
+		}
+	}
+	return ret;
+}
+
+static int scpi_get_transition_latency(struct device *cpu_dev)
+{
+	u8 domain = topology_physical_package_id(cpu_dev->id);
+	struct scpi_dvfs_info *info;
+
+	info = scpi_ops->dvfs_get_info(domain);
+	if (IS_ERR(info))
+		return PTR_ERR(info);
+
+	return info->latency;
+}
+
+static struct cpufreq_arm_bL_ops scpi_cpufreq_ops = {
+	.name	= "scpi",
+	.get_transition_latency = scpi_get_transition_latency,
+	.init_opp_table = scpi_init_opp_table,
+};
+
+static int scpi_cpufreq_probe(struct platform_device *pdev)
+{
+	scpi_ops = get_scpi_ops();
+	if (!scpi_ops)
+		return -EIO;
+
+	return bL_cpufreq_register(&scpi_cpufreq_ops);
+}
+
+static int scpi_cpufreq_remove(struct platform_device *pdev)
+{
+	bL_cpufreq_unregister(&scpi_cpufreq_ops);
+	return 0;
+}
+
+static struct platform_driver scpi_cpufreq_platdrv = {
+	.driver = {
+		.name	= "scpi-cpufreq",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= scpi_cpufreq_probe,
+	.remove		= scpi_cpufreq_remove,
+};
+module_platform_driver(scpi_cpufreq_platdrv);
+
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Viresh Kumar]

On 27 April 2015 at 17:10, Sudeep Holla <sudeep.holla@arm.com> wrote:
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 4f3dbc8cf729..9e678bf1687c 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
>            This add the CPUfreq driver support for Versatile Express
>           big.LITTLE platforms using SPC for power management.
>
> +config ARM_SCPI_CPUFREQ
> +        tristate "SCPI based CPUfreq driver"
> +       depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL
> +        help
> +          This add the CPUfreq driver support for ARM big.LITTLE platforms
> +         using SCPI interface for CPU power management.
> +
> +         This driver works only if firmware the supporting CPU DVFS adhere
> +         to SCPI protocol.

Wanna reword that ?

>
>  config ARM_EXYNOS_CPUFREQ
>         tristate "SAMSUNG EXYNOS CPUfreq Driver"
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index cdce92ae2e8b..02fc9f849d4b 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_ARM_SA1110_CPUFREQ)      += sa1110-cpufreq.o
>  obj-$(CONFIG_ARM_SPEAR_CPUFREQ)                += spear-cpufreq.o
>  obj-$(CONFIG_ARM_TEGRA_CPUFREQ)                += tegra-cpufreq.o
>  obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
> +obj-$(CONFIG_ARM_SCPI_CPUFREQ)         += scpi-cpufreq.o
>
>  ##################################################################################
>  # PowerPC platform drivers
> diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
> new file mode 100644
> index 000000000000..4c2c11a9dfc6
> --- /dev/null
> +++ b/drivers/cpufreq/scpi-cpufreq.c
> @@ -0,0 +1,103 @@
> +/*
> + * SCPI CPUFreq Interface driver
> + *
> + * It provides necessary ops to arm_big_little cpufreq driver.
> + *
> + * Copyright (C) 2015 ARM Ltd.
> + * Sudeep Holla <sudeep.holla@arm.com>
> + *
> + * 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
> +
> +#include <linux/cpufreq.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> +#include <linux/scpi_protocol.h>
> +#include <linux/types.h>
> +
> +#include "arm_big_little.h"
> +
> +static struct scpi_ops *scpi_ops;
> +
> +static int scpi_init_opp_table(struct device *cpu_dev)
> +{
> +       u8 domain = topology_physical_package_id(cpu_dev->id);
> +       struct scpi_dvfs_info *info;
> +       struct scpi_opp *opp;
> +       int idx, ret = 0;
> +
> +       if ((dev_pm_opp_get_opp_count(cpu_dev)) > 0)
> +               return 0;

Why, who would have added it ?

> +       info = scpi_ops->dvfs_get_info(domain);

Isn't calling this twice costly for getting the same information ?

> +       if (IS_ERR(info))
> +               return PTR_ERR(info);
> +
> +       opp = info->opps;
> +       if (!opp)
> +               return -EIO;
> +
> +       for (idx = 0; idx < info->count; idx++, opp++) {
> +               ret = dev_pm_opp_add(cpu_dev, opp->freq, opp->m_volt * 1000);
> +               if (ret) {
> +                       dev_warn(cpu_dev, "failed to add opp %uHz %umV\n",
> +                                opp->freq, opp->m_volt);

Don't you want to free earlier OPPs here ?

> +                       return ret;
> +               }
> +       }
> +       return ret;
> +}
> +
> +static int scpi_get_transition_latency(struct device *cpu_dev)
> +{
> +       u8 domain = topology_physical_package_id(cpu_dev->id);
> +       struct scpi_dvfs_info *info;
> +
> +       info = scpi_ops->dvfs_get_info(domain);
> +       if (IS_ERR(info))
> +               return PTR_ERR(info);
> +
> +       return info->latency;
> +}
> +
> +static struct cpufreq_arm_bL_ops scpi_cpufreq_ops = {
> +       .name   = "scpi",
> +       .get_transition_latency = scpi_get_transition_latency,
> +       .init_opp_table = scpi_init_opp_table,

Don't want to free/remove OPPs ?

> +};
> +
> +static int scpi_cpufreq_probe(struct platform_device *pdev)
> +{
> +       scpi_ops = get_scpi_ops();
> +       if (!scpi_ops)
> +               return -EIO;
> +
> +       return bL_cpufreq_register(&scpi_cpufreq_ops);
> +}
> +
> +static int scpi_cpufreq_remove(struct platform_device *pdev)
> +{
> +       bL_cpufreq_unregister(&scpi_cpufreq_ops);
> +       return 0;
> +}
> +
> +static struct platform_driver scpi_cpufreq_platdrv = {
> +       .driver = {
> +               .name   = "scpi-cpufreq",
> +               .owner  = THIS_MODULE,
> +       },
> +       .probe          = scpi_cpufreq_probe,
> +       .remove         = scpi_cpufreq_remove,
> +};
> +module_platform_driver(scpi_cpufreq_platdrv);
> +
> +MODULE_LICENSE("GPL");

GPL V2 ?

Author/Description missing ..

> --
> 1.9.1
>

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Sudeep Holla]

Hi Viresh,

Thanks for the review.

On 29/04/15 06:44, Viresh Kumar wrote:
> On 27 April 2015 at 17:10, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 4f3dbc8cf729..9e678bf1687c 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
>>             This add the CPUfreq driver support for Versatile Express
>>            big.LITTLE platforms using SPC for power management.
>>
>> +config ARM_SCPI_CPUFREQ
>> +        tristate "SCPI based CPUfreq driver"
>> +       depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL
>> +        help
>> +          This add the CPUfreq driver support for ARM big.LITTLE platforms
>> +         using SCPI interface for CPU power management.
>> +
>> +         This driver works only if firmware the supporting CPU DVFS adhere
>> +         to SCPI protocol.
>
> Wanna reword that ?
>

Ok


[...]

>> +static int scpi_init_opp_table(struct device *cpu_dev)
>> +{
>> +       u8 domain = topology_physical_package_id(cpu_dev->id);
>> +       struct scpi_dvfs_info *info;
>> +       struct scpi_opp *opp;
>> +       int idx, ret = 0;
>> +
>> +       if ((dev_pm_opp_get_opp_count(cpu_dev)) > 0)
>> +               return 0;
>
> Why, who would have added it ?
>

IIRC, it was added to prevent spurious duplicate OPP addition messages
during CPU hotplug. I will check it again.

>> +       info = scpi_ops->dvfs_get_info(domain);
>
> Isn't calling this twice costly for getting the same information ?
>

No the SCPI protocol saves them and return just the pointer if it's
already populated.

>> +       if (IS_ERR(info))
>> +               return PTR_ERR(info);
>> +
>> +       opp = info->opps;
>> +       if (!opp)
>> +               return -EIO;
>> +
>> +       for (idx = 0; idx < info->count; idx++, opp++) {
>> +               ret = dev_pm_opp_add(cpu_dev, opp->freq, opp->m_volt * 1000);
>> +               if (ret) {
>> +                       dev_warn(cpu_dev, "failed to add opp %uHz %umV\n",
>> +                                opp->freq, opp->m_volt);
>
> Don't you want to free earlier OPPs here ?
>

Make sense will fix.

>> +static struct cpufreq_arm_bL_ops scpi_cpufreq_ops = {
>> +       .name   = "scpi",
>> +       .get_transition_latency = scpi_get_transition_latency,
>> +       .init_opp_table = scpi_init_opp_table,
>
> Don't want to free/remove OPPs ?
>

Ah I see a new function is added, will fix it. In-fact this driver was
written before that and was held up since the firmware was not stable.

[...]

>> +static struct platform_driver scpi_cpufreq_platdrv = {
>> +       .driver = {
>> +               .name   = "scpi-cpufreq",
>> +               .owner  = THIS_MODULE,
>> +       },
>> +       .probe          = scpi_cpufreq_probe,
>> +       .remove         = scpi_cpufreq_remove,
>> +};
>> +module_platform_driver(scpi_cpufreq_platdrv);
>> +
>> +MODULE_LICENSE("GPL");
>
> GPL V2 ?
>
> Author/Description missing ..
>

Will fix it in next version.

Regards,
Sudeep

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Jon Medhurst (Tixy)]

On Mon, 2015-04-27 at 12:40 +0100, Sudeep Holla wrote:
> On some ARM based systems, a separate Cortex-M based System Control
> Processor(SCP) provides the overall power, clock, reset and system
> control including CPU DVFS. SCPI Message Protocol is used to
> communicate with the SCPI.
> 
> This patch adds a interface driver for adding OPPs and registering
> the arm_big_little cpufreq driver for such systems.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: linux-pm@vger.kernel.org
> ---
>  drivers/cpufreq/Kconfig.arm    |   9 ++++
>  drivers/cpufreq/Makefile       |   1 +
>  drivers/cpufreq/scpi-cpufreq.c | 103 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 113 insertions(+)
>  create mode 100644 drivers/cpufreq/scpi-cpufreq.c
> 
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 4f3dbc8cf729..9e678bf1687c 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
>            This add the CPUfreq driver support for Versatile Express
>  	  big.LITTLE platforms using SPC for power management.
>  
> +config ARM_SCPI_CPUFREQ
> +        tristate "SCPI based CPUfreq driver"
> +	depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL

And ARM_BIG_LITTLE_CPUFREQ depends on CONFIG_ARM, so we can't build this
for arm64, which is the only platform (Juno) we have to run these
patches on. Unless you're prepared for a horrid hack...
https://github.com/ARM-software/linux/commit/b9ceaa0cbd7c57d57ee7e69146cc627697570f6e
or a pair of less horrid ones...
http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/4b5dd8ff98613b7e90c8f3214522a00ab6900fe9
http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/8641dbfe253f6a9061742ca11f769fc1d82c2aaa

Any reason why the above two aren't suitable for mainline Linux? The
second was actually committed then reverted because it broke arm64
builds, but the other patch fixes that.

-- 
Tixy

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Sudeep Holla]

On 01/05/15 14:19, Jon Medhurst (Tixy) wrote:
> On Mon, 2015-04-27 at 12:40 +0100, Sudeep Holla wrote:
>> On some ARM based systems, a separate Cortex-M based System Control
>> Processor(SCP) provides the overall power, clock, reset and system
>> control including CPU DVFS. SCPI Message Protocol is used to
>> communicate with the SCPI.
>>
>> This patch adds a interface driver for adding OPPs and registering
>> the arm_big_little cpufreq driver for such systems.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Viresh Kumar <viresh.kumar@linaro.org>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: linux-pm@vger.kernel.org
>> ---
>>   drivers/cpufreq/Kconfig.arm    |   9 ++++
>>   drivers/cpufreq/Makefile       |   1 +
>>   drivers/cpufreq/scpi-cpufreq.c | 103 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 113 insertions(+)
>>   create mode 100644 drivers/cpufreq/scpi-cpufreq.c
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 4f3dbc8cf729..9e678bf1687c 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
>>             This add the CPUfreq driver support for Versatile Express
>>   	  big.LITTLE platforms using SPC for power management.
>>
>> +config ARM_SCPI_CPUFREQ
>> +        tristate "SCPI based CPUfreq driver"
>> +	depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL
>
> And ARM_BIG_LITTLE_CPUFREQ depends on CONFIG_ARM, so we can't build this
> for arm64, which is the only platform (Juno) we have to run these
> patches on.

Yes I know

> Unless you're prepared for a horrid hack...
> https://github.com/ARM-software/linux/commit/b9ceaa0cbd7c57d57ee7e69146cc627697570f6e

I need to check if it can be improved before I post on the list and
hence dropped it in the first version of this patch-set.

> or a pair of less horrid ones...
> http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/4b5dd8ff98613b7e90c8f3214522a00ab6900fe9

No not this one, this was already discussed long back I believe.
It doesn't make sense to add bl-switcher for ARM64 IMO.

> http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/8641dbfe253f6a9061742ca11f769fc1d82c2aaa
>

Yes the Kconfig changes looks better than what I have now.

> Any reason why the above two aren't suitable for mainline Linux? The
> second was actually committed then reverted because it broke arm64
> builds, but the other patch fixes that.
>
Yes IMO since arm-big-little also supports multi-cluster cpufreq(yes I
know it can be merged into cpufreq-dt once we have OPPv2 bindings, but
until then), it should handle the config where CONFIG_BL_SWITCHER=n and
that's what I have attempted in my patch above.

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Jon Medhurst (Tixy)]

On Fri, 2015-05-01 at 14:32 +0100, Sudeep Holla wrote:
> 
> On 01/05/15 14:19, Jon Medhurst (Tixy) wrote:
> > On Mon, 2015-04-27 at 12:40 +0100, Sudeep Holla wrote:
> >> On some ARM based systems, a separate Cortex-M based System Control
> >> Processor(SCP) provides the overall power, clock, reset and system
> >> control including CPU DVFS. SCPI Message Protocol is used to
> >> communicate with the SCPI.
> >>
> >> This patch adds a interface driver for adding OPPs and registering
> >> the arm_big_little cpufreq driver for such systems.
> >>
> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >> Cc: Viresh Kumar <viresh.kumar@linaro.org>
> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> Cc: linux-pm@vger.kernel.org
> >> ---
> >>   drivers/cpufreq/Kconfig.arm    |   9 ++++
> >>   drivers/cpufreq/Makefile       |   1 +
> >>   drivers/cpufreq/scpi-cpufreq.c | 103 +++++++++++++++++++++++++++++++++++++++++
> >>   3 files changed, 113 insertions(+)
> >>   create mode 100644 drivers/cpufreq/scpi-cpufreq.c
> >>
> >> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> >> index 4f3dbc8cf729..9e678bf1687c 100644
> >> --- a/drivers/cpufreq/Kconfig.arm
> >> +++ b/drivers/cpufreq/Kconfig.arm
> >> @@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
> >>             This add the CPUfreq driver support for Versatile Express
> >>   	  big.LITTLE platforms using SPC for power management.
> >>
> >> +config ARM_SCPI_CPUFREQ
> >> +        tristate "SCPI based CPUfreq driver"
> >> +	depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL
> >
> > And ARM_BIG_LITTLE_CPUFREQ depends on CONFIG_ARM, so we can't build this
> > for arm64, which is the only platform (Juno) we have to run these
> > patches on.
> 
> Yes I know
> 
> > Unless you're prepared for a horrid hack...
> > https://github.com/ARM-software/linux/commit/b9ceaa0cbd7c57d57ee7e69146cc627697570f6e
> 
> I need to check if it can be improved before I post on the list and
> hence dropped it in the first version of this patch-set.
> 
> > or a pair of less horrid ones...
> > http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/4b5dd8ff98613b7e90c8f3214522a00ab6900fe9
> 
> No not this one, this was already discussed long back I believe.
> It doesn't make sense to add bl-switcher for ARM64 IMO.
> 
> > http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/8641dbfe253f6a9061742ca11f769fc1d82c2aaa
> >
> 
> Yes the Kconfig changes looks better than what I have now.
> 
> > Any reason why the above two aren't suitable for mainline Linux? The
> > second was actually committed then reverted because it broke arm64
> > builds, but the other patch fixes that.
> >
> Yes IMO since arm-big-little also supports multi-cluster cpufreq(yes I
> know it can be merged into cpufreq-dt once we have OPPv2 bindings, but
> until then), it should handle the config where CONFIG_BL_SWITCHER=n and
> that's what I have attempted in my patch above.

OK, that all makes sense, just wanted to make sure things didn't get
overlooked, because having code that has no users in mainline seemed a
bit odd.

-- 
Tixy

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Sudeep Holla]

On 01/05/15 15:12, Jon Medhurst (Tixy) wrote:
> On Fri, 2015-05-01 at 14:32 +0100, Sudeep Holla wrote:
>>
>> On 01/05/15 14:19, Jon Medhurst (Tixy) wrote:
>>> On Mon, 2015-04-27 at 12:40 +0100, Sudeep Holla wrote:
>>>> On some ARM based systems, a separate Cortex-M based System Control
>>>> Processor(SCP) provides the overall power, clock, reset and system
>>>> control including CPU DVFS. SCPI Message Protocol is used to
>>>> communicate with the SCPI.
>>>>
>>>> This patch adds a interface driver for adding OPPs and registering
>>>> the arm_big_little cpufreq driver for such systems.
>>>>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> Cc: Viresh Kumar <viresh.kumar@linaro.org>
>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>>> Cc: linux-pm@vger.kernel.org
>>>> ---
>>>>    drivers/cpufreq/Kconfig.arm    |   9 ++++
>>>>    drivers/cpufreq/Makefile       |   1 +
>>>>    drivers/cpufreq/scpi-cpufreq.c | 103 +++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 113 insertions(+)
>>>>    create mode 100644 drivers/cpufreq/scpi-cpufreq.c
>>>>
>>>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>>>> index 4f3dbc8cf729..9e678bf1687c 100644
>>>> --- a/drivers/cpufreq/Kconfig.arm
>>>> +++ b/drivers/cpufreq/Kconfig.arm
>>>> @@ -24,6 +24,15 @@ config ARM_VEXPRESS_SPC_CPUFREQ
>>>>              This add the CPUfreq driver support for Versatile Express
>>>>    	  big.LITTLE platforms using SPC for power management.
>>>>
>>>> +config ARM_SCPI_CPUFREQ
>>>> +        tristate "SCPI based CPUfreq driver"
>>>> +	depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL
>>>
>>> And ARM_BIG_LITTLE_CPUFREQ depends on CONFIG_ARM, so we can't build this
>>> for arm64, which is the only platform (Juno) we have to run these
>>> patches on.
>>
>> Yes I know
>>
>>> Unless you're prepared for a horrid hack...
>>> https://github.com/ARM-software/linux/commit/b9ceaa0cbd7c57d57ee7e69146cc627697570f6e
>>
>> I need to check if it can be improved before I post on the list and
>> hence dropped it in the first version of this patch-set.
>>
>>> or a pair of less horrid ones...
>>> http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/4b5dd8ff98613b7e90c8f3214522a00ab6900fe9
>>
>> No not this one, this was already discussed long back I believe.
>> It doesn't make sense to add bl-switcher for ARM64 IMO.
>>
>>> http://git.linaro.org/kernel/linux-linaro-tracking.git/commit/8641dbfe253f6a9061742ca11f769fc1d82c2aaa
>>>
>>
>> Yes the Kconfig changes looks better than what I have now.
>>
>>> Any reason why the above two aren't suitable for mainline Linux? The
>>> second was actually committed then reverted because it broke arm64
>>> builds, but the other patch fixes that.
>>>
>> Yes IMO since arm-big-little also supports multi-cluster cpufreq(yes I
>> know it can be merged into cpufreq-dt once we have OPPv2 bindings, but
>> until then), it should handle the config where CONFIG_BL_SWITCHER=n and
>> that's what I have attempted in my patch above.
>
> OK, that all makes sense, just wanted to make sure things didn't get
> overlooked, because having code that has no users in mainline seemed a
> bit odd.
>

No issues, it's better to have remainders like this :). I plan to post
something in the next version.

Regards,
Sudeep

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Jon Medhurst (Tixy)]

On Fri, 2015-05-01 at 15:15 +0100, Sudeep Holla wrote:
> No issues, it's better to have remainders like this :). I plan to post
> something in the next version.

Will that also include the other piece of the puzzle to get the
big.LITTLE cpufreq driver running? ;-)  ...
https://github.com/ARM-software/linux/commit/29c33a5865437f0745e8e3a9beb5f82694257b4d

With that, and device-tree updates [1], I've finally managed to get this
patch series running on Juno with v4.1-rc1 and they pass some basic
smoke testing.

-- 
Tixy

Re: [PATCH 4/4] cpufreq: arm_big_little: add SCPI interface driver

[Sudeep Holla]

On 01/05/15 18:10, Jon Medhurst (Tixy) wrote:
> On Fri, 2015-05-01 at 15:15 +0100, Sudeep Holla wrote:
>> No issues, it's better to have remainders like this :). I plan to post
>> something in the next version.
>
> Will that also include the other piece of the puzzle to get the
> big.LITTLE cpufreq driver running? ;-)  ...
> https://github.com/ARM-software/linux/commit/29c33a5865437f0745e8e3a9beb5f82694257b4d

That is already posted separately[1] and acked-by Viresh.
I am trying to put independent patches(not related to Juno/SCPI)
separate so that they will be no dependency to merge that.

Regards,
Sudeep

[1] https://lkml.org/lkml/2015/4/27/174