| Message ID | 20190701165020.19840-1-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [next] clk: Si5341/Si5340: remove redundant assignment to n_den | expand |
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);
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
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?
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
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!
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);
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
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);