2022-06-16 04:10:24

by Liang He

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

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 <[email protected]>
---
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


2022-06-16 06:08:47

by Stephen Boyd

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

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 <[email protected]>
> ---
> changelog:
>
> v2: use real name for Sob

Thanks!

> v1: fix missing bug

2022-06-16 09:09:57

by Liang He

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







At 2022-06-16 14:00:35, "Stephen Boyd" <[email protected]> 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 <[email protected]>
>> ---
>> 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?

2022-06-16 09:45:41

by Liang He

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




At 2022-06-16 16:42:26, [email protected] wrote:
>On 16/06/2022 09:19, Liang He wrote:
>> At 2022-06-16 14:00:35, "Stephen Boyd" <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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