Re: [PATCH v6 11/11] pwm: Add Raspberry Pi Firmware based PWM bus

Hi Uwe, thanks for the review.

On Tue, 2021-01-12 at 10:18 +0100, Uwe Kleine-König wrote:

[...]

quoted

+                duty_cycle = DIV_ROUND_CLOSEST_ULL(state->duty_cycle * RPI_PWM_MAX_DUTY,
+					           RPI_PWM_PERIOD_NS);

... and round down here.

Just to be sure: writing RPI_PWM_MAX_DUTY (i.e. 255) yields 100% duty
cycle, right?

Yes, at 255 the signal is flat.

quoted

+        else
+                duty_cycle = RPI_PWM_MAX_DUTY;
+
+	if (duty_cycle == rpipwm->duty_cycle)
+		return 0;
+
+	ret = raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_CUR_DUTY_REG,
+					   duty_cycle);
+	if (ret) {
+		dev_err(chip->dev, "Failed to set duty cycle: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * This sets the default duty cycle after resetting the board, we
+	 * updated it every time to mimic Raspberry Pi's downstream's driver
+	 * behaviour.
+	 */
+	ret = raspberrypi_pwm_set_property(rpipwm->firmware, RPI_PWM_DEF_DUTY_REG,
+					   duty_cycle);
+	if (ret) {
+		dev_err(chip->dev, "Failed to set default duty cycle: %d\n", ret);
+		return ret;
+	}
+
+        rpipwm->duty_cycle = duty_cycle;

Please use tabs for indention. (The general hint is to use checkpatch
which (I hope) tells you about problems like this.)

Sorry for that.

I took note of the rest of comments and will update the code.

Regards,
Nicolas