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

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

[Uwe Kleine-König]

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@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,

Compared to (implicit) v1
(https://lore.kernel.org/linux-pwm/20261015085336.4181137-2-u.kleine-koenig@baylibre.com):
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

 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

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

[Karl Mehltretter]

On Tue, Oct 20, 2026 at 03:39:38PM +0200, Uwe Kleine-König wrote:
> Compared to (implicit) v1
> (https://lore.kernel.org/linux-pwm/20261015085336.4181137-2-u.kleine-koenig@baylibre.com):
> 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.

Tested on Jetson Nano (Tegra210, 7000a000.pwm channel 0, PWM_CSR_0 at
0x7000a000) on v7.3-rc2 (5b0764a53235), starting from period = 10000,
duty_cycle = 5000, enable = 1 (PWM_CSR_0 = 0x8080003e):

- Unpatched: writing period = 4294987776 (0x100005000) succeeds and
  truncates period_ns to 0x5000 (20480 ns), programming PWM_CSR_0 to
  0x80fa001f. Setting period = 2415919104 (0x90000000) and duty_cycle =
  2147483648 (0x80000000) casts duty_ns to a negative int and programs
  PWM_CSR_0 to 0x999a001f.
- With v2 applied: both out-of-range writes fail with -EINVAL and
  PWM_CSR_0 stays 0x8080003e.

Tested-by: Karl Mehltretter <kmehltretter@gmail.com>

Thanks,
Karl

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@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,
> 
> Compared to (implicit) v1
> (https://lore.kernel.org/linux-pwm/20261015085336.4181137-2-u.kleine-koenig@baylibre.com):
> 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@nvidia.com>
Tested-by: Mikko Perttunen <mperttunen@nvidia.com> # 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
> 
> 

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

[Uwe Kleine-König]

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

Hello,

On Tue, Oct 20, 2026 at 03:39:38PM +0200, 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@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>

pushed to

https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-nexxt

which I will rebase to v7.4-rc1 once that's available and then put in
my for-next branch as 7.5 material.

Best regards
Uwe

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