linux-kernel - RE: [PATCH 4/4 v4] GPIO: gpio-dwapb: Suspend & Resume PM enabling

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]

Date:	Wed, 17 Sep 2014 06:46:11 +0000
From:	"Chen, Alvin" <alvin.chen@...el.com>
To:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	Alexandre Courbot <gnurou@...il.com>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	atull <atull@...nsource.altera.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"Ong, Boon Leong" <boon.leong.ong@...el.com>,
	"Kweh, Hock Leong" <hock.leong.kweh@...el.com>,
	Darren Hart <dvhart@...ux.intel.com>,
	Sebastian Andrzej Siewior <sebastian@...akpoint.cc>,
	"Westerberg, Mika" <mika.westerberg@...el.com>,
	Arnd Bergmann <arnd@...db.de>
Subject: RE: [PATCH 4/4 v4] GPIO: gpio-dwapb: Suspend & Resume PM enabling

> 
> > Reviewed-by: Hock Leong Kweh <hock.leong.kweh@...el.com>
> 
> You still keep that guy as reviewer in a whole series, however I didn't see a
> word from him here. How is it possible?
In our internal review, he gave me a lot of suggestions. 

> > +	for (i = 0; i < gpio->nr_ports; i++) {
> > +		unsigned int offset;
> > +		unsigned int idx = gpio->ports[i].idx;
> > +		struct dwapb_context *ctx = gpio->ports[i].ctx;
> > +
> > +		if (!ctx) {
> > +			ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > +			gpio->ports[i].ctx = ctx;
> > +		}
> 
> I don't think it's a right place to allocate this resource, especially with devm_
> helper. Can you move this to probe() stage?
> 
> Or you even can embed contex structure inside chip one with #ifdef
> CONFIG_PM_SLEEP.
> 

OK, I will improve it.

> Maybe others could comment on this.
> 
> > +
> > +		offset = GPIO_SWPORTA_DDR + (idx * GPIO_SWPORT_DDR_SIZE);
> 
> No need to have parentheses here. Check the code below as well.
OK. I will remove them.