Re: [PATCH v3 2/2] pwm: loongson: Reload PWM configuration through counter reset

[Keguang Zhang]

On Mon, Sep 7, 2026 at 5:33 PM Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote:
>
> On Wed, Jul 15, 2026 at 07:05:24PM +0800, Keguang Zhang via B4 Relay wrote:
> > From: Keguang Zhang <keguang.zhang@xxxxxxxxx>
> >
> > The Loongson PWM controller latches the LOW and PERIOD registers only at
> > the start of each PWM period. After disabling and re-enabling the PWM,
> > the controller resumes from the previous counter value and completes the
> > current period before re-latching the updated LOW and PERIOD values.
> >
> > Reset the PWM counter when disabling the PWM and release it when
> > enabling the PWM so that the updated LOW and PERIOD values are latched
> > before the PWM starts running again.
> >
> > Fixes: 2b62c89448dd ("pwm: Add Loongson PWM controller support")
> > Signed-off-by: Keguang Zhang <keguang.zhang@xxxxxxxxx>
> > ---
> >  drivers/pwm/pwm-loongson.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-loongson.c b/drivers/pwm/pwm-loongson.c
> > index e703217a6d5e..c317d62fe813 100644
> > --- a/drivers/pwm/pwm-loongson.c
> > +++ b/drivers/pwm/pwm-loongson.c
> > @@ -103,6 +103,7 @@ static void pwm_loongson_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >       struct pwm_loongson_ddata *ddata = to_pwm_loongson_ddata(chip);
> >
> >       val = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_CTRL);
> > +     val |= LOONGSON_PWM_CTRL_REG_RST;
> >       val &= ~LOONGSON_PWM_CTRL_REG_EN;
> >       pwm_loongson_writel(ddata, val, LOONGSON_PWM_REG_CTRL);
> >  }
> > @@ -113,6 +114,7 @@ static int pwm_loongson_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >       struct pwm_loongson_ddata *ddata = to_pwm_loongson_ddata(chip);
> >
> >       val = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_CTRL);
> > +     val &= ~LOONGSON_PWM_CTRL_REG_RST;
> >       val |= LOONGSON_PWM_CTRL_REG_EN;
> >       pwm_loongson_writel(ddata, val, LOONGSON_PWM_REG_CTRL);
>
> I applied this patch to
>
> https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
>
> , but I wonder if the fix is incomplete. If at driver bind time
> LOONGSON_PWM_CTRL_REG_RST isn't set and there are still wrong values in
> the shadowed LOW and PERIOD registers the first period will be wrong.
>
> This would need:
>
>         val = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_CTRL);
> +
> +       /*
> +        * Ensure that the values of LOW and PERIOD are sampled into
> +        * the shadow register when the hardware starts running.
> +        */
> +       if (!(val & LOONGSON_PWM_CTRL_REG_RST))
> +               pwm_loongson_writel(ddata, val | LOONGSON_PWM_CTRL_REG_RST, LOONGSON_PWM_REG_CTRL);
> +
>         val &= ~LOONGSON_PWM_CTRL_REG_RST;
>         val |= LOONGSON_PWM_CTRL_REG_EN;
>         pwm_loongson_writel(ddata, val, LOONGSON_PWM_REG_CTRL);
>
> I think.

You are right that this patch doesn't handle the initial hardware
state when the driver binds.
Would you prefer that I submit a follow-up patch to address this, or
are you planning to fix it yourself?
Thanks!

>
> Best regards
> Uwe



-- 
Best regards,

Keguang Zhang