Hi Uwe, thanks for the review. On Tue, 2021-01-12 at 10:18 +0100, Uwe Kleine-König wrote: [...] > > + 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. > > + 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