| Message ID | 20200323065318.16533-3-rayagonda.kokatanur@broadcom.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Handle return value and remove unnecessary check | expand |
On Mon, Mar 23, 2020 at 12:23:18PM +0530, Rayagonda Kokatanur wrote: > Variable 'duty' is u32. Hence the less-than zero > comparison is never true, remove the check. How did you find that one? I assume it was a compiler warning you fixed here? In this case quoting the warning improves the commit log. Also the commit log suggests that IPROC_PWM_DUTY_CYCLE_MIN is zero. Maybe mentioning that explicitly is a nice addition, too. > Fixes: daa5abc41c80 ("pwm: Add support for Broadcom iProc PWM controller") > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > --- > drivers/pwm/pwm-bcm-iproc.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/pwm/pwm-bcm-iproc.c b/drivers/pwm/pwm-bcm-iproc.c > index 8bbd2a04fead..1bb66721f985 100644 > --- a/drivers/pwm/pwm-bcm-iproc.c > +++ b/drivers/pwm/pwm-bcm-iproc.c > @@ -149,8 +149,7 @@ static int iproc_pwmc_apply(struct pwm_chip *chip, struct pwm_device *pwm, > value = rate * state->duty_cycle; > duty = div64_u64(value, div); > > - if (period < IPROC_PWM_PERIOD_MIN || > - duty < IPROC_PWM_DUTY_CYCLE_MIN) > + if (period < IPROC_PWM_PERIOD_MIN) > return -EINVAL; Best regards Uwe
On Mon, Mar 23, 2020 at 1:58 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > On Mon, Mar 23, 2020 at 12:23:18PM +0530, Rayagonda Kokatanur wrote: > > Variable 'duty' is u32. Hence the less-than zero > > comparison is never true, remove the check. > > How did you find that one? I assume it was a compiler warning you fixed > here? In this case quoting the warning improves the commit log. Its reported by internal coverity tool. > > Also the commit log suggests that IPROC_PWM_DUTY_CYCLE_MIN is zero. > Maybe mentioning that explicitly is a nice addition, too. Okay, will add this to commit message. Thanks. > > > Fixes: daa5abc41c80 ("pwm: Add support for Broadcom iProc PWM controller") > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> > > --- > > drivers/pwm/pwm-bcm-iproc.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/pwm/pwm-bcm-iproc.c b/drivers/pwm/pwm-bcm-iproc.c > > index 8bbd2a04fead..1bb66721f985 100644 > > --- a/drivers/pwm/pwm-bcm-iproc.c > > +++ b/drivers/pwm/pwm-bcm-iproc.c > > @@ -149,8 +149,7 @@ static int iproc_pwmc_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > value = rate * state->duty_cycle; > > duty = div64_u64(value, div); > > > > - if (period < IPROC_PWM_PERIOD_MIN || > > - duty < IPROC_PWM_DUTY_CYCLE_MIN) > > + if (period < IPROC_PWM_PERIOD_MIN) > > return -EINVAL; > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
diff --git a/drivers/pwm/pwm-bcm-iproc.c b/drivers/pwm/pwm-bcm-iproc.c index 8bbd2a04fead..1bb66721f985 100644 --- a/drivers/pwm/pwm-bcm-iproc.c +++ b/drivers/pwm/pwm-bcm-iproc.c @@ -149,8 +149,7 @@ static int iproc_pwmc_apply(struct pwm_chip *chip, struct pwm_device *pwm, value = rate * state->duty_cycle; duty = div64_u64(value, div); - if (period < IPROC_PWM_PERIOD_MIN || - duty < IPROC_PWM_DUTY_CYCLE_MIN) + if (period < IPROC_PWM_PERIOD_MIN) return -EINVAL; if (period <= IPROC_PWM_PERIOD_MAX &&
Variable 'duty' is u32. Hence the less-than zero comparison is never true, remove the check. Fixes: daa5abc41c80 ("pwm: Add support for Broadcom iProc PWM controller") Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com> --- drivers/pwm/pwm-bcm-iproc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)