RE: [PATCH 4/4 v4] GPIO: gpio-dwapb: Suspend & Resume PM enabling
From: Chen, Alvin <hidden>
Date: 2014-09-17 06:46:56
Also in:
linux-gpio, lkml
From: Chen, Alvin <hidden>
Date: 2014-09-17 06:46:56
Also in:
linux-gpio, lkml
quoted
Reviewed-by: Hock Leong Kweh <redacted>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.
quoted
+ 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.quoted
+ + 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.