Re: [PATCH] pwm: imx27: fix overflow for bigger periods

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]

On Mon, Dec 07, 2020 at 03:13:24PM +0100, Uwe Kleine-König wrote:
> The second parameter of do_div is an u32 and NSEC_PER_SEC * prescale
> overflows this for bigger periods. Assuming the usual pwm input clk rate
> of 66 MHz this happens starting at requested period > 606060 ns.
> 
> Splitting the division into two operations doesn't loose any precision.
> It doesn't need to be feared that c / NSEC_PER_SEC doesn't fit into the
> unsigned long variable "duty_cycles" because in this case the assignment
> above to period_cycles would already have been overflowing as
> period >= duty_cycle and then the calculation is moot anyhow.
> 
> Fixes: aef1a3799b5c ("pwm: imx27: Fix rounding behavior")
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
> ---
> Hello,
> 
> for a similar patch I said "that looses more precision than I thought at
> first", but I think this was wrong. And if it looses precision the same
> applies to the calculation of period_cycles which uses the same
> operations.
> 
> I'm a bit at doubt how urgent this patch is. The regression was
> introduced in 5.8-rc1.

Well, given that this has been broken in v5.8 and v5.9 and was only
reported a couple of days ago, it doesn't seem like this is very urgent.
At the same time, it's a regression, so we should get this fixed as soon
as possible.

That said, I'm reluctant to send this to Linus without a bit of soak
time in linux-next. So let me put it in linux-next for now and if
there's an rc8 I'll send a PR next week. Otherwise it'll get picked up
into an early stable release and that's probably okay as well.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]