Hi Linus,
Linus Walleij <linus.walleij@xxxxxxxxxxxxxx> writes:
> 2010/9/15 Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>:
>
>> OMAP SOCs have a standard set of tuples consisting of frequency and
>> voltage pairs that the device will support per voltage domain. These
>> are called Operating Performance Points or OPPs.
>> (...)
>> This introduces a common handling OPP mechanism accross all OMAPs.
>> As a start this is used for OMAP3.
>
> OPPs are a generic concept, it's in silicon construction textbooks and all.
> Should this code not be made generic instead? You wouldn't make
> regulators or even DMA platform-specific these days, so why should
> OPPs be?
You're right.
> What in this code is actually OMAP-specific
Only the users. ;)
We currently register OPPs using an OMAP hwmod name, but we could easily
change that to use a struct device instead which would make this
much more generic (note we manage OPPs per-device, not just for the CPU)
The patch below[1] demonstrates quickly how easily we could remove all OMAP
specific stuff from opp.[ch], and move it to the OMAP-specific code that
does the opp_add()
> more than that you name
> some functions omap_*, and how hard would it be to put it under
> arch/arm/common/*.c
> arch/arm/include/asm/*.h
>
> Possible even higher up in the directory hiearchy in include/linux/opp.h
> for the header and drivers/opp/*.c, because I think SuperH and power
> are not that different in this respect.
Yeah, I guess this isn't ARM specific either, so should be at a higher
level.
Nishanth, can take my hack below and continue this evolution? As I
demonstrate with this hack, this won't really change anything for us.
Kevin
>From 96c4e27ba0cb3d9a056693340c6221bc093bce2c Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
Date: Thu, 16 Sep 2010 07:58:16 -0700
Subject: [PATCH] OPP: remove OMAP specifics to make more generic
Internal OPP management is based on 'struct device *', so
we don't need to have omap_hwmod specific knowledge in this layer.
Change opp_add() to take a 'struct device *' instead of using
the OMAP hwmod in the opp_def to lookup the struct device.
Move OMAP-specific hwmod lookups into the OMAP specific layer
which adds OPPs to the database.
Quickly tested on OMAP3 to see that all OPPs are still registered
correctly with CPUfreq
---
arch/arm/mach-omap2/opp3xxx_data.c | 18 +++++++++++++++++-
arch/arm/plat-omap/include/plat/opp.h | 2 +-
arch/arm/plat-omap/opp.c | 24 ++----------------------
3 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/arch/arm/mach-omap2/opp3xxx_data.c b/arch/arm/mach-omap2/opp3xxx_data.c
index e337aeb..f3f9ae4 100644
--- a/arch/arm/mach-omap2/opp3xxx_data.c
+++ b/arch/arm/mach-omap2/opp3xxx_data.c
@@ -23,6 +23,7 @@
#include <plat/opp.h>
#include <plat/cpu.h>
+#include <plat/omap_device.h>
static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
/* MPU OPP1 */
@@ -114,7 +115,22 @@ int __init omap3_pm_init_opp_table(void)
opp_def = omap3_opp_def_list;
for (i = 0; i < omap3_opp_def_size; i++) {
- r = opp_add(opp_def++);
+ struct omap_hwmod *oh;
+ struct device *dev;
+
+ if (!opp_def->hwmod_name) {
+ pr_err("%s: missing name of omap_hwmod, ignoring.\n", __func__);
+ return -EINVAL;
+ }
+ oh = omap_hwmod_lookup(opp_def->hwmod_name);
+ if (!oh || !oh->od) {
+ pr_warn("%s: no hwmod or odev for %s, cannot add OPPs.\n",
+ __func__, opp_def->hwmod_name);
+ return -EINVAL;
+ }
+ dev = &oh->od->pdev.dev;
+
+ r = opp_add(dev, opp_def++);
if (r)
pr_err("unable to add OPP %ld Hz for %s\n",
opp_def->freq, opp_def->hwmod_name);
diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
index 9af8c83..82cfdd6 100644
--- a/arch/arm/plat-omap/include/plat/opp.h
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -75,7 +75,7 @@ struct omap_opp *opp_find_freq_floor(struct device *dev, unsigned long *freq);
struct omap_opp *opp_find_freq_ceil(struct device *dev, unsigned long *freq);
-int opp_add(const struct omap_opp_def *opp_def);
+int opp_add(struct device *dev, const struct omap_opp_def *opp_def);
int opp_enable(struct omap_opp *opp);
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
index b26326b..f5295ca 100644
--- a/arch/arm/plat-omap/opp.c
+++ b/arch/arm/plat-omap/opp.c
@@ -21,7 +21,6 @@
#include <linux/list.h>
#include <plat/opp.h>
-#include <plat/omap_device.h>
/**
* struct omap_opp - OMAP OPP description structure
@@ -58,7 +57,6 @@ struct omap_opp {
struct device_opp {
struct list_head node;
- struct omap_hwmod *oh;
struct device *dev;
struct list_head opp_list;
@@ -291,29 +289,12 @@ static void omap_opp_populate(struct omap_opp *opp,
*
* This function adds an opp definition to the opp list and returns status.
*/
-int opp_add(const struct omap_opp_def *opp_def)
+int opp_add(struct device *dev, const struct omap_opp_def *opp_def)
{
- struct omap_hwmod *oh;
- struct device *dev = NULL;
struct device_opp *tmp_dev_opp, *dev_opp = NULL;
struct omap_opp *opp, *new_opp;
- struct platform_device *pdev;
struct list_head *head;
- /* find the correct hwmod, and device */
- if (!opp_def->hwmod_name) {
- pr_err("%s: missing name of omap_hwmod, ignoring.\n", __func__);
- return -EINVAL;
- }
- oh = omap_hwmod_lookup(opp_def->hwmod_name);
- if (!oh || !oh->od) {
- pr_warn("%s: no hwmod or odev for %s, cannot add OPPs.\n",
- __func__, opp_def->hwmod_name);
- return -EINVAL;
- }
- pdev = &oh->od->pdev;
- dev = &oh->od->pdev.dev;
-
/* Check for existing list for 'dev' */
list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) {
if (dev == tmp_dev_opp->dev) {
@@ -331,8 +312,7 @@ int opp_add(const struct omap_opp_def *opp_def)
return -ENOMEM;
}
- dev_opp->oh = oh;
- dev_opp->dev = &oh->od->pdev.dev;
+ dev_opp->dev = dev;
INIT_LIST_HEAD(&dev_opp->opp_list);
list_add(&dev_opp->node, &dev_opp_list);
--
1.7.2.1
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
