Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1365909 > unrolled thread

[PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-29 08:50 +0200
    Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support Arnd Bergmann <arnd@arndb.de> - 2016-03-29 17:20 +0200
      Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device  creation support Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-29 18:40 +0200
        Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support Arnd Bergmann <arnd@arndb.de> - 2016-03-29 21:50 +0200
          Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device  creation support Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-30 05:30 +0200
            Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support Arnd Bergmann <arnd@arndb.de> - 2016-03-30 10:00 +0200
              Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device  creation support Viresh Kumar <viresh.kumar@linaro.org> - 2016-04-01 12:30 +0200
                Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device  creation support Mason <slash.tmp@free.fr> - 2016-04-01 14:40 +0200
                  Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device  creation support Viresh Kumar <viresh.kumar@linaro.org> - 2016-04-01 15:00 +0200
                Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device  creation support Rob Herring <robherring2@gmail.com> - 2016-04-01 16:20 +0200
      Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device  creation support Viresh Kumar <viresh.kumar@linaro.org> - 2016-03-29 18:50 +0200
        Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support Arnd Bergmann <arnd@arndb.de> - 2016-03-29 21:50 +0200

#1365909 — [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

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 <viresh.kumar@linaro.org>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 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 <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.
+ */
+
+#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 <viresh.kumar@linaro.org>");
+MODULE_DESCRIPTION("cpufreq-dt platdev driver");
+MODULE_LICENSE("GPL");
-- 
2.7.1.410.g6faf27b

[toc] | [next] | [standalone]

#1366375

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 <viresh.kumar@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  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

[toc] | [prev] | [next] | [standalone]

#1366447 — Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

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

[toc] | [prev] | [next] | [standalone]

#1366596

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

[toc] | [prev] | [next] | [standalone]

#1366882 — Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

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

[toc] | [prev] | [next] | [standalone]

#1366965

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

[toc] | [prev] | [next] | [standalone]

#1369164 — Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

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

[toc] | [prev] | [next] | [standalone]

#1369254 — Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

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.

[toc] | [prev] | [next] | [standalone]

#1369285 — Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

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

[toc] | [prev] | [next] | [standalone]

#1369360 — Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

On Fri, Apr 1, 2016 at 5:23 AM, Viresh Kumar <viresh.kumar@linaro.org> 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

[toc] | [prev] | [next] | [standalone]

#1366459 — Re: [PATCH V1 Resend 2/3] cpufreq: dt: Add generic platform-device creation support

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

[toc] | [prev] | [next] | [standalone]

#1366593

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web