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