Re: [PATCH v10 2/2] pwm: sifive: Add a driver for SiFive SoC PWM

From: Uwe Kleine-König
Date: Tue Mar 19 2019 - 04:10:00 EST


On Tue, Mar 19, 2019 at 12:52:12PM +0530, Yash Shah wrote:
> On Tue, Mar 19, 2019 at 3:16 AM Uwe Kleine-König
> <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
> > On Mon, Mar 18, 2019 at 05:17:14PM +0530, Yash Shah wrote:
> > > + val = PWM_SIFIVE_PWMCFG_EN_ALWAYS |
> > > + FIELD_PREP(PWM_SIFIVE_PWMCFG_SCALE, scale);
> > > + writel(val, pwm->regs + PWM_SIFIVE_PWMCFG);
> >
> > Starting with this write the new period length might be active with the
> > previous duty cycle. Is this worth a comment? I think the window where
> > this can actually happen should be made as small as possible, so it
> > would be great to first calculate both register values and then write
> > them in two consecutive writels.
>
> The comment for this scenario has already been mentioned under the
> limitation on top of this driver.
> Anyway, I will try to implement your suggestion of consecutive writes

Yeah, the comment at the top is for general information about the
shortcomings. The comment here would be to say: The problem occurs *here*.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |