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

On Wed, Aug 22, 2012 at 1:36 PM, Alexander Shiyan [off-list ref] wrote:

The CLPS711X CPUs provide some GPIOs for use in the system. This
driver provides support for these via gpiolib. Due to platform
limitations, driver does not support interrupts, only inputs and
outputs.

OK...

(...)

+#include <mach/hardware.h>

+struct clps711x_gpio {
+       struct gpio_chip        chip[CLPS711X_GPIO_PORTS];
+       struct mutex            lock;
+};

That lock is just protecting a few register reads/writes. Use a
spinlock instead, it's more apropriate.

(...)

+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.

(...)

+static int __devinit gpio_clps711x_probe(struct platform_device *pdev)

This should just be __init, you have made it very certain that this
cannot be loaded and unloaded at runtime. (No remove function!)
and its a bool in the Kconfig.

Yours,
Linus Walleij