Hi,
On 7/7/20 9:34 AM, Uwe Kleine-König wrote:
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.
But that would suggest the output is configured for an infinitely high output frequency, but the frequency is actually 0, the reason why get_state needs to treat a base_unit val of 0 special at all is to avoid a division by 0, and in math dividing by 0 gives infinite, isn't UINT_MAX a better way to represent infinity ?
Regards,
Hans