[PATCH v2] pwm: Soften potential loss of precision in compat code

[PATCH v2] pwm: Soften potential loss of precision in compat code

[Uwe Kleine-König]

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.
+                         */
+			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;
 
-- 
2.30.1

Re: [PATCH v2] pwm: Soften potential loss of precision in compat code

[Guru Das Srinagesh]

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.