lkml.org 
[lkml]   [2014]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 4/4 v4] GPIO: gpio-dwapb: Suspend & Resume PM enabling
Date
> 
> > Reviewed-by: Hock Leong Kweh <hock.leong.kweh@intel.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.
\
 
 \ /
  Last update: 2014-09-17 09:21    [W:0.062 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site