All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Simon Horman <horms+renesas@verge.net.au>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Lee Jones <lee.jones@linaro.org>
Cc: linux-renesas-soc@vger.kernel.org, linux-pwm@vger.kernel.org,
	kernel@pengutronix.de
Subject: [PATCH v2 6/6] pwm: renesas-tpu: Improve precision of period and duty_cycle calculation
Date: Wed, 20 Apr 2022 14:12:40 +0200	[thread overview]
Message-ID: <20220420121240.67781-7-u.kleine-koenig@pengutronix.de> (raw)
In-Reply-To: <20220420121240.67781-1-u.kleine-koenig@pengutronix.de>

Dividing by the result of a division looses precision. Consider for example
clk_rate = 33000000 and period_ns = 500001. Then

	clk_rate / (NSEC_PER_SEC / period_ns)

has the exact value 16500.033, but in C this evaluates to 16508. It gets
worse for even bigger values of period_ns, so with period_ns = 500000001,
the exact result is 16500000.033 while in C we get 33000000.

For that reason use

	clk_rate * period_ns / NSEC_PER_SEC

instead which doesn't suffer from this problem. To ensure this doesn't
overflow add a safeguard check for clk_rate.

Note that duty > period can never happen, so the respective check can be
dropped.

Incidentally this fixes a division by zero if period_ns > NSEC_PER_SEC.
Another side effect is that values bigger than INT_MAX for period and
duty_cyle are not wrongly discarded any more.

Fixes: 99b82abb0a35 ("pwm: Add Renesas TPU PWM driver")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pwm/pwm-renesas-tpu.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/pwm/pwm-renesas-tpu.c b/drivers/pwm/pwm-renesas-tpu.c
index 60ba7cf275c7..9e6978c39788 100644
--- a/drivers/pwm/pwm-renesas-tpu.c
+++ b/drivers/pwm/pwm-renesas-tpu.c
@@ -242,20 +242,29 @@ static void tpu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 }
 
 static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-			  int duty_ns, int period_ns, bool enabled)
+			  u64 duty_ns, u64 period_ns, bool enabled)
 {
 	struct tpu_pwm_device *tpd = pwm_get_chip_data(pwm);
 	struct tpu_device *tpu = to_tpu_device(chip);
 	unsigned int prescaler;
 	bool duty_only = false;
 	u32 clk_rate;
-	u32 period;
+	u64 period;
 	u32 duty;
 	int ret;
 
 	clk_rate = clk_get_rate(tpu->clk);
+	if (unlikely(clk_rate > NSEC_PER_SEC)) {
+		/*
+		 * This won't happen in the nearer future, so this is only a
+		 * safeguard to prevent the following calculation from
+		 * overflowing. With this clk_rate * period_ns / NSEC_PER_SEC is
+		 * not greater than period_ns and so fits into an u64.
+		 */
+		return -EINVAL;
+	}
 
-	period = clk_rate / (NSEC_PER_SEC / period_ns);
+	period = mul_u64_u64_div_u64(clk_rate, period_ns, NSEC_PER_SEC);
 
 	/*
 	 * Find the minimal prescaler in [0..3] such that
@@ -292,18 +301,15 @@ static int tpu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	period >>= 2 * prescaler;
 
-	if (duty_ns) {
-		duty = (clk_rate >> 2 * prescaler)
-		     / (NSEC_PER_SEC / duty_ns);
-		if (duty > period)
-			return -EINVAL;
-	} else {
+	if (duty_ns)
+		duty = mul_u64_u64_div_u64(clk_rate, duty_ns,
+					   (u64)NSEC_PER_SEC << (2 * prescaler));
+	else
 		duty = 0;
-	}
 
 	dev_dbg(&tpu->pdev->dev,
 		"rate %u, prescaler %u, period %u, duty %u\n",
-		clk_rate, 1 << (2 * prescaler), period, duty);
+		clk_rate, 1 << (2 * prescaler), (u32)period, duty);
 
 	if (tpd->prescaler == prescaler && tpd->period == period)
 		duty_only = true;
-- 
2.35.1


  parent reply	other threads:[~2022-04-20 12:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20 12:12 [PATCH v2 0/6] pwm: renesas-tpu: Various improvements Uwe Kleine-König
2022-04-20 12:12 ` [PATCH v2 1/6] pwm: renesas-tpu: Make use of dev_err_probe() Uwe Kleine-König
2022-04-20 12:12 ` [PATCH v2 2/6] pwm: renesas-tpu: Make use of devm functions Uwe Kleine-König
2022-04-20 12:12 ` [PATCH v2 3/6] pwm: renesas-tpu: Implement .apply() callback Uwe Kleine-König
2022-04-20 18:07   ` Geert Uytterhoeven
2022-04-20 12:12 ` [PATCH v2 4/6] pwm: renesas-tpu: Rename variables to match the usual naming Uwe Kleine-König
2022-04-20 12:25   ` Geert Uytterhoeven
2022-04-20 12:12 ` [PATCH v2 5/6] pwm: renesas-tpu: Improve maths to compute register settings Uwe Kleine-König
2022-04-20 18:08   ` Geert Uytterhoeven
2022-06-19 20:03     ` Uwe Kleine-König
2022-04-20 12:12 ` Uwe Kleine-König [this message]
2022-04-20 18:10   ` [PATCH v2 6/6] pwm: renesas-tpu: Improve precision of period and duty_cycle calculation Geert Uytterhoeven
2022-04-20 18:12 ` [PATCH v2 0/6] pwm: renesas-tpu: Various improvements Geert Uytterhoeven
2022-05-20 14:19 ` Thierry Reding

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=20220420121240.67781-7-u.kleine-koenig@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=geert+renesas@glider.be \
    --cc=horms+renesas@verge.net.au \
    --cc=kernel@pengutronix.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=thierry.reding@gmail.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.