[PATCH v2] clk: tegra: (clk-tegra30) Add missing of_node_put()

[PATCH v2] clk: tegra: (clk-tegra30) Add missing of_node_put()

[Liang He]

In tegra30_clock_init, of_find_matching_node() will return a node
pointer with refcount incremented. We should use of_node_put() when
the node pointer is not used anymore.

Signed-off-by: Liang He <windhl@126.com>
---
 changelog:

 v2: use real name for Sob
 v1: fix missing bug

 drivers/clk/tegra/clk-tegra30.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 04b496123820..168c07d5a5f2 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1320,6 +1320,7 @@ static void __init tegra30_clock_init(struct device_node *np)
 	}
 
 	pmc_base = of_iomap(node, 0);
+	of_node_put(node);
 	if (!pmc_base) {
 		pr_err("Can't map pmc registers\n");
 		BUG();
-- 
2.25.1

Re: [PATCH v2] clk: tegra: (clk-tegra30) Add missing of_node_put()

[Stephen Boyd]

The subject should match historical subjects

 $ git log --oneline -3 -- drivers/clk/tegra/clk-tegra30.c

shows mostly "clk: tegra: ". Can you also combine this with the other
tegra patch? Don't think we need two patches for essentially the same
thing.

Quoting Liang He (2022-06-15 20:36:22)
> In tegra30_clock_init, of_find_matching_node() will return a node
> pointer with refcount incremented. We should use of_node_put() when
> the node pointer is not used anymore.
> 
> Signed-off-by: Liang He <windhl@126.com>
> ---
>  changelog:
> 
>  v2: use real name for Sob

Thanks!

>  v1: fix missing bug

Re:Re: [PATCH v2] clk: tegra: (clk-tegra30) Add missing of_node_put()

[Liang He]

At 2022-06-16 14:00:35, "Stephen Boyd" <sboyd@kernel.org> wrote:
>The subject should match historical subjects
>
> $ git log --oneline -3 -- drivers/clk/tegra/clk-tegra30.c
>
>shows mostly "clk: tegra: ". Can you also combine this with the other
>tegra patch? Don't think we need two patches for essentially the same
>thing.
>
>Quoting Liang He (2022-06-15 20:36:22)
>> In tegra30_clock_init, of_find_matching_node() will return a node
>> pointer with refcount incremented. We should use of_node_put() when
>> the node pointer is not used anymore.
>> 
>> Signed-off-by: Liang He <windhl@126.com>
>> ---
>>  changelog:
>> 
>>  v2: use real name for Sob
>
>Thanks!
>
>>  v1: fix missing bug

Sorry for my fault. I have been advised to use real name and I resend a PATCH v2 with my real name, but really with the same patch code.

So how can I withdraw the first patch or resend other thing? I am confused. 

Can you help me, Conor?

Re: [PATCH v2] clk: tegra: (clk-tegra30) Add missing of_node_put()

[Conor.Dooley]

On 16/06/2022 09:19, Liang He wrote:
> At 2022-06-16 14:00:35, "Stephen Boyd" <sboyd@kernel.org> wrote:
>> The subject should match historical subjects
>>
>> $ git log --oneline -3 -- drivers/clk/tegra/clk-tegra30.c
>>
>> shows mostly "clk: tegra: ". Can you also combine this with the other
>> tegra patch? Don't think we need two patches for essentially the same
>> thing.
>>
>> Quoting Liang He (2022-06-15 20:36:22)
>>> In tegra30_clock_init, of_find_matching_node() will return a node
>>> pointer with refcount incremented. We should use of_node_put() when
>>> the node pointer is not used anymore.
>>>
>>> Signed-off-by: Liang He <windhl@126.com>
>>> ---
>>>   changelog:
>>>
>>>   v2: use real name for Sob
>>
>> Thanks!
>>
>>>   v1: fix missing bug
> 
> Sorry for my fault. I have been advised to use real name and I resend a PATCH v2 with my real name, but really with the same patch code.
> 
> So how can I withdraw the first patch or resend other thing? I am confused.
> 
> Can you help me, Conor?

Yeah, sure. I think you're just getting a bit confused by conflicting
responses from different people. Some of the things I said on whatever
the original patch I replied to apply everywhere - like using your real
name or adding changelogs.
However, different subsystem maintainers have a different opinions about
how patches for their subsystem should look. I would imagine that it was
Guenter Roeck that asked you to use the "subsystem: (driver) action"
subject, which is how hwmon patches are done - but not other subsystems.

What Stephen is asking, is that you run
$ git log --oneline -3 -- drivers/clk/tegra/clk-tegra30.c
to figure out what the subject should be, based on previous subjects.
That's good advice to follow for any patch you send :)

This other comment was:
> Don't think we need two patches for essentially the same thing.

I assume you sent two different patches for the same driver, or the
same directory of drivers?
He's just asking that you squash the two commits together into one
commit.

 From your other email:
> Sorry, what do you mean same subject line comment?
> I add a [PATCH v2] tag when I use 'git format-patch -v 2'.

I assume Stephen meant: the same comments about "(clk-tegra30)"
from this patch apply to that patch too. There's nothing wrong
with having "[PATCH v2]".

> You mean the two commit has same subject content?
> Sorry, I am a beginner to submit patch commit.

Ohh don't worry, we have all been there (and in my case not too
long ago either...)

Hope that helps!
Thanks,
Conor.

Re:Re: [PATCH v2] clk: tegra: (clk-tegra30) Add missing of_node_put()

[Liang He]

At 2022-06-16 16:42:26, Conor.Dooley@microchip.com wrote:
>On 16/06/2022 09:19, Liang He wrote:
>> At 2022-06-16 14:00:35, "Stephen Boyd" <sboyd@kernel.org> wrote:
>>> The subject should match historical subjects
>>>
>>> $ git log --oneline -3 -- drivers/clk/tegra/clk-tegra30.c
>>>
>>> shows mostly "clk: tegra: ". Can you also combine this with the other
>>> tegra patch? Don't think we need two patches for essentially the same
>>> thing.
>>>
>>> Quoting Liang He (2022-06-15 20:36:22)
>>>> In tegra30_clock_init, of_find_matching_node() will return a node
>>>> pointer with refcount incremented. We should use of_node_put() when
>>>> the node pointer is not used anymore.
>>>>
>>>> Signed-off-by: Liang He <windhl@126.com>
>>>> ---
>>>>   changelog:
>>>>
>>>>   v2: use real name for Sob
>>>
>>> Thanks!
>>>
>>>>   v1: fix missing bug
>> 
>> Sorry for my fault. I have been advised to use real name and I resend a PATCH v2 with my real name, but really with the same patch code.
>> 
>> So how can I withdraw the first patch or resend other thing? I am confused.
>> 
>> Can you help me, Conor?
>
>Yeah, sure. I think you're just getting a bit confused by conflicting
>responses from different people. Some of the things I said on whatever
>the original patch I replied to apply everywhere - like using your real
>name or adding changelogs.
>However, different subsystem maintainers have a different opinions about
>how patches for their subsystem should look. I would imagine that it was
>Guenter Roeck that asked you to use the "subsystem: (driver) action"
>subject, which is how hwmon patches are done - but not other subsystems.
>
>What Stephen is asking, is that you run
>$ git log --oneline -3 -- drivers/clk/tegra/clk-tegra30.c
>to figure out what the subject should be, based on previous subjects.
>That's good advice to follow for any patch you send :)
>


>This other comment was:
>> Don't think we need two patches for essentially the same thing.
>
>I assume you sent two different patches for the same driver, or the
>same directory of drivers?
>He's just asking that you squash the two commits together into one
>commit.
>

Ok, thanks, Conor, I got it and I will prepare my next merged patch for
these files which are indeed in the same driver directory.

> From your other email:
>> Sorry, what do you mean same subject line comment?
>> I add a [PATCH v2] tag when I use 'git format-patch -v 2'.
>
>I assume Stephen meant: the same comments about "(clk-tegra30)"
>from this patch apply to that patch too. There's nothing wrong
>with having "[PATCH v2]".
>
>> You mean the two commit has same subject content?
>> Sorry, I am a beginner to submit patch commit.
>
>Ohh don't worry, we have all been there (and in my case not too
>long ago either...)
>
>Hope that helps!
>Thanks,
>Conor.
>
>



Thanks again, Conor.

Liang