Thread (5 messages) flat view 5 messages, 2 authors, 2012-09-24
STALE5070d

[PATCH] GPIO: Add support for GPIO on CLPS711X-target platform

From: Alexander Shiyan <hidden>
Date: 2012-08-24 03:45:52

Hello.

On Fri, 24 Aug 2012 00:08:45 +0200
Linus Walleij [off-list ref] wrote:

...
quoted
+static int gpio_clps711x_get(struct gpio_chip *chip, unsigned offset)
+{
+       int ret;
+       struct clps711x_gpio *gpio = dev_get_drvdata(chip->dev);
+
+       mutex_lock(&gpio->lock);
+       ret = clps_readb(gpio_clps711x_port_dataaddr(chip)) & (1 << offset);
+       mutex_unlock(&gpio->lock);
+
+       return !!ret;
+}
Please get rid of this clps_readb() business. I can see it's just this:
#define clps_readb(off)         readb(CLPS711X_VIRT_BASE + (off))

This makes stuff hard to understand, please pass the virtual base as
a memory resource to the platform device, store it in the state holder
and just use readb() in the driver from that offset.
This will break the whole concept of this platform. All the CPU registers
are in one particular area, and therefore access to the registers of all
the drivers performed using the macro.
Maybe just add a short comment?

-- 
Alexander Shiyan [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help