| Message ID | 1507632519-19648-1-git-send-email-sudeep.holla@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node() | expand |
On 10-10-17, 11:48, Sudeep Holla wrote: > Commit 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") > moved away from using cpu_dev->of_node because of some limitations. > However commit 7467c9d95989 ("of: return of_get_cpu_node from > of_cpu_device_node_get if CPUs are not registered") added support to > falls back to of_get_cpu_node if called if CPUs are not registered yet. > > This patch moves back to use of_cpu_device_node_get in > dev_pm_opp_of_get_sharing_cpus to avoid scanning the device tree again. > It also adds the missing of_node_put for the CPU device nodes. > > Cc: Viresh Kumar <vireshk@kernel.org> > Cc: Nishanth Menon <nm@ti.com> > Cc: Stephen Boyd <sboyd@codeaurora.org> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/base/power/opp/of.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c > index 0b718886479b..3505193043fe 100644 > --- a/drivers/base/power/opp/of.c > +++ b/drivers/base/power/opp/of.c > @@ -603,13 +603,14 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, > if (cpu == cpu_dev->id) > continue; > > - cpu_np = of_get_cpu_node(cpu, NULL); > + cpu_np = of_cpu_device_node_get(cpu); > if (!cpu_np) { > dev_err(cpu_dev, "%s: failed to get cpu%d node\n", > __func__, cpu); > ret = -ENOENT; > goto put_cpu_node; > } > + of_node_put(cpu_np); > > /* Get OPP descriptor node */ > tmp_np = _opp_of_get_opp_desc_node(cpu_np); This line is going to use cpu_np, how can we put cpu_np before this ? And you need to rebase this on pm/linux-next. -- viresh
On 10/10/17 12:45, Viresh Kumar wrote: > On 10-10-17, 11:48, Sudeep Holla wrote: >> Commit 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >> moved away from using cpu_dev->of_node because of some limitations. >> However commit 7467c9d95989 ("of: return of_get_cpu_node from >> of_cpu_device_node_get if CPUs are not registered") added support to >> falls back to of_get_cpu_node if called if CPUs are not registered yet. >> >> This patch moves back to use of_cpu_device_node_get in >> dev_pm_opp_of_get_sharing_cpus to avoid scanning the device tree again. >> It also adds the missing of_node_put for the CPU device nodes. >> >> Cc: Viresh Kumar <vireshk@kernel.org> >> Cc: Nishanth Menon <nm@ti.com> >> Cc: Stephen Boyd <sboyd@codeaurora.org> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >> Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> --- >> drivers/base/power/opp/of.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c >> index 0b718886479b..3505193043fe 100644 >> --- a/drivers/base/power/opp/of.c >> +++ b/drivers/base/power/opp/of.c >> @@ -603,13 +603,14 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, >> if (cpu == cpu_dev->id) >> continue; >> >> - cpu_np = of_get_cpu_node(cpu, NULL); >> + cpu_np = of_cpu_device_node_get(cpu); >> if (!cpu_np) { >> dev_err(cpu_dev, "%s: failed to get cpu%d node\n", >> __func__, cpu); >> ret = -ENOENT; >> goto put_cpu_node; >> } >> + of_node_put(cpu_np); >> >> /* Get OPP descriptor node */ >> tmp_np = _opp_of_get_opp_desc_node(cpu_np); > > This line is going to use cpu_np, how can we put cpu_np before this ? > We didn't take the reference before commit 762792913f8c as we were using cpu_dev->of_node directly. The above mentioned commit used of_get_cpu_node() which introduces the reference. So I assumed it shouldn't matter much and in order to keep the change simple, I did it before _opp_of_get_opp_desc_node. I can move just after _opp_of_get_opp_desc_node and before if check to avoid having both before failure goto and after the block. Let me know what is your preference ? > And you need to rebase this on pm/linux-next. > OK -- Regards, Sudeep
On 10/10/17 13:39, Sudeep Holla wrote: > > > On 10/10/17 12:45, Viresh Kumar wrote: >> On 10-10-17, 11:48, Sudeep Holla wrote: >>> Commit 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>> moved away from using cpu_dev->of_node because of some limitations. >>> However commit 7467c9d95989 ("of: return of_get_cpu_node from >>> of_cpu_device_node_get if CPUs are not registered") added support to >>> falls back to of_get_cpu_node if called if CPUs are not registered yet. >>> >>> This patch moves back to use of_cpu_device_node_get in >>> dev_pm_opp_of_get_sharing_cpus to avoid scanning the device tree again. >>> It also adds the missing of_node_put for the CPU device nodes. >>> >>> Cc: Viresh Kumar <vireshk@kernel.org> >>> Cc: Nishanth Menon <nm@ti.com> >>> Cc: Stephen Boyd <sboyd@codeaurora.org> >>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >>> Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>> --- >>> drivers/base/power/opp/of.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c >>> index 0b718886479b..3505193043fe 100644 >>> --- a/drivers/base/power/opp/of.c >>> +++ b/drivers/base/power/opp/of.c >>> @@ -603,13 +603,14 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, >>> if (cpu == cpu_dev->id) >>> continue; >>> >>> - cpu_np = of_get_cpu_node(cpu, NULL); >>> + cpu_np = of_cpu_device_node_get(cpu); >>> if (!cpu_np) { >>> dev_err(cpu_dev, "%s: failed to get cpu%d node\n", >>> __func__, cpu); >>> ret = -ENOENT; >>> goto put_cpu_node; >>> } >>> + of_node_put(cpu_np); >>> >>> /* Get OPP descriptor node */ >>> tmp_np = _opp_of_get_opp_desc_node(cpu_np); >> >> This line is going to use cpu_np, how can we put cpu_np before this ? >> > > We didn't take the reference before commit 762792913f8c as we were using > cpu_dev->of_node directly. The above mentioned commit used > of_get_cpu_node() which introduces the reference. So I assumed it > shouldn't matter much and in order to keep the change simple, I did it > before _opp_of_get_opp_desc_node. I can move just after > _opp_of_get_opp_desc_node and before if check to avoid having both > before failure goto and after the block. > > Let me know what is your preference ? > >> And you need to rebase this on pm/linux-next. >> > > OK > I see the path has changed. I am not sure how you would consider this patch ? as fix for v4.14(the refcount issue is introduced in v4.14) or for v4.15 I can split the patch if use of of_cpu_device_node_get is not a fix material. Let me know. -- Regards, Sudeep
On Tue, Oct 10, 2017 at 5:54 PM, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > On 10/10/17 13:39, Sudeep Holla wrote: >> >> >> On 10/10/17 12:45, Viresh Kumar wrote: >>> On 10-10-17, 11:48, Sudeep Holla wrote: >>>> Commit 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>>> moved away from using cpu_dev->of_node because of some limitations. >>>> However commit 7467c9d95989 ("of: return of_get_cpu_node from >>>> of_cpu_device_node_get if CPUs are not registered") added support to >>>> falls back to of_get_cpu_node if called if CPUs are not registered yet. >>>> >>>> This patch moves back to use of_cpu_device_node_get in >>>> dev_pm_opp_of_get_sharing_cpus to avoid scanning the device tree again. >>>> It also adds the missing of_node_put for the CPU device nodes. >>>> >>>> Cc: Viresh Kumar <vireshk@kernel.org> >>>> Cc: Nishanth Menon <nm@ti.com> >>>> Cc: Stephen Boyd <sboyd@codeaurora.org> >>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >>>> Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>> --- >>>> drivers/base/power/opp/of.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c >>>> index 0b718886479b..3505193043fe 100644 >>>> --- a/drivers/base/power/opp/of.c >>>> +++ b/drivers/base/power/opp/of.c >>>> @@ -603,13 +603,14 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, >>>> if (cpu == cpu_dev->id) >>>> continue; >>>> >>>> - cpu_np = of_get_cpu_node(cpu, NULL); >>>> + cpu_np = of_cpu_device_node_get(cpu); >>>> if (!cpu_np) { >>>> dev_err(cpu_dev, "%s: failed to get cpu%d node\n", >>>> __func__, cpu); >>>> ret = -ENOENT; >>>> goto put_cpu_node; >>>> } >>>> + of_node_put(cpu_np); >>>> >>>> /* Get OPP descriptor node */ >>>> tmp_np = _opp_of_get_opp_desc_node(cpu_np); >>> >>> This line is going to use cpu_np, how can we put cpu_np before this ? >>> >> >> We didn't take the reference before commit 762792913f8c as we were using >> cpu_dev->of_node directly. The above mentioned commit used >> of_get_cpu_node() which introduces the reference. So I assumed it >> shouldn't matter much and in order to keep the change simple, I did it >> before _opp_of_get_opp_desc_node. I can move just after >> _opp_of_get_opp_desc_node and before if check to avoid having both >> before failure goto and after the block. >> >> Let me know what is your preference ? >> >>> And you need to rebase this on pm/linux-next. >>> >> >> OK >> > I see the path has changed. I am not sure how you would consider this > patch ? as fix for v4.14(the refcount issue is introduced in v4.14) > or for v4.15 > > I can split the patch if use of of_cpu_device_node_get is not a fix > material. Let me know. It will go into 4.15, no need to resend.
On Tue, Oct 10, 2017 at 7:12 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Tue, Oct 10, 2017 at 5:54 PM, Sudeep Holla <sudeep.holla@arm.com> wrote: >> >> >> On 10/10/17 13:39, Sudeep Holla wrote: >>> >>> >>> On 10/10/17 12:45, Viresh Kumar wrote: >>>> On 10-10-17, 11:48, Sudeep Holla wrote: >>>>> Commit 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>>>> moved away from using cpu_dev->of_node because of some limitations. >>>>> However commit 7467c9d95989 ("of: return of_get_cpu_node from >>>>> of_cpu_device_node_get if CPUs are not registered") added support to >>>>> falls back to of_get_cpu_node if called if CPUs are not registered yet. >>>>> >>>>> This patch moves back to use of_cpu_device_node_get in >>>>> dev_pm_opp_of_get_sharing_cpus to avoid scanning the device tree again. >>>>> It also adds the missing of_node_put for the CPU device nodes. >>>>> >>>>> Cc: Viresh Kumar <vireshk@kernel.org> >>>>> Cc: Nishanth Menon <nm@ti.com> >>>>> Cc: Stephen Boyd <sboyd@codeaurora.org> >>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >>>>> Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>>> --- >>>>> drivers/base/power/opp/of.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c >>>>> index 0b718886479b..3505193043fe 100644 >>>>> --- a/drivers/base/power/opp/of.c >>>>> +++ b/drivers/base/power/opp/of.c >>>>> @@ -603,13 +603,14 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, >>>>> if (cpu == cpu_dev->id) >>>>> continue; >>>>> >>>>> - cpu_np = of_get_cpu_node(cpu, NULL); >>>>> + cpu_np = of_cpu_device_node_get(cpu); >>>>> if (!cpu_np) { >>>>> dev_err(cpu_dev, "%s: failed to get cpu%d node\n", >>>>> __func__, cpu); >>>>> ret = -ENOENT; >>>>> goto put_cpu_node; >>>>> } >>>>> + of_node_put(cpu_np); >>>>> >>>>> /* Get OPP descriptor node */ >>>>> tmp_np = _opp_of_get_opp_desc_node(cpu_np); >>>> >>>> This line is going to use cpu_np, how can we put cpu_np before this ? >>>> >>> >>> We didn't take the reference before commit 762792913f8c as we were using >>> cpu_dev->of_node directly. The above mentioned commit used >>> of_get_cpu_node() which introduces the reference. So I assumed it >>> shouldn't matter much and in order to keep the change simple, I did it >>> before _opp_of_get_opp_desc_node. I can move just after >>> _opp_of_get_opp_desc_node and before if check to avoid having both >>> before failure goto and after the block. >>> >>> Let me know what is your preference ? >>> >>>> And you need to rebase this on pm/linux-next. >>>> >>> >>> OK >>> >> I see the path has changed. I am not sure how you would consider this >> patch ? as fix for v4.14(the refcount issue is introduced in v4.14) >> or for v4.15 >> >> I can split the patch if use of of_cpu_device_node_get is not a fix >> material. Let me know. > > It will go into 4.15, no need to resend. I mean no need to resend just because of the path change. It looks like the patch itself needs to be modified, though. Anyway, 4.15 material.
On 10/10/17 18:13, Rafael J. Wysocki wrote: > On Tue, Oct 10, 2017 at 7:12 PM, Rafael J. Wysocki <rafael@kernel.org> wrote: >> On Tue, Oct 10, 2017 at 5:54 PM, Sudeep Holla <sudeep.holla@arm.com> wrote: >>> >>> >>> On 10/10/17 13:39, Sudeep Holla wrote: >>>> >>>> >>>> On 10/10/17 12:45, Viresh Kumar wrote: >>>>> On 10-10-17, 11:48, Sudeep Holla wrote: >>>>>> Commit 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>>>>> moved away from using cpu_dev->of_node because of some limitations. >>>>>> However commit 7467c9d95989 ("of: return of_get_cpu_node from >>>>>> of_cpu_device_node_get if CPUs are not registered") added support to >>>>>> falls back to of_get_cpu_node if called if CPUs are not registered yet. >>>>>> >>>>>> This patch moves back to use of_cpu_device_node_get in >>>>>> dev_pm_opp_of_get_sharing_cpus to avoid scanning the device tree again. >>>>>> It also adds the missing of_node_put for the CPU device nodes. >>>>>> >>>>>> Cc: Viresh Kumar <vireshk@kernel.org> >>>>>> Cc: Nishanth Menon <nm@ti.com> >>>>>> Cc: Stephen Boyd <sboyd@codeaurora.org> >>>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >>>>>> Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") >>>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>>>> --- >>>>>> drivers/base/power/opp/of.c | 3 ++- >>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c >>>>>> index 0b718886479b..3505193043fe 100644 >>>>>> --- a/drivers/base/power/opp/of.c >>>>>> +++ b/drivers/base/power/opp/of.c >>>>>> @@ -603,13 +603,14 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, >>>>>> if (cpu == cpu_dev->id) >>>>>> continue; >>>>>> >>>>>> - cpu_np = of_get_cpu_node(cpu, NULL); >>>>>> + cpu_np = of_cpu_device_node_get(cpu); >>>>>> if (!cpu_np) { >>>>>> dev_err(cpu_dev, "%s: failed to get cpu%d node\n", >>>>>> __func__, cpu); >>>>>> ret = -ENOENT; >>>>>> goto put_cpu_node; >>>>>> } >>>>>> + of_node_put(cpu_np); >>>>>> >>>>>> /* Get OPP descriptor node */ >>>>>> tmp_np = _opp_of_get_opp_desc_node(cpu_np); >>>>> >>>>> This line is going to use cpu_np, how can we put cpu_np before this ? >>>>> >>>> >>>> We didn't take the reference before commit 762792913f8c as we were using >>>> cpu_dev->of_node directly. The above mentioned commit used >>>> of_get_cpu_node() which introduces the reference. So I assumed it >>>> shouldn't matter much and in order to keep the change simple, I did it >>>> before _opp_of_get_opp_desc_node. I can move just after >>>> _opp_of_get_opp_desc_node and before if check to avoid having both >>>> before failure goto and after the block. >>>> >>>> Let me know what is your preference ? >>>> >>>>> And you need to rebase this on pm/linux-next. >>>>> >>>> >>>> OK >>>> >>> I see the path has changed. I am not sure how you would consider this >>> patch ? as fix for v4.14(the refcount issue is introduced in v4.14) >>> or for v4.15 >>> >>> I can split the patch if use of of_cpu_device_node_get is not a fix >>> material. Let me know. >> >> It will go into 4.15, no need to resend. > > I mean no need to resend just because of the path change. > > It looks like the patch itself needs to be modified, though. > > Anyway, 4.15 material. > Got it. I will base it on pm/linux-next then when I repost. -- Regards, Sudeep
On 10-10-17, 13:39, Sudeep Holla wrote: > We didn't take the reference before commit 762792913f8c as we were using > cpu_dev->of_node directly. Yeah, and so the refcount was never screwed for us. > The above mentioned commit used > of_get_cpu_node() which introduces the reference. And it missing putting it down and we missed catching that in reviews. > So I assumed it > shouldn't matter much and in order to keep the change simple, I did it > before _opp_of_get_opp_desc_node. I can move just after > _opp_of_get_opp_desc_node and before if check to avoid having both > before failure goto and after the block. So the right thing to do based on current code is to put the reference only after we are done using the pointer. -- viresh
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c index 0b718886479b..3505193043fe 100644 --- a/drivers/base/power/opp/of.c +++ b/drivers/base/power/opp/of.c @@ -603,13 +603,14 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, if (cpu == cpu_dev->id) continue; - cpu_np = of_get_cpu_node(cpu, NULL); + cpu_np = of_cpu_device_node_get(cpu); if (!cpu_np) { dev_err(cpu_dev, "%s: failed to get cpu%d node\n", __func__, cpu); ret = -ENOENT; goto put_cpu_node; } + of_node_put(cpu_np); /* Get OPP descriptor node */ tmp_np = _opp_of_get_opp_desc_node(cpu_np);
Commit 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") moved away from using cpu_dev->of_node because of some limitations. However commit 7467c9d95989 ("of: return of_get_cpu_node from of_cpu_device_node_get if CPUs are not registered") added support to falls back to of_get_cpu_node if called if CPUs are not registered yet. This patch moves back to use of_cpu_device_node_get in dev_pm_opp_of_get_sharing_cpus to avoid scanning the device tree again. It also adds the missing of_node_put for the CPU device nodes. Cc: Viresh Kumar <vireshk@kernel.org> Cc: Nishanth Menon <nm@ti.com> Cc: Stephen Boyd <sboyd@codeaurora.org> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used") Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/base/power/opp/of.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.7.4