overflow and wrong timeout errors in pwm-atmel

[Uwe Kleine-König]

On Wed, Apr 21, 2021 at 01:03:36PM +0200, Uwe Kleine-König wrote:
> On Wed, Apr 21, 2021 at 11:26:08AM +0200, Uwe Kleine-König wrote:
> > With these three patches PWM_DEBUG is now happy. (At least I couldn't
> > trigger a warning any more. I think there are still a few problems with
> > integer overflows.)
> 
> BTW, setting the period to 138350580899 (with a clock rate of 133333333
> Hz) results in setting period=0 because
> 
> 	state->period * clkrate =
> 	138350580899 * 133333333 =
> 	40254751 (discarded from 18446744073749806367).

As a first remedy the following could be done:

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 38d86340201c..02d69fa5f7d2 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -199,6 +199,11 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
 	unsigned long long cycles = state->period;
 	int shift;
 
+	if (fls(cycles) + fls(clkrate) > 64) {
+		dev_err(chip->dev, "period to big to calculate HW parameters\n");
+		return -EINVAL;
+	}
+
 	/* Calculate the period cycles and prescale value */
 	cycles *= clkrate;
 	do_div(cycles, NSEC_PER_SEC);

Is this sensible? (Actually I'd prefer to just continue with

	period = (ULL(1) << (64 - fls(clkrate))) - 1

according to the motto to yield the highest possible period, but this
function has another error path that returns -EINVAL so this would be
inconsistent.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20210421/b1814cb8/attachment-0001.sig>