linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: "Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	"Len Brown" <lenb@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>,
	linux-pwm@vger.kernel.org,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel@lists.freedesktop.org,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	linux-acpi@vger.kernel.org
Subject: [PATCH v10 04/17] pwm: lpss: Add range limit check for the base_unit register value
Date: Thu,  3 Sep 2020 13:23:24 +0200	[thread overview]
Message-ID: <20200903112337.4113-5-hdegoede@redhat.com> (raw)
In-Reply-To: <20200903112337.4113-1-hdegoede@redhat.com>

When the user requests a high enough period ns value, then the
calculations in pwm_lpss_prepare() might result in a base_unit value of 0.

But according to the data-sheet the way the PWM controller works is that
each input clock-cycle the base_unit gets added to a N bit counter and
that counter overflowing determines the PWM output frequency. Adding 0
to the counter is a no-op. The data-sheet even explicitly states that
writing 0 to the base_unit bits will result in the PWM outputting a
continuous 0 signal.

When the user requestes a low enough period ns value, then the
calculations in pwm_lpss_prepare() might result in a base_unit value
which is bigger then base_unit_range - 1. Currently the codes for this
deals with this by applying a mask:

	base_unit &= (base_unit_range - 1);

But this means that we let the value overflow the range, we throw away the
higher bits and store whatever value is left in the lower bits into the
register leading to a random output frequency, rather then clamping the
output frequency to the highest frequency which the hardware can do.

This commit fixes both issues by clamping the base_unit value to be
between 1 and (base_unit_range - 1).

Fixes: 684309e5043e ("pwm: lpss: Avoid potential overflow of base_unit")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v5:
- Use clamp_val(... instead of clam_t(unsigned long long, ...

Changes in v3:
- Change upper limit of clamp to (base_unit_range - 1)
- Add Fixes tag
---
 drivers/pwm/pwm-lpss.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index 43b1fc634af1..da9bc3d10104 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -97,6 +97,8 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm,
 	freq *= base_unit_range;
 
 	base_unit = DIV_ROUND_CLOSEST_ULL(freq, c);
+	/* base_unit must not be 0 and we also want to avoid overflowing it */
+	base_unit = clamp_val(base_unit, 1, base_unit_range - 1);
 
 	on_time_div = 255ULL * duty_ns;
 	do_div(on_time_div, period_ns);
@@ -105,7 +107,6 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm,
 	orig_ctrl = ctrl = pwm_lpss_read(pwm);
 	ctrl &= ~PWM_ON_TIME_DIV_MASK;
 	ctrl &= ~((base_unit_range - 1) << PWM_BASE_UNIT_SHIFT);
-	base_unit &= (base_unit_range - 1);
 	ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT;
 	ctrl |= on_time_div;
 
-- 
2.28.0


  parent reply	other threads:[~2020-09-03 15:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-03 11:23 [PATCH v10 00/17] acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API Hans de Goede
2020-09-03 11:23 ` [PATCH v10 01/17] ACPI / LPSS: Resume Cherry Trail PWM controller in no-irq phase Hans de Goede
2020-09-03 11:23 ` [PATCH v10 02/17] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (at activation) Hans de Goede
2020-09-03 11:23 ` [PATCH v10 03/17] pwm: lpss: Fix off by one error in base_unit math in pwm_lpss_prepare() Hans de Goede
2020-09-03 11:23 ` Hans de Goede [this message]
2020-09-03 11:23 ` [PATCH v10 05/17] pwm: lpss: Add pwm_lpss_prepare_enable() helper Hans de Goede
2020-09-03 11:23 ` [PATCH v10 06/17] pwm: lpss: Make pwm_lpss_apply() not rely on existing hardware state Hans de Goede
2020-09-03 12:44   ` Andy Shevchenko
2020-09-03 11:23 ` [PATCH v10 07/17] pwm: lpss: Remove suspend/resume handlers Hans de Goede
2020-09-03 12:48   ` Andy Shevchenko
2020-09-03 12:56     ` Andy Shevchenko
2020-09-03 13:09       ` Hans de Goede
2020-09-03 11:23 ` [PATCH v10 08/17] pwm: crc: Fix period / duty_cycle times being off by a factor of 256 Hans de Goede
2020-09-03 11:23 ` [PATCH v10 09/17] pwm: crc: Fix off-by-one error in the clock-divider calculations Hans de Goede
2020-09-03 11:23 ` [PATCH v10 10/17] pwm: crc: Fix period changes not having any effect Hans de Goede
2020-09-03 11:23 ` [PATCH v10 11/17] pwm: crc: Enable/disable PWM output on enable/disable Hans de Goede
2020-09-03 11:23 ` [PATCH v10 12/17] pwm: crc: Implement apply() method to support the new atomic PWM API Hans de Goede
2020-09-03 11:23 ` [PATCH v10 13/17] pwm: crc: Implement get_state() method Hans de Goede
2020-09-03 11:23 ` [PATCH v10 14/17] drm/i915: panel: Add get_vbt_pwm_freq() helper Hans de Goede
2020-09-03 11:23 ` [PATCH v10 15/17] drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller Hans de Goede
2020-09-03 11:23 ` [PATCH v10 16/17] drm/i915: panel: Honor the VBT PWM min setting " Hans de Goede
2020-09-03 11:23 ` [PATCH v10 17/17] drm/i915: panel: Use atomic PWM API " Hans de Goede

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=20200903112337.4113-5-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=rodrigo.vivi@intel.com \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=ville.syrjala@linux.intel.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;
as well as URLs for NNTP newsgroup(s).