On 03/10/13 15:20, Nishanth Menon wrote: > On 10/01/2013 11:46 AM, Sudeep KarkadaNagesha wrote: >> On 01/10/13 14:32, Sudeep KarkadaNagesha wrote: [...] >> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@xxxxxxx> >> >> Currently we need to replicate the OPP entries in all the nodes even >> though they share OPPs being in the same clock domain. >> >> Few drivers like cpufreq depend on physical cpu0 node to specify the >> OPPs and only that node is referred irrespective of the logical cpu >> accessing it. Alternatively to support cpuhotplug path, few drivers >> parse all the cpu nodes for OPPs. Instead we can specify the phandle >> of the node which contains the OPP tuples. >> >> This patch adds support to the new property 'operating-points-phandle' >> which specifies the phandle pointing to another node which contains the >> actual OPP tuples. >> >> Cc: Rob Herring <rob.herring@xxxxxxxxxxx> >> Cc: Pawel Moll <pawel.moll@xxxxxxx> >> Cc: Mark Rutland <mark.rutland@xxxxxxx> >> Cc: Stephen Warren <swarren@xxxxxxxxxxxxx> >> Cc: "Rafael J. Wysocki" <rjw@xxxxxxx> >> Cc: Nishanth Menon <nm@xxxxxx> >> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@xxxxxxx> >> --- >> drivers/base/power/opp.c | 34 +++++++++++++++++++++++++--------- >> 1 file changed, 25 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c >> index ef89897..cd4dbb3 100644 >> --- a/drivers/base/power/opp.c >> +++ b/drivers/base/power/opp.c >> @@ -708,14 +708,25 @@ struct srcu_notifier_head *opp_get_notifier(struct device >> *dev) > ^^ > <minor crib> > When reposting, could you avoid having word wrap? - kinda hard to get > https://patchwork.kernel.org/patch/2971091/ to apply clean :( > > wget -O a.patch 'https://patchwork.kernel.org/patch/2971091/mbox/' > patch -p1< a.patch --dry-run > patching file drivers/base/power/opp.c > patch: **** malformed patch at line 143: *dev) > > also when i look at the mbox patch, I see it split up.. did not dig in > why.. manually fixed it up. Sorry for this, since I replied over my original patch(didn't use git mail) I messed it up. It won't happen again :) > </minor crib> > > minor comments follow (other than squashing it with patch #1) >> int of_init_opp_table(struct device *dev) >> { >> const struct property *prop; >> + struct device_node *opp_node; >> const __be32 *val; >> - int nr; >> - >> - prop = of_find_property(dev->of_node, "operating-points", NULL); >> - if (!prop) >> - return -ENODEV; >> - if (!prop->value) >> - return -ENODATA; >> + int nr, ret = 0; >> + >> + opp_node = of_parse_phandle(dev->of_node, >> + "operating-points-phandle", 0); >> + if (!opp_node) /* if no OPP phandle, search for OPPs in current node */ > If you do not have a strong opinion, could you move the comment above > the if? >> + opp_node = dev->of_node; > add an eol here. >> + prop = of_find_property(opp_node, "operating-points", NULL); >> + if (!prop) { >> + dev_warn(dev, "node %s missing operating-points property\n", >> + opp_node->full_name); > ^^ align the opp_node->full_name to the d in dev in dev_warn(dev? > Checkpatch.sh --strict reports > CHECK: Alignment should match open parenthesis > #57: FILE: drivers/base/power/opp.c:722: > + dev_warn(dev, "node %s missing operating-points property\n", > + opp_node->full_name); > > total: 0 errors, 0 warnings, 1 checks, 53 lines checked > All the other coding style comments and checkpatch errors reported in all the patches are now fixed. I will post the next version once I get response for DT binding updates from DT maintainers. > >> + ret = -ENODEV; >> + goto put_node; >> + } >> + if (!prop->value) { >> + ret = -ENODATA; >> + goto put_node; >> + } >> >> /* >> * Each OPP is a set of tuples consisting of frequency and >> @@ -724,7 +735,8 @@ int of_init_opp_table(struct device *dev) >> nr = prop->length / sizeof(u32); >> if (nr % 2) { >> dev_err(dev, "%s: Invalid OPP list\n", __func__); >> - return -EINVAL; >> + ret = -EINVAL; >> + goto put_node; >> } >> >> val = prop->value; >> @@ -740,7 +752,11 @@ int of_init_opp_table(struct device *dev) >> nr -= 2; >> } >> >> - return 0; >> +put_node: >> + if (opp_node != dev->of_node) >> + of_node_put(opp_node); >> + >> + return ret; >> } >> EXPORT_SYMBOL_GPL(of_init_opp_table); >> #endif >> > other than that, please feel free to add > Acked-by: Nishanth Menon <nm@xxxxxx> > Thanks for the review. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
