[PATCH] clk: socfpga: remove set but not used variable 'rc'

[PATCH] clk: socfpga: remove set but not used variable 'rc'

[Yu Kuai]

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/clk/socfpga/clk-pll.c:83:6: warning: variable ‘rc’ set but
not used [-Wunused-but-set-variable]

The returned value of of_clk_add_provider() is never used, and so
can be removed.

Fixes: 97259e99bdc9 ("clk: socfpga: split clk code")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/clk/socfpga/clk-pll.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
index dcb573d44034..127cc849c5ee 100644
--- a/drivers/clk/socfpga/clk-pll.c
+++ b/drivers/clk/socfpga/clk-pll.c
@@ -80,7 +80,6 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
 	const char *parent_name[SOCFPGA_MAX_PARENTS];
 	struct clk_init_data init;
 	struct device_node *clkmgr_np;
-	int rc;
 	int err;
 
 	of_property_read_u32(node, "reg", &reg);
@@ -114,7 +113,7 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
 		kfree(pll_clk);
 		return ERR_PTR(err);
 	}
-	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
+	of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
 	return hw_clk;
 }
 
-- 
2.25.4

Re: [PATCH] clk: socfpga: remove set but not used variable 'rc'

[Dinh Nguyen]

On 5/14/21 1:22 AM, Yu Kuai wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/clk/socfpga/clk-pll.c:83:6: warning: variable ‘rc’ set but
> not used [-Wunused-but-set-variable]
> 
> The returned value of of_clk_add_provider() is never used, and so
> can be removed.
> 
> Fixes: 97259e99bdc9 ("clk: socfpga: split clk code")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>   drivers/clk/socfpga/clk-pll.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
> index dcb573d44034..127cc849c5ee 100644
> --- a/drivers/clk/socfpga/clk-pll.c
> +++ b/drivers/clk/socfpga/clk-pll.c
> @@ -80,7 +80,6 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>   	const char *parent_name[SOCFPGA_MAX_PARENTS];
>   	struct clk_init_data init;
>   	struct device_node *clkmgr_np;
> -	int rc;
>   	int err;
>   
>   	of_property_read_u32(node, "reg", &reg);
> @@ -114,7 +113,7 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>   		kfree(pll_clk);
>   		return ERR_PTR(err);
>   	}
> -	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
> +	of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
>   	return hw_clk;
>   }
>   
> 

Acked-by: Dinh Nguyen <dinguyen@kernel.org>

Re: [PATCH] clk: socfpga: remove set but not used variable 'rc'

[yukuai (C)]

ping ...

On 2021/05/14 23:11, Dinh Nguyen wrote:
> 
> 
> On 5/14/21 1:22 AM, Yu Kuai wrote:
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> drivers/clk/socfpga/clk-pll.c:83:6: warning: variable ‘rc’ set but
>> not used [-Wunused-but-set-variable]
>>
>> The returned value of of_clk_add_provider() is never used, and so
>> can be removed.
>>
>> Fixes: 97259e99bdc9 ("clk: socfpga: split clk code")
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   drivers/clk/socfpga/clk-pll.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/socfpga/clk-pll.c 
>> b/drivers/clk/socfpga/clk-pll.c
>> index dcb573d44034..127cc849c5ee 100644
>> --- a/drivers/clk/socfpga/clk-pll.c
>> +++ b/drivers/clk/socfpga/clk-pll.c
>> @@ -80,7 +80,6 @@ static __init struct clk_hw 
>> *__socfpga_pll_init(struct device_node *node,
>>       const char *parent_name[SOCFPGA_MAX_PARENTS];
>>       struct clk_init_data init;
>>       struct device_node *clkmgr_np;
>> -    int rc;
>>       int err;
>>       of_property_read_u32(node, "reg", &reg);
>> @@ -114,7 +113,7 @@ static __init struct clk_hw 
>> *__socfpga_pll_init(struct device_node *node,
>>           kfree(pll_clk);
>>           return ERR_PTR(err);
>>       }
>> -    rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
>> +    of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
>>       return hw_clk;
>>   }
>>
> 
> Acked-by: Dinh Nguyen <dinguyen@kernel.org>
> .
> 

Re: [PATCH] clk: socfpga: remove set but not used variable 'rc'

[Stephen Boyd]

Quoting Yu Kuai (2021-05-13 23:22:34)
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/clk/socfpga/clk-pll.c:83:6: warning: variable ‘rc’ set but
> not used [-Wunused-but-set-variable]
> 
> The returned value of of_clk_add_provider() is never used, and so
> can be removed.
> 
> Fixes: 97259e99bdc9 ("clk: socfpga: split clk code")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/clk/socfpga/clk-pll.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
> index dcb573d44034..127cc849c5ee 100644
> --- a/drivers/clk/socfpga/clk-pll.c
> +++ b/drivers/clk/socfpga/clk-pll.c
> @@ -80,7 +80,6 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>         const char *parent_name[SOCFPGA_MAX_PARENTS];
>         struct clk_init_data init;
>         struct device_node *clkmgr_np;
> -       int rc;
>         int err;
>  
>         of_property_read_u32(node, "reg", &reg);
> @@ -114,7 +113,7 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>                 kfree(pll_clk);
>                 return ERR_PTR(err);
>         }
> -       rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
> +       of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);

Shouldn't we fail if of_clk_add_provider() fails? So return some sort of
error pointer and unwind allocations, etc.

>         return hw_clk;
>  }
>

Re: [PATCH] clk: socfpga: remove set but not used variable 'rc'

[yukuai (C)]

On 2021/06/02 15:48, Stephen Boyd wrote:
> Quoting Yu Kuai (2021-05-13 23:22:34)
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> drivers/clk/socfpga/clk-pll.c:83:6: warning: variable ‘rc’ set but
>> not used [-Wunused-but-set-variable]
>>
>> The returned value of of_clk_add_provider() is never used, and so
>> can be removed.
>>
>> Fixes: 97259e99bdc9 ("clk: socfpga: split clk code")
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   drivers/clk/socfpga/clk-pll.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
>> index dcb573d44034..127cc849c5ee 100644
>> --- a/drivers/clk/socfpga/clk-pll.c
>> +++ b/drivers/clk/socfpga/clk-pll.c
>> @@ -80,7 +80,6 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>>          const char *parent_name[SOCFPGA_MAX_PARENTS];
>>          struct clk_init_data init;
>>          struct device_node *clkmgr_np;
>> -       int rc;
>>          int err;
>>   
>>          of_property_read_u32(node, "reg", &reg);
>> @@ -114,7 +113,7 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>>                  kfree(pll_clk);
>>                  return ERR_PTR(err);
>>          }
>> -       rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
>> +       of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
> 
> Shouldn't we fail if of_clk_add_provider() fails? So return some sort of
> error pointer and unwind allocations, etc.

Will send a v2 patch. By the way, do you think it's better to
return error pointer instead of NULL pointer when kzalloc() failed?

Thanks
Yu Kuai
> 
>>          return hw_clk;
>>   }
>>
> .
> 

[PATCH V2] clk: socfpga: err out if of_clk_add_provider() failed in __socfpga_pll_init()

[Yu Kuai]

__socfpga_pll_init() should fail if of_clk_add_provider() failed.
remove 'rc' in the meantime to avoid gcc
'-Wunused-but-set-variable' warning.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
changes in V2:
 - remove 'rc' and use err' instead of 'rc'
 - err out if of_clk_add_provider() failed

 drivers/clk/socfpga/clk-pll.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
index dcb573d44034..5a9eec2eca80 100644
--- a/drivers/clk/socfpga/clk-pll.c
+++ b/drivers/clk/socfpga/clk-pll.c
@@ -80,7 +80,6 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
 	const char *parent_name[SOCFPGA_MAX_PARENTS];
 	struct clk_init_data init;
 	struct device_node *clkmgr_np;
-	int rc;
 	int err;
 
 	of_property_read_u32(node, "reg", &reg);
@@ -110,12 +109,16 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
 	hw_clk = &pll_clk->hw.hw;
 
 	err = clk_hw_register(NULL, hw_clk);
-	if (err) {
-		kfree(pll_clk);
-		return ERR_PTR(err);
-	}
-	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
+	if (err)
+		goto err_out;
+	err = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
+	if (err)
+		goto err_out;
 	return hw_clk;
+
+err_out:
+	kfree(pll_clk);
+	return ERR_PTR(err);
 }
 
 void __init socfpga_pll_init(struct device_node *node)
-- 
2.31.1

Re: [PATCH V2] clk: socfpga: err out if of_clk_add_provider() failed in __socfpga_pll_init()

[yukuai (C)]

ping ...

On 2021/06/02 16:42, Yu Kuai wrote:
> __socfpga_pll_init() should fail if of_clk_add_provider() failed.
> remove 'rc' in the meantime to avoid gcc
> '-Wunused-but-set-variable' warning.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
> changes in V2:
>   - remove 'rc' and use err' instead of 'rc'
>   - err out if of_clk_add_provider() failed
> 
>   drivers/clk/socfpga/clk-pll.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
> index dcb573d44034..5a9eec2eca80 100644
> --- a/drivers/clk/socfpga/clk-pll.c
> +++ b/drivers/clk/socfpga/clk-pll.c
> @@ -80,7 +80,6 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>   	const char *parent_name[SOCFPGA_MAX_PARENTS];
>   	struct clk_init_data init;
>   	struct device_node *clkmgr_np;
> -	int rc;
>   	int err;
>   
>   	of_property_read_u32(node, "reg", &reg);
> @@ -110,12 +109,16 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>   	hw_clk = &pll_clk->hw.hw;
>   
>   	err = clk_hw_register(NULL, hw_clk);
> -	if (err) {
> -		kfree(pll_clk);
> -		return ERR_PTR(err);
> -	}
> -	rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
> +	if (err)
> +		goto err_out;
> +	err = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
> +	if (err)
> +		goto err_out;
>   	return hw_clk;
> +
> +err_out:
> +	kfree(pll_clk);
> +	return ERR_PTR(err);
>   }
>   
>   void __init socfpga_pll_init(struct device_node *node)
> 

Re: [PATCH V2] clk: socfpga: err out if of_clk_add_provider() failed in __socfpga_pll_init()

[Stephen Boyd]

Quoting Yu Kuai (2021-06-02 01:42:59)
> __socfpga_pll_init() should fail if of_clk_add_provider() failed.
> remove 'rc' in the meantime to avoid gcc
> '-Wunused-but-set-variable' warning.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---

Please don't send as in-reply-to previous patches. Makes it hard for me
to find them on the list.

> changes in V2:
>  - remove 'rc' and use err' instead of 'rc'
>  - err out if of_clk_add_provider() failed
> 
>  drivers/clk/socfpga/clk-pll.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/socfpga/clk-pll.c b/drivers/clk/socfpga/clk-pll.c
> index dcb573d44034..5a9eec2eca80 100644
> --- a/drivers/clk/socfpga/clk-pll.c
> +++ b/drivers/clk/socfpga/clk-pll.c
> @@ -80,7 +80,6 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>         const char *parent_name[SOCFPGA_MAX_PARENTS];
>         struct clk_init_data init;
>         struct device_node *clkmgr_np;
> -       int rc;
>         int err;
>  
>         of_property_read_u32(node, "reg", &reg);
> @@ -110,12 +109,16 @@ static __init struct clk_hw *__socfpga_pll_init(struct device_node *node,
>         hw_clk = &pll_clk->hw.hw;
>  
>         err = clk_hw_register(NULL, hw_clk);
> -       if (err) {
> -               kfree(pll_clk);
> -               return ERR_PTR(err);
> -       }
> -       rc = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
> +       if (err)
> +               goto err_out;
> +       err = of_clk_add_provider(node, of_clk_src_simple_get, hw_clk);
> +       if (err)

Now we don't unregister the clk on failure? Can you use
of_clk_add_hw_provider() as well? That would make this more modern. It
can be done as a followup patch to this one.

> +               goto err_out;
>         return hw_clk;
> +
> +err_out:
> +       kfree(pll_clk);
> +       return ERR_PTR(err);
>  }