[Intel-gfx] [PATCH v2 04/15] pwm: lpss: Fix off by one error in base_unit math in pwm_lpss_prepare()

[Andy Shevchenko]

On Mon, Jun 08, 2020 at 01:13:01PM +0200, Hans de Goede wrote:
> On 6/8/20 5:55 AM, Andy Shevchenko wrote:
> > On Sun, Jun 07, 2020 at 08:18:29PM +0200, Hans de Goede wrote:
> > > 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.
> > > 
> > > So assuming e.g. a 16 bit counter this means that if base_unit is set to 1,
> > > after 65535 input clock-cycles the counter has been increased from 0 to
> > > 65535 and it will overflow on the next cycle, so it will overflow after
> > > every 65536 clock cycles and thus the calculations done in
> > > pwm_lpss_prepare() should use 65536 and not 65535.
> > > 
> > > This commit fixes this. Note this also aligns the calculations in
> > > pwm_lpss_prepare() with those in pwm_lpss_get_state().
> > 
> > This one sounds like a bug which I have noticed on Broxton (but thought as a
> > hardware issue). In any case it has to be tested on various platforms to see
> > how it affects on them.
> 
> If you like at the datasheet / read my commit description then it
> becomes obvious that because of the way the PWM controller works that
> it takes the full 2^(base-unit-bits) for the counter to overflow,
> not 2^(base-unit-bits) - 1. This will make a difference of a factor
> 65535/65536 in the output frequency which will be tricky to measure.
> 
> IOW I'm not sure we can really test if this helps, but it is
> obviously the right thing to do and it aligns the pwm_apply code
> with the pwm_get_state code which already does not have the - 1.

Yes. It seems I did a mistake in the commit
684309e5043e ("pwm: lpss: Avoid potential overflow of base_unit")
when missed multiplication.

For this one

Reviewed-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko