Re: [v3,PATCH 3/3] pwm: mtk_disp: implement .get_state()

From: Thierry Reding
Date: Fri Apr 09 2021 - 08:24:12 EST


On Tue, Apr 06, 2021 at 12:27:56PM +0200, Uwe Kleine-König wrote:
> On Tue, Apr 06, 2021 at 05:57:42PM +0800, Rex-BC Chen wrote:
> > implement get_state function for pwm-mtk-disp
> >
> > Signed-off-by: Rex-BC Chen <rex-bc.chen@xxxxxxxxxxxx>
> > Signed-off-by: Jitao Shi <jitao.shi@xxxxxxxxxxxx>
>
> Ideally you S-o-b line is the last one to show the order in which this
> patch went from one person to another.
>
> > ---
> > drivers/pwm/pwm-mtk-disp.c | 46 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 46 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> > index 502228adf718..166e0a8ca703 100644
> > --- a/drivers/pwm/pwm-mtk-disp.c
> > +++ b/drivers/pwm/pwm-mtk-disp.c
> > @@ -179,8 +179,54 @@ static int mtk_disp_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > return mtk_disp_pwm_enable(chip, state);
> > }
> >
> > +static void mtk_disp_pwm_get_state(struct pwm_chip *chip,
> > + struct pwm_device *pwm,
> > + struct pwm_state *state)
> > +{
> > + struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> > + u32 clk_div, period, high_width, con0, con1;
> > + u64 rate;
> > + int err;
> > +
> > + err = clk_prepare_enable(mdp->clk_main);
> > + if (err < 0) {
> > + dev_err(chip->dev, "Can't enable mdp->clk_main: %d\n", err);
> > + return;
> > + }
> > + err = clk_prepare_enable(mdp->clk_mm);
> > + if (err < 0) {
> > + dev_err(chip->dev, "Can't enable mdp->clk_mm: %d\n", err);
> > + clk_disable_unprepare(mdp->clk_main);
>
> As before: %pe please

According to the documentation %pe only works on pointers for which
IS_ERR() is true, so I'm not sure it can be used with plain integer
error codes.

Looks like there's a bunch of drivers that will do %pe and then use
ERR_PTR(err) to make this work, but to be honest, that seems like
jumping through hoops.

But I don't feel strongly either way, so choose whichever you want.

Thierry

Attachment: signature.asc
Description: PGP signature