Message ID | 20170821131332.GA14662@verge.net.au |
---|---|
State | Accepted |
Commit | edeec420de2407618de097977e76b572d9de1bcf |
Headers | show |
Series | [1/2] cpufreq: dt-platdev: Automatically create cpufreq device with OPP v2 | expand |
On Mon, Aug 21, 2017 at 03:13:32PM +0200, Viresh Kumar wrote: > The initial idea of creating the cpufreq-dt-platdev.c file was to keep a > list of platforms that use the "operating-points" (V1) bindings and > create cpufreq device for them only, as we weren't sure which platforms > would want the device to get created automatically as some had their own > cpufreq drivers as well, or wanted to initialize cpufreq after doing > some stuff from platform code. > > But that wasn't the case with platforms using "operating-points-v2" > property. We wanted the device to get created automatically without the > need of adding them to the whitelist. Though, we will still have some > exceptions where we don't want to create the device automatically. > > Rename the earlier platform list as *whitelist* and create a new > *blacklist* as well. > > The cpufreq-dt device will get created if: > - The platform is there in the whitelist OR > - The platform has "operating-points-v2" property in CPU0's DT node and > isn't part of the blacklist . > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > I have exercised this on the r8a7795 and r8a7795 with the following reverted: > > * 034def597bb7 ("cpufreq: rcar: Add support for R8A7795 SoC") > * bea2ebca6b91 ("cpufreq: dt: Add r8a7796 support to to use generic cpufreq driver") > > Tested-by: Simon Horman <horms+renesas@verge.net.au> Sorry, I seem to have accidently sent this email as Virish rather than myself. I will try again. > > > --- > drivers/cpufreq/cpufreq-dt-platdev.c | 45 ++++++++++++++++++++++++++++++++---- > 1 file changed, 40 insertions(+), 5 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c > index bcee384b3251..061b468512a2 100644 > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > @@ -9,11 +9,16 @@ > > #include <linux/err.h> > #include <linux/of.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > > #include "cpufreq-dt.h" > > -static const struct of_device_id machines[] __initconst = { > +/* > + * Machines for which the cpufreq device is *always* created, mostly used for > + * platforms using "operating-points" (V1) property. > + */ > +static const struct of_device_id whitelist[] __initconst = { > { .compatible = "allwinner,sun4i-a10", }, > { .compatible = "allwinner,sun5i-a10s", }, > { .compatible = "allwinner,sun5i-a13", }, > @@ -101,21 +106,51 @@ static const struct of_device_id machines[] __initconst = { > { } > }; > > +/* > + * Machines for which the cpufreq device is *not* created, mostly used for > + * platforms using "operating-points-v2" property. > + */ > +static const struct of_device_id blacklist[] __initconst = { > + { } > +}; > + > +static bool __init cpu0_node_has_opp_v2_prop(void) > +{ > + struct device_node *np = of_cpu_device_node_get(0); > + bool ret = false; > + > + if (of_get_property(np, "operating-points-v2", NULL)) > + ret = true; > + > + of_node_put(np); > + return ret; > +} > + > static int __init cpufreq_dt_platdev_init(void) > { > struct device_node *np = of_find_node_by_path("/"); > const struct of_device_id *match; > + const void *data = NULL; > > if (!np) > return -ENODEV; > > - match = of_match_node(machines, np); > + match = of_match_node(whitelist, np); > + if (match) { > + data = match->data; > + goto create_pdev; > + } > + > + if (cpu0_node_has_opp_v2_prop() && !of_match_node(blacklist, np)) > + goto create_pdev; > + > of_node_put(np); > - if (!match) > - return -ENODEV; > + return -ENODEV; > > +create_pdev: > + of_node_put(np); > return PTR_ERR_OR_ZERO(platform_device_register_data(NULL, "cpufreq-dt", > - -1, match->data, > + -1, data, > sizeof(struct cpufreq_dt_platform_data))); > } > device_initcall(cpufreq_dt_platdev_init);
Hi, On 09/11/2017 07:13 AM, Keerthy wrote: > > > On Tuesday 29 August 2017 03:41 PM, Viresh Kumar wrote: >> On 21-08-17, 15:17, Simon Horman wrote: >>> Sorry, I seem to have accidently sent this email as Virish rather than >>> myself. I will try again. >> >> No issues. I see that Rafael has already applied my patches, you can send >> additional patches over that to make sure everything is clear. > > Viresh, > > I see below warning with this patch on am57xx-beagle > > [ 3.545247] WARNING: CPU: 1 PID: 1 at fs/sysfs/dir.c:31 > sysfs_warn_dup+0x58/0x78 > [ 3.553099] sysfs: cannot create duplicate filename > '/devices/platform/cpufreq-dt' > [ 3.561126] Modules linked in: > [ 3.564402] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W > 4.13.0-next-20170911-12690-ga31cc45-dirty #7 > [ 3.575071] Hardware name: Generic DRA74X (Flattened Device Tree) > [ 3.581484] [<c0110a88>] (unwind_backtrace) from [<c010cae4>] > (show_stack+0x10/0x14) > [ 3.589620] [<c010cae4>] (show_stack) from [<c0828184>] > (dump_stack+0xac/0xe0) > [ 3.597209] [<c0828184>] (dump_stack) from [<c01374d0>] > (__warn+0xd8/0x104) > [ 3.604532] [<c01374d0>] (__warn) from [<c0137530>] > (warn_slowpath_fmt+0x34/0x44) > [ 3.612390] [<c0137530>] (warn_slowpath_fmt) from [<c0345b30>] > (sysfs_warn_dup+0x58/0x78) > [ 3.620988] [<c0345b30>] (sysfs_warn_dup) from [<c0345c18>] > (sysfs_create_dir_ns+0x80/0x98) > [ 3.629773] [<c0345c18>] (sysfs_create_dir_ns) from [<c082c974>] > (kobject_add_internal+0x9c/0x2d4) > [ 3.639177] [<c082c974>] (kobject_add_internal) from [<c082cbf8>] > (kobject_add+0x4c/0x9c) > [ 3.647764] [<c082cbf8>] (kobject_add) from [<c05805a8>] > (device_add+0xcc/0x57c) > [ 3.655542] [<c05805a8>] (device_add) from [<c0584d38>] > (platform_device_add+0x100/0x220) > [ 3.664138] [<c0584d38>] (platform_device_add) from [<c058576c>] > (platform_device_register_full+0xf4/0x118) > [ 3.674378] [<c058576c>] (platform_device_register_full) from > [<c0670514>] (ti_cpufreq_init+0x150/0x22c) > [ 3.684326] [<c0670514>] (ti_cpufreq_init) from [<c0101df4>] > (do_one_initcall+0x3c/0x170) > [ 3.692916] [<c0101df4>] (do_one_initcall) from [<c0c00eb4>] > (kernel_init_freeable+0x1fc/0x2c4) > [ 3.702049] [<c0c00eb4>] (kernel_init_freeable) from [<c083c0cc>] > (kernel_init+0x8/0x110) > [ 3.710639] [<c083c0cc>] (kernel_init) from [<c0107d78>] > (ret_from_fork+0x14/0x3c) > [ 3.718680] ---[ end trace 33482508cbb50156 ]--- > [ 3.720850] ata1: SATA link down (SStatus 0 SControl 300) > [ 3.729278] ------------[ cut here ]------------ > [ 3.734147] WARNING: CPU: 1 PID: 1 at lib/kobject.c:240 > kobject_add_internal+0x254/0x2d4 > [ 3.742709] kobject_add_internal failed for cpufreq-dt with -EEXIST, > don't try to register things with the same name in the same directory. > [ 3.755900] Modules linked in: > [ 3.759239] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W > 4.13.0-next-20170911-12690-ga31cc45-dirty #7 > [ 3.769907] Hardware name: Generic DRA74X (Flattened Device Tree) > [ 3.776321] [<c0110a88>] (unwind_backtrace) from [<c010cae4>] > (show_stack+0x10/0x14) > [ 3.784454] [<c010cae4>] (show_stack) from [<c0828184>] > (dump_stack+0xac/0xe0) > [ 3.792043] [<c0828184>] (dump_stack) from [<c01374d0>] > (__warn+0xd8/0x104) > [ 3.799364] [<c01374d0>] (__warn) from [<c0137530>] > (warn_slowpath_fmt+0x34/0x44) > [ 3.807227] [<c0137530>] (warn_slowpath_fmt) from [<c082cb2c>] > (kobject_add_internal+0x254/0x2d4) > [ 3.816552] [<c082cb2c>] (kobject_add_internal) from [<c082cbf8>] > (kobject_add+0x4c/0x9c) > [ 3.825139] [<c082cbf8>] (kobject_add) from [<c05805a8>] > (device_add+0xcc/0x57c) > [ 3.832913] [<c05805a8>] (device_add) from [<c0584d38>] > (platform_device_add+0x100/0x220) > [ 3.841504] [<c0584d38>] (platform_device_add) from [<c058576c>] > (platform_device_register_full+0xf4/0x118) > [ 3.851729] [<c058576c>] (platform_device_register_full) from > [<c0670514>] (ti_cpufreq_init+0x150/0x22c) > [ 3.861686] [<c0670514>] (ti_cpufreq_init) from [<c0101df4>] > (do_one_initcall+0x3c/0x170) > [ 3.870273] [<c0101df4>] (do_one_initcall) from [<c0c00eb4>] > (kernel_init_freeable+0x1fc/0x2c4) > [ 3.879412] [<c0c00eb4>] (kernel_init_freeable) from [<c083c0cc>] > (kernel_init+0x8/0x110) > [ 3.888002] [<c083c0cc>] (kernel_init) from [<c0107d78>] > (ret_from_fork+0x14/0x3c) > [ 3.896035] ---[ end trace 33482508cbb50157 ]--- > > I believe in two places platform_device_register_* calls are made with > the same name 'cpufreq-dt' > > First place is: cpufreq_dt_platdev_init and the other i believe is > ti_cpufreq_init. > > Dave, > > Should the ti-cpufreq driver also be calling > platform_device_register_simple? Yes, ti-cpufreq registers the cpufreq-dt platdev in order to ensure the ti-cpufreq driver probes first and provides the necessary opp-supported-hw for cpufreq-dt. This applies to am335x, am437x, dra7xx, and am57xx. I suppose these platforms will need to be added to the 'blacklist' to prevent the cpufreq-dt platdev from being created automatically, unless there is a better way to handle this dependency... Regards, Dave > > Regards, > Keerthy > >>
On 11-09-17, 09:18, Dave Gerlach wrote: > Yes, ti-cpufreq registers the cpufreq-dt platdev in order to ensure the > ti-cpufreq driver probes first and provides the necessary opp-supported-hw for > cpufreq-dt. This applies to am335x, am437x, dra7xx, and am57xx. I suppose these > platforms will need to be added to the 'blacklist' to prevent the cpufreq-dt > platdev from being created automatically, unless there is a better way to handle > this dependency... Right. Patches are already on the list to fix all platforms that had such issues. -- viresh
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index bcee384b3251..061b468512a2 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -9,11 +9,16 @@ #include <linux/err.h> #include <linux/of.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include "cpufreq-dt.h" -static const struct of_device_id machines[] __initconst = { +/* + * Machines for which the cpufreq device is *always* created, mostly used for + * platforms using "operating-points" (V1) property. + */ +static const struct of_device_id whitelist[] __initconst = { { .compatible = "allwinner,sun4i-a10", }, { .compatible = "allwinner,sun5i-a10s", }, { .compatible = "allwinner,sun5i-a13", }, @@ -101,21 +106,51 @@ static const struct of_device_id machines[] __initconst = { { } }; +/* + * Machines for which the cpufreq device is *not* created, mostly used for + * platforms using "operating-points-v2" property. + */ +static const struct of_device_id blacklist[] __initconst = { + { } +}; + +static bool __init cpu0_node_has_opp_v2_prop(void) +{ + struct device_node *np = of_cpu_device_node_get(0); + bool ret = false; + + if (of_get_property(np, "operating-points-v2", NULL)) + ret = true; + + of_node_put(np); + return ret; +} + static int __init cpufreq_dt_platdev_init(void) { struct device_node *np = of_find_node_by_path("/"); const struct of_device_id *match; + const void *data = NULL; if (!np) return -ENODEV; - match = of_match_node(machines, np); + match = of_match_node(whitelist, np); + if (match) { + data = match->data; + goto create_pdev; + } + + if (cpu0_node_has_opp_v2_prop() && !of_match_node(blacklist, np)) + goto create_pdev; + of_node_put(np); - if (!match) - return -ENODEV; + return -ENODEV; +create_pdev: + of_node_put(np); return PTR_ERR_OR_ZERO(platform_device_register_data(NULL, "cpufreq-dt", - -1, match->data, + -1, data, sizeof(struct cpufreq_dt_platform_data))); } device_initcall(cpufreq_dt_platdev_init);