linux-kernel - Re: [PATCH] pwm: pwm-imx27: Use 'dev' instead of dereferencing it repeatedly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190924105209.GC14924@ulmo>
Date:   Tue, 24 Sep 2019 12:52:09 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     'Anson Huang' <Anson.Huang@....com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Linux-imx@....com" <Linux-imx@....com>
Subject: Re: [PATCH] pwm: pwm-imx27: Use 'dev' instead of dereferencing it
 repeatedly

On Tue, Sep 24, 2019 at 09:46:20AM +0000, David Laight wrote:
> From: Anson Huang
> > Sent: 24 September 2019 10:00
> > Add helper variable dev = &pdev->dev to simply the code.
> > 
> > Signed-off-by: Anson Huang <Anson.Huang@....com>
> > ---
> >  drivers/pwm/pwm-imx27.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
> > index 434a351..3afee29 100644
> > --- a/drivers/pwm/pwm-imx27.c
> > +++ b/drivers/pwm/pwm-imx27.c
> > @@ -290,27 +290,28 @@ MODULE_DEVICE_TABLE(of, pwm_imx27_dt_ids);
> > 
> >  static int pwm_imx27_probe(struct platform_device *pdev)
> >  {
> > +	struct device *dev = &pdev->dev;
> >  	struct pwm_imx27_chip *imx;
> > 
> > -	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
> > +	imx = devm_kzalloc(dev, sizeof(*imx), GFP_KERNEL);
> >  	if (imx == NULL)
> >  		return -ENOMEM;
> > 
> >  	platform_set_drvdata(pdev, imx);
> > 
> > -	imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
> > +	imx->clk_ipg = devm_clk_get(dev, "ipg");
> >  	if (IS_ERR(imx->clk_ipg)) {
> > -		dev_err(&pdev->dev, "getting ipg clock failed with %ld\n",
> > +		dev_err(dev, "getting ipg clock failed with %ld\n",
> >  				PTR_ERR(imx->clk_ipg));
> >  		return PTR_ERR(imx->clk_ipg);
> >  	}
> > 
> > -	imx->clk_per = devm_clk_get(&pdev->dev, "per");
> > +	imx->clk_per = devm_clk_get(dev, "per");
> >  	if (IS_ERR(imx->clk_per)) {
> >  		int ret = PTR_ERR(imx->clk_per);
> > 
> >  		if (ret != -EPROBE_DEFER)
> > -			dev_err(&pdev->dev,
> > +			dev_err(dev,
> >  				"failed to get peripheral clock: %d\n",
> >  				ret);
> 
> Hopefully the compiler will optimise this back otherwise you've added another
> local variable which may cause spilling to stack.
> For a setup function it probably doesn't matter, but in general it might
> have a small negative performance impact.
> 
> In any case this doesn't shorten any lines enough to remove line-wrap
> and using &pdev->dev is really one less variable to mentally track
> when reading the code.
> 
> 	David

I agree. A positive diffstat is often a good indication that it's not
worth it. Don't get me wrong, I think there are cases where an extra
local variable can be worth it, but this isn't one of them, so it's
really just unnecessary churn.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)