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
