| Message ID | CAKohpo=6qXer+RRS-F6x=O1VLzbWq+_a-b29MaUzZBvt7XzDwQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 13, 2014 at 9:22 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 13 May 2014 13:11, [Chander Kashyap <chander.kashyap@linaro.org> wrote: > > What happened to your name ? "[" > >> From: Chander Kashyap <k.chander@samsung.com> >> >> It may be possible to unregister and re-register the cpufreq driver. >> One such example is arm big-little IKS cpufreq driver. While >> re-registering the driver, same OPPs may get added again. >> >> This patch detects the duplicacy and discards them. >> >> Signed-off-by: Chander Kashyap <k.chander@samsung.com> >> Signed-off-by: Inderpal Singh <inderpal.s@samsung.com> >> --- >> drivers/base/power/opp.c | 28 +++++++++++++++++++--------- >> 1 file changed, 19 insertions(+), 9 deletions(-) > > I wouldn't say that this approach is particularly bad or wrong, but what > about this instead? > Yes I prefer this and this exactly what I had[1] in my OPP DT series which we could not conclude on the bindings. You also need patch[2] for DT version. Regards, Sudeep [1] http://www.spinics.net/lists/cpufreq/msg07914.html [2] http://www.spinics.net/lists/cpufreq/msg07916.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 13 May 2014 16:00, Sudeep Holla <sudeep.holla@arm.com> wrote: > On Tue, May 13, 2014 at 9:22 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> On 13 May 2014 13:11, [Chander Kashyap <chander.kashyap@linaro.org> wrote: >> >> What happened to your name ? "[" >> >>> From: Chander Kashyap <k.chander@samsung.com> >>> >>> It may be possible to unregister and re-register the cpufreq driver. >>> One such example is arm big-little IKS cpufreq driver. While >>> re-registering the driver, same OPPs may get added again. >>> >>> This patch detects the duplicacy and discards them. >>> >>> Signed-off-by: Chander Kashyap <k.chander@samsung.com> >>> Signed-off-by: Inderpal Singh <inderpal.s@samsung.com> >>> --- >>> drivers/base/power/opp.c | 28 +++++++++++++++++++--------- >>> 1 file changed, 19 insertions(+), 9 deletions(-) >> >> I wouldn't say that this approach is particularly bad or wrong, but what >> about this instead? >> > > Yes I prefer this and this exactly what I had[1] in my OPP DT series which > we could not conclude on the bindings. You also need patch[2] for DT version. Ahh, I have just reinvented the wheel. Though I can see now that I have Acked those patches as well :) So, what are the plans for those patches then? As Chander also needs few of those. Probably split the series to get the non-blockers upstream Atleast ? Another thing that I thought later, though the problem can be fixed by your version of patches, the version from chander had something good as well. It would get rid of duplicate entries coming from dtb. Would it make sense to get that part in as well? -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Viresh, On 13/05/14 12:05, Viresh Kumar wrote: > On 13 May 2014 16:00, Sudeep Holla <sudeep.holla@arm.com> wrote: >> On Tue, May 13, 2014 at 9:22 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >>> On 13 May 2014 13:11, [Chander Kashyap <chander.kashyap@linaro.org> wrote: >>> >>> What happened to your name ? "[" >>> >>>> From: Chander Kashyap <k.chander@samsung.com> >>>> >>>> It may be possible to unregister and re-register the cpufreq driver. >>>> One such example is arm big-little IKS cpufreq driver. While >>>> re-registering the driver, same OPPs may get added again. >>>> >>>> This patch detects the duplicacy and discards them. >>>> >>>> Signed-off-by: Chander Kashyap <k.chander@samsung.com> >>>> Signed-off-by: Inderpal Singh <inderpal.s@samsung.com> >>>> --- >>>> drivers/base/power/opp.c | 28 +++++++++++++++++++--------- >>>> 1 file changed, 19 insertions(+), 9 deletions(-) >>> >>> I wouldn't say that this approach is particularly bad or wrong, but what >>> about this instead? >>> >> >> Yes I prefer this and this exactly what I had[1] in my OPP DT series which >> we could not conclude on the bindings. You also need patch[2] for DT version. > > Ahh, I have just reinvented the wheel. Though I can see now that I have > Acked those patches as well :) > Yes correct, it was stalled due to no discussions on the bindings. > So, what are the plans for those patches then? As Chander also needs few > of those. > > Probably split the series to get the non-blockers upstream Atleast ? > I can do that. > Another thing that I thought later, though the problem can be fixed by > your version of patches, the version from chander had something good as > well. It would get rid of duplicate entries coming from dtb. Would it make > sense to get that part in as well? > That requires agreement on the bindings. The main issue with shared opp bindings is that in general we don't want to modify the standard bindings without covering all aspects. It will avoid carrying around buggy bindings forever as its ABI. There was another thread from Samsung modifying bindings[0] and Rob insisted to join the discussion[1] I started, but I never got any feedback. Regards, Sudeep [0] http://marc.info/?l=devicetree&m=139152254008892&w=2 [1] http://www.spinics.net/lists/arm-kernel/msg303977.html -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13 May 2014 16:35, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 13 May 2014 16:00, Sudeep Holla <sudeep.holla@arm.com> wrote: >> On Tue, May 13, 2014 at 9:22 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >>> On 13 May 2014 13:11, [Chander Kashyap <chander.kashyap@linaro.org> wrote: >>> >>> What happened to your name ? "[" >>> >>>> From: Chander Kashyap <k.chander@samsung.com> >>>> >>>> It may be possible to unregister and re-register the cpufreq driver. >>>> One such example is arm big-little IKS cpufreq driver. While >>>> re-registering the driver, same OPPs may get added again. >>>> >>>> This patch detects the duplicacy and discards them. >>>> >>>> Signed-off-by: Chander Kashyap <k.chander@samsung.com> >>>> Signed-off-by: Inderpal Singh <inderpal.s@samsung.com> >>>> --- >>>> drivers/base/power/opp.c | 28 +++++++++++++++++++--------- >>>> 1 file changed, 19 insertions(+), 9 deletions(-) >>> >>> I wouldn't say that this approach is particularly bad or wrong, but what >>> about this instead? >>> >> >> Yes I prefer this and this exactly what I had[1] in my OPP DT series which >> we could not conclude on the bindings. You also need patch[2] for DT version. > > Ahh, I have just reinvented the wheel. Though I can see now that I have > Acked those patches as well :) > > So, what are the plans for those patches then? As Chander also needs few > of those. > > Probably split the series to get the non-blockers upstream Atleast ? > > Another thing that I thought later, though the problem can be fixed by > your version of patches, the version from chander had something good as > well. It would get rid of duplicate entries coming from dtb. Would it make > sense to get that part in as well? This patch takes care for duplicate entries even without dt. Hence i feel it can go as separate patch. > > -- > viresh
On 13/05/14 12:57, Chander Kashyap wrote: > On 13 May 2014 16:35, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> On 13 May 2014 16:00, Sudeep Holla <sudeep.holla@arm.com> wrote: >>> On Tue, May 13, 2014 at 9:22 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >>>> On 13 May 2014 13:11, [Chander Kashyap <chander.kashyap@linaro.org> wrote: >>>> >>>> What happened to your name ? "[" >>>> >>>>> From: Chander Kashyap <k.chander@samsung.com> >>>>> >>>>> It may be possible to unregister and re-register the cpufreq driver. >>>>> One such example is arm big-little IKS cpufreq driver. While >>>>> re-registering the driver, same OPPs may get added again. >>>>> >>>>> This patch detects the duplicacy and discards them. >>>>> >>>>> Signed-off-by: Chander Kashyap <k.chander@samsung.com> >>>>> Signed-off-by: Inderpal Singh <inderpal.s@samsung.com> >>>>> --- >>>>> drivers/base/power/opp.c | 28 +++++++++++++++++++--------- >>>>> 1 file changed, 19 insertions(+), 9 deletions(-) >>>> >>>> I wouldn't say that this approach is particularly bad or wrong, but what >>>> about this instead? >>>> >>> >>> Yes I prefer this and this exactly what I had[1] in my OPP DT series which >>> we could not conclude on the bindings. You also need patch[2] for DT version. >> >> Ahh, I have just reinvented the wheel. Though I can see now that I have >> Acked those patches as well :) >> >> So, what are the plans for those patches then? As Chander also needs few >> of those. >> >> Probably split the series to get the non-blockers upstream Atleast ? >> >> Another thing that I thought later, though the problem can be fixed by >> your version of patches, the version from chander had something good as >> well. It would get rid of duplicate entries coming from dtb. Would it make >> sense to get that part in as well? > > This patch takes care for duplicate entries even without dt. Hence i > feel it can go as separate patch. > Sorry if I added any confusion. My original patch series addressed both the issues: 1. duplicate entries added by OPP library(same as addressed by your patch) and 2. another to avoid duplication in the devicetree for OPPs IIUC Exynos don't use ST for OPPs(yet), so you need to address only (1). But we still need (2) before any bL platforms use DT for OPPs and need CPUFreq support. And yes we can address this separately. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/13/2014 03:22 AM, Viresh Kumar wrote: > On 13 May 2014 13:11, [Chander Kashyap <chander.kashyap@linaro.org> wrote: > > What happened to your name ? "[" > >> From: Chander Kashyap <k.chander@samsung.com> >> >> It may be possible to unregister and re-register the cpufreq driver. >> One such example is arm big-little IKS cpufreq driver. While >> re-registering the driver, same OPPs may get added again. >> >> This patch detects the duplicacy and discards them. >> >> Signed-off-by: Chander Kashyap <k.chander@samsung.com> >> Signed-off-by: Inderpal Singh <inderpal.s@samsung.com> >> --- >> drivers/base/power/opp.c | 28 +++++++++++++++++++--------- >> 1 file changed, 19 insertions(+), 9 deletions(-) > > I wouldn't say that this approach is particularly bad or wrong, but what > about this instead? > > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c > index 2553867..7efdaf3 100644 > --- a/drivers/base/power/opp.c > +++ b/drivers/base/power/opp.c > @@ -713,6 +713,11 @@ int of_init_opp_table(struct device *dev) > const __be32 *val; > int nr; > > + if (!IS_ERR(find_device_opp(dev))) { > + dev_warn("%s: opp table already exists\n", __func__); > + return -EEXIST; > + } > + > prop = of_find_property(dev->of_node, "operating-points", NULL); > if (!prop) > return -ENODEV; yes - this is good but should be an additional patch IMHO, since it solves a different issue:"prevent opp table re-creation attempt". the $subject patch addresses an issue where dev_pm_opp_add can be invoked independently as well -> So, we seem to have have three issues to solve: a) do we continue to ensure that OPP table is created one time? b) while creating OPP table, duplicate entries should be rejected ($subject) c) prevent recreation of OPP table once created (if we decide not to have a cleanup logic as part of (a)) - this is what you propose. Now, considering that OPP table definition is an SoC behavior, that behavior is NOT going to change just because we are reinserting driver modules - so if cpufreq drivers are resulting in that behavior, then we should fix that. and consider OPP tables are created one time (at boot) and do the necessary changes for the same. just my 2 cents.
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 2553867..7efdaf3 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -713,6 +713,11 @@ int of_init_opp_table(struct device *dev) const __be32 *val; int nr; + if (!IS_ERR(find_device_opp(dev))) { + dev_warn("%s: opp table already exists\n", __func__); + return -EEXIST; + } + prop = of_find_property(dev->of_node, "operating-points", NULL); if (!prop) return -ENODEV;