All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.