Message ID | 20200830125753.230420-5-hdegoede@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API | expand |
On Sun, Aug 30, 2020 at 02:57:40PM +0200, Hans de Goede wrote: > 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> > 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(-) Acked-by: Thierry Reding <thierry.reding@gmail.com>
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;