Hello,
On Tue, Feb 11, 2025 at 02:02:03PM +0600, Binbin Zhou wrote:
> On Tue, Feb 11, 2025 at 12:26 AM Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote:
> > On Tue, Dec 10, 2024 at 08:37:06PM +0800, Binbin Zhou wrote:
> > > +static int pwm_loongson_suspend(struct device *dev)
> > > +{
> > > + struct pwm_chip *chip = dev_get_drvdata(dev);
> > > + struct pwm_loongson_ddata *ddata = to_pwm_loongson_ddata(chip);
> > > +
> > > + ddata->lss.ctrl = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_CTRL);
> > > + ddata->lss.duty = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_DUTY);
> > > + ddata->lss.period = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_PERIOD);
> > > +
> > > + clk_disable_unprepare(ddata->clk);
> > > +
> > > + return 0;
> >
> > Is this needed assuming that before suspend the consumer stopped the
> > PWM?
>
> Actually, I don't quite understand the problem you're pointing out. It
> seems to me that the register and clk operations are required
> regardless of the state of the pwm.
> At least from the experimental results, the logic is now as expected.
> Of course, I may be missing some critical information.
When a PWM goes into suspend it's expected that its consumer already
disabled it.
Until I come around to do that properly in the core for all drivers, I
think the right approach in a driver is:
for (i = 0; i < chip->npwm; ++i) {
if (chip->pwms[i].state.enabled)
return -EBUSY;
}
and if you then know that all PWMs are disabled, maybe you don't need to
store all the registers you did in pwm_loongson_suspend()?
Best regards
Uwe
Attachment:
signature.asc
Description: PGP signature
