Linux Tegra architecture development
 help / color / mirror / Atom feed
From: Mikko Perttunen <mperttunen@nvidia.com>
To: Thierry Reding <thierry.reding@kernel.org>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	linux-pwm@vger.kernel.org
Cc: linux-pwm@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, Sashiko <sashiko-bot@kernel.org>,
	"Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Subject: Re: [PATCH v2] pwm: tegra: Fix value discarding for duty and period
Date: Fri, 30 Oct 2026 14:54:03 +0900	[thread overview]
Message-ID: <Zd2GpvGWSu-r_sJpx49U1g@nvidia.com> (raw)
In-Reply-To: <20261020133938.191013-2-u.kleine-koenig@baylibre.com>

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





  parent reply	other threads:[~2026-10-30  5:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 13:39 [PATCH v2] pwm: tegra: Fix value discarding for duty and period Uwe Kleine-König
2026-10-27  6:54 ` Karl Mehltretter
2026-10-30  5:54 ` Mikko Perttunen [this message]
2026-10-30 10:04 ` Uwe Kleine-König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zd2GpvGWSu-r_sJpx49U1g@nvidia.com \
    --to=mperttunen@nvidia.com \
    --cc=jonathanh@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=thierry.reding@kernel.org \
    --cc=u.kleine-koenig@baylibre.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox