| Message ID | 20210312212119.1342666-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] pwm: Soften potential loss of precision in compat code | expand |
Hi Uwe, Just a minor comment: On Fri, Mar 12, 2021 at 10:21:19PM +0100, Uwe Kleine-König wrote: > The legacy callback .config() only uses int for period and duty_cycle > while the corresponding values in struct pwm_state are u64. To prevent > that a value bigger than INT_MAX is discarded to a very small value, > explicitly check for big values and pass INT_MAX instead of discarding. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > compared to (implicit) v1 I added a comment and used min instead of open > coding the calculation. > > Best regards > Uwe > > drivers/pwm/core.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index 4b3779d58c5a..7d0266bc5fcb 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -605,9 +605,18 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state) > > if (state->period != pwm->state.period || > state->duty_cycle != pwm->state.duty_cycle) { > + int duty_cycle, period; > + > + /* > + * The legacy callbacks use only (signed!) int for > + * period and duty_cycle compared to u64 in struct > + * pwm_state. So clamp the values to INT_MAX. > + */ Minor: misaligned end of comment block. Other than that, looks good to me. You may add my Acked-by to your v3 once this is fixed. Acked-by: Guru Das Srinagesh <gurus@codeaurora.org> Guru Das.
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 4b3779d58c5a..7d0266bc5fcb 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -605,9 +605,18 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state) if (state->period != pwm->state.period || state->duty_cycle != pwm->state.duty_cycle) { + int duty_cycle, period; + + /* + * The legacy callbacks use only (signed!) int for + * period and duty_cycle compared to u64 in struct + * pwm_state. So clamp the values to INT_MAX. + */ + period = min(state->period, (u64)INT_MAX); + duty_cycle = min(state->duty_cycle, (u64)INT_MAX); + err = chip->ops->config(pwm->chip, pwm, - state->duty_cycle, - state->period); + duty_cycle, period); if (err) return err;
The legacy callback .config() only uses int for period and duty_cycle while the corresponding values in struct pwm_state are u64. To prevent that a value bigger than INT_MAX is discarded to a very small value, explicitly check for big values and pass INT_MAX instead of discarding. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Hello, compared to (implicit) v1 I added a comment and used min instead of open coding the calculation. Best regards Uwe drivers/pwm/core.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)