[PATCH] pwm: tegra: fix doubled output frequency due to divider truncation

[PATCH] pwm: tegra: fix doubled output frequency due to divider truncation

[Ola Chr. Vaage]

From: "Ola Chr. Vaage" <ola.christoffer.vage@scoutdi.com>

The PWM_SCALE frequency divider is computed by rounding down

	clk_rate * period_ns / (NSEC_PER_SEC << PWM_DUTY_WIDTH)

With dynamic clock scaling (Tegra186 and later), the driver doubles its
clock-rate request when the provider cannot meet it. The provider then
typically grants slightly less than the doubled request, the exact
divider lands just below 2, and the round-down truncates it to 1: the
output runs at double the requested frequency. On Tegra234 the BPMP
grants PWM clock rates as 408 MHz / N, so nearly every requested period
is affected: requesting 40000 ns produces 20078 ns, 100000 ns produces
50195 ns. The duty ratio is computed independently and stays correct,
which hides the problem from duty-only consumers such as backlights;
frequency-sensitive loads break.

Measured on a Jetson Orin NX board with the 45334 ns fan period used in
NVIDIA device trees:

	required_clk_rate = ceil((1e9 << 8) / 45334) = 5646977
	clk_round_rate() = 5589041 (408 MHz / 73) -> request doubled
	dev_pm_opp_set_rate(11293954) grants 11027028 (408 MHz / 37)
	divider = trunc(1.953) = 1 -> output period 23217 ns, 43.07 kHz

43 kHz is outside the 21-28 kHz band that 4-wire fan PWM inputs are
designed for. The fan on this board cannot start below duty 110/255 and
stalls below 88/255, so closed-loop fan control oscillates between a
stalled fan and full speed.

Round the divider to the closest integer instead. This bounds the
period error to half a divider step rather than a full one, and
restores what this configuration did before commit 8c193f4714df
("pwm: tegra: Optimize period calculation") switched the rounding from
closest to down: divider round(0.99) = 1 on the undoubled 5589041 Hz
grant, period 45802 ns, +1.0%. Between that commit and commit
5eccd0d9fabc ("pwm: tegra: Ensure the clock rate is not less than
needed") the same request failed with -EINVAL instead. Verified on the
Orin NX with this patch applied: PWM_SCALE reads 1 (21.54 kHz, +2.4%
period), the fan starts at duty <= 50/255 and sustains 80/255, and
closed-loop control is stable. There is no round-closest variant of
mul_u64_u64_div_u64(), so compute twice the quotient and round up the
halving.

Fixes: 5eccd0d9fabc ("pwm: tegra: Ensure the clock rate is not less than needed")
Signed-off-by: Ola Chr. Vaage <ola.christoffer.vage@scoutdi.com>
---
 drivers/pwm/pwm-tegra.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 172063b51d4..a5adc4f3ce6 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -163,9 +163,15 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		pc->clk_rate = clk_get_rate(pc->clk);
 	}
 
-	/* Consider precision in PWM_SCALE_WIDTH rate calculation */
-	rate = mul_u64_u64_div_u64(pc->clk_rate, period_ns,
+	/*
+	 * Consider precision in PWM_SCALE_WIDTH rate calculation. Round to
+	 * the closest integer: there is no round-closest variant of
+	 * mul_u64_u64_div_u64(), so compute twice the quotient and round up
+	 * the halving.
+	 */
+	rate = mul_u64_u64_div_u64(pc->clk_rate, 2 * (u64)period_ns,
 				   (u64)NSEC_PER_SEC << PWM_DUTY_WIDTH);
+	rate = DIV_ROUND_UP_ULL(rate, 2);
 
 	/*
 	 * Since the actual PWM divider is the register's frequency divider
-- 
2.54.0

Re: [PATCH] pwm: tegra: fix doubled output frequency due to divider truncation

[Uwe Kleine-König]

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

Hello Ola,

On Mon, Jul 13, 2026 at 01:15:41PM +0200, Ola Chr. Vaage wrote:
> From: "Ola Chr. Vaage" <ola.christoffer.vage@scoutdi.com>
> 
> The PWM_SCALE frequency divider is computed by rounding down
> 
> 	clk_rate * period_ns / (NSEC_PER_SEC << PWM_DUTY_WIDTH)
> 
> With dynamic clock scaling (Tegra186 and later), the driver doubles its
> clock-rate request when the provider cannot meet it. The provider then
> typically grants slightly less than the doubled request, the exact
> divider lands just below 2, and the round-down truncates it to 1: the
> output runs at double the requested frequency. On Tegra234 the BPMP
> grants PWM clock rates as 408 MHz / N, so nearly every requested period
> is affected: requesting 40000 ns produces 20078 ns, 100000 ns produces
> 50195 ns. The duty ratio is computed independently and stays correct,
> which hides the problem from duty-only consumers such as backlights;
> frequency-sensitive loads break.
> 
> Measured on a Jetson Orin NX board with the 45334 ns fan period used in
> NVIDIA device trees:
> 
> 	required_clk_rate = ceil((1e9 << 8) / 45334) = 5646977
> 	clk_round_rate() = 5589041 (408 MHz / 73) -> request doubled
> 	dev_pm_opp_set_rate(11293954) grants 11027028 (408 MHz / 37)
> 	divider = trunc(1.953) = 1 -> output period 23217 ns, 43.07 kHz
> 
> 43 kHz is outside the 21-28 kHz band that 4-wire fan PWM inputs are
> designed for. The fan on this board cannot start below duty 110/255 and
> stalls below 88/255, so closed-loop fan control oscillates between a
> stalled fan and full speed.
> 
> Round the divider to the closest integer instead. This bounds the
> period error to half a divider step rather than a full one, and
> restores what this configuration did before commit 8c193f4714df
> ("pwm: tegra: Optimize period calculation") switched the rounding from
> closest to down: divider round(0.99) = 1 on the undoubled 5589041 Hz
> grant, period 45802 ns, +1.0%. Between that commit and commit
> 5eccd0d9fabc ("pwm: tegra: Ensure the clock rate is not less than
> needed") the same request failed with -EINVAL instead. Verified on the
> Orin NX with this patch applied: PWM_SCALE reads 1 (21.54 kHz, +2.4%
> period), the fan starts at duty <= 50/255 and sustains 80/255, and
> closed-loop control is stable. There is no round-closest variant of
> mul_u64_u64_div_u64(), so compute twice the quotient and round up the
> halving.
> 
> Fixes: 5eccd0d9fabc ("pwm: tegra: Ensure the clock rate is not less than needed")
> Signed-off-by: Ola Chr. Vaage <ola.christoffer.vage@scoutdi.com>
> ---
>  drivers/pwm/pwm-tegra.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 172063b51d4..a5adc4f3ce6 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -163,9 +163,15 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  		pc->clk_rate = clk_get_rate(pc->clk);
>  	}
>  
> -	/* Consider precision in PWM_SCALE_WIDTH rate calculation */
> -	rate = mul_u64_u64_div_u64(pc->clk_rate, period_ns,
> +	/*
> +	 * Consider precision in PWM_SCALE_WIDTH rate calculation. Round to
> +	 * the closest integer: there is no round-closest variant of
> +	 * mul_u64_u64_div_u64(), so compute twice the quotient and round up
> +	 * the halving.
> +	 */
> +	rate = mul_u64_u64_div_u64(pc->clk_rate, 2 * (u64)period_ns,
>  				   (u64)NSEC_PER_SEC << PWM_DUTY_WIDTH);
> +	rate = DIV_ROUND_UP_ULL(rate, 2);
>  
>  	/*
>  	 * Since the actual PWM divider is the register's frequency divider

The usual behaviour for .apply() is to pick the largest possible period
not larger than the requested period (and similar for duty_cycle). The
tegra PWM driver doesn't do that and instead trys to pick a nearest
match. If you change how the configuration happens, the only acceptable
way is to migrate to the usual behaviour, still better, convert to the
waveform callbacks.

The driver also lacks a .get_state() callback and should not use
pwm_is_enabled(). Also calling tegra_pwm_config() discards bits from
state->duty_cycle and state->period if they are bigger than INT_MAX.

So there are some things to work on in that driver, but don't make the
clock selection not more complicated as it already is and as is
necessary.

Best regards
Uwe

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

Re: [PATCH] pwm: tegra: fix doubled output frequency due to divider truncation

[Uwe Kleine-König]

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

Hello again,

On Tue, Jul 14, 2026 at 11:14:58AM +0200, Uwe Kleine-König wrote:
> The usual behaviour for .apply() is to pick the largest possible period
> not larger than the requested period (and similar for duty_cycle). The
> tegra PWM driver doesn't do that and instead trys to pick a nearest
> match. If you change how the configuration happens, the only acceptable
> way is to migrate to the usual behaviour, still better, convert to the
> waveform callbacks.
> 
> The driver also lacks a .get_state() callback and should not use
> pwm_is_enabled(). Also calling tegra_pwm_config() discards bits from
> state->duty_cycle and state->period if they are bigger than INT_MAX.
> 
> So there are some things to work on in that driver, but don't make the
> clock selection not more complicated as it already is and as is
> necessary.

I started on that, but failed to Cc: you. Please see
https://lore.kernel.org/linux-pwm/cover.1784030076.git.ukleinek@kernel.org

Best regards
Uwe

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