On 1 December 2014 at 18:24, Arnd Bergmann arnd@arndb.de wrote:
Thanks a lot for working on this, we really need to figure it out one day!
:)
Your patches seem well-implemented, so if everybody thinks the general approach is the best solution, we should do that. From my point of view, there are two things I would do differently:
- In the DT binding, I would strongly prefer anything but the root compatible property as the key for the new platforms. Clearly we have to keep using it for the backwards-compatibility case, as you do, but I think there are more appropriate places to put it. Sorting from most favorite to least favorite, my list would be: 1. a new property in /cpus/ 2. a new property each /cpus/cpu@... node.
I did it this way earlier and named it dvfs-method but probably putting this into the /cpus/ node is far better. But then Sudeep asked to utilize compatible property only..
Are you fine with the name here? "dvfs-method"
3. the compatible property of the /cpus node 4. a top-level device node that gets turned into a platform device 5. a new property in the / node 6. a new property in the /chosen node 7. the compatible property in the / node
- Implementation-wise, I don't think it's helpful to have a global function that registers a platform device to be consumed by the device driver. I'd rather just see a module_init function in each driver that rather than
Okay, this might work better in longer run. I am fine with it.
[PATCH, RFC] cpufreq: dt: simplify and generalize probing
We should not have to create a platform device from platform specific code in order to use the completely generic cpufreq-dt driver. This adds a simpler method by creating a new standard property of the "/cpus" node to look for, with a fallback for existing users. The list of existing users needs to be completed, and the same change done for the other DT based drivers.
Signed-off-by: Arnd Bergmann arnd@arndb.de
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c index 6f5f5615fbf1..697b4dc47715 100644 --- a/drivers/cpufreq/cpufreq-dt.c +++ b/drivers/cpufreq/cpufreq-dt.c @@ -345,13 +345,50 @@ static struct cpufreq_driver dt_cpufreq_driver = { .attr = cpufreq_generic_attr, };
-static int dt_cpufreq_probe(struct platform_device *pdev) +/*
- these machines are using the driver but provide no standard
- probing method, only for old machines with existing dtbs.
- */
+static struct of_device_id legacy_machines = {
{ .compatible = "calxeda,highbank" },{ .compatible = "renesas,sh7372" },{ .compatible = "renesas,sh73a0" },{ .compatible = "samsung,exynos5250" },{ .compatible = "samsung,exynos4210" },{ .compatible = "xlnx,zynq-7000" },+};
+static bool dt_cpufreq_available(void) +{
struct device_node *node;bool ret;node = of_find_node_by_path("/cpus");if (!node)return 0;/* the specific property needs to be debated */ret = of_property_read_bool("linux,cpu-frequency-scaling");
How can this be a bool? We need to differentiate on which binding wants to go for arm-bl or cupfreq-dt or any other driver. So we surely need a string ?
of_node_put(node);if (ret)return 1;node = of_find_node_by_path("/");ret = of_match_device(&legacy_machines, node);of_node_put(node);return ret;+}
+static int __init dt_cpufreq_probe(void) { struct device *cpu_dev; struct regulator *cpu_reg; struct clk *cpu_clk; int ret;
if (!dt_cpufreq_available())return -ENXIO; /* * All per-cluster (CPUs sharing clock/voltages) initialization is done * from ->init(). In probe(), we just need to make sure that clk and@@ -367,29 +404,19 @@ static int dt_cpufreq_probe(struct platform_device *pdev) if (!IS_ERR(cpu_reg)) regulator_put(cpu_reg);
dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
We still need this, and its about how clocks are shared between CPUs.
ret = cpufreq_register_driver(&dt_cpufreq_driver); if (ret) dev_err(cpu_dev, "failed register driver: %d\n", ret); return ret;} +module_init(dt_cpufreq_probe);
-static int dt_cpufreq_remove(struct platform_device *pdev) +static void __exit dt_cpufreq_remove(void) { cpufreq_unregister_driver(&dt_cpufreq_driver);
return 0;}
-static struct platform_driver dt_cpufreq_platdrv = {
.driver = {.name = "cpufreq-dt",},.probe = dt_cpufreq_probe,.remove = dt_cpufreq_remove,-}; -module_platform_driver(dt_cpufreq_platdrv); +module_exit(dt_cpufreq_remove);
MODULE_AUTHOR("Viresh Kumar viresh.kumar@linaro.org"); MODULE_AUTHOR("Shawn Guo shawn.guo@linaro.org");