[PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Sudeep Holla]

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);
--
2.7.4

Re: [PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Viresh Kumar]

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

Re: [PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Sudeep Holla]

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

Re: [PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Sudeep Holla]

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

Re: [PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Rafael J. Wysocki]

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.

Re: [PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Rafael J. Wysocki]

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.

Re: [PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Sudeep Holla]

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

Re: [PATCH] PM / OPP: use of_cpu_device_node_get() instead of of_get_cpu_node()

[Viresh Kumar]

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

[PATCH v2] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Sudeep Holla]

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.

It adds the missing of_node_put for the CPU device nodes. This patch also
moves back to use of_cpu_device_node_get in dev_pm_opp_of_get_sharing_cpus
to avoid scanning the device tree again.

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/opp/of.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

v1->v2:
	- Moved of_node_put after using cpu_np
	- Renamed the $subject as fixing refcount is key part of the part
	  and correctly relates to "Fixes" tag
	- Also added include change which was accidentally missed

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 0b718886479b..6a486ad81259 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -16,7 +16,7 @@
 #include <linux/cpu.h>
 #include <linux/errno.h>
 #include <linux/device.h>
-#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/export.h>

@@ -603,7 +603,7 @@ 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);
@@ -614,10 +614,12 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev,
 		/* Get OPP descriptor node */
 		tmp_np = _opp_of_get_opp_desc_node(cpu_np);
 		if (!tmp_np) {
+			of_node_put(cpu_np);
 			pr_err("%pOF: Couldn't find opp node\n", cpu_np);
 			ret = -ENOENT;
 			goto put_cpu_node;
 		}
+		of_node_put(cpu_np);

 		/* CPUs are sharing opp node */
 		if (np == tmp_np)
--
2.7.4

Re: [PATCH v2] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Viresh Kumar]

On 11-10-17, 11:11, 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.
> 
> It adds the missing of_node_put for the CPU device nodes. This patch also
> moves back to use of_cpu_device_node_get in dev_pm_opp_of_get_sharing_cpus
> to avoid scanning the device tree again.
> 
> 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>

Cc: 4.14+ <stable@vger.kernel.org> # 4.14+

> ---
>  drivers/opp/of.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> v1->v2:
> 	- Moved of_node_put after using cpu_np
> 	- Renamed the $subject as fixing refcount is key part of the part
> 	  and correctly relates to "Fixes" tag
> 	- Also added include change which was accidentally missed

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Sudeep Holla]

On 11/10/17 11:21, Viresh Kumar wrote:
> On 11-10-17, 11:11, 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.
>>
>> It adds the missing of_node_put for the CPU device nodes. This patch also
>> moves back to use of_cpu_device_node_get in dev_pm_opp_of_get_sharing_cpus
>> to avoid scanning the device tree again.
>>
>> 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>
> 
> Cc: 4.14+ <stable@vger.kernel.org> # 4.14+
> 

Generally Greg pulls them based on fixes tags, so I didn't bother
to specify that explicitly :)

>> ---
>>  drivers/opp/of.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> v1->v2:
>> 	- Moved of_node_put after using cpu_np
>> 	- Renamed the $subject as fixing refcount is key part of the part
>> 	  and correctly relates to "Fixes" tag
>> 	- Also added include change which was accidentally missed
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> 


Thanks.

-- 
Regards,
Sudeep

Re: [PATCH v2] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Viresh Kumar]

On 11-10-17, 11:35, Sudeep Holla wrote:
> Generally Greg pulls them based on fixes tags, so I didn't bother
> to specify that explicitly :)

Hmm, I didn't knew that :)

-- 
viresh

Re: [PATCH v2] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Sudeep Holla]

On 11/10/17 11:36, Viresh Kumar wrote:
> On 11-10-17, 11:35, Sudeep Holla wrote:
>> Generally Greg pulls them based on fixes tags, so I didn't bother
>> to specify that explicitly :)
> 
> Hmm, I didn't knew that :)
> 

It's more than just that. Check this thread [1]

-- 
Regards,
Sudeep

[1] https://lkml.org/lkml/2017/10/6/923

Re: [PATCH v2] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Stephen Boyd]

On 10/11, Sudeep Holla wrote:
> @@ -603,7 +603,7 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev,
> @@ -614,10 +614,12 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev,
>  		/* Get OPP descriptor node */
>  		tmp_np = _opp_of_get_opp_desc_node(cpu_np);
>  		if (!tmp_np) {
> +			of_node_put(cpu_np);
>  			pr_err("%pOF: Couldn't find opp node\n", cpu_np);
>  			ret = -ENOENT;
>  			goto put_cpu_node;
>  		}
> +		of_node_put(cpu_np);
> 

Why not?


		tmp_np = _opp_of_get_opp_desc_node(cpu_np);
		of_node_put(cpu_np);
		if (!tmp_np) {
			pr_err("%pOF: Couldn't find opp node\n", cpu_np);
			ret = -ENOENT;
			goto put_cpu_node;
		}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Sudeep Holla]

On 11/10/17 21:20, Stephen Boyd wrote:
> On 10/11, Sudeep Holla wrote:
>> @@ -603,7 +603,7 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev,
>> @@ -614,10 +614,12 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev,
>>  		/* Get OPP descriptor node */
>>  		tmp_np = _opp_of_get_opp_desc_node(cpu_np);
>>  		if (!tmp_np) {
>> +			of_node_put(cpu_np);
>>  			pr_err("%pOF: Couldn't find opp node\n", cpu_np);
>>  			ret = -ENOENT;
>>  			goto put_cpu_node;
>>  		}
>> +		of_node_put(cpu_np);
>>
> 
> Why not?
> 
> 
> 		tmp_np = _opp_of_get_opp_desc_node(cpu_np);
> 		of_node_put(cpu_np);
> 		if (!tmp_np) {
> 			pr_err("%pOF: Couldn't find opp node\n", cpu_np);
> 			ret = -ENOENT;
> 			goto put_cpu_node;
> 		}
> 

Yes that's exactly what I asked in v1 and the did it other way as I
didn't get any response. I can do this way, not a problem.

-- 
Regards,
Sudeep

[PATCH v3] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Sudeep Holla]

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.

It adds the missing of_node_put for the CPU device nodes. This patch also
moves back to use of_cpu_device_node_get in dev_pm_opp_of_get_sharing_cpus
to avoid scanning the device tree again.

Cc: Nishanth Menon <nm@ti.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Acked-by: Viresh Kumar <vireshk@kernel.org>
Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/opp/of.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

v1->v2:
	- Moved of_node_put after using cpu_np
	- Renamed the $subject as fixing refcount is key part of the part
	  and correctly relates to "Fixes" tag
	- Also added include change which was accidentally missed

v2->v3:
	- Moved of_node_put before if condition

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 87509cb69f79..cb716aa2f44b 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -16,7 +16,7 @@
 #include <linux/cpu.h>
 #include <linux/errno.h>
 #include <linux/device.h>
-#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/export.h>

@@ -604,7 +604,7 @@ 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);
@@ -614,6 +614,7 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev,

 		/* Get OPP descriptor node */
 		tmp_np = _opp_of_get_opp_desc_node(cpu_np);
+		of_node_put(cpu_np);
 		if (!tmp_np) {
 			pr_err("%pOF: Couldn't find opp node\n", cpu_np);
 			ret = -ENOENT;
--
2.7.4

Re: [PATCH v3] PM / OPP: add missing of_node_put() for of_get_cpu_node()

[Stephen Boyd]

On 10/12/2017 03:32 AM, 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.
>
> It adds the missing of_node_put for the CPU device nodes. This patch also
> moves back to use of_cpu_device_node_get in dev_pm_opp_of_get_sharing_cpus
> to avoid scanning the device tree again.
>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Acked-by: Viresh Kumar <vireshk@kernel.org>
> Fixes: 762792913f8c ("PM / OPP: Fix get sharing CPUs when hotplug is used")
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---


Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project