Re: [PATCH 1/1] clk: tegra: fix WARN_ON in PLL_RE registration
On 05/16/2015 01:12 AM, Benson Leung wrote:
On Fri, May 15, 2015 at 5:07 AM, Bill Huang wrote:
This fixes two things.
- Read the correct IDDQ register
- Check the correct IDDQ bit position
Signed-off-by: Bill Huang
Reviewed-by: Benson Leung
By the way, does it also make sense to do the same thing for
tegra_clk_register_pllss, which also reads the base register instead
of the specific iddq_reg from params?
Yes thanks for catching this, I've sent another fix in
https://patchwork.ozlabs.org/patch/473329/
---
drivers/clk/tegra/clk-pll.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 05c6d08..734340e 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1630,7 +1630,8 @@ struct clk *tegra_clk_register_pllre(const char *name,
const char *parent_name,
val = pll_readl_base(pll);
if (val & PLL_BASE_ENABLE)
- WARN_ON(val & pll_params->iddq_bit_idx);
+ WARN_ON(readl_relaxed(clk_base + pll_params->iddq_reg) &
+ BIT(pll_params->iddq_bit_idx));
else {
int m;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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/1] clk: tegra: fix WARN_ON in PLL_RE registration
On 05/16/2015 01:12 AM, Benson Leung wrote:
On Fri, May 15, 2015 at 5:07 AM, Bill Huang bilhu...@nvidia.com wrote:
This fixes two things.
- Read the correct IDDQ register
- Check the correct IDDQ bit position
Signed-off-by: Bill Huang bilhu...@nvidia.com
Reviewed-by: Benson Leung ble...@chromium.org
By the way, does it also make sense to do the same thing for
tegra_clk_register_pllss, which also reads the base register instead
of the specific iddq_reg from params?
Yes thanks for catching this, I've sent another fix in
https://patchwork.ozlabs.org/patch/473329/
---
drivers/clk/tegra/clk-pll.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 05c6d08..734340e 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1630,7 +1630,8 @@ struct clk *tegra_clk_register_pllre(const char *name,
const char *parent_name,
val = pll_readl_base(pll);
if (val PLL_BASE_ENABLE)
- WARN_ON(val pll_params-iddq_bit_idx);
+ WARN_ON(readl_relaxed(clk_base + pll_params-iddq_reg)
+ BIT(pll_params-iddq_bit_idx));
else {
int m;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-tegra in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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 v5 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/23/2013 01:06 PM, Viresh Kumar wrote: Ccc'ing Grant and Rob as well. On 20 December 2013 21:59, Stephen Warren wrote: No, I definitely don't agree here. The rules for arch/arm64 are: no platform-specific code. We should immediately start planning for that. If this means renaming the file that creates the virtual device from tegra-cpufreq.c to something else, so be it, but we shouldn't go backwards and push stuff into the arch directories. I don't mind doing this now as well if it is generic enough. I wasn't sure if you guys wanted to take it on now.. @Bill: So, please create a separate commit for creating such file which would create a virtual device for probing cpufreq drivers with name picked from root-node. Compilation of such a file should be configurable but if it is compiled, then it shouldn't cause any problems if that device isn't used, for multiplatform kernels specially.. Probably then you can widen the scope of your patchset by modifying some of the existing drivers which require a device to get cpufreq driver probed. Currently they are all making such a device from their arch/ stuff. Actually, I don't have plan or resource on doing this, would it be better that you help to do that instead? Thanks. I am not sure about the location of such file. Should this be placed in DT code somewhere or kept in cpufreq? Rob/Grant ?? Do we have consensus on where to create such file? -- 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 v5 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/23/2013 01:06 PM, Viresh Kumar wrote: Ccc'ing Grant and Rob as well. On 20 December 2013 21:59, Stephen Warren swar...@wwwdotorg.org wrote: No, I definitely don't agree here. The rules for arch/arm64 are: no platform-specific code. We should immediately start planning for that. If this means renaming the file that creates the virtual device from tegra-cpufreq.c to something else, so be it, but we shouldn't go backwards and push stuff into the arch directories. I don't mind doing this now as well if it is generic enough. I wasn't sure if you guys wanted to take it on now.. @Bill: So, please create a separate commit for creating such file which would create a virtual device for probing cpufreq drivers with name picked from root-node. Compilation of such a file should be configurable but if it is compiled, then it shouldn't cause any problems if that device isn't used, for multiplatform kernels specially.. Probably then you can widen the scope of your patchset by modifying some of the existing drivers which require a device to get cpufreq driver probed. Currently they are all making such a device from their arch/ stuff. Actually, I don't have plan or resource on doing this, would it be better that you help to do that instead? Thanks. I am not sure about the location of such file. Should this be placed in DT code somewhere or kept in cpufreq? Rob/Grant ?? Do we have consensus on where to create such file? -- 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 v5 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/20/2013 06:33 PM, Viresh Kumar wrote: On 20 December 2013 15:55, bilhuang wrote: Don't you think it worth creating a file here so this can be shared to arm64? We will see how to handle virtual devices when we will start getting arm64 SoCs. Probably we might end up writing a single file in cpufreq, if required, that will create virtual devices for every arm64 platform.. So, some people might use it and others wouldn't.. But no platform specific files for such stuff. So, the best we can do for now is to move these to platform code as we are talking about arm32 SoC's for now which do have a mach-* directory.. OK thanks, this is suggested by Stephen earlier, I'll let him comment in case he might think otherwise. -- 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 v5 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/20/2013 05:31 PM, Viresh Kumar wrote:
On 19 December 2013 16:48, Bill Huang wrote:
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ce52ed9..22dfc43 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -225,6 +225,18 @@ config ARM_TEGRA_CPUFREQ
help
This adds the CPUFreq driver support for TEGRA SOCs.
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
Probably just in case you agree to my next comment:
depends on ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA_CPUFREQ.
+
+ If in doubt, say N.
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
/*
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
*
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
*/
#include
+#include
+#include
+#include
+
+int __init tegra_cpufreq_init(void)
+{
+ struct device_node *root;
+
+ root = of_find_node_by_path("/");
+ if (root) {
+ struct platform_device_info devinfo;
+ const char *compat;
+ int i;
+
+ memset(, 0, sizeof(devinfo));
+ i = of_property_count_strings(root, "compatible");
+ if (i > 0) {
+ of_property_read_string_index(
+ root, "compatible", i - 1, );
+ devinfo.name = kasprintf(
+ GFP_KERNEL, "%s-cpufreq", compat);
+ platform_device_register_full();
+ }
}
+EXPORT_SYMBOL(tegra_cpufreq_init);
Probably above is all that is present in this file. This is just about adding
the right cpufreq device so that right driver can get probed.
I don't think this code is present at the right place. We don't need a file
in cpufreq/ which is there just to add a device :) ..
Probably move this piece of code to arch/mach-tegra where you are calling
init.
And so remove ARM_TEGRA_CPUFREQ config option completely.
Don't you think it worth creating a file here so this can be shared to
arm64?
--
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 v5 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/20/2013 05:31 PM, Viresh Kumar wrote:
On 19 December 2013 16:48, Bill Huang bilhu...@nvidia.com wrote:
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ce52ed9..22dfc43 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -225,6 +225,18 @@ config ARM_TEGRA_CPUFREQ
help
This adds the CPUFreq driver support for TEGRA SOCs.
+config ARM_TEGRA20_CPUFREQ
+ bool NVIDIA TEGRA20
+ depends on ARM_TEGRA_CPUFREQ ARCH_TEGRA_2x_SOC
Probably just in case you agree to my next comment:
depends on ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA_CPUFREQ.
+
+ If in doubt, say N.
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
/*
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
*
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
*/
#include linux/kernel.h
+#include linux/platform_device.h
+#include linux/of.h
+#include linux/tegra-cpufreq.h
+
+int __init tegra_cpufreq_init(void)
+{
+ struct device_node *root;
+
+ root = of_find_node_by_path(/);
+ if (root) {
+ struct platform_device_info devinfo;
+ const char *compat;
+ int i;
+
+ memset(devinfo, 0, sizeof(devinfo));
+ i = of_property_count_strings(root, compatible);
+ if (i 0) {
+ of_property_read_string_index(
+ root, compatible, i - 1, compat);
+ devinfo.name = kasprintf(
+ GFP_KERNEL, %s-cpufreq, compat);
+ platform_device_register_full(devinfo);
+ }
}
+EXPORT_SYMBOL(tegra_cpufreq_init);
Probably above is all that is present in this file. This is just about adding
the right cpufreq device so that right driver can get probed.
I don't think this code is present at the right place. We don't need a file
in cpufreq/ which is there just to add a device :) ..
Probably move this piece of code to arch/mach-tegra where you are calling
init.
And so remove ARM_TEGRA_CPUFREQ config option completely.
Don't you think it worth creating a file here so this can be shared to
arm64?
--
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 v5 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/20/2013 06:33 PM, Viresh Kumar wrote: On 20 December 2013 15:55, bilhuang bilhu...@nvidia.com wrote: Don't you think it worth creating a file here so this can be shared to arm64? We will see how to handle virtual devices when we will start getting arm64 SoCs. Probably we might end up writing a single file in cpufreq, if required, that will create virtual devices for every arm64 platform.. So, some people might use it and others wouldn't.. But no platform specific files for such stuff. So, the best we can do for now is to move these to platform code as we are talking about arm32 SoC's for now which do have a mach-* directory.. OK thanks, this is suggested by Stephen earlier, I'll let him comment in case he might think otherwise. -- 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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/19/2013 01:29 PM, Viresh Kumar wrote: On 19 December 2013 10:56, bilhuang wrote: I'm not sure virtual regulator for CPU is a good idea, in addition to that, we don't have a single SoC OPP table, we need several which are speedo-id and process-id dependant, but generic cpufreq-cpu0 is assuming there is only one statically Can't that be handled via DT ? I don't think it can be handled via DT unless we separate DTB according to different CPU speedo/process-id but that is not a good idea. for some SoC the frequency table is not fixed, they are created at runtime combining our fast and slow CPU frequency table and dvfs table. So I'm really not sure is it worth adding so many tweaks in order to use the generic cpufreq-cpu0 driver. Hmm, maybe I got confused because I don't have a clear picture in my mind. It might be better to go ahead with your implementation for now and after everything is set, we can choose to use cpufreq-cpu0 if it is worth it. This makes more sense to me, thanks. -- 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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/18/2013 10:39 PM, Viresh Kumar wrote: On 18 December 2013 17:03, bilhuang wrote: cpufreq-cpu0 driver will call regulator_set_voltage_tol() directly according to the pre-defined OPP freq/volt pairs, the regulator drivers could be shared by other SoC so is not suitable to handle this, or do I misunderstand? In case regulator's driver is shared, then you can probably add another virtual regulator for CPU which would have the special code you want. I'm not sure virtual regulator for CPU is a good idea, in addition to that, we don't have a single SoC OPP table, we need several which are speedo-id and process-id dependant, but generic cpufreq-cpu0 is assuming there is only one statically, for some SoC the frequency table is not fixed, they are created at runtime combining our fast and slow CPU frequency table and dvfs table. So I'm really not sure is it worth adding so many tweaks in order to use the generic cpufreq-cpu0 driver. -- 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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/18/2013 07:11 PM, Viresh Kumar wrote: On 17 December 2013 16:22, bilhuang wrote: Tegra20 DVFS is a little bit complicated due to the fact that we can't scale VDD_CPU directly, there are constraints or relationship to other power rails so I don't think it is a good idea to use generic cpufreq-cpu0 driver if we're going to support voltage scaling. But why can't we handle that in a CPU specific regulator code? cpufreq-cpu0 driver will call regulator_set_voltage_tol() directly according to the pre-defined OPP freq/volt pairs, the regulator drivers could be shared by other SoC so is not suitable to handle this, or do I misunderstand? -- 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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/18/2013 07:11 PM, Viresh Kumar wrote: On 17 December 2013 16:22, bilhuang bilhu...@nvidia.com wrote: Tegra20 DVFS is a little bit complicated due to the fact that we can't scale VDD_CPU directly, there are constraints or relationship to other power rails so I don't think it is a good idea to use generic cpufreq-cpu0 driver if we're going to support voltage scaling. But why can't we handle that in a CPU specific regulator code? cpufreq-cpu0 driver will call regulator_set_voltage_tol() directly according to the pre-defined OPP freq/volt pairs, the regulator drivers could be shared by other SoC so is not suitable to handle this, or do I misunderstand? -- 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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/18/2013 10:39 PM, Viresh Kumar wrote: On 18 December 2013 17:03, bilhuang bilhu...@nvidia.com wrote: cpufreq-cpu0 driver will call regulator_set_voltage_tol() directly according to the pre-defined OPP freq/volt pairs, the regulator drivers could be shared by other SoC so is not suitable to handle this, or do I misunderstand? In case regulator's driver is shared, then you can probably add another virtual regulator for CPU which would have the special code you want. I'm not sure virtual regulator for CPU is a good idea, in addition to that, we don't have a single SoC OPP table, we need several which are speedo-id and process-id dependant, but generic cpufreq-cpu0 is assuming there is only one statically, for some SoC the frequency table is not fixed, they are created at runtime combining our fast and slow CPU frequency table and dvfs table. So I'm really not sure is it worth adding so many tweaks in order to use the generic cpufreq-cpu0 driver. -- 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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/19/2013 01:29 PM, Viresh Kumar wrote: On 19 December 2013 10:56, bilhuang bilhu...@nvidia.com wrote: I'm not sure virtual regulator for CPU is a good idea, in addition to that, we don't have a single SoC OPP table, we need several which are speedo-id and process-id dependant, but generic cpufreq-cpu0 is assuming there is only one statically Can't that be handled via DT ? I don't think it can be handled via DT unless we separate DTB according to different CPU speedo/process-id but that is not a good idea. for some SoC the frequency table is not fixed, they are created at runtime combining our fast and slow CPU frequency table and dvfs table. So I'm really not sure is it worth adding so many tweaks in order to use the generic cpufreq-cpu0 driver. Hmm, maybe I got confused because I don't have a clear picture in my mind. It might be better to go ahead with your implementation for now and after everything is set, we can choose to use cpufreq-cpu0 if it is worth it. This makes more sense to me, thanks. -- 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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/17/2013 02:54 PM, Viresh Kumar wrote:
On 5 December 2013 13:14, Bill Huang wrote:
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
I strongly feel we must reuse cpufreq-cpu0 driver here after adding a
clk/regulator driver for tegra to support all that.
Tegra20 DVFS is a little bit complicated due to the fact that we can't
scale VDD_CPU directly, there are constraints or relationship to other
power rails so I don't think it is a good idea to use generic
cpufreq-cpu0 driver if we're going to support voltage scaling.
@Stephen: If you want we can keep all that tegra specific stuff
(clk/regulator) in
tegra-cpufreq.c, but we can easily use cpufreq-cpu0 driver without much
complications..
I have tried it earlier, got some comments and then got busy in other stuff..
https://lkml.org/lkml/2013/8/7/364
static int tegra_cpu_exit(struct cpufreq_policy *policy)
{
- clk_disable_unprepare(cpu_clk);
- clk_disable_unprepare(emc_clk);
+ cpufreq_frequency_table_cpuinfo(policy, tegra_data->freq_table);
Btw, why do you need this here?
Actually the latest version is v4 which is quite different against v3.
--
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 v3 1/2] cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code
On 12/17/2013 02:31 PM, Viresh Kumar wrote:
On 5 December 2013 13:14, Bill Huang wrote:
Move the call from module_init to Tegra machine codes so it won't be
called in a multi-platform kernel running on non-Tegra SoCs.
Signed-off-by: Bill Huang
---
arch/arm/mach-tegra/tegra.c |2 ++
drivers/cpufreq/tegra-cpufreq.c | 13 ++---
include/linux/tegra-soc.h | 11 ++-
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 7336817..14490ad 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -160,6 +161,7 @@ static void __init tegra_dt_init_late(void)
{
int i;
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index 63f0059..ae1c0f1 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -155,7 +155,7 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
#endif
};
Remove module.h as well ??
-static int __init tegra_cpufreq_init(void)
+int __init tegra_cpufreq_init(void)
{
cpu_clk = clk_get_sys(NULL, "cclk");
if (IS_ERR(cpu_clk))
@@ -177,17 +177,8 @@ static int __init tegra_cpufreq_init(void)
return cpufreq_register_driver(_cpufreq_driver);
}
-
-static void __exit tegra_cpufreq_exit(void)
-{
-cpufreq_unregister_driver(_cpufreq_driver);
- clk_put(emc_clk);
- clk_put(cpu_clk);
-}
-
+EXPORT_SYMBOL(tegra_cpufreq_init);
MODULE_AUTHOR("Colin Cross ");
MODULE_DESCRIPTION("cpufreq driver for Nvidia Tegra2");
MODULE_LICENSE("GPL");
Remove these as well? As they don't have a meaning for Tegra
cpufreq driver which can't be compiled as module.
OK thanks.
-module_init(tegra_cpufreq_init);
-module_exit(tegra_cpufreq_exit);
Rest as what Stephen suggested.
--
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 v3 0/2] Remodel Tegra cpufreq drivers to support Tegra series SoC
On 12/17/2013 02:26 PM, Viresh Kumar wrote: On 5 December 2013 13:14, Bill Huang wrote: This patch series remodel Tegra cpufreq driver to make it more easy to add new SoC support, in addition to that, adding probe function in the driver to let probe defer can be used to control init sequence when we are going to support DVFS. Changes since v2: - Fix Kconfig. - Rebase on top of branch 'cpufreq-next' on git://git.linaro.org/people/vireshk/linux.git. That's wrong base.. This branch is used only for Rafael to pull in stuff and so it doesn't get updated everyday.. You must rebase on Rafael's linux-next branch all the time. OK thanks, is it branch pm-cpufreq? -- 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 v4 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/17/2013 12:58 AM, Stephen Warren wrote:
On 12/16/2013 03:52 AM, bilhuang wrote:
On 12/14/2013 07:21 AM, Stephen Warren wrote:
On 12/12/2013 02:33 AM, Bill Huang wrote:
Re-model Tegra20 cpufreq driver as below.
* Rename tegra-cpufreq.c to tegra20-cpufreq.c since this file supports
only Tegra20.
* Add probe function so defer probe can be used when we're going to
support DVFS.
* Create a fake cpufreq platform device with its name being
"${root_compatible}-cpufreq" so SoC cpufreq driver can bind to it
accordingly.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
+config ARM_TEGRA20_CPUFREQ
+bool "NVIDIA TEGRA20"
+depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
+default y
+help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA20_CPUFREQ.
I think that last sentence is no longer true in this patch version. Or,
did you mean to write ARM_TEGRA_CPUFREQ rather than ARM_TEGRA20_CPUFREQ?
Right, should be ARM_TEGRA_CPUFREQ, thanks for catching this.
diff --git a/drivers/cpufreq/tegra-cpufreq.c
b/drivers/cpufreq/tegra-cpufreq.c
+static const char * const tegra_soc_compat[] = {
+"nvidia,tegra124",
+"nvidia,tegra114",
+"nvidia,tegra30",
+"nvidia,tegra20",
+NULL
};
That table will need editing for each chip. I wonder if you can do
something like always use the very last entry in /compatible. That would
assume a particular ordering of the compatible entries, but they should
be in the order $board, $soc anyway...
How do we get subset of a string and making sure it is the last? There
must be some assumptions here (though they will possibly be true) I
guess, for example, they should be in the order $board, $soc... and the
last "nvidia" should be the start of the last compatible id we would
like to get.
The compatible property is an array of strings, rather than one big
string, so you can just keep getting index 0, 1, 2, ... until you find
there aren't any more entries. I think it's reasonable to assume that
the SoC compatible value must be last, and it should be in practice,
although perhaps that is fragile. If you want, we can leave this as-is
for now, and modify it later if it becomes a problem.
Right, thanks.
--
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 v3 1/2] cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code
On 12/17/2013 02:31 PM, Viresh Kumar wrote:
On 5 December 2013 13:14, Bill Huang bilhu...@nvidia.com wrote:
Move the call from module_init to Tegra machine codes so it won't be
called in a multi-platform kernel running on non-Tegra SoCs.
Signed-off-by: Bill Huang bilhu...@nvidia.com
---
arch/arm/mach-tegra/tegra.c |2 ++
drivers/cpufreq/tegra-cpufreq.c | 13 ++---
include/linux/tegra-soc.h | 11 ++-
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 7336817..14490ad 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -34,6 +34,7 @@
#include linux/usb/tegra_usb_phy.h
#include linux/clk/tegra.h
#include linux/irqchip.h
+#include linux/tegra-soc.h
#include asm/hardware/cache-l2x0.h
#include asm/mach-types.h
@@ -160,6 +161,7 @@ static void __init tegra_dt_init_late(void)
{
int i;
+ tegra_cpufreq_init();
tegra_init_suspend();
tegra_cpuidle_init();
tegra_powergate_debugfs_init();
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
index 63f0059..ae1c0f1 100644
--- a/drivers/cpufreq/tegra-cpufreq.c
+++ b/drivers/cpufreq/tegra-cpufreq.c
@@ -155,7 +155,7 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
#endif
};
Remove module.h as well ??
-static int __init tegra_cpufreq_init(void)
+int __init tegra_cpufreq_init(void)
{
cpu_clk = clk_get_sys(NULL, cclk);
if (IS_ERR(cpu_clk))
@@ -177,17 +177,8 @@ static int __init tegra_cpufreq_init(void)
return cpufreq_register_driver(tegra_cpufreq_driver);
}
-
-static void __exit tegra_cpufreq_exit(void)
-{
-cpufreq_unregister_driver(tegra_cpufreq_driver);
- clk_put(emc_clk);
- clk_put(cpu_clk);
-}
-
+EXPORT_SYMBOL(tegra_cpufreq_init);
MODULE_AUTHOR(Colin Cross ccr...@android.com);
MODULE_DESCRIPTION(cpufreq driver for Nvidia Tegra2);
MODULE_LICENSE(GPL);
Remove these as well? As they don't have a meaning for Tegra
cpufreq driver which can't be compiled as module.
OK thanks.
-module_init(tegra_cpufreq_init);
-module_exit(tegra_cpufreq_exit);
Rest as what Stephen suggested.
--
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 v3 0/2] Remodel Tegra cpufreq drivers to support Tegra series SoC
On 12/17/2013 02:26 PM, Viresh Kumar wrote: On 5 December 2013 13:14, Bill Huang bilhu...@nvidia.com wrote: This patch series remodel Tegra cpufreq driver to make it more easy to add new SoC support, in addition to that, adding probe function in the driver to let probe defer can be used to control init sequence when we are going to support DVFS. Changes since v2: - Fix Kconfig. - Rebase on top of branch 'cpufreq-next' on git://git.linaro.org/people/vireshk/linux.git. That's wrong base.. This branch is used only for Rafael to pull in stuff and so it doesn't get updated everyday.. You must rebase on Rafael's linux-next branch all the time. OK thanks, is it branch pm-cpufreq? -- 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 v4 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/17/2013 12:58 AM, Stephen Warren wrote:
On 12/16/2013 03:52 AM, bilhuang wrote:
On 12/14/2013 07:21 AM, Stephen Warren wrote:
On 12/12/2013 02:33 AM, Bill Huang wrote:
Re-model Tegra20 cpufreq driver as below.
* Rename tegra-cpufreq.c to tegra20-cpufreq.c since this file supports
only Tegra20.
* Add probe function so defer probe can be used when we're going to
support DVFS.
* Create a fake cpufreq platform device with its name being
${root_compatible}-cpufreq so SoC cpufreq driver can bind to it
accordingly.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
+config ARM_TEGRA20_CPUFREQ
+bool NVIDIA TEGRA20
+depends on ARM_TEGRA_CPUFREQ ARCH_TEGRA_2x_SOC
+default y
+help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA20_CPUFREQ.
I think that last sentence is no longer true in this patch version. Or,
did you mean to write ARM_TEGRA_CPUFREQ rather than ARM_TEGRA20_CPUFREQ?
Right, should be ARM_TEGRA_CPUFREQ, thanks for catching this.
diff --git a/drivers/cpufreq/tegra-cpufreq.c
b/drivers/cpufreq/tegra-cpufreq.c
+static const char * const tegra_soc_compat[] = {
+nvidia,tegra124,
+nvidia,tegra114,
+nvidia,tegra30,
+nvidia,tegra20,
+NULL
};
That table will need editing for each chip. I wonder if you can do
something like always use the very last entry in /compatible. That would
assume a particular ordering of the compatible entries, but they should
be in the order $board, $soc anyway...
How do we get subset of a string and making sure it is the last? There
must be some assumptions here (though they will possibly be true) I
guess, for example, they should be in the order $board, $soc... and the
last nvidia should be the start of the last compatible id we would
like to get.
The compatible property is an array of strings, rather than one big
string, so you can just keep getting index 0, 1, 2, ... until you find
there aren't any more entries. I think it's reasonable to assume that
the SoC compatible value must be last, and it should be in practice,
although perhaps that is fragile. If you want, we can leave this as-is
for now, and modify it later if it becomes a problem.
Right, thanks.
--
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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/17/2013 02:54 PM, Viresh Kumar wrote:
On 5 December 2013 13:14, Bill Huang bilhu...@nvidia.com wrote:
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
I strongly feel we must reuse cpufreq-cpu0 driver here after adding a
clk/regulator driver for tegra to support all that.
Tegra20 DVFS is a little bit complicated due to the fact that we can't
scale VDD_CPU directly, there are constraints or relationship to other
power rails so I don't think it is a good idea to use generic
cpufreq-cpu0 driver if we're going to support voltage scaling.
@Stephen: If you want we can keep all that tegra specific stuff
(clk/regulator) in
tegra-cpufreq.c, but we can easily use cpufreq-cpu0 driver without much
complications..
I have tried it earlier, got some comments and then got busy in other stuff..
https://lkml.org/lkml/2013/8/7/364
static int tegra_cpu_exit(struct cpufreq_policy *policy)
{
- clk_disable_unprepare(cpu_clk);
- clk_disable_unprepare(emc_clk);
+ cpufreq_frequency_table_cpuinfo(policy, tegra_data-freq_table);
Btw, why do you need this here?
Actually the latest version is v4 which is quite different against v3.
--
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 v4 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/14/2013 07:21 AM, Stephen Warren wrote:
On 12/12/2013 02:33 AM, Bill Huang wrote:
Re-model Tegra20 cpufreq driver as below.
* Rename tegra-cpufreq.c to tegra20-cpufreq.c since this file supports
only Tegra20.
* Add probe function so defer probe can be used when we're going to
support DVFS.
* Create a fake cpufreq platform device with its name being
"${root_compatible}-cpufreq" so SoC cpufreq driver can bind to it
accordingly.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
+config ARM_TEGRA20_CPUFREQ
+ bool "NVIDIA TEGRA20"
+ depends on ARM_TEGRA_CPUFREQ && ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA20_CPUFREQ.
I think that last sentence is no longer true in this patch version. Or,
did you mean to write ARM_TEGRA_CPUFREQ rather than ARM_TEGRA20_CPUFREQ?
Right, should be ARM_TEGRA_CPUFREQ, thanks for catching this.
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
+static const char * const tegra_soc_compat[] = {
+ "nvidia,tegra124",
+ "nvidia,tegra114",
+ "nvidia,tegra30",
+ "nvidia,tegra20",
+ NULL
};
That table will need editing for each chip. I wonder if you can do
something like always use the very last entry in /compatible. That would
assume a particular ordering of the compatible entries, but they should
be in the order $board, $soc anyway...
How do we get subset of a string and making sure it is the last? There
must be some assumptions here (though they will possibly be true) I
guess, for example, they should be in the order $board, $soc... and the
last "nvidia" should be the start of the last compatible id we would
like to get.
+int __init tegra_cpufreq_init(void)
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(tegra_soc_compat); i++) {
+ if (of_machine_is_compatible(tegra_soc_compat[i])) {
+ struct platform_device_info devinfo;
+ char buf[40];
+
+ memset(, 0, sizeof(devinfo));
+ strcpy(buf, tegra_soc_compat[i]);
+ strcat(buf, "-cpufreq");
kasprintf() might be simpler, and would avoid the arbitrary 39-character
string limit and possibility of overflow.
Ah yeah, thanks.
+ devinfo.name = buf;
+ platform_device_register_full();
Does the devinfo struct need to stick around, i.e. does
platform_device_register_full keep the pointer, or take a copy of the
struct? If it keeps the pointer, it'd be best to make devinfo a static
global variable.
devinfo is used to provide dev info to create platform device structure,
so I think it is OK here.
diff --git a/drivers/cpufreq/tegra20-cpufreq.c
b/drivers/cpufreq/tegra20-cpufreq.c
Please pass the "-C" option to "git format-patch"; I assume that almost
all the code in this file is simply cut/paste verbatim from
tegra-cpufreq.c where it was deleted.
OK, thanks.
+ * Copyright (C) 2010 Google, Inc.
It's worth adding NV (c) here too.
OK.
+static int tegra20_cpufreq_remove(struct platform_device *pdev)
+{
+ cpufreq_unregister_driver(_cpufreq_driver);
+ return 0;
+}
That leaks all the clk_get_sys() calls. Does building this as a module
work OK?
I should add back those clk_put here.
+MODULE_LICENSE("GPL");
That should be "GPL v2".
OK.
diff --git a/include/linux/tegra-cpufreq.h b/include/linux/tegra-cpufreq.h
+#ifdef CONFIG_ARM_TEGRA_CPUFREQ
+int tegra_cpufreq_init(void);
+#else
+static inline int tegra_cpufreq_init(void)
+{ return; }
+#endif
If you're going to wrap the { } onto one line, then I think it'd be best
to wrap the whole thing (prototype and body) onto one line. Otherwise,
write:
{
return;
}
Oh, and you need "return 0" not just "return".
Thanks for catching.
--
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 v4 1/1] cpufreq: tegra: Re-model Tegra20 cpufreq driver
On 12/14/2013 07:21 AM, Stephen Warren wrote:
On 12/12/2013 02:33 AM, Bill Huang wrote:
Re-model Tegra20 cpufreq driver as below.
* Rename tegra-cpufreq.c to tegra20-cpufreq.c since this file supports
only Tegra20.
* Add probe function so defer probe can be used when we're going to
support DVFS.
* Create a fake cpufreq platform device with its name being
${root_compatible}-cpufreq so SoC cpufreq driver can bind to it
accordingly.
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
+config ARM_TEGRA20_CPUFREQ
+ bool NVIDIA TEGRA20
+ depends on ARM_TEGRA_CPUFREQ ARCH_TEGRA_2x_SOC
+ default y
+ help
+ This enables Tegra20 cpufreq functionality, it adds
+ Tegra20 CPU frequency ladder and the call back functions
+ to set CPU rate. All the non-SoC dependant codes are
+ controlled by the config ARM_TEGRA20_CPUFREQ.
I think that last sentence is no longer true in this patch version. Or,
did you mean to write ARM_TEGRA_CPUFREQ rather than ARM_TEGRA20_CPUFREQ?
Right, should be ARM_TEGRA_CPUFREQ, thanks for catching this.
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
+static const char * const tegra_soc_compat[] = {
+ nvidia,tegra124,
+ nvidia,tegra114,
+ nvidia,tegra30,
+ nvidia,tegra20,
+ NULL
};
That table will need editing for each chip. I wonder if you can do
something like always use the very last entry in /compatible. That would
assume a particular ordering of the compatible entries, but they should
be in the order $board, $soc anyway...
How do we get subset of a string and making sure it is the last? There
must be some assumptions here (though they will possibly be true) I
guess, for example, they should be in the order $board, $soc... and the
last nvidia should be the start of the last compatible id we would
like to get.
+int __init tegra_cpufreq_init(void)
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(tegra_soc_compat); i++) {
+ if (of_machine_is_compatible(tegra_soc_compat[i])) {
+ struct platform_device_info devinfo;
+ char buf[40];
+
+ memset(devinfo, 0, sizeof(devinfo));
+ strcpy(buf, tegra_soc_compat[i]);
+ strcat(buf, -cpufreq);
kasprintf() might be simpler, and would avoid the arbitrary 39-character
string limit and possibility of overflow.
Ah yeah, thanks.
+ devinfo.name = buf;
+ platform_device_register_full(devinfo);
Does the devinfo struct need to stick around, i.e. does
platform_device_register_full keep the pointer, or take a copy of the
struct? If it keeps the pointer, it'd be best to make devinfo a static
global variable.
devinfo is used to provide dev info to create platform device structure,
so I think it is OK here.
diff --git a/drivers/cpufreq/tegra20-cpufreq.c
b/drivers/cpufreq/tegra20-cpufreq.c
Please pass the -C option to git format-patch; I assume that almost
all the code in this file is simply cut/paste verbatim from
tegra-cpufreq.c where it was deleted.
OK, thanks.
+ * Copyright (C) 2010 Google, Inc.
It's worth adding NV (c) here too.
OK.
+static int tegra20_cpufreq_remove(struct platform_device *pdev)
+{
+ cpufreq_unregister_driver(tegra20_cpufreq_driver);
+ return 0;
+}
That leaks all the clk_get_sys() calls. Does building this as a module
work OK?
I should add back those clk_put here.
+MODULE_LICENSE(GPL);
That should be GPL v2.
OK.
diff --git a/include/linux/tegra-cpufreq.h b/include/linux/tegra-cpufreq.h
+#ifdef CONFIG_ARM_TEGRA_CPUFREQ
+int tegra_cpufreq_init(void);
+#else
+static inline int tegra_cpufreq_init(void)
+{ return; }
+#endif
If you're going to wrap the { } onto one line, then I think it'd be best
to wrap the whole thing (prototype and body) onto one line. Otherwise,
write:
{
return;
}
Oh, and you need return 0 not just return.
Thanks for catching.
--
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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/10/2013 01:32 AM, Stephen Warren wrote:
On 12/09/2013 01:44 AM, bilhuang wrote:
On 12/06/2013 07:04 AM, Stephen Warren wrote:
On 12/05/2013 12:44 AM, Bill Huang wrote:
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
diff --git a/drivers/cpufreq/tegra-cpufreq.c
b/drivers/cpufreq/tegra-cpufreq.c
@@ -91,14 +40,10 @@ static int tegra_update_cpu_speed(struct
cpufreq_policy *policy,
...
+if (soc_config->vote_emc_on_cpu_rate)
+soc_config->vote_emc_on_cpu_rate(rate);
+
+ret = soc_config->cpu_clk_set_rate(rate * 1000);
if (ret)
pr_err("cpu-tegra: Failed to set cpu frequency to %lu kHz\n",
rate);
Is there any/much shared code left in this file after this patch? It
seems like all this file does now is make each cpufreq callback function
call soc_config->the_same_function_name(). If so, wouldn't it be better
to simply implement completely separate tegar20-cpufreq and
tegra30-cpufreq drivers, and register them each directly with the
cpufreq core, to avoid this file doing all the indirection?
I think this file is needed since we can shared the registration and
probe logic for different SoCs.
But there's basically nothing in probe() already, and if we have a
separate driver for each SoC, then there's even less code; just a call
to devm_kzalloc() for the device-specific data (which will be
SoC-specific in size anyway), and a call to cpufreq_register_driver(). I
don't think it's worth sharing that if it means that every other
function needs to be an indirect function call.
OK that makes sense.
-int __init tegra_cpufreq_init(void)
+static struct {
+char *compat;
+int (*init)(struct tegra_cpufreq_data *,
+const struct tegra_cpufreq_config **);
+} tegra_init_funcs[] = {
+{ "nvidia,tegra20", tegra20_cpufreq_init },
+};
+
+static int tegra_cpufreq_probe(struct platform_device *pdev)
...
+for (i = 0; i < ARRAY_SIZE(tegra_init_funcs); i++) {
+if (of_machine_is_compatible(tegra_init_funcs[i].compat)) {
+ret = tegra_init_funcs[i].init(tegra_data, _config);
+if (!ret)
+break;
+else
+goto out;
+}
}
+if (i == ARRAY_SIZE(tegra_init_funcs))
+goto out;
I think there are better ways of doing this than open-coding it. Perhaps
of_match_device() or the platform-driver equivalent could be made to
work?
Open coding is everywhere in OF helper functions actually. I doubt if we
can use of_match_device() if we're not adding node in DT.
If we're matching the platform device then we might need open coding, no?
For platform devices, you can set up the id_table of struct
platform_driver, and then simply call platform_get_device_id(pdev)
inside probe() to find the matching entry. drivers/i2c/busses/i2c-at91.c
is an example of how this works (just some random driver I found using
grep).
If we're going to have separate driver for each SoC, then we don't need
platform_get_device_id(pdev) stuffs...
What I would like to do is creating platform cpufreq device with name
"${root_compatible}-cpufreq" then each SoC cpufreq driver can bind to
it, but the question is, which file is the best place to do this? Create
a new file for this or use existing file like arch/arm/mach-tegra/tegra.c?
+int __init tegra_cpufreq_init(void)
+{
+struct platform_device_info devinfo = { .name = "tegra-cpufreq", };
+
+platform_device_register_full();
+
+return 0;
}
EXPORT_SYMBOL(tegra_cpufreq_init);
Perhaps instead of hard-coding the name "tegra-cpufreq" here, you could
dynamically construct the device name based on the DT's root compatible
value, register "${root_compatible}-cpufreq", e.g.
"nvidia,tegra20-cpufreq" or "nvidia,tegra30-cpufreq". That would allow
the kernel's standard device/driver matching mechanism to pick the
correct driver to instantiate. Perhaps you could even dynamically
register an OF device so that you can use of_match_device() in probe, if
I guess what you meant dynamically register an OF device is registering
an fake OF device by calling of_device_add(), no? If yes then what
of_node should we give?
Yes. Good question about which node. I guess the root node would be the
only one that made any sense at all, and admittedly it doesn't make a
huge amount of sense. Perhaps registers a platform device rather than an
OF device would make more sense. See platform_device_register() I think.
--
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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/10/2013 01:32 AM, Stephen Warren wrote:
On 12/09/2013 01:44 AM, bilhuang wrote:
On 12/06/2013 07:04 AM, Stephen Warren wrote:
On 12/05/2013 12:44 AM, Bill Huang wrote:
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
diff --git a/drivers/cpufreq/tegra-cpufreq.c
b/drivers/cpufreq/tegra-cpufreq.c
@@ -91,14 +40,10 @@ static int tegra_update_cpu_speed(struct
cpufreq_policy *policy,
...
+if (soc_config-vote_emc_on_cpu_rate)
+soc_config-vote_emc_on_cpu_rate(rate);
+
+ret = soc_config-cpu_clk_set_rate(rate * 1000);
if (ret)
pr_err(cpu-tegra: Failed to set cpu frequency to %lu kHz\n,
rate);
Is there any/much shared code left in this file after this patch? It
seems like all this file does now is make each cpufreq callback function
call soc_config-the_same_function_name(). If so, wouldn't it be better
to simply implement completely separate tegar20-cpufreq and
tegra30-cpufreq drivers, and register them each directly with the
cpufreq core, to avoid this file doing all the indirection?
I think this file is needed since we can shared the registration and
probe logic for different SoCs.
But there's basically nothing in probe() already, and if we have a
separate driver for each SoC, then there's even less code; just a call
to devm_kzalloc() for the device-specific data (which will be
SoC-specific in size anyway), and a call to cpufreq_register_driver(). I
don't think it's worth sharing that if it means that every other
function needs to be an indirect function call.
OK that makes sense.
-int __init tegra_cpufreq_init(void)
+static struct {
+char *compat;
+int (*init)(struct tegra_cpufreq_data *,
+const struct tegra_cpufreq_config **);
+} tegra_init_funcs[] = {
+{ nvidia,tegra20, tegra20_cpufreq_init },
+};
+
+static int tegra_cpufreq_probe(struct platform_device *pdev)
...
+for (i = 0; i ARRAY_SIZE(tegra_init_funcs); i++) {
+if (of_machine_is_compatible(tegra_init_funcs[i].compat)) {
+ret = tegra_init_funcs[i].init(tegra_data, soc_config);
+if (!ret)
+break;
+else
+goto out;
+}
}
+if (i == ARRAY_SIZE(tegra_init_funcs))
+goto out;
I think there are better ways of doing this than open-coding it. Perhaps
of_match_device() or the platform-driver equivalent could be made to
work?
Open coding is everywhere in OF helper functions actually. I doubt if we
can use of_match_device() if we're not adding node in DT.
If we're matching the platform device then we might need open coding, no?
For platform devices, you can set up the id_table of struct
platform_driver, and then simply call platform_get_device_id(pdev)
inside probe() to find the matching entry. drivers/i2c/busses/i2c-at91.c
is an example of how this works (just some random driver I found using
grep).
If we're going to have separate driver for each SoC, then we don't need
platform_get_device_id(pdev) stuffs...
What I would like to do is creating platform cpufreq device with name
${root_compatible}-cpufreq then each SoC cpufreq driver can bind to
it, but the question is, which file is the best place to do this? Create
a new file for this or use existing file like arch/arm/mach-tegra/tegra.c?
+int __init tegra_cpufreq_init(void)
+{
+struct platform_device_info devinfo = { .name = tegra-cpufreq, };
+
+platform_device_register_full(devinfo);
+
+return 0;
}
EXPORT_SYMBOL(tegra_cpufreq_init);
Perhaps instead of hard-coding the name tegra-cpufreq here, you could
dynamically construct the device name based on the DT's root compatible
value, register ${root_compatible}-cpufreq, e.g.
nvidia,tegra20-cpufreq or nvidia,tegra30-cpufreq. That would allow
the kernel's standard device/driver matching mechanism to pick the
correct driver to instantiate. Perhaps you could even dynamically
register an OF device so that you can use of_match_device() in probe, if
I guess what you meant dynamically register an OF device is registering
an fake OF device by calling of_device_add(), no? If yes then what
of_node should we give?
Yes. Good question about which node. I guess the root node would be the
only one that made any sense at all, and admittedly it doesn't make a
huge amount of sense. Perhaps registers a platform device rather than an
OF device would make more sense. See platform_device_register() I think.
--
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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/06/2013 07:04 AM, Stephen Warren wrote:
On 12/05/2013 12:44 AM, Bill Huang wrote:
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
@@ -91,14 +40,10 @@ static int tegra_update_cpu_speed(struct cpufreq_policy
*policy,
...
+ if (soc_config->vote_emc_on_cpu_rate)
+ soc_config->vote_emc_on_cpu_rate(rate);
+
+ ret = soc_config->cpu_clk_set_rate(rate * 1000);
if (ret)
pr_err("cpu-tegra: Failed to set cpu frequency to %lu kHz\n",
rate);
Is there any/much shared code left in this file after this patch? It
seems like all this file does now is make each cpufreq callback function
call soc_config->the_same_function_name(). If so, wouldn't it be better
to simply implement completely separate tegar20-cpufreq and
tegra30-cpufreq drivers, and register them each directly with the
cpufreq core, to avoid this file doing all the indirection?
I think this file is needed since we can shared the registration and
probe logic for different SoCs.
-int __init tegra_cpufreq_init(void)
+static struct {
+ char *compat;
+ int (*init)(struct tegra_cpufreq_data *,
+ const struct tegra_cpufreq_config **);
+} tegra_init_funcs[] = {
+ { "nvidia,tegra20", tegra20_cpufreq_init },
+};
+
+static int tegra_cpufreq_probe(struct platform_device *pdev)
...
+ for (i = 0; i < ARRAY_SIZE(tegra_init_funcs); i++) {
+ if (of_machine_is_compatible(tegra_init_funcs[i].compat)) {
+ ret = tegra_init_funcs[i].init(tegra_data, _config);
+ if (!ret)
+ break;
+ else
+ goto out;
+ }
}
+ if (i == ARRAY_SIZE(tegra_init_funcs))
+ goto out;
I think there are better ways of doing this than open-coding it. Perhaps
of_match_device() or the platform-driver equivalent could be made to work?
Open coding is everywhere in OF helper functions actually. I doubt if we
can use of_match_device() if we're not adding node in DT.
If we're matching the platform device then we might need open coding, no?
+int __init tegra_cpufreq_init(void)
+{
+ struct platform_device_info devinfo = { .name = "tegra-cpufreq", };
+
+ platform_device_register_full();
+
+ return 0;
}
EXPORT_SYMBOL(tegra_cpufreq_init);
Perhaps instead of hard-coding the name "tegra-cpufreq" here, you could
dynamically construct the device name based on the DT's root compatible
value, register "${root_compatible}-cpufreq", e.g.
"nvidia,tegra20-cpufreq" or "nvidia,tegra30-cpufreq". That would allow
the kernel's standard device/driver matching mechanism to pick the
correct driver to instantiate. Perhaps you could even dynamically
register an OF device so that you can use of_match_device() in probe, if
I guess what you meant dynamically register an OF device is registering
an fake OF device by calling of_device_add(), no? If yes then what
of_node should we give?
there's some advantage of having a single driver that supports N chips.
--
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 v3 1/2] cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code
On 12/06/2013 06:54 AM, Stephen Warren wrote:
On 12/05/2013 12:44 AM, Bill Huang wrote:
Move the call from module_init to Tegra machine codes so it won't be
called in a multi-platform kernel running on non-Tegra SoCs.
diff --git a/include/linux/tegra-soc.h b/include/linux/tegra-soc.h
It might be better to create for the interface
to the cpufreq driver; tegra-soc.h is for the interface to core Tegra
code *from* other drivers.
Thanks, will do.
+#ifdef CONFIG_ARM_TEGRA_CPUFREQ
+int tegra_cpufreq_init(void);
+#else
+static inline int tegra_cpufreq_init(void)
+{
+ return -EINVAL;
+}
+#endif
Probably best to "return 0" from the !CONFIG_ARM_TEGRA_CPUFREQ case; the
whole point is to isolate callers from having to care whether
CONFIG_ARM_TEGRA_CPUFREQ is enabled, and making the function act like it
worked OK is part of that isolation.
OK thanks.
--
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 v3 1/2] cpufreq: tegra: Call tegra_cpufreq_init() specifically in machine code
On 12/06/2013 06:54 AM, Stephen Warren wrote:
On 12/05/2013 12:44 AM, Bill Huang wrote:
Move the call from module_init to Tegra machine codes so it won't be
called in a multi-platform kernel running on non-Tegra SoCs.
diff --git a/include/linux/tegra-soc.h b/include/linux/tegra-soc.h
It might be better to create linux/tegra-cpufreq.h for the interface
to the cpufreq driver; tegra-soc.h is for the interface to core Tegra
code *from* other drivers.
Thanks, will do.
+#ifdef CONFIG_ARM_TEGRA_CPUFREQ
+int tegra_cpufreq_init(void);
+#else
+static inline int tegra_cpufreq_init(void)
+{
+ return -EINVAL;
+}
+#endif
Probably best to return 0 from the !CONFIG_ARM_TEGRA_CPUFREQ case; the
whole point is to isolate callers from having to care whether
CONFIG_ARM_TEGRA_CPUFREQ is enabled, and making the function act like it
worked OK is part of that isolation.
OK thanks.
--
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 v3 2/2] cpufreq: tegra: Re-model Tegra cpufreq driver
On 12/06/2013 07:04 AM, Stephen Warren wrote:
On 12/05/2013 12:44 AM, Bill Huang wrote:
Re-model Tegra cpufreq driver to support all Tegra series of SoCs.
* Make tegra-cpufreq.c a generic Tegra cpufreq driver.
* Move Tegra20 specific codes into tegra20-cpufreq.c.
* Bind Tegra cpufreq dirver with a fake device so defer probe would work
when we're going to get regulator in the driver to support voltage
scaling (DVFS).
diff --git a/drivers/cpufreq/tegra-cpufreq.c b/drivers/cpufreq/tegra-cpufreq.c
@@ -91,14 +40,10 @@ static int tegra_update_cpu_speed(struct cpufreq_policy
*policy,
...
+ if (soc_config-vote_emc_on_cpu_rate)
+ soc_config-vote_emc_on_cpu_rate(rate);
+
+ ret = soc_config-cpu_clk_set_rate(rate * 1000);
if (ret)
pr_err(cpu-tegra: Failed to set cpu frequency to %lu kHz\n,
rate);
Is there any/much shared code left in this file after this patch? It
seems like all this file does now is make each cpufreq callback function
call soc_config-the_same_function_name(). If so, wouldn't it be better
to simply implement completely separate tegar20-cpufreq and
tegra30-cpufreq drivers, and register them each directly with the
cpufreq core, to avoid this file doing all the indirection?
I think this file is needed since we can shared the registration and
probe logic for different SoCs.
-int __init tegra_cpufreq_init(void)
+static struct {
+ char *compat;
+ int (*init)(struct tegra_cpufreq_data *,
+ const struct tegra_cpufreq_config **);
+} tegra_init_funcs[] = {
+ { nvidia,tegra20, tegra20_cpufreq_init },
+};
+
+static int tegra_cpufreq_probe(struct platform_device *pdev)
...
+ for (i = 0; i ARRAY_SIZE(tegra_init_funcs); i++) {
+ if (of_machine_is_compatible(tegra_init_funcs[i].compat)) {
+ ret = tegra_init_funcs[i].init(tegra_data, soc_config);
+ if (!ret)
+ break;
+ else
+ goto out;
+ }
}
+ if (i == ARRAY_SIZE(tegra_init_funcs))
+ goto out;
I think there are better ways of doing this than open-coding it. Perhaps
of_match_device() or the platform-driver equivalent could be made to work?
Open coding is everywhere in OF helper functions actually. I doubt if we
can use of_match_device() if we're not adding node in DT.
If we're matching the platform device then we might need open coding, no?
+int __init tegra_cpufreq_init(void)
+{
+ struct platform_device_info devinfo = { .name = tegra-cpufreq, };
+
+ platform_device_register_full(devinfo);
+
+ return 0;
}
EXPORT_SYMBOL(tegra_cpufreq_init);
Perhaps instead of hard-coding the name tegra-cpufreq here, you could
dynamically construct the device name based on the DT's root compatible
value, register ${root_compatible}-cpufreq, e.g.
nvidia,tegra20-cpufreq or nvidia,tegra30-cpufreq. That would allow
the kernel's standard device/driver matching mechanism to pick the
correct driver to instantiate. Perhaps you could even dynamically
register an OF device so that you can use of_match_device() in probe, if
I guess what you meant dynamically register an OF device is registering
an fake OF device by calling of_device_add(), no? If yes then what
of_node should we give?
there's some advantage of having a single driver that supports N chips.
--
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/
