Thread (12 messages) 12 messages, 4 authors, 2014-09-17

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

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help