Re: [PATCH v8 09/17] pwm: crc: Fix off-by-one error in the clock-divider calculations

Thierry Reding
2020-08-31T11:29:40Z

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On Sun, Aug 30, 2020 at 02:57:45PM +0200, Hans de Goede wrote:
> The CRC PWM controller has a clock-divider which divides the clock with
> a value between 1-128. But as can seen from the PWM_DIV_CLK_xxx
> defines, this range maps to a register value of 0-127.
>
> So after calculating the clock-divider we must subtract 1 to get the
> register value, unless the requested frequency was so high that the
> calculation has already resulted in a (rounded) divider value of 0.
>
> Note that before this fix, setting a period of PWM_MAX_PERIOD_NS which
> corresponds to the max. divider value of 128 could have resulted in a
> bug where the code would use 128 as divider-register value which would
> have resulted in an actual divider value of 0 (and the enable bit being
> set). A rounding error stopped this bug from actually happen. This
> same rounding error means that after the subtraction of 1 it is impossible
> to set the divider to 128. Also bump PWM_MAX_PERIOD_NS by 1 ns to allow
> setting a divider of 128 (register-value 127).
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v3:
> - Introduce crc_pwm_calc_clk_div() here instead of later in the patch-set
> to reduce the amount of churn in the patch-set a bit
> ---
> drivers/pwm/pwm-crc.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)

Acked-by: Thierry Reding <thierry.reding@gmail.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]