Switch to a more robust approach to automatically release the node when
it goes out of scope, dropping the need for explicit calls to
of_node_put().

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/soc/atmel/soc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/atmel/soc.c b/drivers/soc/atmel/soc.c
index 64b1ad063073..298b542dd1c0 100644
--- a/drivers/soc/atmel/soc.c
+++ b/drivers/soc/atmel/soc.c
@@ -399,15 +399,12 @@ static const struct of_device_id at91_soc_allowed_list[] __initconst = {
 
 static int __init atmel_soc_device_init(void)
 {
-	struct device_node *np = of_find_node_by_path("/");
+	struct device_node *np __free(device_node) = of_find_node_by_path("/");
 
-	if (!of_match_node(at91_soc_allowed_list, np)) {
-		of_node_put(np);
+	if (!of_match_node(at91_soc_allowed_list, np))
 		return 0;
-	}
 
 	at91_soc_init(socs);
-	of_node_put(np);
 
 	return 0;
 }

-- 
2.43.0

On 30/10/2024 18:10, Javier Carrasco wrote:
> Switch to a more robust approach to automatically release the node when
> it goes out of scope, dropping the need for explicit calls to
> of_node_put().

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

There is never a "drivers" prefix. Especially not first (because as
middle appears for FEW subsystems, not for SoC though).

> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  drivers/soc/atmel/soc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/soc/atmel/soc.c b/drivers/soc/atmel/soc.c
> index 64b1ad063073..298b542dd1c0 100644
> --- a/drivers/soc/atmel/soc.c
> +++ b/drivers/soc/atmel/soc.c
> @@ -399,15 +399,12 @@ static const struct of_device_id at91_soc_allowed_list[] __initconst = {
>  
>  static int __init atmel_soc_device_init(void)
>  {
> -	struct device_node *np = of_find_node_by_path("/");
> +	struct device_node *np __free(device_node) = of_find_node_by_path("/");
>  
> -	if (!of_match_node(at91_soc_allowed_list, np)) {
> -		of_node_put(np);

You just added this code. Don't add code which immediately you remove.
Squash two patches.

Best regards,
Krzysztof

On 31/10/2024 12:07, Krzysztof Kozlowski wrote:
> On 30/10/2024 18:10, Javier Carrasco wrote:
>> Switch to a more robust approach to automatically release the node when
>> it goes out of scope, dropping the need for explicit calls to
>> of_node_put().
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> There is never a "drivers" prefix. Especially not first (because as
> middle appears for FEW subsystems, not for SoC though).
> 

Thanks, I added that by mistake. I will fix that for v2.

>>
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>> ---
>>  drivers/soc/atmel/soc.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/soc/atmel/soc.c b/drivers/soc/atmel/soc.c
>> index 64b1ad063073..298b542dd1c0 100644
>> --- a/drivers/soc/atmel/soc.c
>> +++ b/drivers/soc/atmel/soc.c
>> @@ -399,15 +399,12 @@ static const struct of_device_id at91_soc_allowed_list[] __initconst = {
>>  
>>  static int __init atmel_soc_device_init(void)
>>  {
>> -	struct device_node *np = of_find_node_by_path("/");
>> +	struct device_node *np __free(device_node) = of_find_node_by_path("/");
>>  
>> -	if (!of_match_node(at91_soc_allowed_list, np)) {
>> -		of_node_put(np);
> 
> You just added this code. Don't add code which immediately you remove.
> Squash two patches.
> 
> Best regards,
> Krzysztof
> 


As I said in another thread, I split the solution into a first one to be
applied to stable kernels, and a second one that uses a more robust
approach that is not supported by all stable kernels.

Best regards,
Javier Carrasco

On 31/10/2024 12:14, Javier Carrasco wrote:
> On 31/10/2024 12:07, Krzysztof Kozlowski wrote:
>> On 30/10/2024 18:10, Javier Carrasco wrote:
>>> Switch to a more robust approach to automatically release the node when
>>> it goes out of scope, dropping the need for explicit calls to
>>> of_node_put().
>>
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching. For bindings, the preferred subjects are
>> explained here:
>> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>>
>> There is never a "drivers" prefix. Especially not first (because as
>> middle appears for FEW subsystems, not for SoC though).
>>
> 
> Thanks, I added that by mistake. I will fix that for v2.
> 
>>>
>>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>>> ---
>>>  drivers/soc/atmel/soc.c | 7 ++-----
>>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/soc/atmel/soc.c b/drivers/soc/atmel/soc.c
>>> index 64b1ad063073..298b542dd1c0 100644
>>> --- a/drivers/soc/atmel/soc.c
>>> +++ b/drivers/soc/atmel/soc.c
>>> @@ -399,15 +399,12 @@ static const struct of_device_id at91_soc_allowed_list[] __initconst = {
>>>  
>>>  static int __init atmel_soc_device_init(void)
>>>  {
>>> -	struct device_node *np = of_find_node_by_path("/");
>>> +	struct device_node *np __free(device_node) = of_find_node_by_path("/");
>>>  
>>> -	if (!of_match_node(at91_soc_allowed_list, np)) {
>>> -		of_node_put(np);
>>
>> You just added this code. Don't add code which immediately you remove.
>> Squash two patches.
>>
>> Best regards,
>> Krzysztof
>>
> 
> 
> As I said in another thread, I split the solution into a first one to be
> applied to stable kernels, and a second one that uses a more robust
> approach that is not supported by all stable kernels.
> 

Same response as in other thread:

1. Then send backport for stable, if you care about stable. You inflate
history with irrational commits just instead of doing proper stable
backport.

2. Creating some commits purely for stable in the mainline kernel is not
a correct approach. We work here on mainline and in mainline this is one
logical change: fixing issue. Whether you fix issue with of_node_put or
cleanup or by removing of_find_node_by_path() call, it does not matter.
All of these are fixing the same, one issue.

If you think about stable kernels, then work on backports, not inflate
mainline kernel with multiple commits doing the same, creating
artificial history.

Best regards,
Krzysztof

On 31/10/2024 12:07, Krzysztof Kozlowski wrote:
> On 30/10/2024 18:10, Javier Carrasco wrote:
>> Switch to a more robust approach to automatically release the node when
>> it goes out of scope, dropping the need for explicit calls to
>> of_node_put().
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> There is never a "drivers" prefix. Especially not first (because as
> middle appears for FEW subsystems, not for SoC though).
> 

Interestingly, 10 out of 20 patches for this file use the "drivers"
prefix first, so I guess it was an error that propagated for some time.
I will stick to soc: atmel: for v2.

> 
> Best regards,
> Krzysztof
>