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

From: Thierry Reding <thierry.reding@gmail.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Rex-BC Chen <rex-bc.chen@mediatek.com>,
	lee.jones@linaro.org, matthias.bgg@gmail.com,
	linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	Project_Global_Chrome_Upstream_Group@mediatek.com,
	Jitao Shi <jitao.shi@mediatek.com>
Subject: Re: [v3,PATCH 3/3] pwm: mtk_disp: implement .get_state()
Date: Fri, 9 Apr 2021 14:24:43 +0200	[thread overview]
Message-ID: <YHBHizYI7oyULZx9@orome.fritz.box> (raw)
In-Reply-To: <20210406102756.4kncasxe4um3q6wi@pengutronix.de>

[-- Attachment #1.1: Type: text/plain, Size: 1987 bytes --]

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@mediatek.com>
> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> 
> 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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel