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

[Mikko Perttunen]

On Tuesday, October 20, 2026 10:39 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. While touching these, also limit the
> scope of the required_clk_rate variable.
> 
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> 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@xxxxxxxxxxxx>
> ---
> Hello,
> 
> Compared to (implicit) v1
> (https://lore.kernel.org/linux-pwm/20261015085336.4181137-2-u.kleine-koenig@xxxxxxxxxxxx):
> this adds more adaptions to handle the bigger variable types, as found
> by both Sashiko and Mikko. I didn't think a lot about the
> required_clk_rate calculations, but I guess the saturating math I
> implemented should be fine.
> 
> Best regards
> Uwe

Reviewed-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
Tested-by: Mikko Perttunen <mperttunen@xxxxxxxxxx> # Jetson AGX Thor (fan)

> 
>  drivers/pwm/pwm-tegra.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 5cdbe120ba2d..6d468a673ded 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -138,22 +138,23 @@ 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;
> +	u64 c, rate;
>  	u32 val = 0;
>  	int err;
>  
>  	/*
>  	 * 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 given that we have
> +	 * duty_ns ≤ period_ns above.
> +	 */
>  	val = (u32)c << TEGRA_PWM_DUTY_SHIFT;
>  
>  	/*
> @@ -176,6 +177,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  	 * dynamically (e.g. Tegra186 or Tegra194).
>  	 */
>  	if (pc->soc->num_channels == 1) {
> +		u64 required_clk_rate;
>  		/*
>  		 * Rate is multiplied with TEGRA_PWM_DEPTH so that it matches
>  		 * with the maximum possible rate that the controller can
> @@ -187,10 +189,18 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  		 * source clock rate as required_clk_rate, PWM controller will
>  		 * be able to configure the requested period.
>  		 */
> -		required_clk_rate = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC * TEGRA_PWM_DEPTH,
> -						     period_ns);
> +		required_clk_rate = mul_u64_u64_div_u64_roundup(NSEC_PER_SEC, TEGRA_PWM_DEPTH,
> +								period_ns);
>  
> -		if (required_clk_rate > clk_round_rate(pc->clk, required_clk_rate))
> +		/*
> +		 * clk_round_rate() and dev_pm_opp_set_rate() both take an unsigned
> +		 * long as frequency parameter, so cap required_clk_rate
> +		 * accordingly.
> +		 */
> +		if (required_clk_rate > ULONG_MAX)
> +			required_clk_rate = ULONG_MAX;
> +
> +		if (required_clk_rate > clk_round_rate(pc->clk, required_clk_rate)) {
>  			/*
>  			 * required_clk_rate is a lower bound for the input
>  			 * rate; for lower rates there is no value for PWM_SCALE
> @@ -201,6 +211,10 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  			 */
>  			required_clk_rate *= 2;
>  
> +			if (required_clk_rate > ULONG_MAX)
> +				required_clk_rate = ULONG_MAX;
> +		}
> +
>  		err = dev_pm_opp_set_rate(pwmchip_parent(chip), required_clk_rate);
>  		if (err < 0)
>  			return -EINVAL;
> -- 
> 2.56.0.rc1
> 
>