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


Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to