* [PATCH] pwm: soften potential loss of precision in compat code
@ 2021-03-08 9:23 Uwe Kleine-König
2021-03-11 21:06 ` Guru Das Srinagesh
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2021-03-08 9:23 UTC (permalink / raw)
To: Thierry Reding, Lee Jones; +Cc: linux-pwm, kernel
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>
---
drivers/pwm/core.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index c4d5c0667137..4058d3c86a45 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -599,9 +599,20 @@ 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;
+
+ if (state->period < INT_MAX)
+ period = state->period;
+ else
+ period = INT_MAX;
+
+ if (state->duty_cycle < INT_MAX)
+ duty_cycle = state->duty_cycle;
+ else
+ duty_cycle = INT_MAX;
+
err = chip->ops->config(pwm->chip, pwm,
- state->duty_cycle,
- state->period);
+ duty_cycle, period);
if (err)
return err;
--
2.30.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] pwm: soften potential loss of precision in compat code
2021-03-08 9:23 [PATCH] pwm: soften potential loss of precision in compat code Uwe Kleine-König
@ 2021-03-11 21:06 ` Guru Das Srinagesh
2021-03-12 7:12 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Guru Das Srinagesh @ 2021-03-11 21:06 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Thierry Reding, Lee Jones, linux-pwm, kernel
On Mon, Mar 08, 2021 at 10:23:22AM +0100, Uwe Kleine-König wrote:
> if (state->period != pwm->state.period ||
> state->duty_cycle != pwm->state.duty_cycle) {
> + int duty_cycle, period;
> +
> + if (state->period < INT_MAX)
> + period = state->period;
> + else
> + period = INT_MAX;
Using a MIN() macro here might improve readability:
period = MIN(state->period, INT_MAX);
> +
> + if (state->duty_cycle < INT_MAX)
> + duty_cycle = state->duty_cycle;
> + else
> + duty_cycle = INT_MAX;
Same here.
> +
> err = chip->ops->config(pwm->chip, pwm,
> - state->duty_cycle,
> - state->period);
> + duty_cycle, period);
> if (err)
> return err;
>
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pwm: soften potential loss of precision in compat code
2021-03-11 21:06 ` Guru Das Srinagesh
@ 2021-03-12 7:12 ` Uwe Kleine-König
2021-03-15 1:53 ` Guru Das Srinagesh
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2021-03-12 7:12 UTC (permalink / raw)
To: Guru Das Srinagesh; +Cc: linux-pwm, Thierry Reding, Lee Jones, kernel
[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]
On Thu, Mar 11, 2021 at 01:06:39PM -0800, Guru Das Srinagesh wrote:
> On Mon, Mar 08, 2021 at 10:23:22AM +0100, Uwe Kleine-König wrote:
> > if (state->period != pwm->state.period ||
> > state->duty_cycle != pwm->state.duty_cycle) {
> > + int duty_cycle, period;
> > +
> > + if (state->period < INT_MAX)
> > + period = state->period;
> > + else
> > + period = INT_MAX;
>
> Using a MIN() macro here might improve readability:
> period = MIN(state->period, INT_MAX);
Which MIN macro. There are 17 defined in the kernel and none of them in
a header that could be sensibly included by this code.
There are some helpers in <linux/minmax.h> which would result in:
period = min_t(u64, state->period, INT_MAX)
or
period = min(state->period, (u64)INT_MAX);
. I don't feel strong here, my initial variant needs more vertical space
but might be a tad easier to understand. In retrospect I'd say that
adding a comment would be more imporant than how to actually calculate
the value, something like:
/*
* 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.
*/
Sounds sensible?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pwm: soften potential loss of precision in compat code
2021-03-12 7:12 ` Uwe Kleine-König
@ 2021-03-15 1:53 ` Guru Das Srinagesh
0 siblings, 0 replies; 4+ messages in thread
From: Guru Das Srinagesh @ 2021-03-15 1:53 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: linux-pwm, Thierry Reding, Lee Jones, kernel
On Fri, Mar 12, 2021 at 08:12:33AM +0100, Uwe Kleine-König wrote:
> On Thu, Mar 11, 2021 at 01:06:39PM -0800, Guru Das Srinagesh wrote:
> > On Mon, Mar 08, 2021 at 10:23:22AM +0100, Uwe Kleine-König wrote:
> > > if (state->period != pwm->state.period ||
> > > state->duty_cycle != pwm->state.duty_cycle) {
> > > + int duty_cycle, period;
> > > +
> > > + if (state->period < INT_MAX)
> > > + period = state->period;
> > > + else
> > > + period = INT_MAX;
> >
> > Using a MIN() macro here might improve readability:
> > period = MIN(state->period, INT_MAX);
>
> Which MIN macro. There are 17 defined in the kernel and none of them in
> a header that could be sensibly included by this code.
>
> There are some helpers in <linux/minmax.h> which would result in:
>
> period = min_t(u64, state->period, INT_MAX)
>
> or
>
> period = min(state->period, (u64)INT_MAX);
>
> . I don't feel strong here, my initial variant needs more vertical space
> but might be a tad easier to understand. In retrospect I'd say that
> adding a comment would be more imporant than how to actually calculate
> the value, something like:
>
> /*
> * 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.
> */
>
> Sounds sensible?
Yes, I agree.
Guru Das.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-15 1:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 9:23 [PATCH] pwm: soften potential loss of precision in compat code Uwe Kleine-König
2021-03-11 21:06 ` Guru Das Srinagesh
2021-03-12 7:12 ` Uwe Kleine-König
2021-03-15 1:53 ` Guru Das Srinagesh
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.