On Mon, Jul 06, 2020 at 10:53:08PM +0200, Hans de Goede wrote: > Hi, > > Thank you for your review and sorry for the slow reply. No problem for me, I didn't hold my breath :-) > > > diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c > > > index 43b1fc634af1..80d0f9c64f9d 100644 > > > --- a/drivers/pwm/pwm-lpss.c > > > +++ b/drivers/pwm/pwm-lpss.c > > > @@ -97,6 +97,9 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, > > > freq *= base_unit_range; > > > base_unit = DIV_ROUND_CLOSEST_ULL(freq, c); > > > > DIV_ROUND_CLOSEST_ULL is most probably wrong, too. But I didn't spend > > the time to actually confirm that. > > Yes I saw your comment elsewhere that the PWM API defines rounding > in a certain direction, but fixing that falls outside of this patch. Yeah, sure. > [...] > I hope this helps to explain what is going on a bit. I will try to make sense of that and reply to the patch directly when I succeeded. > ### > > As for the behavior on base_unit==0 in the get_state method, > as mentioned above I wrote that when I did not fully understood > how the controller works. > > We really should never encounter this. > > But if we do then I think closest to the truth would be: > > state->period = UINT_MAX; > state->duty_cycle = 0; I'd say state->period = 1 & state->duty_cycle = 0 is a better representation. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature