[PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The variable n_den is initialized however that value is never read
as n_den is re-assigned a little later in the two paths of a
following if-statement.  Remove the redundant assignment.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/clk/clk-si5341.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
index 72424eb7e5f8..6e780c2a9e6b 100644
--- a/drivers/clk/clk-si5341.c
+++ b/drivers/clk/clk-si5341.c
@@ -547,7 +547,6 @@ static int si5341_synth_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	bool is_integer;
 
 	n_num = synth->data->freq_vco;
-	n_den = rate;
 
 	/* see if there's an integer solution */
 	r = do_div(n_num, rate);
-- 
2.20.1

Re: [PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Stephen Boyd]

Please Cc authors of drivers so they can ack/review.

Adding Mike to take a look.

Quoting Colin King (2019-07-01 09:50:20)
> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable n_den is initialized however that value is never read
> as n_den is re-assigned a little later in the two paths of a
> following if-statement.  Remove the redundant assignment.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/clk/clk-si5341.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
> index 72424eb7e5f8..6e780c2a9e6b 100644
> --- a/drivers/clk/clk-si5341.c
> +++ b/drivers/clk/clk-si5341.c
> @@ -547,7 +547,6 @@ static int si5341_synth_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>         bool is_integer;
>  
>         n_num = synth->data->freq_vco;
> -       n_den = rate;
>  
>         /* see if there's an integer solution */
>         r = do_div(n_num, rate);

Re: [PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Christophe JAILLET]

Le 22/07/2019 à 23:24, Stephen Boyd a écrit :
> Please Cc authors of drivers so they can ack/review.
>
> Adding Mike to take a look.
>
> Quoting Colin King (2019-07-01 09:50:20)
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The variable n_den is initialized however that value is never read
>> as n_den is re-assigned a little later in the two paths of a
>> following if-statement.  Remove the redundant assignment.
>>
>> Addresses-Coverity: ("Unused value")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   drivers/clk/clk-si5341.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
>> index 72424eb7e5f8..6e780c2a9e6b 100644
>> --- a/drivers/clk/clk-si5341.c
>> +++ b/drivers/clk/clk-si5341.c
>> @@ -547,7 +547,6 @@ static int si5341_synth_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>>          bool is_integer;
>>   
>>          n_num = synth->data->freq_vco;
>> -       n_den = rate;
>>   
>>          /* see if there's an integer solution */
>>          r = do_div(n_num, rate);
>
Hi,

I got the same advise from some else no later than yesterday (i.e. email 
the author...)
Maybe 'get_maintainer.pl' could be improved to search for it and propose 
the mail automatically?

just my 2c.


CJ

Re: [PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Stephen Boyd]

Quoting Christophe JAILLET (2019-07-22 14:43:32)
> Le 22/07/2019 à 23:24, Stephen Boyd a écrit :
> > Please Cc authors of drivers so they can ack/review.
> >
> > Adding Mike to take a look.
> >
> > Quoting Colin King (2019-07-01 09:50:20)
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> The variable n_den is initialized however that value is never read
> >> as n_den is re-assigned a little later in the two paths of a
> >> following if-statement.  Remove the redundant assignment.
> >>
> >> Addresses-Coverity: ("Unused value")
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >>   drivers/clk/clk-si5341.c | 1 -
> >>   1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
> >> index 72424eb7e5f8..6e780c2a9e6b 100644
> >> --- a/drivers/clk/clk-si5341.c
> >> +++ b/drivers/clk/clk-si5341.c
> >> @@ -547,7 +547,6 @@ static int si5341_synth_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> >>          bool is_integer;
> >>   
> >>          n_num = synth->data->freq_vco;
> >> -       n_den = rate;
> >>   
> >>          /* see if there's an integer solution */
> >>          r = do_div(n_num, rate);
> >
> Hi,
> 
> I got the same advise from some else no later than yesterday (i.e. email 
> the author...)
> Maybe 'get_maintainer.pl' could be improved to search for it and propose 
> the mail automatically?
> 
> just my 2c.
> 

Use --git option of get_maintainer.pl?

Re: [PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Christophe JAILLET]

Le 22/07/2019 à 23:53, Stephen Boyd a écrit :
> Quoting Christophe JAILLET (2019-07-22 14:43:32)
>> Le 22/07/2019 à 23:24, Stephen Boyd a écrit :
>>> Please Cc authors of drivers so they can ack/review.
>>>
>>> Adding Mike to take a look.
>>>
>>> Quoting Colin King (2019-07-01 09:50:20)
>>>> From: Colin Ian King <colin.king@canonical.com>
>>>>
>>>> The variable n_den is initialized however that value is never read
>>>> as n_den is re-assigned a little later in the two paths of a
>>>> following if-statement.  Remove the redundant assignment.
>>>>
>>>> Addresses-Coverity: ("Unused value")
>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>> ---
>>>>    drivers/clk/clk-si5341.c | 1 -
>>>>    1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
>>>> index 72424eb7e5f8..6e780c2a9e6b 100644
>>>> --- a/drivers/clk/clk-si5341.c
>>>> +++ b/drivers/clk/clk-si5341.c
>>>> @@ -547,7 +547,6 @@ static int si5341_synth_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>>>>           bool is_integer;
>>>>    
>>>>           n_num = synth->data->freq_vco;
>>>> -       n_den = rate;
>>>>    
>>>>           /* see if there's an integer solution */
>>>>           r = do_div(n_num, rate);
>> Hi,
>>
>> I got the same advise from some else no later than yesterday (i.e. email
>> the author...)
>> Maybe 'get_maintainer.pl' could be improved to search for it and propose
>> the mail automatically?
>>
>> just my 2c.
>>
> Use --git option of get_maintainer.pl?
>
>
I don't use it explicitly, but the suggestions I get include some git 
history, so I guess that it is on by default.

I was thinking at parsing files to see if MODULE_AUTHOR includes an email.

CJ

Re: [PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Stephen Boyd]

Quoting Christophe JAILLET (2019-07-22 15:00:24)
>
> I don't use it explicitly, but the suggestions I get include some git 
> history, so I guess that it is on by default.
> 
> I was thinking at parsing files to see if MODULE_AUTHOR includes an email.
> 

Ok. Feel free to write a patch. Just know that MODULE_AUTHOR isn't
always there so it's not a substitute for looking at git history or git
blame to figure out who wrote the code.

I suspect it's better to try to work on code and infrastructure to make
these sorts of patches and questions irrelevant by detecting these
problems before the code is merged, instead of after, by trawling the
mailing lists and trying to apply patches and test them for common
problems and then notifying the people working on the code. I don't have
unlimited time in my life, so getting patches like this just makes me
spend more time doing mundane tasks I don't want to do.

TL;DR: Please help automate this sort of stuff!

Re: [PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Mike Looijmans]

Good catch, thanks. You have my

Acked-by: Mike Looijmans <mike.looijmans@topic.nl>


On 22-07-19 23:24, Stephen Boyd wrote:
> Please Cc authors of drivers so they can ack/review.
> 
> Adding Mike to take a look.
> 
> Quoting Colin King (2019-07-01 09:50:20)
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The variable n_den is initialized however that value is never read
>> as n_den is re-assigned a little later in the two paths of a
>> following if-statement.  Remove the redundant assignment.
>>
>> Addresses-Coverity: ("Unused value")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   drivers/clk/clk-si5341.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
>> index 72424eb7e5f8..6e780c2a9e6b 100644
>> --- a/drivers/clk/clk-si5341.c
>> +++ b/drivers/clk/clk-si5341.c
>> @@ -547,7 +547,6 @@ static int si5341_synth_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>>          bool is_integer;
>>   
>>          n_num = synth->data->freq_vco;
>> -       n_den = rate;
>>   
>>          /* see if there's an integer solution */
>>          r = do_div(n_num, rate);


-- 
Mike Looijmans

Re: [PATCH][next] clk: Si5341/Si5340: remove redundant assignment to n_den

[Stephen Boyd]

Quoting Colin King (2019-07-01 09:50:20)
> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable n_den is initialized however that value is never read
> as n_den is re-assigned a little later in the two paths of a
> following if-statement.  Remove the redundant assignment.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---

Applied to clk-next