linux-kernel - RE: [PATCH 1/2] pinctrl: tegra: add suspend-resume support

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: <77F7DB30C698A44DA22FB222C89DE941A6692E62BC@BGMAIL01.nvidia.com>
Date:	Sun, 28 Apr 2013 19:08:15 +0530
From:	Bibek Basu <bbasu@...dia.com>
To:	Thierry Reding <thierry.reding@...onic-design.de>
CC:	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	"swarren@...dotorg.org" <swarren@...dotorg.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Pritesh Raithatha <praithatha@...dia.com>
Subject: RE: [PATCH 1/2] pinctrl: tegra: add suspend-resume support

> -----Original Message-----
> From: Thierry Reding [mailto:thierry.reding@...onic-design.de]
> Sent: Wednesday, April 24, 2013 12:14 AM
> To: Bibek Basu
> Cc: linus.walleij@...aro.org; swarren@...dotorg.org; linux-
> tegra@...r.kernel.org; linux-kernel@...r.kernel.org; Pritesh Raithatha
> Subject: Re: [PATCH 1/2] pinctrl: tegra: add suspend-resume support
> 
> * PGP Signed by an unknown key
> 
> > diff --git a/drivers/pinctrl/pinctrl-tegra.c
> > b/drivers/pinctrl/pinctrl-tegra.c
> [...]
> > @@ -41,6 +42,8 @@ struct tegra_pmx {
> >
> >  	int nbanks;
> >  	void __iomem **regs;
> > +	int *regs_size;
> 
> Perhaps this should be unsigned int *. The values stored in this array will
> never be negative, right?
Agree. 
> 
> >  int tegra_pinctrl_probe(struct platform_device *pdev,
> >  			const struct tegra_pinctrl_soc_data *soc_data)  {
> > -	struct tegra_pmx *pmx;
> >  	struct resource *res;
> > -	int i;
> > +	struct tegra_pmx *pmx;
> > +	int i, pg_data_size = 0;
> 
> There's a needless move of the pmx variable declaration here.
Agree
> 
> > @@ -735,6 +769,21 @@ int tegra_pinctrl_probe(struct platform_device
> *pdev,
> >  		return -ENODEV;
> >  	}
> >
> > +if (IS_ENABLED(CONFIG_PM_SLEEP))
> > +	pmx->regs_size = devm_kzalloc(&pdev->dev,
> > +				pmx->nbanks * sizeof(*(pmx->regs_size)),
> > +				GFP_KERNEL);
> > +	if (!pmx->regs_size) {
> > +		dev_err(&pdev->dev, "Can't alloc regs pointer\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	pmx->pg_data = devm_kzalloc(&pdev->dev, pg_data_size,
> GFP_KERNEL);
> > +	if (!pmx->pg_data) {
> > +		dev_err(&pdev->dev, "Can't alloc pingroup data pointer\n");
> > +		return -ENODEV;
> > +	}
> 
> I don't think this works the way you expect it to. The line
> 
> 	if (IS_ENABLED(CONFIG_PM_SLEEP))
> 
> is a standard conditional and therefore needs to be properly indented and
> use { and } to delimit the block.
> 
My Bad. Will fix
> > @@ -756,6 +805,9 @@ int tegra_pinctrl_probe(struct platform_device
> *pdev,
> >  			dev_err(&pdev->dev, "Couldn't ioremap regs %d\n",
> i);
> >  			return -ENODEV;
> >  		}
> > +
> > +if (IS_ENABLED(CONFIG_PM_SLEEP))
> > +		pmx->regs_size[i] = resource_size(res);
> 
> In this case it will actually work as expected, but the if () should be properly
> indented.
> 
> Thierry
> 
Thanks for the review
Bibek
> * Unknown Key
> * 0x7F3EB3A1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/