Re: [PATCH] pwm: tegra: Fix value discarding for duty and period

[Mikko Perttunen <mperttunen@nvidia.com>]

On Thursday, October 15, 2026 5:53 PM Uwe Kleine-König wrote:
> tegra_pwm_config() is called with u64 values passed for duty_ns and
> period_ns in tegra_pwm_apply(). Make these parameters u64 instead of int
> to not lose the upper bits for bigger values. If such a discard happens
> the values that are written to hardware become wrong, or (if the
> discarded period value becomes 0) a division by zero happens.
> 
> Adapt the math functions accordingly to not let the now potentially
> bigger values result in overflows.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/bug/linux-5c5b97d6-4da6-4d06-9dc7-957419dcd832
> Fixes: fd3ddd4355c0 ("pwm: tegra: Implement .apply() callback")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> Hello,
> 
> actually the Fixes: line isn't quite accurate. The problem that the
> arguments are cropped to int from u64 exists since v4.7-rc1 (commit
> 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic updates"))
> in the pwm core, but the blamed commit above is the commit where this
> problem was moved to the pwm-tegra driver and thus it's fixable there.
> The problem in the core doesn't exist any more since commit 0829c35dc534
> ("pwm: Drop support for legacy drivers") in v6.0-rc1.
> 
> Note this patch changes the rounding of the duty cycle (from somewhat
> round closest to somewhat round down). It would not be hard to keep the
> behaviour by introducing a variant of mul_u64_u64_div_u64() that rounds
> closest, but the rounding is spoiled anyhow by later rounding of the
> period. To actually fix that it would be best to convert the driver to
> the waveform callbacks.
> 
> Example calculation: Let's assume an input clock (pc->clk_rate) of
> 100000 Hz and a request with period_ns = 230399744 and
> duty_ns = 128249857. To translate that into register settings the
> DUTY_SHIFT values will be round down with both approaches and result in
> an actual duty_cycle of 127658000 ns (Δ = -591857 ns) while 128557000 ns
> (Δ = 307143 ns) would also be possible.
> 
> Best regards
> Uwe
> 
>  drivers/pwm/pwm-tegra.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 5cdbe120ba2d..8b5ed7998fdd 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -138,22 +138,21 @@ static inline void tegra_pwm_writel(struct pwm_device *pwm, unsigned int offset,
>  }
>  
>  static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> -			    int duty_ns, int period_ns)
> +			    u64 duty_ns, u64 period_ns)
>  {
>  	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
> -	unsigned long long c = duty_ns;
>  	unsigned long rate, required_clk_rate;
>  	u32 val = 0;
>  	int err;
> +	u64 c;
>  
>  	/*
>  	 * Convert from duty_ns / period_ns to a fixed number of duty ticks
> -	 * per TEGRA_PWM_DEPTH cycles and make sure to round to the
> -	 * nearest integer during division.
> +	 * per TEGRA_PWM_DEPTH cycles.
>  	 */
> -	c *= TEGRA_PWM_DEPTH;
> -	c = DIV_ROUND_CLOSEST_ULL(c, period_ns);
> +	c = mul_u64_u64_div_u64(duty_ns, TEGRA_PWM_DEPTH, period_ns);
>  
> +	/* The cast here is fine as c <= TEGRA_PWM_DEPTH */
>  	val = (u32)c << TEGRA_PWM_DUTY_SHIFT;

The change looks good to me, but the required clock rate calculation 
further below should also be updated accordingly.

Thanks
Mikko

>  
>  	/*
> 
> base-commit: 00e917df050d79ac26ec700717680ad6421d2808
> -- 
> 2.56.0.rc1
> 
>