Re: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On Fri, May 2, 2014 at 12:22 AM, Viresh Kumar wrote: > On 2 May 2014 10:48, Nishanth Menon wrote: >> On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar >> wrote: diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c new file mode 100644 index 000..2602ff8 --- /dev/null +++ b/drivers/cpufreq/cpufreq_opp.c @@ -0,0 +1,102 @@ +/* + * Generic OPP Interface for CPUFREQ drivers + * + * Copyright (C) 2009-2014 Texas Instruments Incorporated. + * Nishanth Menon + * Romit Dasgupta + * Kevin Hilman + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ >>> >>> I hope you have just copy pasted routines to this file, and haven't done >>> even the most minor modification in those, as its hard to review it. >> >> there is code replacement ofcourse -> >> * the logic of walking down the list holding a mutex has been replaced >> with rcu locks, >> * instead of reading internal data structure and generating the list, >> use the existing search API that does exactly the same. >> * Documentation update for the same. > > Hmm, actually if I would have written this patch, then probably I would > have done the same thing, but looking from the reviewers perspective, > it would be much more easy if we can separate things into patches. > > So, maybe do these changes first in opp.c only and then finally a > patch that just moves things around. > >> Both are needed if you have to move the code out. functionally, both >> are equivalent > > That's an assumption and we never know when we might have screwed > the code :) .. And so more careful review of those parts is required :) True. Will do the same as suggested for the formal series. Thanks for your feedback and review. Regards, Nishanth Menon -- 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: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On Fri, May 2, 2014 at 12:22 AM, Viresh Kumar viresh.ku...@linaro.org wrote: On 2 May 2014 10:48, Nishanth Menon n...@ti.com wrote: On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar viresh.ku...@linaro.org wrote: diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c new file mode 100644 index 000..2602ff8 --- /dev/null +++ b/drivers/cpufreq/cpufreq_opp.c @@ -0,0 +1,102 @@ +/* + * Generic OPP Interface for CPUFREQ drivers + * + * Copyright (C) 2009-2014 Texas Instruments Incorporated. + * Nishanth Menon + * Romit Dasgupta + * Kevin Hilman + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ I hope you have just copy pasted routines to this file, and haven't done even the most minor modification in those, as its hard to review it. there is code replacement ofcourse - * the logic of walking down the list holding a mutex has been replaced with rcu locks, * instead of reading internal data structure and generating the list, use the existing search API that does exactly the same. * Documentation update for the same. Hmm, actually if I would have written this patch, then probably I would have done the same thing, but looking from the reviewers perspective, it would be much more easy if we can separate things into patches. So, maybe do these changes first in opp.c only and then finally a patch that just moves things around. Both are needed if you have to move the code out. functionally, both are equivalent That's an assumption and we never know when we might have screwed the code :) .. And so more careful review of those parts is required :) True. Will do the same as suggested for the formal series. Thanks for your feedback and review. Regards, Nishanth Menon -- 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: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On 2 May 2014 10:48, Nishanth Menon wrote: > On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar wrote: >>> diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c >>> new file mode 100644 >>> index 000..2602ff8 >>> --- /dev/null >>> +++ b/drivers/cpufreq/cpufreq_opp.c >>> @@ -0,0 +1,102 @@ >>> +/* >>> + * Generic OPP Interface for CPUFREQ drivers >>> + * >>> + * Copyright (C) 2009-2014 Texas Instruments Incorporated. >>> + * Nishanth Menon >>> + * Romit Dasgupta >>> + * Kevin Hilman >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + */ >> >> I hope you have just copy pasted routines to this file, and haven't done >> even the most minor modification in those, as its hard to review it. > > there is code replacement ofcourse -> > * the logic of walking down the list holding a mutex has been replaced > with rcu locks, > * instead of reading internal data structure and generating the list, > use the existing search API that does exactly the same. > * Documentation update for the same. Hmm, actually if I would have written this patch, then probably I would have done the same thing, but looking from the reviewers perspective, it would be much more easy if we can separate things into patches. So, maybe do these changes first in opp.c only and then finally a patch that just moves things around. > Both are needed if you have to move the code out. functionally, both > are equivalent That's an assumption and we never know when we might have screwed the code :) .. And so more careful review of those parts is required :) >>> diff --git a/drivers/cpufreq/cpufreq_opp.h b/drivers/cpufreq/cpufreq_opp.h >> >> Two problems, driver may lie in arch/ as well, though we don't recommend >> them, secondly move these in cpufreq.h, don't need a header here for sure. > > There are none at the moment. ideally, we'd like to discourage folks Yes, we do. :) > putting cpufreq drivers in arch/ given the amount of effort you have > undertaken in bringing them all here. but I have personally no strong > objection to getting rid of the private header and using the generic > cpufreq header. > > Otherwise, I assume you are ok with this approach and will post a > formal patch by monday. Yep. -- 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: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar wrote: > On 2 May 2014 06:36, Nishanth Menon wrote: >> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig >> index 1fbe11f..281ccfb 100644 >> --- a/drivers/cpufreq/Kconfig >> +++ b/drivers/cpufreq/Kconfig >> @@ -17,6 +17,11 @@ config CPU_FREQ >> >> if CPU_FREQ >> >> +config CPU_FREQ_PM_OPP >> + bool >> + depends on PM_OPP >> + default y >> + > > Don't need this ok. > >> config CPU_FREQ_GOV_COMMON >> bool >> >> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile >> index 0dbb963..16eea68 100644 >> --- a/drivers/cpufreq/Makefile >> +++ b/drivers/cpufreq/Makefile >> @@ -1,5 +1,7 @@ >> # CPUfreq core >> obj-$(CONFIG_CPU_FREQ) += cpufreq.o freq_table.o >> +obj-$(CONFIG_CPU_FREQ_PM_OPP) += cpufreq_opp.o > > Just use: obj-$(CONFIG_PM_OPP) ok. > >> + >> # CPUfreq stats >> obj-$(CONFIG_CPU_FREQ_STAT) += cpufreq_stats.o >> > >> diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c >> new file mode 100644 >> index 000..2602ff8 >> --- /dev/null >> +++ b/drivers/cpufreq/cpufreq_opp.c >> @@ -0,0 +1,102 @@ >> +/* >> + * Generic OPP Interface for CPUFREQ drivers >> + * >> + * Copyright (C) 2009-2014 Texas Instruments Incorporated. >> + * Nishanth Menon >> + * Romit Dasgupta >> + * Kevin Hilman >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ > > I hope you have just copy pasted routines to this file, and haven't done > even the most minor modification in those, as its hard to review it. there is code replacement ofcourse -> * the logic of walking down the list holding a mutex has been replaced with rcu locks, * instead of reading internal data structure and generating the list, use the existing search API that does exactly the same. * Documentation update for the same. Both are needed if you have to move the code out. functionally, both are equivalent > >> +#include > > Sure? That's it, nothing else required to compile this file independently? > As a rule include all the files directly which might be required for > compilation > of this file and don't expect them to be included by some other header > files indirectly. Alrite. will do, I try to trim the headers down to bare minimum, but will take care of it in the formal post. > >> diff --git a/drivers/cpufreq/cpufreq_opp.h b/drivers/cpufreq/cpufreq_opp.h > > Two problems, driver may lie in arch/ as well, though we don't recommend > them, secondly move these in cpufreq.h, don't need a header here for sure. There are none at the moment. ideally, we'd like to discourage folks putting cpufreq drivers in arch/ given the amount of effort you have undertaken in bringing them all here. but I have personally no strong objection to getting rid of the private header and using the generic cpufreq header. Otherwise, I assume you are ok with this approach and will post a formal patch by monday. Regards, Nishanth Menon -- 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: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On 2 May 2014 06:36, Nishanth Menon wrote: > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig > index 1fbe11f..281ccfb 100644 > --- a/drivers/cpufreq/Kconfig > +++ b/drivers/cpufreq/Kconfig > @@ -17,6 +17,11 @@ config CPU_FREQ > > if CPU_FREQ > > +config CPU_FREQ_PM_OPP > + bool > + depends on PM_OPP > + default y > + Don't need this > config CPU_FREQ_GOV_COMMON > bool > > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile > index 0dbb963..16eea68 100644 > --- a/drivers/cpufreq/Makefile > +++ b/drivers/cpufreq/Makefile > @@ -1,5 +1,7 @@ > # CPUfreq core > obj-$(CONFIG_CPU_FREQ) += cpufreq.o freq_table.o > +obj-$(CONFIG_CPU_FREQ_PM_OPP) += cpufreq_opp.o Just use: obj-$(CONFIG_PM_OPP) > + > # CPUfreq stats > obj-$(CONFIG_CPU_FREQ_STAT) += cpufreq_stats.o > > diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c > new file mode 100644 > index 000..2602ff8 > --- /dev/null > +++ b/drivers/cpufreq/cpufreq_opp.c > @@ -0,0 +1,102 @@ > +/* > + * Generic OPP Interface for CPUFREQ drivers > + * > + * Copyright (C) 2009-2014 Texas Instruments Incorporated. > + * Nishanth Menon > + * Romit Dasgupta > + * Kevin Hilman > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ I hope you have just copy pasted routines to this file, and haven't done even the most minor modification in those, as its hard to review it. > +#include Sure? That's it, nothing else required to compile this file independently? As a rule include all the files directly which might be required for compilation of this file and don't expect them to be included by some other header files indirectly. > diff --git a/drivers/cpufreq/cpufreq_opp.h b/drivers/cpufreq/cpufreq_opp.h Two problems, driver may lie in arch/ as well, though we don't recommend them, secondly move these in cpufreq.h, don't need a header here for sure. -- 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/
[RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
CPUFREQ specific functions for OPP (Operating Performance Points) can
be isolated to just cpufreq. This allows for independent modifications
as needed. The functionality desired by cpufreq can easily be provided
by existing functions and any future "special handling" needed for
cpufreq drivers can similarly be handled.
With this change the internal storage order of OPP entries are no
longer a limiting factor for how we need cpufreq table to look like.
Cc: "Rafael J. Wysocki"
Cc: Viresh Kumar
Cc: Kevin Hilman
Cc: cpuf...@vger.kernel.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Nishanth Menon
---
patch based on v3.15-rc1 tag.
Test log: OMAP5uEVM: http://slexy.org/view/s20WzLXI7K
drivers/base/power/opp.c | 91
drivers/cpufreq/Kconfig|5 ++
drivers/cpufreq/Makefile |2 +
drivers/cpufreq/arm_big_little.c |2 +-
drivers/cpufreq/arm_big_little_dt.c|2 +-
drivers/cpufreq/cpufreq-cpu0.c |3 +-
drivers/cpufreq/cpufreq_opp.c | 102
drivers/cpufreq/cpufreq_opp.h | 39
drivers/cpufreq/exynos5440-cpufreq.c |3 +-
drivers/cpufreq/imx6q-cpufreq.c|3 +-
drivers/cpufreq/omap-cpufreq.c |3 +-
drivers/cpufreq/vexpress-spc-cpufreq.c |2 +-
include/linux/pm_opp.h | 20 ---
13 files changed, 159 insertions(+), 118 deletions(-)
create mode 100644 drivers/cpufreq/cpufreq_opp.c
create mode 100644 drivers/cpufreq/cpufreq_opp.h
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..d9e376a 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -15,7 +15,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -596,96 +595,6 @@ int dev_pm_opp_disable(struct device *dev, unsigned long
freq)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
-#ifdef CONFIG_CPU_FREQ
-/**
- * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
- * @dev: device for which we do this operation
- * @table: Cpufreq table returned back to caller
- *
- * Generate a cpufreq table for a provided device- this assumes that the
- * opp list is already initialized and ready for usage.
- *
- * This function allocates required memory for the cpufreq table. It is
- * expected that the caller does the required maintenance such as freeing
- * the table as required.
- *
- * Returns -EINVAL for bad pointers, -ENODEV if the device is not found,
-ENOMEM
- * if no memory available for the operation (table is not populated), returns 0
- * if successful and table is populated.
- *
- * WARNING: It is important for the callers to ensure refreshing their copy of
- * the table if any of the mentioned functions have been invoked in the
interim.
- *
- * Locking: The internal device_opp and opp structures are RCU protected.
- * To simplify the logic, we pretend we are updater and hold relevant mutex
here
- * Callers should ensure that this function is *NOT* called under RCU
protection
- * or in contexts where mutex locking cannot be used.
- */
-int dev_pm_opp_init_cpufreq_table(struct device *dev,
- struct cpufreq_frequency_table **table)
-{
- struct device_opp *dev_opp;
- struct dev_pm_opp *opp;
- struct cpufreq_frequency_table *freq_table;
- int i = 0;
-
- /* Pretend as if I am an updater */
- mutex_lock(_opp_list_lock);
-
- dev_opp = find_device_opp(dev);
- if (IS_ERR(dev_opp)) {
- int r = PTR_ERR(dev_opp);
- mutex_unlock(_opp_list_lock);
- dev_err(dev, "%s: Device OPP not found (%d)\n", __func__, r);
- return r;
- }
-
- freq_table = kzalloc(sizeof(struct cpufreq_frequency_table) *
-(dev_pm_opp_get_opp_count(dev) + 1), GFP_KERNEL);
- if (!freq_table) {
- mutex_unlock(_opp_list_lock);
- dev_warn(dev, "%s: Unable to allocate frequency table\n",
- __func__);
- return -ENOMEM;
- }
-
- list_for_each_entry(opp, _opp->opp_list, node) {
- if (opp->available) {
- freq_table[i].driver_data = i;
- freq_table[i].frequency = opp->rate / 1000;
- i++;
- }
- }
- mutex_unlock(_opp_list_lock);
-
- freq_table[i].driver_data = i;
- freq_table[i].frequency = CPUFREQ_TABLE_END;
-
- *table = _table[0];
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
-
-/**
- * dev_pm_opp_free_cpufreq_table() - free the cpufreq table
- * @dev: device for which we do this operation
- * @table: table to free
- *
- * Free up the table
[RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
CPUFREQ specific functions for OPP (Operating Performance Points) can
be isolated to just cpufreq. This allows for independent modifications
as needed. The functionality desired by cpufreq can easily be provided
by existing functions and any future special handling needed for
cpufreq drivers can similarly be handled.
With this change the internal storage order of OPP entries are no
longer a limiting factor for how we need cpufreq table to look like.
Cc: Rafael J. Wysocki r...@rjwysocki.net
Cc: Viresh Kumar viresh.ku...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: cpuf...@vger.kernel.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Nishanth Menon n...@ti.com
---
patch based on v3.15-rc1 tag.
Test log: OMAP5uEVM: http://slexy.org/view/s20WzLXI7K
drivers/base/power/opp.c | 91
drivers/cpufreq/Kconfig|5 ++
drivers/cpufreq/Makefile |2 +
drivers/cpufreq/arm_big_little.c |2 +-
drivers/cpufreq/arm_big_little_dt.c|2 +-
drivers/cpufreq/cpufreq-cpu0.c |3 +-
drivers/cpufreq/cpufreq_opp.c | 102
drivers/cpufreq/cpufreq_opp.h | 39
drivers/cpufreq/exynos5440-cpufreq.c |3 +-
drivers/cpufreq/imx6q-cpufreq.c|3 +-
drivers/cpufreq/omap-cpufreq.c |3 +-
drivers/cpufreq/vexpress-spc-cpufreq.c |2 +-
include/linux/pm_opp.h | 20 ---
13 files changed, 159 insertions(+), 118 deletions(-)
create mode 100644 drivers/cpufreq/cpufreq_opp.c
create mode 100644 drivers/cpufreq/cpufreq_opp.h
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 2553867..d9e376a 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -15,7 +15,6 @@
#include linux/errno.h
#include linux/err.h
#include linux/slab.h
-#include linux/cpufreq.h
#include linux/device.h
#include linux/list.h
#include linux/rculist.h
@@ -596,96 +595,6 @@ int dev_pm_opp_disable(struct device *dev, unsigned long
freq)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
-#ifdef CONFIG_CPU_FREQ
-/**
- * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
- * @dev: device for which we do this operation
- * @table: Cpufreq table returned back to caller
- *
- * Generate a cpufreq table for a provided device- this assumes that the
- * opp list is already initialized and ready for usage.
- *
- * This function allocates required memory for the cpufreq table. It is
- * expected that the caller does the required maintenance such as freeing
- * the table as required.
- *
- * Returns -EINVAL for bad pointers, -ENODEV if the device is not found,
-ENOMEM
- * if no memory available for the operation (table is not populated), returns 0
- * if successful and table is populated.
- *
- * WARNING: It is important for the callers to ensure refreshing their copy of
- * the table if any of the mentioned functions have been invoked in the
interim.
- *
- * Locking: The internal device_opp and opp structures are RCU protected.
- * To simplify the logic, we pretend we are updater and hold relevant mutex
here
- * Callers should ensure that this function is *NOT* called under RCU
protection
- * or in contexts where mutex locking cannot be used.
- */
-int dev_pm_opp_init_cpufreq_table(struct device *dev,
- struct cpufreq_frequency_table **table)
-{
- struct device_opp *dev_opp;
- struct dev_pm_opp *opp;
- struct cpufreq_frequency_table *freq_table;
- int i = 0;
-
- /* Pretend as if I am an updater */
- mutex_lock(dev_opp_list_lock);
-
- dev_opp = find_device_opp(dev);
- if (IS_ERR(dev_opp)) {
- int r = PTR_ERR(dev_opp);
- mutex_unlock(dev_opp_list_lock);
- dev_err(dev, %s: Device OPP not found (%d)\n, __func__, r);
- return r;
- }
-
- freq_table = kzalloc(sizeof(struct cpufreq_frequency_table) *
-(dev_pm_opp_get_opp_count(dev) + 1), GFP_KERNEL);
- if (!freq_table) {
- mutex_unlock(dev_opp_list_lock);
- dev_warn(dev, %s: Unable to allocate frequency table\n,
- __func__);
- return -ENOMEM;
- }
-
- list_for_each_entry(opp, dev_opp-opp_list, node) {
- if (opp-available) {
- freq_table[i].driver_data = i;
- freq_table[i].frequency = opp-rate / 1000;
- i++;
- }
- }
- mutex_unlock(dev_opp_list_lock);
-
- freq_table[i].driver_data = i;
- freq_table[i].frequency = CPUFREQ_TABLE_END;
-
- *table = freq_table[0];
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
-
Re: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On 2 May 2014 06:36, Nishanth Menon n...@ti.com wrote: diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index 1fbe11f..281ccfb 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -17,6 +17,11 @@ config CPU_FREQ if CPU_FREQ +config CPU_FREQ_PM_OPP + bool + depends on PM_OPP + default y + Don't need this config CPU_FREQ_GOV_COMMON bool diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 0dbb963..16eea68 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -1,5 +1,7 @@ # CPUfreq core obj-$(CONFIG_CPU_FREQ) += cpufreq.o freq_table.o +obj-$(CONFIG_CPU_FREQ_PM_OPP) += cpufreq_opp.o Just use: obj-$(CONFIG_PM_OPP) + # CPUfreq stats obj-$(CONFIG_CPU_FREQ_STAT) += cpufreq_stats.o diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c new file mode 100644 index 000..2602ff8 --- /dev/null +++ b/drivers/cpufreq/cpufreq_opp.c @@ -0,0 +1,102 @@ +/* + * Generic OPP Interface for CPUFREQ drivers + * + * Copyright (C) 2009-2014 Texas Instruments Incorporated. + * Nishanth Menon + * Romit Dasgupta + * Kevin Hilman + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ I hope you have just copy pasted routines to this file, and haven't done even the most minor modification in those, as its hard to review it. +#include linux/slab.h Sure? That's it, nothing else required to compile this file independently? As a rule include all the files directly which might be required for compilation of this file and don't expect them to be included by some other header files indirectly. diff --git a/drivers/cpufreq/cpufreq_opp.h b/drivers/cpufreq/cpufreq_opp.h Two problems, driver may lie in arch/ as well, though we don't recommend them, secondly move these in cpufreq.h, don't need a header here for sure. -- 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: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar viresh.ku...@linaro.org wrote: On 2 May 2014 06:36, Nishanth Menon n...@ti.com wrote: diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index 1fbe11f..281ccfb 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -17,6 +17,11 @@ config CPU_FREQ if CPU_FREQ +config CPU_FREQ_PM_OPP + bool + depends on PM_OPP + default y + Don't need this ok. config CPU_FREQ_GOV_COMMON bool diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index 0dbb963..16eea68 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -1,5 +1,7 @@ # CPUfreq core obj-$(CONFIG_CPU_FREQ) += cpufreq.o freq_table.o +obj-$(CONFIG_CPU_FREQ_PM_OPP) += cpufreq_opp.o Just use: obj-$(CONFIG_PM_OPP) ok. + # CPUfreq stats obj-$(CONFIG_CPU_FREQ_STAT) += cpufreq_stats.o diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c new file mode 100644 index 000..2602ff8 --- /dev/null +++ b/drivers/cpufreq/cpufreq_opp.c @@ -0,0 +1,102 @@ +/* + * Generic OPP Interface for CPUFREQ drivers + * + * Copyright (C) 2009-2014 Texas Instruments Incorporated. + * Nishanth Menon + * Romit Dasgupta + * Kevin Hilman + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ I hope you have just copy pasted routines to this file, and haven't done even the most minor modification in those, as its hard to review it. there is code replacement ofcourse - * the logic of walking down the list holding a mutex has been replaced with rcu locks, * instead of reading internal data structure and generating the list, use the existing search API that does exactly the same. * Documentation update for the same. Both are needed if you have to move the code out. functionally, both are equivalent +#include linux/slab.h Sure? That's it, nothing else required to compile this file independently? As a rule include all the files directly which might be required for compilation of this file and don't expect them to be included by some other header files indirectly. Alrite. will do, I try to trim the headers down to bare minimum, but will take care of it in the formal post. diff --git a/drivers/cpufreq/cpufreq_opp.h b/drivers/cpufreq/cpufreq_opp.h Two problems, driver may lie in arch/ as well, though we don't recommend them, secondly move these in cpufreq.h, don't need a header here for sure. There are none at the moment. ideally, we'd like to discourage folks putting cpufreq drivers in arch/ given the amount of effort you have undertaken in bringing them all here. but I have personally no strong objection to getting rid of the private header and using the generic cpufreq header. Otherwise, I assume you are ok with this approach and will post a formal patch by monday. Regards, Nishanth Menon -- 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: [RFC PATCH] PM / OPP: move cpufreq specific OPP functions out of generic OPP library
On 2 May 2014 10:48, Nishanth Menon n...@ti.com wrote: On Thu, May 1, 2014 at 11:30 PM, Viresh Kumar viresh.ku...@linaro.org wrote: diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c new file mode 100644 index 000..2602ff8 --- /dev/null +++ b/drivers/cpufreq/cpufreq_opp.c @@ -0,0 +1,102 @@ +/* + * Generic OPP Interface for CPUFREQ drivers + * + * Copyright (C) 2009-2014 Texas Instruments Incorporated. + * Nishanth Menon + * Romit Dasgupta + * Kevin Hilman + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ I hope you have just copy pasted routines to this file, and haven't done even the most minor modification in those, as its hard to review it. there is code replacement ofcourse - * the logic of walking down the list holding a mutex has been replaced with rcu locks, * instead of reading internal data structure and generating the list, use the existing search API that does exactly the same. * Documentation update for the same. Hmm, actually if I would have written this patch, then probably I would have done the same thing, but looking from the reviewers perspective, it would be much more easy if we can separate things into patches. So, maybe do these changes first in opp.c only and then finally a patch that just moves things around. Both are needed if you have to move the code out. functionally, both are equivalent That's an assumption and we never know when we might have screwed the code :) .. And so more careful review of those parts is required :) diff --git a/drivers/cpufreq/cpufreq_opp.h b/drivers/cpufreq/cpufreq_opp.h Two problems, driver may lie in arch/ as well, though we don't recommend them, secondly move these in cpufreq.h, don't need a header here for sure. There are none at the moment. ideally, we'd like to discourage folks Yes, we do. :) putting cpufreq drivers in arch/ given the amount of effort you have undertaken in bringing them all here. but I have personally no strong objection to getting rid of the private header and using the generic cpufreq header. Otherwise, I assume you are ok with this approach and will post a formal patch by monday. Yep. -- 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/
