Hi Uwe:
On Fri, May 24, 2024 at 10:01 PM Uwe Kleine-König
<u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
>
> 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.
Yes, you are right, I will fix it.
>
> The suggestion to put this in a struct is nice. I'd call it something
> with "suspend" though, maybe "pwm_loongson_suspend_store"?
>
Ah, The name sounds more readable.
Thanks.
Binbin
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | https://www.pengutronix.de/ |
