| Message ID | 20221004064521.498510-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: supply: remove less-than-zero comparison of unsigned variables | expand |
Hi, On Tue, Oct 04, 2022 at 11:45:21AM +0500, Muhammad Usama Anjum wrote: > max_chg_vol_reg and max_chg_cur_reg are unsigned variables. The > less-than-zero comparison of an unsigned value is never true. Remove > these checks. > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- I'm a bit hesitant to apply this. While the analysis is correct max_chg_cur_reg is sourced from rk817_chg_cur_to_reg(). That has has a 'u8' return value as function signature, but tries to return -EINVAL. I think it makes sense to either fix this at the same time or change the variable type to signed. Also please Cc the driver author (done now). -- Sebastian > drivers/power/supply/rk817_charger.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c > index 635f051b0821..aa4b33f1bb83 100644 > --- a/drivers/power/supply/rk817_charger.c > +++ b/drivers/power/supply/rk817_charger.c > @@ -951,12 +951,12 @@ static int rk817_battery_init(struct rk817_charger *charger, > > max_chg_cur_reg = rk817_chg_cur_to_reg(max_chg_cur_ma); > > - if (max_chg_vol_reg < 0 || max_chg_vol_reg > 7) { > + if (max_chg_vol_reg > 7) { > return dev_err_probe(charger->dev, -EINVAL, > "invalid max charger voltage, value %u unsupported\n", > max_chg_vol_mv * 1000); > } > - if (max_chg_cur_reg < 0 || max_chg_cur_reg > 7) { > + if (max_chg_cur_reg > 7) { > return dev_err_probe(charger->dev, -EINVAL, > "invalid max charger current, value %u unsupported\n", > max_chg_cur_ma * 1000); > -- > 2.30.2 >
On Sat, Oct 29, 2022 at 02:17:08AM +0200, Sebastian Reichel wrote: > Hi, > > On Tue, Oct 04, 2022 at 11:45:21AM +0500, Muhammad Usama Anjum wrote: > > max_chg_vol_reg and max_chg_cur_reg are unsigned variables. The > > less-than-zero comparison of an unsigned value is never true. Remove > > these checks. > > > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > > --- > > I'm a bit hesitant to apply this. While the analysis is correct > max_chg_cur_reg is sourced from rk817_chg_cur_to_reg(). That has > has a 'u8' return value as function signature, but tries to return > -EINVAL. I think it makes sense to either fix this at the same > time or change the variable type to signed. Also please Cc the > driver author (done now). I think this fixes it, can you confirm? https://lore.kernel.org/linux-pm/20221019234505.5viojwmk6ksqr4gb@mercury.elektranox.org/ Thank you. > > -- Sebastian > > > drivers/power/supply/rk817_charger.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c > > index 635f051b0821..aa4b33f1bb83 100644 > > --- a/drivers/power/supply/rk817_charger.c > > +++ b/drivers/power/supply/rk817_charger.c > > @@ -951,12 +951,12 @@ static int rk817_battery_init(struct rk817_charger *charger, > > > > max_chg_cur_reg = rk817_chg_cur_to_reg(max_chg_cur_ma); > > > > - if (max_chg_vol_reg < 0 || max_chg_vol_reg > 7) { > > + if (max_chg_vol_reg > 7) { > > return dev_err_probe(charger->dev, -EINVAL, > > "invalid max charger voltage, value %u unsupported\n", > > max_chg_vol_mv * 1000); > > } > > - if (max_chg_cur_reg < 0 || max_chg_cur_reg > 7) { > > + if (max_chg_cur_reg > 7) { > > return dev_err_probe(charger->dev, -EINVAL, > > "invalid max charger current, value %u unsupported\n", > > max_chg_cur_ma * 1000); > > -- > > 2.30.2 > >
diff --git a/drivers/power/supply/rk817_charger.c b/drivers/power/supply/rk817_charger.c index 635f051b0821..aa4b33f1bb83 100644 --- a/drivers/power/supply/rk817_charger.c +++ b/drivers/power/supply/rk817_charger.c @@ -951,12 +951,12 @@ static int rk817_battery_init(struct rk817_charger *charger, max_chg_cur_reg = rk817_chg_cur_to_reg(max_chg_cur_ma); - if (max_chg_vol_reg < 0 || max_chg_vol_reg > 7) { + if (max_chg_vol_reg > 7) { return dev_err_probe(charger->dev, -EINVAL, "invalid max charger voltage, value %u unsupported\n", max_chg_vol_mv * 1000); } - if (max_chg_cur_reg < 0 || max_chg_cur_reg > 7) { + if (max_chg_cur_reg > 7) { return dev_err_probe(charger->dev, -EINVAL, "invalid max charger current, value %u unsupported\n", max_chg_cur_ma * 1000);
max_chg_vol_reg and max_chg_cur_reg are unsigned variables. The less-than-zero comparison of an unsigned value is never true. Remove these checks. Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- drivers/power/supply/rk817_charger.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)