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:

quoted

+	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

quoted

+		if (pre_div > pwm_div)
+			break;

A comment here why we can end the search would be good.

quoted

+		/*
+		 * 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@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -