Multiple platforms are using the generic cpufreq-dt driver now, and all
of them are required to create a platform device with name "cpufreq-dt",
in order to get the cpufreq-dt probed.
Many of them do it from platform code, others have special drivers just
to do that.
It would be more sensible to do this at a generic place, where all such
platform can mark their entries.
This patch adds a separate file to get this device created. Currently
the compat list of platforms that we support is empty, and will be
filled in as and when we move platforms to use it.
It always compiles as part of the kernel and so doesn't need a
module-exit operation.
Signed-off-by: Viresh Kumar <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
drivers/cpufreq/Kconfig | 11 +++++++++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/cpufreq-dt-platdev.c | 48 ++++++++++++++++++++++++++++++++++++
3 files changed, 60 insertions(+)
create mode 100644 drivers/cpufreq/cpufreq-dt-platdev.c
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index a7f45853c103..08573d54105b 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -191,6 +191,7 @@ config CPUFREQ_DT
depends on HAVE_CLK && OF
# if CPU_THERMAL is on and THERMAL=m, CPUFREQ_DT cannot be =y:
depends on !CPU_THERMAL || THERMAL
+ select CPUFREQ_DT_PLATDEV
select PM_OPP
help
This adds a generic DT based cpufreq driver for frequency management.
@@ -199,6 +200,16 @@ config CPUFREQ_DT
If in doubt, say N.
+config CPUFREQ_DT_PLATDEV
+ bool
+ depends on CPUFREQ_DT
+ help
+ This adds a generic DT based cpufreq platdev driver for frequency
+ management. This creates a 'cpufreq-dt' platform device, on the
+ supported platforms.
+
+ If in doubt, say N.
+
if X86
source "drivers/cpufreq/Kconfig.x86"
endif
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 9e63fb1b09f8..b9224fdb8322 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_CONSERVATIVE) += cpufreq_conservative.o
obj-$(CONFIG_CPU_FREQ_GOV_COMMON) += cpufreq_governor.o
obj-$(CONFIG_CPUFREQ_DT) += cpufreq-dt.o
+obj-$(CONFIG_CPUFREQ_DT_PLATDEV) += cpufreq-dt-platdev.o
##################################################################################
# x86 drivers.
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
new file mode 100644
index 000000000000..18b81724ca0b
--- /dev/null
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2016 Linaro.
+ * Viresh Kumar <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/cpufreq-dt.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+struct cpufreq_dt_compat {
+ const char *compatible;
+ const void *data;
+ size_t size;
+};
+
+static struct cpufreq_dt_compat compat[] = {
+};
+
+static int __init cpufreq_dt_platdev_init(void)
+{
+ struct platform_device *pdev;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(compat); i++) {
+ if (!of_machine_is_compatible(compat[i].compatible))
+ continue;
+
+ pdev = platform_device_register_data(NULL, "cpufreq-dt", -1,
+ compat[i].data,
+ compat[i].size);
+
+ return PTR_ERR_OR_ZERO(pdev);
+ }
+
+ return -ENODEV;
+}
+module_init(cpufreq_dt_platdev_init);
+
+MODULE_ALIAS("cpufreq-dt-platdev");
+MODULE_AUTHOR("Viresh Kumar <[email protected]>");
+MODULE_DESCRIPTION("cpufreq-dt platdev driver");
+MODULE_LICENSE("GPL");
--
2.7.1.410.g6faf27b
On Tuesday 29 March 2016 12:09:48 Viresh Kumar wrote:
> Multiple platforms are using the generic cpufreq-dt driver now, and all
> of them are required to create a platform device with name "cpufreq-dt",
> in order to get the cpufreq-dt probed.
>
> Many of them do it from platform code, others have special drivers just
> to do that.
>
> It would be more sensible to do this at a generic place, where all such
> platform can mark their entries.
>
> This patch adds a separate file to get this device created. Currently
> the compat list of platforms that we support is empty, and will be
> filled in as and when we move platforms to use it.
>
> It always compiles as part of the kernel and so doesn't need a
> module-exit operation.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> ---
> drivers/cpufreq/Kconfig | 11 +++++++++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/cpufreq-dt-platdev.c | 48 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 60 insertions(+)
> create mode 100644 drivers/cpufreq/cpufreq-dt-platdev.c
>
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index a7f45853c103..08573d54105b 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -191,6 +191,7 @@ config CPUFREQ_DT
> depends on HAVE_CLK && OF
> # if CPU_THERMAL is on and THERMAL=m, CPUFREQ_DT cannot be =y:
> depends on !CPU_THERMAL || THERMAL
> + select CPUFREQ_DT_PLATDEV
> select PM_OPP
> help
> This adds a generic DT based cpufreq driver for frequency management.
> @@ -199,6 +200,16 @@ config CPUFREQ_DT
>
> If in doubt, say N.
>
> +config CPUFREQ_DT_PLATDEV
> + bool
> + depends on CPUFREQ_DT
The 'depends on' line is redundant as you always 'select' the code
from CPUFREQ_DT. Since they are always set together, you can also
just drop the new symbol, or put the new code into the same file.
> +struct cpufreq_dt_compat {
> + const char *compatible;
> + const void *data;
> + size_t size;
> +};
> +
> +static struct cpufreq_dt_compat compat[] = {
> +};
The 'data' here is alway 'struct cpufreq_dt_platform_data' or NULL,
and the size can be derived from that. If we add this into the opp
platform interfaces, both can go away.
> +static int __init cpufreq_dt_platdev_init(void)
> +{
> + struct platform_device *pdev;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(compat); i++) {
> + if (!of_machine_is_compatible(compat[i].compatible))
> + continue;
> +
> + pdev = platform_device_register_data(NULL, "cpufreq-dt", -1,
> + compat[i].data,
> + compat[i].size);
> +
and then this can become
if (of_device_match(of_root, compat))
platform_device_register_simple(NULL, "cpufreq-dt", 0, NULL, 0);
Arnd
On 29-03-16, 17:14, Arnd Bergmann wrote:
> On Tuesday 29 March 2016 12:09:48 Viresh Kumar wrote:
> > +config CPUFREQ_DT_PLATDEV
> > + bool
> > + depends on CPUFREQ_DT
>
> The 'depends on' line is redundant as you always 'select' the code
> from CPUFREQ_DT. Since they are always set together, you can also
> just drop the new symbol, or put the new code into the same file.
I would like to keep a separate file for this, it looks awkward to update the
driver for any new user platform.
I added a separate symbol to take care of the Modular case of cpufreq-dt. i.e. I
didn't wanted to add module_exit() code in the new file. Currently, it gets
included in the kernel even if cpufreq-dt is selected as a Module, so we just
create the platform-device just once and never touch it again.
--
viresh
On 29-03-16, 17:14, Arnd Bergmann wrote:
> if (of_device_match(of_root, compat))
You meant of_match_node() here, right?
> platform_device_register_simple(NULL, "cpufreq-dt", 0, NULL, 0);
--
viresh
On Tuesday 29 March 2016 22:06:49 Viresh Kumar wrote:
> On 29-03-16, 17:14, Arnd Bergmann wrote:
> > On Tuesday 29 March 2016 12:09:48 Viresh Kumar wrote:
> > > +config CPUFREQ_DT_PLATDEV
> > > + bool
> > > + depends on CPUFREQ_DT
> >
> > The 'depends on' line is redundant as you always 'select' the code
> > from CPUFREQ_DT. Since they are always set together, you can also
> > just drop the new symbol, or put the new code into the same file.
>
> I would like to keep a separate file for this, it looks awkward to update the
> driver for any new user platform.
>
> I added a separate symbol to take care of the Modular case of cpufreq-dt. i.e. I
> didn't wanted to add module_exit() code in the new file. Currently, it gets
> included in the kernel even if cpufreq-dt is selected as a Module, so we just
> create the platform-device just once and never touch it again.
Ok, that makes sense.
Regarding new platforms, I'd hope that we could manage to define an extension
to the oppv2 binding that marks a machine as compatible with opp, so we can
just look at that instead of the root compatible string. Your current series
is nice in the way it gets the hack outside of my code, but it would be ideal
if we could avoid propagating it further.
Arnd
On Tuesday 29 March 2016 22:12:09 you wrote:
> On 29-03-16, 17:14, Arnd Bergmann wrote:
> > if (of_device_match(of_root, compat))
>
> You meant of_match_node() here, right?
>
Yes, sorry for the confusion.
Arnd
On 29-03-16, 21:45, Arnd Bergmann wrote:
> Regarding new platforms, I'd hope that we could manage to define an extension
> to the oppv2 binding that marks a machine as compatible with opp, so we can
The extension of oppv2 binding or compatible string is for platforms that really
need extend oppv2 (like ST, which already have a new compatible string). I am
not sure if we should be putting that into the Machine's compatible area, as I
though it is something internal to OPP layer or their driver.
But we can see that when we handle it.
--
viresh
On Wednesday 30 March 2016 08:52:40 Viresh Kumar wrote:
> On 29-03-16, 21:45, Arnd Bergmann wrote:
> > Regarding new platforms, I'd hope that we could manage to define an extension
> > to the oppv2 binding that marks a machine as compatible with opp, so we can
>
> The extension of oppv2 binding or compatible string is for platforms that really
> need extend oppv2 (like ST, which already have a new compatible string). I am
> not sure if we should be putting that into the Machine's compatible area, as I
> though it is something internal to OPP layer or their driver.
>
> But we can see that when we handle it.
>
I think it should be something in the /cpus or the /opp_table hierarchy,
not the root of the device tree, but other than that I don't care much
whether it's a variation of the oppv2 compatible string or an additional
property in any of the nodes.
Arnd
Cc'ing Rob and Mason.
On 30-03-16, 09:53, Arnd Bergmann wrote:
> I think it should be something in the /cpus or the /opp_table hierarchy,
> not the root of the device tree, but other than that I don't care much
> whether it's a variation of the oppv2 compatible string or an additional
> property in any of the nodes.
So you mean for future DT files we can have something like this:
cpus {
compatible = "operation-points-v2";
#address-cells = <1>;
#size-cells = <0>;
cpu@0 {
compatible = "arm,cortex-a9";
reg = <0>;
next-level-cache = <&L2>;
operating-points-v2 = <&cpu0_opp_table>;
};
};
cpu0_opp_table: opp_table0 {
opp@1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <970000 975000 985000>;
opp-microamp = <70000>;
clock-latency-ns = <300000>;
opp-suspend;
};
opp@1100000000 {
opp-hz = /bits/ 64 <1100000000>;
opp-microvolt = <980000 1000000 1010000>;
opp-microamp = <80000>;
clock-latency-ns = <310000>;
};
};
};
And the cpufreq-dt driver can match /cpus node's compatible string against
"operating-points-v2" and create a device at runtime ?
@Rob: Will that be acceptable to you? We are discussing (again) about how to
probe cpufreq-dt driver automatically for platforms :)
The cpus node doesn't have any 'compatible' property today, and I will be
required to add that in this case.
--
viresh
On 01/04/2016 12:23, Viresh Kumar wrote:
> Cc'ing Rob and Mason.
>
> On 30-03-16, 09:53, Arnd Bergmann wrote:
>
>> I think it should be something in the /cpus or the /opp_table hierarchy,
>> not the root of the device tree, but other than that I don't care much
>> whether it's a variation of the oppv2 compatible string or an additional
>> property in any of the nodes.
>
> So you mean for future DT files we can have something like this:
>
> cpus {
> compatible = "operation-points-v2";
> [...]
>
> And the cpufreq-dt driver can match /cpus node's compatible string against
> "operating-points-v2" and create a device at runtime ?
Hmmm... I'm using the older operating-points prop in my platform's DT.
Why can't we define a new property (e.g. "enable-generic-cpufreq")
which registers the "cpufreq-dt" pseudo-device?
And platforms that manually register "cpufreq-dt" would be
automatically white-listed, even if they don't have the new
property, to maintain backward-compat?
> @Rob: Will that be acceptable to you? We are discussing (again) about how to
> probe cpufreq-dt driver automatically for platforms :)
>
> The cpus node doesn't have any 'compatible' property today, and I will be
> required to add that in this case.
Why does it need a compatible prop?
Why isn't a bool prop enough?
Regards.
On 01-04-16, 14:30, Mason wrote:
> Hmmm... I'm using the older operating-points prop in my platform's DT.
> Why can't we define a new property (e.g. "enable-generic-cpufreq")
> which registers the "cpufreq-dt" pseudo-device?
DT is all about expressing hardware in a file. The same bindings
should be usable across any operating system, not just Linux. And so
we shouldn't have any OS or software-implementation specific
properties here.
> And platforms that manually register "cpufreq-dt" would be
> automatically white-listed, even if they don't have the new
> property, to maintain backward-compat?
Its not about just making it work, otherwise we would have done it
long time back by creating a DT node for cpufreq-dt driver.
> > @Rob: Will that be acceptable to you? We are discussing (again) about how to
> > probe cpufreq-dt driver automatically for platforms :)
> >
> > The cpus node doesn't have any 'compatible' property today, and I will be
> > required to add that in this case.
>
> Why does it need a compatible prop?
That compatible property will describe how to parse/describe
operating-points for a CPU. Any driver, which has the ability to parse
those bindings can do things base on such a compatibility string.
I hope you got the idea.
--
viresh
On Fri, Apr 1, 2016 at 5:23 AM, Viresh Kumar <[email protected]> wrote:
> Cc'ing Rob and Mason.
>
> On 30-03-16, 09:53, Arnd Bergmann wrote:
>> I think it should be something in the /cpus or the /opp_table hierarchy,
>> not the root of the device tree, but other than that I don't care much
>> whether it's a variation of the oppv2 compatible string or an additional
>> property in any of the nodes.
>
> So you mean for future DT files we can have something like this:
>
> cpus {
> compatible = "operation-points-v2";
> #address-cells = <1>;
> #size-cells = <0>;
>
> cpu@0 {
> compatible = "arm,cortex-a9";
> reg = <0>;
> next-level-cache = <&L2>;
> operating-points-v2 = <&cpu0_opp_table>;
> };
> };
>
> cpu0_opp_table: opp_table0 {
> opp@1000000000 {
> opp-hz = /bits/ 64 <1000000000>;
> opp-microvolt = <970000 975000 985000>;
> opp-microamp = <70000>;
> clock-latency-ns = <300000>;
> opp-suspend;
> };
> opp@1100000000 {
> opp-hz = /bits/ 64 <1100000000>;
> opp-microvolt = <980000 1000000 1010000>;
> opp-microamp = <80000>;
> clock-latency-ns = <310000>;
> };
> };
> };
>
>
> And the cpufreq-dt driver can match /cpus node's compatible string against
> "operating-points-v2" and create a device at runtime ?
>
> @Rob: Will that be acceptable to you? We are discussing (again) about how to
> probe cpufreq-dt driver automatically for platforms :)
No, I don't think that belongs in /cpus.
Part of the problem is this requires a DT change if you switch between
a platform-specific driver and generic driver.
I don't understand the issue having a little bit of code to parse the
DT and create the device. If you are worried about having a long list
of platforms, you could instead check the tree for operating-points-v2
property in the cpu node and create the device unless the platform is
black-listed.
Rob
On 01-04-16, 09:15, Rob Herring wrote:
> On Fri, Apr 1, 2016 at 5:23 AM, Viresh Kumar <[email protected]> wrote:
> > So you mean for future DT files we can have something like this:
> >
> > cpus {
> > compatible = "operation-points-v2";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > cpu@0 {
> > compatible = "arm,cortex-a9";
> > reg = <0>;
> > next-level-cache = <&L2>;
> > operating-points-v2 = <&cpu0_opp_table>;
> > };
> > };
> >
> > cpu0_opp_table: opp_table0 {
> > opp@1000000000 {
> > opp-hz = /bits/ 64 <1000000000>;
> > opp-microvolt = <970000 975000 985000>;
> > opp-microamp = <70000>;
> > clock-latency-ns = <300000>;
> > opp-suspend;
> > };
> > opp@1100000000 {
> > opp-hz = /bits/ 64 <1100000000>;
> > opp-microvolt = <980000 1000000 1010000>;
> > opp-microamp = <80000>;
> > clock-latency-ns = <310000>;
> > };
> > };
> > };
> >
> >
> > And the cpufreq-dt driver can match /cpus node's compatible string against
> > "operating-points-v2" and create a device at runtime ?
> >
> > @Rob: Will that be acceptable to you? We are discussing (again) about how to
> > probe cpufreq-dt driver automatically for platforms :)
>
> No, I don't think that belongs in /cpus.
>
> Part of the problem is this requires a DT change if you switch between
> a platform-specific driver and generic driver.
Right.
> I don't understand the issue having a little bit of code to parse the
> DT and create the device.
I am fine with that, we were just re-evaluating our options :)
> If you are worried about having a long list
> of platforms,
At least I am not.
> you could instead check the tree for operating-points-v2
> property in the cpu node and create the device unless the platform is
> black-listed.
I don't really like the black-list idea much. It forces a Non
cpufreq-dt platform to edit cpufreq-dt related file, just to make its
own cpufreq driver work.
I find that ugly somehow :)
--
viresh
On Thursday 07 April 2016 14:00:36 Viresh Kumar wrote:
> On 01-04-16, 09:15, Rob Herring wrote:
> > On Fri, Apr 1, 2016 at 5:23 AM, Viresh Kumar <[email protected]> wrote:
> > >
> > >
> > > And the cpufreq-dt driver can match /cpus node's compatible string against
> > > "operating-points-v2" and create a device at runtime ?
> > >
> > > @Rob: Will that be acceptable to you? We are discussing (again) about how to
> > > probe cpufreq-dt driver automatically for platforms
> >
> > No, I don't think that belongs in /cpus.
> >
> > Part of the problem is this requires a DT change if you switch between
> > a platform-specific driver and generic driver.
>
> Right.
How likely is that to happen for future platforms though?
I'd rather see a whitelist for the existing platforms (as started
by Viresh), because we don't know what other out-of-tree platforms
use the "operating-points-v2" binding that are in fact not compatible
with today's driver.
If we add one way to identify machines that are known to follow
the binding to the degree necessary to make them work with the
Linux driver (whatever way that would be), then the only remaining
worry is about machines suddenly breaking because of a change in
the Linux driver, and we can work around that in the kernel if
necessary (or revert whatever broke the platform).
> > I don't understand the issue having a little bit of code to parse the
> > DT and create the device.
>
> I am fine with that, we were just re-evaluating our options
>
> > If you are worried about having a long list
> > of platforms,
>
> At least I am not.
The length of the list is not the issue here, it's the requirement
to edit the list for each new SoC that gets added. That causes conflicts
and constant churn.
> > you could instead check the tree for operating-points-v2
> > property in the cpu node and create the device unless the platform is
> > black-listed.
>
> I don't really like the black-list idea much. It forces a Non
> cpufreq-dt platform to edit cpufreq-dt related file, just to make its
> own cpufreq driver work.
>
> I find that ugly somehow
Agreed.
Sorry for the late reply, I'm still catching up with email from two
weeks of travelling.
Arnd