[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230517102030.b4nyo2dmpfl7v7fk@pengutronix.de>
Date: Wed, 17 May 2023 12:20:30 +0200
From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To: Conor Dooley <conor.dooley@...rochip.com>
Cc: Thierry Reding <thierry.reding@...il.com>,
Daire McNamara <daire.mcnamara@...rochip.com>,
linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v17 1/2] pwm: add microchip soft ip corePWM driver
Hello Conor,
I found one remaining issue:
On Fri, Apr 21, 2023 at 10:27:09AM +0100, Conor Dooley wrote:
> +static u64 mchp_core_pwm_calc_duty(const struct pwm_state *state, u64 clk_rate,
> + u8 prescale, u8 period_steps)
> +{
> + u64 duty_steps, tmp;
> +
> + /*
> + * Calculate the duty cycle in multiples of the prescaled period:
> + * duty_steps = duty_in_ns / step_in_ns
> + * step_in_ns = (prescale * NSEC_PER_SEC) / clk_rate
> + * The code below is rearranged slightly to only divide once.
> + */
> + tmp = (prescale + 1) * NSEC_PER_SEC;
If prescale > 4 this overflows on 32bit archs, doesn't it?
(I think prescale + 1 is promoted to unsigned int, then the
multiplication is done and only then the range is extended to u64.
> + duty_steps = mul_u64_u64_div_u64(state->duty_cycle, clk_rate, tmp);
> +
> + return duty_steps;
> +}
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists