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

[Linus Walleij]

On Wed, Aug 22, 2012 at 1:36 PM, Alexander Shiyan <shc_work at mail.ru> 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