Messages in this thread | | | From | Martin Blumenstingl <> | Date | Tue, 2 Apr 2019 21:22:55 +0200 | Subject | Re: [PATCH 1/2] pwm: meson: consider 128 a valid pre-divider |
| |
Hello Uwe,
On Mon, Apr 1, 2019 at 8:38 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > Hello Martin, > > On Mon, Apr 01, 2019 at 08:18:16PM +0200, Martin Blumenstingl wrote: > > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c > > index f6e738ad7bd9..4b708c1fcb1d 100644 > > --- a/drivers/pwm/pwm-meson.c > > +++ b/drivers/pwm/pwm-meson.c > > @@ -188,7 +188,7 @@ static int meson_pwm_calc(struct meson_pwm *meson, > > do_div(fin_ps, fin_freq); > > > > /* Calc pre_div with the period */ > > - for (pre_div = 0; pre_div < MISC_CLK_DIV_MASK; pre_div++) { > > + for (pre_div = 0; pre_div <= MISC_CLK_DIV_MASK; pre_div++) { > > cnt = DIV_ROUND_CLOSEST_ULL((u64)period * 1000, > > fin_ps * (pre_div + 1)); > > dev_dbg(meson->chip.dev, "fin_ps=%llu pre_div=%u cnt=%u\n", > > You could even calculate pre_div without the loop. > > Something like: > > u64 pre_div = (u64)period * rate; > do_div_round_up(pre_div, NSEC_PER_SEC * 0xffff); > pre_div--; > > (I didn't check rounding and maybe its off by one and ...) This would > also get rid of the strange 1000 that is currently used in the > calculation without a real benefit (unless I missed something). personally I prefer using this simple patch applied first as it is easy to review and (due to the Fixes tag) may get backported to stable kernels. I'm not saying I don't like your suggestion, I propose to postpone implementing this cleanup. I need to have a closer look at the calculation because three values are derived from the input clock rate (pre_div, cnt, duty_cnt) and I don't want to mess up the cases that are already working as of today.
Please let me know what you think.
Regards Martin
| |