Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 9 August 2013 00:20, Stephen Warren wrote: > I don't think so. I think it's reasonable to have a per-SoC cpufreq > driver whose primary content is the parameterization data and/or custom > hooks for a unified core cpufreq driver. The duplicate parts of each > cpufreq driver can be moved into the core cpufreq driver, but the > non-duplicate parts remain. That's how many other subsystems work (MMC, > USB, ASoC spring to mind). Guys in the --to list: Please shout before its too late... I can understand why Stephen is asking not to implement a virtual clock driver for cpu as there is no clock corresponding to that.. We are just playing with existing clocks there.. But I thought these clock APIs can be considered as hooks that we were looking for and so shouldn't be a problem.. But yes, different people see things differently.. So, if I take Stephen's suggestions then I need to implement hooks into cpufreq-cpu0 driver for taking freq-table/ setting clk rates, etc... Let me know if anybody has a issue with that before we actually implement that.. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 08:42 PM, Viresh Kumar wrote: > On 8 August 2013 00:20, Stephen Warren wrote: >> Right, and that's *exactly* what having a cpufreq driver is for; to >> implement the details of CPU clock management. > > cpufreq drivers used to keep such information since a long time, > probably because there wasn't another place to keep them and > provide generic API's (like generic clock framework).. And so this > replication started to get in place which we are trying to get rid of > now. > > All cpufreq drivers share a lot of common code which can go > away and so cpufreq-cpu0 was introduced.. > > With this patchset this replication goes away for tegra atleast at > the cost of a platform specific clk-cpu driver.. I think that's a good > deal, isn't it? I think this patch series is simply moving the custom per-SoC code somewhere else (clock driver) so that the cpufreq drivers can be simpler. However, the clock drivers are more complex, and now represent concepts that aren't really clocks. So, no I'm not sure it's good. > And that's the only way you can use these generic drivers that we > have... I don't think so. I think it's reasonable to have a per-SoC cpufreq driver whose primary content is the parameterization data and/or custom hooks for a unified core cpufreq driver. The duplicate parts of each cpufreq driver can be moved into the core cpufreq driver, but the non-duplicate parts remain. That's how many other subsystems work (MMC, USB, ASoC spring to mind). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 08:42 PM, Viresh Kumar wrote: On 8 August 2013 00:20, Stephen Warren swar...@wwwdotorg.org wrote: Right, and that's *exactly* what having a cpufreq driver is for; to implement the details of CPU clock management. cpufreq drivers used to keep such information since a long time, probably because there wasn't another place to keep them and provide generic API's (like generic clock framework).. And so this replication started to get in place which we are trying to get rid of now. All cpufreq drivers share a lot of common code which can go away and so cpufreq-cpu0 was introduced.. With this patchset this replication goes away for tegra atleast at the cost of a platform specific clk-cpu driver.. I think that's a good deal, isn't it? I think this patch series is simply moving the custom per-SoC code somewhere else (clock driver) so that the cpufreq drivers can be simpler. However, the clock drivers are more complex, and now represent concepts that aren't really clocks. So, no I'm not sure it's good. And that's the only way you can use these generic drivers that we have... I don't think so. I think it's reasonable to have a per-SoC cpufreq driver whose primary content is the parameterization data and/or custom hooks for a unified core cpufreq driver. The duplicate parts of each cpufreq driver can be moved into the core cpufreq driver, but the non-duplicate parts remain. That's how many other subsystems work (MMC, USB, ASoC spring to mind). -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 9 August 2013 00:20, Stephen Warren swar...@wwwdotorg.org wrote: I don't think so. I think it's reasonable to have a per-SoC cpufreq driver whose primary content is the parameterization data and/or custom hooks for a unified core cpufreq driver. The duplicate parts of each cpufreq driver can be moved into the core cpufreq driver, but the non-duplicate parts remain. That's how many other subsystems work (MMC, USB, ASoC spring to mind). Guys in the --to list: Please shout before its too late... I can understand why Stephen is asking not to implement a virtual clock driver for cpu as there is no clock corresponding to that.. We are just playing with existing clocks there.. But I thought these clock APIs can be considered as hooks that we were looking for and so shouldn't be a problem.. But yes, different people see things differently.. So, if I take Stephen's suggestions then I need to implement hooks into cpufreq-cpu0 driver for taking freq-table/ setting clk rates, etc... Let me know if anybody has a issue with that before we actually implement that.. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 8 August 2013 00:20, Stephen Warren wrote: > Right, and that's *exactly* what having a cpufreq driver is for; to > implement the details of CPU clock management. cpufreq drivers used to keep such information since a long time, probably because there wasn't another place to keep them and provide generic API's (like generic clock framework).. And so this replication started to get in place which we are trying to get rid of now. All cpufreq drivers share a lot of common code which can go away and so cpufreq-cpu0 was introduced.. With this patchset this replication goes away for tegra atleast at the cost of a platform specific clk-cpu driver.. I think that's a good deal, isn't it? And that's the only way you can use these generic drivers that we have... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 11:54 AM, Viresh Kumar wrote: > On 7 August 2013 23:18, Stephen Warren wrote: >> On 08/07/2013 11:45 AM, Viresh Kumar wrote: >>> On 7 August 2013 23:08, Stephen Warren wrote: On 08/07/2013 08:46 AM, Viresh Kumar wrote: > This patch adds CPU0's clk driver for Tegra. It will be used by the > generic > cpufreq-cpu0 driver to get/set cpu clk. > > Most of the platform specific bits are picked from tegra-cpufreq.c. Hmmm. I'm not sure if it makes sense to represent this as a clock object; isn't this more of a virtual construct that manages the rate of the clock, rather than an actual clock? The actual clock already exists as "cpu". >>> >>> I see it as this: There is a clock in system for cpu, call it "cpu". Now we >>> must be able to provide get/set routines for it. A set should set the >>> frequency to whatever is asked for and should really worry about how >>> that is being set. This part is internal to "cpu" clk. >> >> Sure. >> >>> This is what cpufreq-cpu0 driver should expect and does. Current "cpu" >>> clock implemented doesn't provide this facility ? And so this wrapper >>> made sense to me. >> >> But the additional management logic on top of the raw clock is exactly >> what the cpufreq driver is for. This patch series is basically moving >> the cpufreq driver code inside the clock code instead. > > Above "sure" didn't go very well with what you wrote here :) > > The additional management that we are required to do isn't cpufreq > driver specific but cpu or platform specific. Right, and that's *exactly* what having a cpufreq driver is for; to implement the details of CPU clock management. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 7 August 2013 23:18, Stephen Warren wrote: > On 08/07/2013 11:45 AM, Viresh Kumar wrote: >> On 7 August 2013 23:08, Stephen Warren wrote: >>> On 08/07/2013 08:46 AM, Viresh Kumar wrote: This patch adds CPU0's clk driver for Tegra. It will be used by the generic cpufreq-cpu0 driver to get/set cpu clk. Most of the platform specific bits are picked from tegra-cpufreq.c. >>> >>> Hmmm. I'm not sure if it makes sense to represent this as a clock >>> object; isn't this more of a virtual construct that manages the rate of >>> the clock, rather than an actual clock? The actual clock already exists >>> as "cpu". >> >> I see it as this: There is a clock in system for cpu, call it "cpu". Now we >> must be able to provide get/set routines for it. A set should set the >> frequency to whatever is asked for and should really worry about how >> that is being set. This part is internal to "cpu" clk. > > Sure. > >> This is what cpufreq-cpu0 driver should expect and does. Current "cpu" >> clock implemented doesn't provide this facility ? And so this wrapper >> made sense to me. > > But the additional management logic on top of the raw clock is exactly > what the cpufreq driver is for. This patch series is basically moving > the cpufreq driver code inside the clock code instead. Above "sure" didn't go very well with what you wrote here :) The additional management that we are required to do isn't cpufreq driver specific but cpu or platform specific. cpufreq shouldn't care about how CPU's clock is set to a particular frequency, its headache of CPU's clk driver instead. cpu is yet another device and so clk_set_rate() must be enough to set its frequency There might be other frameworks that need to set frequency of this device later on and surely we don't want to replicate such piece of code to every user.. Does it make sense to you? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 11:45 AM, Viresh Kumar wrote: > On 7 August 2013 23:08, Stephen Warren wrote: >> On 08/07/2013 08:46 AM, Viresh Kumar wrote: >>> This patch adds CPU0's clk driver for Tegra. It will be used by the generic >>> cpufreq-cpu0 driver to get/set cpu clk. >>> >>> Most of the platform specific bits are picked from tegra-cpufreq.c. >> >> Hmmm. I'm not sure if it makes sense to represent this as a clock >> object; isn't this more of a virtual construct that manages the rate of >> the clock, rather than an actual clock? The actual clock already exists >> as "cpu". > > I see it as this: There is a clock in system for cpu, call it "cpu". Now we > must be able to provide get/set routines for it. A set should set the > frequency to whatever is asked for and should really worry about how > that is being set. This part is internal to "cpu" clk. Sure. > This is what cpufreq-cpu0 driver should expect and does. Current "cpu" > clock implemented doesn't provide this facility ? And so this wrapper > made sense to me. But the additional management logic on top of the raw clock is exactly what the cpufreq driver is for. This patch series is basically moving the cpufreq driver code inside the clock code instead. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 7 August 2013 23:08, Stephen Warren wrote: > On 08/07/2013 08:46 AM, Viresh Kumar wrote: >> This patch adds CPU0's clk driver for Tegra. It will be used by the generic >> cpufreq-cpu0 driver to get/set cpu clk. >> >> Most of the platform specific bits are picked from tegra-cpufreq.c. > > Hmmm. I'm not sure if it makes sense to represent this as a clock > object; isn't this more of a virtual construct that manages the rate of > the clock, rather than an actual clock? The actual clock already exists > as "cpu". I see it as this: There is a clock in system for cpu, call it "cpu". Now we must be able to provide get/set routines for it. A set should set the frequency to whatever is asked for and should really worry about how that is being set. This part is internal to "cpu" clk. This is what cpufreq-cpu0 driver should expect and does. Current "cpu" clock implemented doesn't provide this facility ? And so this wrapper made sense to me. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 08:46 AM, Viresh Kumar wrote: > This patch adds CPU0's clk driver for Tegra. It will be used by the generic > cpufreq-cpu0 driver to get/set cpu clk. > > Most of the platform specific bits are picked from tegra-cpufreq.c. Hmmm. I'm not sure if it makes sense to represent this as a clock object; isn't this more of a virtual construct that manages the rate of the clock, rather than an actual clock? The actual clock already exists as "cpu". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
Quoting Viresh Kumar (2013-08-07 07:46:43)
> This patch adds CPU0's clk driver for Tegra. It will be used by the generic
> cpufreq-cpu0 driver to get/set cpu clk.
>
> Most of the platform specific bits are picked from tegra-cpufreq.c.
>
> Signed-off-by: Viresh Kumar
Hi Viresh,
It is nice to see more CPUfreq consolidation.
I'm currently hacking on a patch to introduce clk_coordinate_rates().
That function may be a better fit for this sort of thing compared to
overloading the .set_rate callback. I'll try to get the patches on the
list ASAP and will Cc you.
Regards,
Mike
> ---
> drivers/clk/tegra/Makefile | 1 +
> drivers/clk/tegra/clk-cpu.c | 164
>
> drivers/clk/tegra/clk-tegra30.c | 4 +
> include/linux/clk/tegra.h | 1 +
> 4 files changed, 170 insertions(+)
> create mode 100644 drivers/clk/tegra/clk-cpu.c
>
> diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
> index f49fac2..0e818c0 100644
> --- a/drivers/clk/tegra/Makefile
> +++ b/drivers/clk/tegra/Makefile
> @@ -10,3 +10,4 @@ obj-y += clk-super.o
> obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += clk-tegra20.o
> obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra30.o
> obj-$(CONFIG_ARCH_TEGRA_114_SOC) += clk-tegra114.o
> +obj-$(CONFIG_GENERIC_CPUFREQ_CPU0) += clk-cpu.o
> diff --git a/drivers/clk/tegra/clk-cpu.c b/drivers/clk/tegra/clk-cpu.c
> new file mode 100644
> index 000..01716d6
> --- /dev/null
> +++ b/drivers/clk/tegra/clk-cpu.c
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (C) 2013 Linaro
> + *
> + * Author: Viresh Kumar
> + *
> + * 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.
> + */
> +
> +/*
> + * Responsible for setting cpu0 clk as requested by cpufreq-cpu0 driver
> + *
> + * All platform specific bits are taken from tegra-cpufreq driver.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include
> +#include
> +#include
> +
> +#define to_clk_cpu0(_hw) container_of(_hw, struct clk_cpu0, hw)
> +
> +struct clk_cpu0 {
> + struct clk_hw hw;
> + spinlock_t *lock;
> +};
> +
> +static struct clk *cpu_clk;
> +static struct clk *pll_x_clk;
> +static struct clk *pll_p_clk;
> +static struct clk *emc_clk;
> +
> +static unsigned long cpu0_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + return clk_get_rate(cpu_clk);
> +}
> +
> +static long cpu0_round_rate(struct clk_hw *hw, unsigned long drate,
> + unsigned long *parent_rate)
> +{
> + return clk_round_rate(cpu_clk, drate);
> +}
> +
> +static int cpu0_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + int ret;
> +
> + /*
> +* Vote on memory bus frequency based on cpu frequency
> +* This sets the minimum frequency, display or avp may request higher
> +*/
> + if (rate >= 81600)
> + clk_set_rate(emc_clk, 6); /* cpu 816 MHz, emc max */
> + else if (rate >= 45600)
> + clk_set_rate(emc_clk, 3); /* cpu 456 MHz, emc 150Mhz
> */
> + else
> + clk_set_rate(emc_clk, 1); /* emc 50Mhz */
> +
> + /*
> +* Take an extra reference to the main pll so it doesn't turn
> +* off when we move the cpu off of it
> +*/
> + clk_prepare_enable(pll_x_clk);
> +
> + ret = clk_set_parent(cpu_clk, pll_p_clk);
> + if (ret) {
> + pr_err("%s: Failed to switch cpu to clock pll_p\n", __func__);
> + goto out;
> + }
> +
> + if (rate == clk_get_rate(pll_p_clk))
> + goto out;
> +
> + ret = clk_set_rate(pll_x_clk, rate);
> + if (ret) {
> + pr_err("Failed to change pll_x to %lu\n", rate);
> + goto out;
> + }
> +
> + ret = clk_set_parent(cpu_clk, pll_x_clk);
> + if (ret) {
> + pr_err("Failed to switch cpu to clock pll_x\n");
> + goto out;
> + }
> +
> +out:
> + clk_disable_unprepare(pll_x_clk);
> + return ret;
> +}
> +
> +static struct clk_ops clk_cpu0_ops = {
> + .recalc_rate = cpu0_recalc_rate,
> + .round_rate = cpu0_round_rate,
> + .set_rate = cpu0_set_rate,
> +};
> +
> +struct clk *tegra_clk_register_cpu0(void)
> +{
> + struct clk_init_data init;
> + struct clk_cpu0 *cpu0;
> + struct clk *clk;
> +
> + cpu0 = kzalloc(sizeof(*cpu0), GFP_KERNEL);
> + if (!cpu0) {
> + pr_err("%s: could not allocate cpu0 clk\n", __func__);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + cpu_clk = clk_get_sys(NULL, "cpu");
> + if (IS_ERR(cpu_clk)) {
> + clk = cpu_clk;
> + goto free_mem;
> + }
> +
> +
[PATCH 1/6] clk: Tegra: Add CPU0 clock driver
This patch adds CPU0's clk driver for Tegra. It will be used by the generic
cpufreq-cpu0 driver to get/set cpu clk.
Most of the platform specific bits are picked from tegra-cpufreq.c.
Signed-off-by: Viresh Kumar
---
drivers/clk/tegra/Makefile | 1 +
drivers/clk/tegra/clk-cpu.c | 164
drivers/clk/tegra/clk-tegra30.c | 4 +
include/linux/clk/tegra.h | 1 +
4 files changed, 170 insertions(+)
create mode 100644 drivers/clk/tegra/clk-cpu.c
diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index f49fac2..0e818c0 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -10,3 +10,4 @@ obj-y += clk-super.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += clk-tegra20.o
obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra30.o
obj-$(CONFIG_ARCH_TEGRA_114_SOC) += clk-tegra114.o
+obj-$(CONFIG_GENERIC_CPUFREQ_CPU0) += clk-cpu.o
diff --git a/drivers/clk/tegra/clk-cpu.c b/drivers/clk/tegra/clk-cpu.c
new file mode 100644
index 000..01716d6
--- /dev/null
+++ b/drivers/clk/tegra/clk-cpu.c
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2013 Linaro
+ *
+ * Author: Viresh Kumar
+ *
+ * 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.
+ */
+
+/*
+ * Responsible for setting cpu0 clk as requested by cpufreq-cpu0 driver
+ *
+ * All platform specific bits are taken from tegra-cpufreq driver.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+
+#define to_clk_cpu0(_hw) container_of(_hw, struct clk_cpu0, hw)
+
+struct clk_cpu0 {
+ struct clk_hw hw;
+ spinlock_t *lock;
+};
+
+static struct clk *cpu_clk;
+static struct clk *pll_x_clk;
+static struct clk *pll_p_clk;
+static struct clk *emc_clk;
+
+static unsigned long cpu0_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return clk_get_rate(cpu_clk);
+}
+
+static long cpu0_round_rate(struct clk_hw *hw, unsigned long drate,
+ unsigned long *parent_rate)
+{
+ return clk_round_rate(cpu_clk, drate);
+}
+
+static int cpu0_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int ret;
+
+ /*
+* Vote on memory bus frequency based on cpu frequency
+* This sets the minimum frequency, display or avp may request higher
+*/
+ if (rate >= 81600)
+ clk_set_rate(emc_clk, 6); /* cpu 816 MHz, emc max */
+ else if (rate >= 45600)
+ clk_set_rate(emc_clk, 3); /* cpu 456 MHz, emc 150Mhz */
+ else
+ clk_set_rate(emc_clk, 1); /* emc 50Mhz */
+
+ /*
+* Take an extra reference to the main pll so it doesn't turn
+* off when we move the cpu off of it
+*/
+ clk_prepare_enable(pll_x_clk);
+
+ ret = clk_set_parent(cpu_clk, pll_p_clk);
+ if (ret) {
+ pr_err("%s: Failed to switch cpu to clock pll_p\n", __func__);
+ goto out;
+ }
+
+ if (rate == clk_get_rate(pll_p_clk))
+ goto out;
+
+ ret = clk_set_rate(pll_x_clk, rate);
+ if (ret) {
+ pr_err("Failed to change pll_x to %lu\n", rate);
+ goto out;
+ }
+
+ ret = clk_set_parent(cpu_clk, pll_x_clk);
+ if (ret) {
+ pr_err("Failed to switch cpu to clock pll_x\n");
+ goto out;
+ }
+
+out:
+ clk_disable_unprepare(pll_x_clk);
+ return ret;
+}
+
+static struct clk_ops clk_cpu0_ops = {
+ .recalc_rate = cpu0_recalc_rate,
+ .round_rate = cpu0_round_rate,
+ .set_rate = cpu0_set_rate,
+};
+
+struct clk *tegra_clk_register_cpu0(void)
+{
+ struct clk_init_data init;
+ struct clk_cpu0 *cpu0;
+ struct clk *clk;
+
+ cpu0 = kzalloc(sizeof(*cpu0), GFP_KERNEL);
+ if (!cpu0) {
+ pr_err("%s: could not allocate cpu0 clk\n", __func__);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ cpu_clk = clk_get_sys(NULL, "cpu");
+ if (IS_ERR(cpu_clk)) {
+ clk = cpu_clk;
+ goto free_mem;
+ }
+
+ pll_x_clk = clk_get_sys(NULL, "pll_x");
+ if (IS_ERR(pll_x_clk)) {
+ clk = pll_x_clk;
+ goto put_cpu_clk;
+ }
+
+ pll_p_clk = clk_get_sys(NULL, "pll_p_cclk");
+ if (IS_ERR(pll_p_clk)) {
+ clk = pll_p_clk;
+ goto put_pll_x_clk;
+ }
+
+ emc_clk = clk_get_sys("cpu", "emc");
+ if (IS_ERR(emc_clk)) {
+ clk = emc_clk;
+ goto put_pll_p_clk;
+ }
+
+ cpu0->hw.init =
+
+ init.name = "cpu0";
+ init.ops = _cpu0_ops;
+ init.flags = CLK_IS_ROOT | CLK_GET_RATE_NOCACHE;
+ init.num_parents = 0;
+
+ clk
[PATCH 1/6] clk: Tegra: Add CPU0 clock driver
This patch adds CPU0's clk driver for Tegra. It will be used by the generic
cpufreq-cpu0 driver to get/set cpu clk.
Most of the platform specific bits are picked from tegra-cpufreq.c.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
drivers/clk/tegra/Makefile | 1 +
drivers/clk/tegra/clk-cpu.c | 164
drivers/clk/tegra/clk-tegra30.c | 4 +
include/linux/clk/tegra.h | 1 +
4 files changed, 170 insertions(+)
create mode 100644 drivers/clk/tegra/clk-cpu.c
diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index f49fac2..0e818c0 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -10,3 +10,4 @@ obj-y += clk-super.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += clk-tegra20.o
obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra30.o
obj-$(CONFIG_ARCH_TEGRA_114_SOC) += clk-tegra114.o
+obj-$(CONFIG_GENERIC_CPUFREQ_CPU0) += clk-cpu.o
diff --git a/drivers/clk/tegra/clk-cpu.c b/drivers/clk/tegra/clk-cpu.c
new file mode 100644
index 000..01716d6
--- /dev/null
+++ b/drivers/clk/tegra/clk-cpu.c
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2013 Linaro
+ *
+ * Author: Viresh Kumar viresh.ku...@linaro.org
+ *
+ * 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.
+ */
+
+/*
+ * Responsible for setting cpu0 clk as requested by cpufreq-cpu0 driver
+ *
+ * All platform specific bits are taken from tegra-cpufreq driver.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
+#include linux/clk-provider.h
+#include linux/err.h
+#include linux/slab.h
+
+#define to_clk_cpu0(_hw) container_of(_hw, struct clk_cpu0, hw)
+
+struct clk_cpu0 {
+ struct clk_hw hw;
+ spinlock_t *lock;
+};
+
+static struct clk *cpu_clk;
+static struct clk *pll_x_clk;
+static struct clk *pll_p_clk;
+static struct clk *emc_clk;
+
+static unsigned long cpu0_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return clk_get_rate(cpu_clk);
+}
+
+static long cpu0_round_rate(struct clk_hw *hw, unsigned long drate,
+ unsigned long *parent_rate)
+{
+ return clk_round_rate(cpu_clk, drate);
+}
+
+static int cpu0_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int ret;
+
+ /*
+* Vote on memory bus frequency based on cpu frequency
+* This sets the minimum frequency, display or avp may request higher
+*/
+ if (rate = 81600)
+ clk_set_rate(emc_clk, 6); /* cpu 816 MHz, emc max */
+ else if (rate = 45600)
+ clk_set_rate(emc_clk, 3); /* cpu 456 MHz, emc 150Mhz */
+ else
+ clk_set_rate(emc_clk, 1); /* emc 50Mhz */
+
+ /*
+* Take an extra reference to the main pll so it doesn't turn
+* off when we move the cpu off of it
+*/
+ clk_prepare_enable(pll_x_clk);
+
+ ret = clk_set_parent(cpu_clk, pll_p_clk);
+ if (ret) {
+ pr_err(%s: Failed to switch cpu to clock pll_p\n, __func__);
+ goto out;
+ }
+
+ if (rate == clk_get_rate(pll_p_clk))
+ goto out;
+
+ ret = clk_set_rate(pll_x_clk, rate);
+ if (ret) {
+ pr_err(Failed to change pll_x to %lu\n, rate);
+ goto out;
+ }
+
+ ret = clk_set_parent(cpu_clk, pll_x_clk);
+ if (ret) {
+ pr_err(Failed to switch cpu to clock pll_x\n);
+ goto out;
+ }
+
+out:
+ clk_disable_unprepare(pll_x_clk);
+ return ret;
+}
+
+static struct clk_ops clk_cpu0_ops = {
+ .recalc_rate = cpu0_recalc_rate,
+ .round_rate = cpu0_round_rate,
+ .set_rate = cpu0_set_rate,
+};
+
+struct clk *tegra_clk_register_cpu0(void)
+{
+ struct clk_init_data init;
+ struct clk_cpu0 *cpu0;
+ struct clk *clk;
+
+ cpu0 = kzalloc(sizeof(*cpu0), GFP_KERNEL);
+ if (!cpu0) {
+ pr_err(%s: could not allocate cpu0 clk\n, __func__);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ cpu_clk = clk_get_sys(NULL, cpu);
+ if (IS_ERR(cpu_clk)) {
+ clk = cpu_clk;
+ goto free_mem;
+ }
+
+ pll_x_clk = clk_get_sys(NULL, pll_x);
+ if (IS_ERR(pll_x_clk)) {
+ clk = pll_x_clk;
+ goto put_cpu_clk;
+ }
+
+ pll_p_clk = clk_get_sys(NULL, pll_p_cclk);
+ if (IS_ERR(pll_p_clk)) {
+ clk = pll_p_clk;
+ goto put_pll_x_clk;
+ }
+
+ emc_clk = clk_get_sys(cpu, emc);
+ if (IS_ERR(emc_clk)) {
+ clk = emc_clk;
+ goto put_pll_p_clk;
+ }
+
+ cpu0-hw.init = init;
+
+ init.name = cpu0;
+ init.ops = clk_cpu0_ops;
+ init.flags =
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
Quoting Viresh Kumar (2013-08-07 07:46:43)
This patch adds CPU0's clk driver for Tegra. It will be used by the generic
cpufreq-cpu0 driver to get/set cpu clk.
Most of the platform specific bits are picked from tegra-cpufreq.c.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Hi Viresh,
It is nice to see more CPUfreq consolidation.
I'm currently hacking on a patch to introduce clk_coordinate_rates().
That function may be a better fit for this sort of thing compared to
overloading the .set_rate callback. I'll try to get the patches on the
list ASAP and will Cc you.
Regards,
Mike
---
drivers/clk/tegra/Makefile | 1 +
drivers/clk/tegra/clk-cpu.c | 164
drivers/clk/tegra/clk-tegra30.c | 4 +
include/linux/clk/tegra.h | 1 +
4 files changed, 170 insertions(+)
create mode 100644 drivers/clk/tegra/clk-cpu.c
diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
index f49fac2..0e818c0 100644
--- a/drivers/clk/tegra/Makefile
+++ b/drivers/clk/tegra/Makefile
@@ -10,3 +10,4 @@ obj-y += clk-super.o
obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += clk-tegra20.o
obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra30.o
obj-$(CONFIG_ARCH_TEGRA_114_SOC) += clk-tegra114.o
+obj-$(CONFIG_GENERIC_CPUFREQ_CPU0) += clk-cpu.o
diff --git a/drivers/clk/tegra/clk-cpu.c b/drivers/clk/tegra/clk-cpu.c
new file mode 100644
index 000..01716d6
--- /dev/null
+++ b/drivers/clk/tegra/clk-cpu.c
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2013 Linaro
+ *
+ * Author: Viresh Kumar viresh.ku...@linaro.org
+ *
+ * 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.
+ */
+
+/*
+ * Responsible for setting cpu0 clk as requested by cpufreq-cpu0 driver
+ *
+ * All platform specific bits are taken from tegra-cpufreq driver.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
+#include linux/clk-provider.h
+#include linux/err.h
+#include linux/slab.h
+
+#define to_clk_cpu0(_hw) container_of(_hw, struct clk_cpu0, hw)
+
+struct clk_cpu0 {
+ struct clk_hw hw;
+ spinlock_t *lock;
+};
+
+static struct clk *cpu_clk;
+static struct clk *pll_x_clk;
+static struct clk *pll_p_clk;
+static struct clk *emc_clk;
+
+static unsigned long cpu0_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return clk_get_rate(cpu_clk);
+}
+
+static long cpu0_round_rate(struct clk_hw *hw, unsigned long drate,
+ unsigned long *parent_rate)
+{
+ return clk_round_rate(cpu_clk, drate);
+}
+
+static int cpu0_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int ret;
+
+ /*
+* Vote on memory bus frequency based on cpu frequency
+* This sets the minimum frequency, display or avp may request higher
+*/
+ if (rate = 81600)
+ clk_set_rate(emc_clk, 6); /* cpu 816 MHz, emc max */
+ else if (rate = 45600)
+ clk_set_rate(emc_clk, 3); /* cpu 456 MHz, emc 150Mhz
*/
+ else
+ clk_set_rate(emc_clk, 1); /* emc 50Mhz */
+
+ /*
+* Take an extra reference to the main pll so it doesn't turn
+* off when we move the cpu off of it
+*/
+ clk_prepare_enable(pll_x_clk);
+
+ ret = clk_set_parent(cpu_clk, pll_p_clk);
+ if (ret) {
+ pr_err(%s: Failed to switch cpu to clock pll_p\n, __func__);
+ goto out;
+ }
+
+ if (rate == clk_get_rate(pll_p_clk))
+ goto out;
+
+ ret = clk_set_rate(pll_x_clk, rate);
+ if (ret) {
+ pr_err(Failed to change pll_x to %lu\n, rate);
+ goto out;
+ }
+
+ ret = clk_set_parent(cpu_clk, pll_x_clk);
+ if (ret) {
+ pr_err(Failed to switch cpu to clock pll_x\n);
+ goto out;
+ }
+
+out:
+ clk_disable_unprepare(pll_x_clk);
+ return ret;
+}
+
+static struct clk_ops clk_cpu0_ops = {
+ .recalc_rate = cpu0_recalc_rate,
+ .round_rate = cpu0_round_rate,
+ .set_rate = cpu0_set_rate,
+};
+
+struct clk *tegra_clk_register_cpu0(void)
+{
+ struct clk_init_data init;
+ struct clk_cpu0 *cpu0;
+ struct clk *clk;
+
+ cpu0 = kzalloc(sizeof(*cpu0), GFP_KERNEL);
+ if (!cpu0) {
+ pr_err(%s: could not allocate cpu0 clk\n, __func__);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ cpu_clk = clk_get_sys(NULL, cpu);
+ if (IS_ERR(cpu_clk)) {
+ clk = cpu_clk;
+ goto free_mem;
+ }
+
+ pll_x_clk = clk_get_sys(NULL, pll_x);
+ if (IS_ERR(pll_x_clk)) {
+
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 08:46 AM, Viresh Kumar wrote: This patch adds CPU0's clk driver for Tegra. It will be used by the generic cpufreq-cpu0 driver to get/set cpu clk. Most of the platform specific bits are picked from tegra-cpufreq.c. Hmmm. I'm not sure if it makes sense to represent this as a clock object; isn't this more of a virtual construct that manages the rate of the clock, rather than an actual clock? The actual clock already exists as cpu. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 7 August 2013 23:08, Stephen Warren swar...@wwwdotorg.org wrote: On 08/07/2013 08:46 AM, Viresh Kumar wrote: This patch adds CPU0's clk driver for Tegra. It will be used by the generic cpufreq-cpu0 driver to get/set cpu clk. Most of the platform specific bits are picked from tegra-cpufreq.c. Hmmm. I'm not sure if it makes sense to represent this as a clock object; isn't this more of a virtual construct that manages the rate of the clock, rather than an actual clock? The actual clock already exists as cpu. I see it as this: There is a clock in system for cpu, call it cpu. Now we must be able to provide get/set routines for it. A set should set the frequency to whatever is asked for and should really worry about how that is being set. This part is internal to cpu clk. This is what cpufreq-cpu0 driver should expect and does. Current cpu clock implemented doesn't provide this facility ? And so this wrapper made sense to me. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 11:45 AM, Viresh Kumar wrote: On 7 August 2013 23:08, Stephen Warren swar...@wwwdotorg.org wrote: On 08/07/2013 08:46 AM, Viresh Kumar wrote: This patch adds CPU0's clk driver for Tegra. It will be used by the generic cpufreq-cpu0 driver to get/set cpu clk. Most of the platform specific bits are picked from tegra-cpufreq.c. Hmmm. I'm not sure if it makes sense to represent this as a clock object; isn't this more of a virtual construct that manages the rate of the clock, rather than an actual clock? The actual clock already exists as cpu. I see it as this: There is a clock in system for cpu, call it cpu. Now we must be able to provide get/set routines for it. A set should set the frequency to whatever is asked for and should really worry about how that is being set. This part is internal to cpu clk. Sure. This is what cpufreq-cpu0 driver should expect and does. Current cpu clock implemented doesn't provide this facility ? And so this wrapper made sense to me. But the additional management logic on top of the raw clock is exactly what the cpufreq driver is for. This patch series is basically moving the cpufreq driver code inside the clock code instead. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 7 August 2013 23:18, Stephen Warren swar...@wwwdotorg.org wrote: On 08/07/2013 11:45 AM, Viresh Kumar wrote: On 7 August 2013 23:08, Stephen Warren swar...@wwwdotorg.org wrote: On 08/07/2013 08:46 AM, Viresh Kumar wrote: This patch adds CPU0's clk driver for Tegra. It will be used by the generic cpufreq-cpu0 driver to get/set cpu clk. Most of the platform specific bits are picked from tegra-cpufreq.c. Hmmm. I'm not sure if it makes sense to represent this as a clock object; isn't this more of a virtual construct that manages the rate of the clock, rather than an actual clock? The actual clock already exists as cpu. I see it as this: There is a clock in system for cpu, call it cpu. Now we must be able to provide get/set routines for it. A set should set the frequency to whatever is asked for and should really worry about how that is being set. This part is internal to cpu clk. Sure. This is what cpufreq-cpu0 driver should expect and does. Current cpu clock implemented doesn't provide this facility ? And so this wrapper made sense to me. But the additional management logic on top of the raw clock is exactly what the cpufreq driver is for. This patch series is basically moving the cpufreq driver code inside the clock code instead. Above sure didn't go very well with what you wrote here :) The additional management that we are required to do isn't cpufreq driver specific but cpu or platform specific. cpufreq shouldn't care about how CPU's clock is set to a particular frequency, its headache of CPU's clk driver instead. cpu is yet another device and so clk_set_rate() must be enough to set its frequency There might be other frameworks that need to set frequency of this device later on and surely we don't want to replicate such piece of code to every user.. Does it make sense to you? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 08/07/2013 11:54 AM, Viresh Kumar wrote: On 7 August 2013 23:18, Stephen Warren swar...@wwwdotorg.org wrote: On 08/07/2013 11:45 AM, Viresh Kumar wrote: On 7 August 2013 23:08, Stephen Warren swar...@wwwdotorg.org wrote: On 08/07/2013 08:46 AM, Viresh Kumar wrote: This patch adds CPU0's clk driver for Tegra. It will be used by the generic cpufreq-cpu0 driver to get/set cpu clk. Most of the platform specific bits are picked from tegra-cpufreq.c. Hmmm. I'm not sure if it makes sense to represent this as a clock object; isn't this more of a virtual construct that manages the rate of the clock, rather than an actual clock? The actual clock already exists as cpu. I see it as this: There is a clock in system for cpu, call it cpu. Now we must be able to provide get/set routines for it. A set should set the frequency to whatever is asked for and should really worry about how that is being set. This part is internal to cpu clk. Sure. This is what cpufreq-cpu0 driver should expect and does. Current cpu clock implemented doesn't provide this facility ? And so this wrapper made sense to me. But the additional management logic on top of the raw clock is exactly what the cpufreq driver is for. This patch series is basically moving the cpufreq driver code inside the clock code instead. Above sure didn't go very well with what you wrote here :) The additional management that we are required to do isn't cpufreq driver specific but cpu or platform specific. Right, and that's *exactly* what having a cpufreq driver is for; to implement the details of CPU clock management. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver
On 8 August 2013 00:20, Stephen Warren swar...@wwwdotorg.org wrote: Right, and that's *exactly* what having a cpufreq driver is for; to implement the details of CPU clock management. cpufreq drivers used to keep such information since a long time, probably because there wasn't another place to keep them and provide generic API's (like generic clock framework).. And so this replication started to get in place which we are trying to get rid of now. All cpufreq drivers share a lot of common code which can go away and so cpufreq-cpu0 was introduced.. With this patchset this replication goes away for tegra atleast at the cost of a platform specific clk-cpu driver.. I think that's a good deal, isn't it? And that's the only way you can use these generic drivers that we have... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
