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

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2470 bytes --]

On Fri, Apr 09, 2021 at 02:24:43PM +0200, Thierry Reding wrote:
> 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.

It cannot.

> 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.

When I suggested to implement %dE to print error codes this was shot
down by the printk guys who's position is that %pe has to be good enough
for everybody. And yes, you'd need to pass ERR_PTR(err) then.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]