| Message ID | 1404467103-29644-3-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 4 July 2014 15:14, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > This commit adds a simple cpufreq driver for the Armada XP SoC, which > has one separately controllable clock for each CPU. For this reason, > the existing cpufreq-cpu0 generic driver cannot be used because it > currently assumes that there is only one clock controlling the > frequency of all CPUs. > > There are on-going discussions on extending the cpufreq-cpu0 to cover > the case of having one clock for each CPU, but there are still some > unresolved issues to get this extended cpufreq-cpu0 driver merged. Exactly, and those changes would get merged in one form or the other. Surely in 3.17 atleast, if not 3.16 as we are already reaching rc4.. So, it would be great if you can test on top of those changes to see if something isn't solved for your platform yet? git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v3 -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Viresh Kumar, Thanks for your feedback! On Fri, 4 Jul 2014 15:25:35 +0530, Viresh Kumar wrote: > On 4 July 2014 15:14, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: > > This commit adds a simple cpufreq driver for the Armada XP SoC, which > > has one separately controllable clock for each CPU. For this reason, > > the existing cpufreq-cpu0 generic driver cannot be used because it > > currently assumes that there is only one clock controlling the > > frequency of all CPUs. > > > > There are on-going discussions on extending the cpufreq-cpu0 to cover > > the case of having one clock for each CPU, but there are still some > > unresolved issues to get this extended cpufreq-cpu0 driver merged. > > Exactly, and those changes would get merged in one form or the other. > Surely in 3.17 atleast, if not 3.16 as we are already reaching rc4.. > > So, it would be great if you can test on top of those changes to see if > something isn't solved for your platform yet? > > git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v3 One other problem with cpufreq-cpu0 is that it assumes the Device Tree can contain a static OPP table, with the frequency/voltage points. Unfortunately, in the case of the Armada XP platform, this is not possible, because we cannot hardcode into the Device Tree the possible CPU frequencies. The nominal CPU frequency is selected by Sample on Reset pins (i.e pins of the CPUs that are sampled during the reset sequence) and can therefore change from one board to the other, and they can also be changed via special U-Boot commands. And the frequencies supported by cpufreq are the nominal frequency of the CPU as determined by the sample at reset pins, and half of this frequency. That's why the proposed armadaxp-cpufreq driver does not hardcode the possible frequencies, but instead creates the frequency table by using the current CPU frequency (nominal frequency) and half of it. This doesn't work very well with the idea of having the OPP table statically encoded in to the Device Tree. Options are: - Improve the cpufreq-cpu0 driver so that the OPP table can be passed through platform_data, and therefore built dynamically by the platform code and passed when registering the cpufreq platform_device. - Dynamically build/update the OPP table in the Device Tree. Which option do you think is appropriate here? Thanks, Thomas
> This doesn't work very well with the idea of having the OPP table > statically encoded in to the Device Tree. Options are: > > - Improve the cpufreq-cpu0 driver so that the OPP table can be passed > through platform_data, and therefore built dynamically by the > platform code and passed when registering the cpufreq > platform_device. > > - Dynamically build/update the OPP table in the Device Tree. > > Which option do you think is appropriate here? It should also be noted that that applies to many Marvell SoCs. Kirkwood and Dove are the same. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Freitag, den 04.07.2014, 13:12 +0200 schrieb Thomas Petazzoni: [...] > This doesn't work very well with the idea of having the OPP table > statically encoded in to the Device Tree. Options are: > > - Improve the cpufreq-cpu0 driver so that the OPP table can be passed > through platform_data, and therefore built dynamically by the > platform code and passed when registering the cpufreq > platform_device. > > - Dynamically build/update the OPP table in the Device Tree. This sounds like the right thing to do. Ideally your bootloader would do this for you, so you don't have to encode those properties statically at all. Barebox already has facilities to fixup any loaded device tree. Have a look at of_register_fixup() there. Regards, Lucas
On Fri, Jul 04, 2014 at 03:52:14PM +0200, Lucas Stach wrote: > Am Freitag, den 04.07.2014, 13:12 +0200 schrieb Thomas Petazzoni: > [...] > > This doesn't work very well with the idea of having the OPP table > > statically encoded in to the Device Tree. Options are: > > > > - Improve the cpufreq-cpu0 driver so that the OPP table can be passed > > through platform_data, and therefore built dynamically by the > > platform code and passed when registering the cpufreq > > platform_device. > > > > - Dynamically build/update the OPP table in the Device Tree. > > This sounds like the right thing to do. Ideally your bootloader would do > this for you, so you don't have to encode those properties statically at > all. > > Barebox already has facilities to fixup any loaded device tree. Have a > look at of_register_fixup() there. We are talking about legacy devices here, which have been in the field for years. There is very little chance of a boot loader upgrade. Appended DT is the norm for these devices, and modifying the DT would have to happen in kernel. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4 July 2014 19:45, Andrew Lunn <andrew@lunn.ch> wrote: > We are talking about legacy devices here, which have been in the field > for years. There is very little chance of a boot loader > upgrade. Appended DT is the norm for these devices, and modifying the > DT would have to happen in kernel. FWIW, I should mention that we don't need to fix DT's at all in your case. Just add OPPs at runtime using dev_pm_opp_add() API from within kernel, maybe from your arch/arm/mach-* folder and cpufreq-cpu0 would be able to parse them. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Viresh Kumar, On Fri, 4 Jul 2014 15:25:35 +0530, Viresh Kumar wrote: > On 4 July 2014 15:14, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: > > This commit adds a simple cpufreq driver for the Armada XP SoC, which > > has one separately controllable clock for each CPU. For this reason, > > the existing cpufreq-cpu0 generic driver cannot be used because it > > currently assumes that there is only one clock controlling the > > frequency of all CPUs. > > > > There are on-going discussions on extending the cpufreq-cpu0 to cover > > the case of having one clock for each CPU, but there are still some > > unresolved issues to get this extended cpufreq-cpu0 driver merged. > > Exactly, and those changes would get merged in one form or the other. > Surely in 3.17 atleast, if not 3.16 as we are already reaching rc4.. > > So, it would be great if you can test on top of those changes to see if > something isn't solved for your platform yet? > > git://git.linaro.org/people/viresh.kumar/linux.git cpufreq/cpu0-krait-v3 Are these changes in linux-next ? The mvebu maintainer Jason Cooper has pulled the cpufreq support for Armada XP, so it will appear in linux-next tomorrow. It would be good of the cpufreq-generic driver improvements (to support one clock per CPU) were also part of linux-next so that we could test that the combination works as expected (of course, I did test locally, but I'd like to ensure it still works in linux-next). Could you get your tree in linux-next, or get your patches merged by Rafael in his linux-next branch so that they will appear in linux-next ? Thanks, Thomas
On 16 July 2014 19:32, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Could you get your tree in linux-next, or get your patches merged by > Rafael in his linux-next branch so that they will appear in linux-next ? They aren't in yet and Rafael haven't started pushing stuff for 3.16 yet. But it might be pushed sometime soon, no deadlines decided yet. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Viresh Kumar, On Wed, 16 Jul 2014 20:55:21 +0530, Viresh Kumar wrote: > On 16 July 2014 19:32, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: > > Could you get your tree in linux-next, or get your patches merged by > > Rafael in his linux-next branch so that they will appear in linux-next ? > > They aren't in yet and Rafael haven't started pushing stuff for 3.16 yet. I guess you meant 3.17, right? > But it might be pushed sometime soon, no deadlines decided yet. Would be good to have this soon, since the 3.17 merge window is approaching. You asked me to use this cpufreq-generic driver instead of a custom cpufreq driver, and I did the work to do that. So now I clearly hope that cpufreq-generic will be merged in 3.17. Thanks! Thomas
On 16 July 2014 20:58, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > I guess you meant 3.17, right? Yeah, sorry. > Would be good to have this soon, since the 3.17 merge window is > approaching. You asked me to use this cpufreq-generic driver instead of > a custom cpufreq driver, and I did the work to do that. So now I clearly > hope that cpufreq-generic will be merged in 3.17. Yeah, I have already pinged Rafael few days back on this. He came back from holidays last week and this is what he told me: Jul 11 19:58:34 <vireshk> rafael, when are you going to pick stuff for 3.17? Jul 11 19:59:11 <rafael> vireshk: next week i suppose Do you depend on this patch btw? https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg675398.html Actually there had been some objections to comparing clocks this way and we *might* hold this off, unless the bindings are finalized. Will get all other patches merged though. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Viresh Kumar, On Wed, 16 Jul 2014 21:02:29 +0530, Viresh Kumar wrote: > > Would be good to have this soon, since the 3.17 merge window is > > approaching. You asked me to use this cpufreq-generic driver instead of > > a custom cpufreq driver, and I did the work to do that. So now I clearly > > hope that cpufreq-generic will be merged in 3.17. > > Yeah, I have already pinged Rafael few days back on this. He came back > from holidays last week and this is what he told me: > > Jul 11 19:58:34 <vireshk> rafael, when are you going to pick stuff for 3.17? > Jul 11 19:59:11 <rafael> vireshk: next week i suppose Ok. > Do you depend on this patch btw? > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg675398.html > > Actually there had been some objections to comparing clocks this way and > we *might* hold this off, unless the bindings are finalized. Will get all other > patches merged though. I'm not sure to fully understand what this patch is doing, but I believe I depend on it. My system has one clock for each CPU, and therefore each CPU has a separately controllable frequency. See http://git.infradead.org/linux-mvebu.git/blob/3843607838cc5436d02a6771e661969a54c2fee0:/arch/arm/boot/dts/armada-xp-mv78460.dtsi#l30 for the Device Tree part describing the CPUs. So the existing cpufreq-cpu0 (as of 3.16) doesn't work because it assumes there is one single clock controlling the frequency. Best regards, Thomas
On 16 July 2014 21:17, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > I'm not sure to fully understand what this patch is doing, but I > believe I depend on it. Yes, you do. I have cc'd you to the other thread where this was in discussion. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index ebac671..825b273 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -24,6 +24,12 @@ config ARM_VEXPRESS_SPC_CPUFREQ This add the CPUfreq driver support for Versatile Express big.LITTLE platforms using SPC for power management. +config ARM_ARMADAXP_CPUFREQ + bool "Marvell Armada XP" + depends on MACH_ARMADA_XP + default y + help + This adds the CPUFreq driver for Marvell Armada XP SoC. config ARM_EXYNOS_CPUFREQ bool diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 738c8b7..e260a0c 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -49,6 +49,7 @@ obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ) += arm_big_little.o # LITTLE drivers, so that it is probed last. obj-$(CONFIG_ARM_DT_BL_CPUFREQ) += arm_big_little_dt.o +obj-$(CONFIG_ARM_ARMADAXP_CPUFREQ) += armadaxp-cpufreq.o obj-$(CONFIG_ARCH_DAVINCI_DA850) += davinci-cpufreq.o obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += exynos-cpufreq.o diff --git a/drivers/cpufreq/armadaxp-cpufreq.c b/drivers/cpufreq/armadaxp-cpufreq.c new file mode 100644 index 0000000..e59581f --- /dev/null +++ b/drivers/cpufreq/armadaxp-cpufreq.c @@ -0,0 +1,112 @@ +/* + * Copyright (C) 2014 Marvell + * + * Thomas Petazzoni <thomas.petazzoni@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public + * License version 2. This program is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/clk.h> +#include <linux/cpufreq.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> + +#define MAX_CPU_CLKS 4 + +struct priv { + struct clk *cpu_clks[MAX_CPU_CLKS]; +} priv; + +#define STATE_CPU_FREQ 0x01 +#define STATE_FABRIC_FREQ 0x02 + +static struct cpufreq_frequency_table armadaxp_freq_table[] = { + {0, STATE_CPU_FREQ, 0}, + {0, STATE_FABRIC_FREQ, 0}, + {0, 0, CPUFREQ_TABLE_END}, +}; + +static unsigned int armadaxp_cpufreq_get_cpu_frequency(unsigned int cpu) +{ + return clk_get_rate(priv.cpu_clks[cpu]) / 1000; +} + +static int armadaxp_cpufreq_target(struct cpufreq_policy *policy, + unsigned int index) +{ + clk_set_rate(priv.cpu_clks[policy->cpu], + armadaxp_freq_table[index].frequency * 1000); + return 0; +} + +static int armadaxp_cpufreq_cpu_init(struct cpufreq_policy *policy) +{ + if (!cpu_online(policy->cpu)) + return -ENODEV; + policy->cpuinfo.transition_latency = 5000; + return cpufreq_table_validate_and_show(policy, armadaxp_freq_table); +} + +static struct cpufreq_driver armadaxp_cpufreq_driver = { + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK, + .get = armadaxp_cpufreq_get_cpu_frequency, + .verify = cpufreq_generic_frequency_table_verify, + .target_index = armadaxp_cpufreq_target, + .init = armadaxp_cpufreq_cpu_init, + .name = "armadaxp-cpufreq", + .attr = cpufreq_generic_attr, +}; + +static int armadaxp_cpufreq_probe(struct platform_device *pdev) +{ + struct device_node *np; + unsigned long cpu_freq; + int i; + + for (i = 0; i < MAX_CPU_CLKS; i++) { + np = of_get_cpu_node(i, NULL); + if (!np) + continue; + + priv.cpu_clks[i] = of_clk_get(np, 0); + of_node_put(np); + } + + cpu_freq = clk_get_rate(priv.cpu_clks[0]) / 1000; + + /* + * The available frequencies are the nominal CPU frequency, + * and half of it, which is the fabric frequency. + */ + armadaxp_freq_table[0].frequency = cpu_freq; + armadaxp_freq_table[1].frequency = cpu_freq / 2; + + return cpufreq_register_driver(&armadaxp_cpufreq_driver); + +} + +static int armadaxp_cpufreq_remove(struct platform_device *pdev) +{ + cpufreq_unregister_driver(&armadaxp_cpufreq_driver); + return 0; +} + +static struct platform_driver armadaxp_cpufreq_platform_driver = { + .probe = armadaxp_cpufreq_probe, + .remove = armadaxp_cpufreq_remove, + .driver = { + .name = "armadaxp-cpufreq", + .owner = THIS_MODULE, + }, +}; + +module_platform_driver(armadaxp_cpufreq_platform_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Thomas Petazzoni <thomas.petazzoni@free-electrons.com"); +MODULE_DESCRIPTION("cpufreq driver for Marvell's Armada XP CPU"); +MODULE_ALIAS("platform:armadaxp-cpufreq");
This commit adds a simple cpufreq driver for the Armada XP SoC, which has one separately controllable clock for each CPU. For this reason, the existing cpufreq-cpu0 generic driver cannot be used because it currently assumes that there is only one clock controlling the frequency of all CPUs. There are on-going discussions on extending the cpufreq-cpu0 to cover the case of having one clock for each CPU, but there are still some unresolved issues to get this extended cpufreq-cpu0 driver merged. In the mean time, we propose to have an Armada XP specific cpufreq driver, and since the choice of the cpufreq driver does not affect the DT binding at all, it will always be time to get rid of this SoC-specific driver and use a more generic one when it becomes available. In terms of implementation, this cpufreq driver supports two frequencies: the nominal frequency of the CPU (which cannot be hardcoded, since it can be changed through Sample At Reset pins) and half of this frequency. The frequency change itself is implemented by simply calling clk_set_rate() on the appropriate CPU clock. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/cpufreq/Kconfig.arm | 6 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/armadaxp-cpufreq.c | 112 +++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 drivers/cpufreq/armadaxp-cpufreq.c