Hi Uwe,
On Tue, Sep 14 2021, Uwe Kleine-König wrote:
> On Mon, Aug 30, 2021 at 02:46:25PM +0300, Baruch Siach wrote:
>> + for (; pre_div <= IPQ_PWM_MAX_DIV; pre_div++) {
>> + pwm_div = DIV64_U64_ROUND_UP(period_ns * rate,
>> + (u64)NSEC_PER_SEC * (pre_div + 1));
>> + pwm_div--;
>
> Can it happen that pwm_div is zero before it is decreased by one? Also
> you need to round down here; with rounding up the resulting period is
> bigger than the requested period (unless the division yields an exact
> integer).
I followed your round down advice on v9, but it turned out to be
wrong. Round down means that the divider is smaller so the period is
larger. This means that 'diff' below can not be positive. So only exact
match (diff == 0) works. When there is no exact match, best_* values are
left in their initial setting.
I'll fix that in v10 along with another bug I introduced in v9.
baruch
>> + if (pre_div > pwm_div)
>> + break;
>
> A comment here why we can end the search would be good.
>
>> + /*
>> + * Make sure we can do 100% duty cycle where
>> + * hi_dur == pwm_div + 1
>> + */
>> + if (pwm_div > IPQ_PWM_MAX_DIV - 1)
>> + continue;
>> +
>> + diff = ((uint64_t)freq * (pre_div + 1) * (pwm_div + 1))
>> + - (uint64_t)rate;
>> +
>> + if (diff < 0) /* period larger than requested */
>> + continue;
>> + if (diff == 0) { /* bingo */
>> + best_pre_div = pre_div;
>> + best_pwm_div = pwm_div;
>> + break;
>> + }
>> + if (diff < min_diff) {
>> + min_diff = diff;
>> + best_pre_div = pre_div;
>> + best_pwm_div = pwm_div;
>> + }
>> + }
>> +
>> + /* config divider values for the closest possible frequency */
>> + config_div_and_duty(pwm, best_pre_div, best_pwm_div,
>> + rate, duty_ns, state->enabled);
>> +
>> + return 0;
>> +}
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@xxxxxxxxxx - tel: +972.52.368.4656, http://www.tkos.co.il -
