I don't think device_node() is a correct freeing function here:
'struct device_node *np __free(device_node);'
even if I'm not an expert in Kernel.
And I think it could be more convincing to explain 'why', rather than
simply highlighting the errors repeatedly.
Best,
Zichen
On Tue, Oct 1, 2024 at 2:58 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 01/10/2024 21:42, Krzysztof Kozlowski wrote:
> > On 01/10/2024 21:35, Zichen Xie wrote:
> >> Thank you for your feedback!
> >>
> >> You can see from commit 90ca6956d383 ("ice: Fix freeing uninitialized
> >> pointers") or any other usages of __free.
> >> Initialization is a necessary and standard way to protect your memory.
> >
> > You did not understand that commit.
> >
> >>
> >> Additionally, the proper freeing function should be of_node_put()
> >> rather than device_node().
> >
> > What?
> >
> >> In commit 8812b8689ee6 ("firmware: tegra: bpmp: Use scoped device node
> >> handling to simplify error paths"),
> >> of_node_put() was originally set to free the memory.
> >
> > What?!?!
> >
> > You don't understand this code, do you?
>
> To be clear:
> 1. Neither your code nor explanation make any sense.
> 2. Patch is wrong - buggy.
> 3. Patch was never even compiled.
>
> That's a NAK, before anyone tries to pick it up.
>
> Best regards,
> Krzysztof
>
