Hello Binbin,
On Fri, May 24, 2024 at 02:29:35PM +0600, Binbin Zhou wrote:
> > > + ddata->duty = pwm_loongson_readl(ddata, PWM_DUTY);
> > > + ddata->period = pwm_loongson_readl(ddata, PWM_PERIOD);
> >
> > The rounding looks wrong. Did you test with PWM_DEBUG enabled?
> >
> > I think the value assigned to ddata->period and the other members isn't
> > used. Unless I'm mistaken, please drop the assignment.
> >
>
> The period, duty and ctrl are prepared for PM. I plan to put these
> three parameters separately into the pwm_loongson_context structure. I
> think it will look clearer:
>
> struct pwm_loongson_context {
> u32 ctrl;
> u32 duty;
> u32 period;
> };
But .suspend() reads the value from the registers and rewrites these
three members itself, too. So the write in .apply() is unused and can be
dropped.
The suggestion to put this in a struct is nice. I'd call it something
with "suspend" though, maybe "pwm_loongson_suspend_store"?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
