[PATCH] gpio: Add generic driver for simple memory mapped controllers

[PATCH] gpio: Add generic driver for simple memory mapped controllers

[Anton Vorontsov]

The simple memory mapped GPIO controllers may be found in various
onboard FPGAs that are used to control board's switches, LEDs,
chip-selects, Ethernet/USB PHY power, etc.

Usually these controllers do not privide any means of pin setup
(in/out/open drain).

The driver provides:
- Support for 8/16/32/64 bits registers;
- Support for GPIO controllers with clear/set registers;
- Support for GPIO controllers with a single "data" register;
- Support for big endian bits/GPIOs ordering (mostly used on PowerPC).

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/gpio/Kconfig          |    5 +
 drivers/gpio/Makefile         |    1 +
 drivers/gpio/simple_mm_gpio.c |  243 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/simple_mm_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f623953..a145063 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -82,6 +82,11 @@ config GPIO_PL061
 	help
 	  Say yes here to support the PrimeCell PL061 GPIO device
 
+config GPIO_SIMPLE_MM
+	tristate "Simple memory mapped GPIO controllers support"
+	help
+	  Say yes here to support simple memory mapped GPIO controllers.
+
 config GPIO_XILINX
 	bool "Xilinx GPIO support"
 	depends on PPC_OF || MICROBLAZE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a69e060..bfd43bb 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_GPIO_MCP23S08)	+= mcp23s08.o
 obj-$(CONFIG_GPIO_PCA953X)	+= pca953x.o
 obj-$(CONFIG_GPIO_PCF857X)	+= pcf857x.o
 obj-$(CONFIG_GPIO_PL061)	+= pl061.o
+obj-$(CONFIG_GPIO_SIMPLE_MM)	+= simple_mm_gpio.o
 obj-$(CONFIG_GPIO_TC35892)	+= tc35892-gpio.o
 obj-$(CONFIG_GPIO_TIMBERDALE)	+= timbgpio.o
 obj-$(CONFIG_GPIO_TWL4030)	+= twl4030-gpio.o
diff --git a/drivers/gpio/simple_mm_gpio.c b/drivers/gpio/simple_mm_gpio.c
new file mode 100644
index 0000000..028dce8
--- /dev/null
+++ b/drivers/gpio/simple_mm_gpio.c
@@ -0,0 +1,243 @@
+/*
+ * Simple memory mapped GPIOs
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/init.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/log2.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+
+struct sgpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_dat;
+	void __iomem *reg_set;
+	void __iomem *reg_clr;
+	spinlock_t lock;
+
+	int bits;
+	int big_endian_bits;
+
+	/* shadowed data register to clear/set bits safely */
+	unsigned long data;
+};
+
+static struct sgpio_chip *to_sgpio_chip(struct gpio_chip *gc)
+{
+	return container_of(gc, struct sgpio_chip, gc);
+}
+
+static unsigned long sgpio_in(struct sgpio_chip *sgc)
+{
+	switch (sgc->bits) {
+	case 8:
+		return __raw_readb(sgc->reg_dat);
+	case 16:
+		return __raw_readw(sgc->reg_dat);
+	case 32:
+		return __raw_readl(sgc->reg_dat);
+#if BITS_PER_LONG >= 64
+	case 64:
+		return __raw_readq(sgc->reg_dat);
+#endif
+	}
+	BUG();
+}
+
+static void sgpio_out(struct sgpio_chip *sgc, void __iomem *reg,
+		      unsigned long data)
+{
+	switch (sgc->bits) {
+	case 8:
+		__raw_writeb(data, reg);
+		return;
+	case 16:
+		__raw_writew(data, reg);
+		return;
+	case 32:
+		__raw_writel(data, reg);
+		return;
+#if BITS_PER_LONG >= 64
+	case 64:
+		__raw_writeq(data, reg);
+		return;
+#endif
+	}
+	BUG();
+}
+
+static unsigned long sgpio_pin2mask(struct sgpio_chip *sgc, unsigned int pin)
+{
+	if (sgc->big_endian_bits)
+		return 1 << (sgc->bits - 1 - pin);
+	else
+		return 1 << pin;
+}
+
+static int sgpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct sgpio_chip *sgc = to_sgpio_chip(gc);
+
+	return sgpio_in(sgc) & sgpio_pin2mask(sgc, gpio);
+}
+
+static void sgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct sgpio_chip *sgc = to_sgpio_chip(gc);
+	unsigned long mask = sgpio_pin2mask(sgc, gpio);
+	unsigned long flags;
+
+	if (sgc->reg_set) {
+		if (val)
+			sgpio_out(sgc, sgc->reg_set, mask);
+		else
+			sgpio_out(sgc, sgc->reg_clr, mask);
+		return;
+	}
+
+	spin_lock_irqsave(&sgc->lock, flags);
+
+	if (val)
+		sgc->data |= mask;
+	else
+		sgc->data &= ~mask;
+
+	sgpio_out(sgc, sgc->reg_dat, sgc->data);
+
+	spin_unlock_irqrestore(&sgc->lock, flags);
+}
+
+static int sgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int sgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	sgpio_set(gc, gpio, val);
+	return 0;
+}
+
+static int __devinit sgpio_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	struct device *dev = &pdev->dev;
+	struct sgpio_chip *sgc;
+	struct resource *res_dat;
+	struct resource *res_set;
+	struct resource *res_clr;
+	resource_size_t dat_sz;
+	int bits;
+
+	res_dat = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res_dat)
+		return -EINVAL;
+
+	dat_sz = resource_size(res_dat);
+	if (!is_power_of_2(dat_sz))
+		return -EINVAL;
+
+	bits = dat_sz * 8;
+	if (bits > BITS_PER_LONG)
+		return -EINVAL;
+
+	sgc = devm_kzalloc(dev, sizeof(*sgc), GFP_KERNEL);
+	if (!sgc)
+		return -ENOMEM;
+
+	sgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+	if (!sgc->reg_dat)
+		return -ENOMEM;
+
+	res_set = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	res_clr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (res_set && res_clr) {
+		if (resource_size(res_set) != resource_size(res_clr) ||
+				resource_size(res_set) != dat_sz)
+			return -EINVAL;
+
+		sgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
+		sgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+		if (!sgc->reg_set || !sgc->reg_clr)
+			return -ENOMEM;
+	} else if (res_set || res_clr) {
+		return -EINVAL;
+	}
+
+	spin_lock_init(&sgc->lock);
+
+	sgc->bits = bits;
+	sgc->big_endian_bits = !strcmp(platid->name, "simple-mm-gpio-be");
+	sgc->data = sgpio_in(sgc);
+
+	sgc->gc.ngpio = bits;
+	sgc->gc.direction_input = sgpio_dir_in;
+	sgc->gc.direction_output = sgpio_dir_out;
+	sgc->gc.get = sgpio_get;
+	sgc->gc.set = sgpio_set;
+
+	if (pdev->dev.platform_data)
+		sgc->gc.base = (unsigned long)pdev->dev.platform_data;
+	else
+		sgc->gc.base = -1;
+
+	dev_set_drvdata(dev, sgc);
+
+	return gpiochip_add(&sgc->gc);
+}
+
+static int __devexit sgpio_remove(struct platform_device *pdev)
+{
+	struct sgpio_chip *sgc = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&sgc->gc);
+}
+
+static const struct platform_device_id sgpio_id_table[] = {
+	{ "simple-mm-gpio", },
+	{ "simple-mm-gpio-be", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, sgpio_id_table);
+
+static struct platform_driver sgpio_driver = {
+	.driver = {
+		.name = "simple-mm-gpio",
+	},
+	.id_table = sgpio_id_table,
+	.probe = sgpio_probe,
+	.remove = __devexit_p(sgpio_remove),
+};
+
+static int __init sgpio_init(void)
+{
+	return platform_driver_register(&sgpio_driver);
+}
+module_init(sgpio_init);
+
+static void __exit sgpio_exit(void)
+{
+	platform_driver_unregister(&sgpio_driver);
+}
+module_exit(sgpio_exit);
+
+MODULE_DESCRIPTION("Driver for simple memory mapped GPIO controllers");
+MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.5

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[Anton Vorontsov]

On Wed, Aug 25, 2010 at 05:11:23PM -0700, David Brownell wrote:
> From: Anton Vorontsov <cbouatmailru@gmail.com>
> Subject: [PATCH] gpio: Add generic driver for simple memory mapped controllers
> 
> NAK.  As you said, it matches the programming of
> certain FPGAs ... so it's NOT "generic".  Rename to match
> the Verilog/VHDL module name or somesuch, if there's
> really much reuse of that module.

I know at least two completely different platforms that
might reuse this driver.

One is PowerPC with ~10 different boards, and another is
an ARM board.

On PowerPC we use arch/powerpc/sysdev/simple_gpio.c, but
the new driver is much more generic, and is aimed to
replace the PowerPC specific one. So it is 'generic' in
this exact sense.

I also think that it is impractical to name the driver
after the particular FPGA IP codename, or even depend
on 'FPGA' word at all (as the GPIOs might be in ASIC
someday).

> There are other FPGA/CPLD GPIO modules, and most of the
> GPIOs in SOCs also match your description, but aren't
> even vaguely compatible with this specific interface.

Well, good. They don't have to use this driver. The driver
is for 'simple' GPIOs, just as the driver name states. I.e.
just a 'data' register or data + set + clr. The name
'simple', IMHO, perfectly matches the purpose of that
driver, no?

If you don't like the word 'generic' in the email subject
(the word does not appear anywhere else), I can remove it,
but I don't see why the name 'simple' doesn't work for
this driver.

Thanks!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[David Brownell]

> If you don't like the word 'generic' in the
 email subject

Or patch description.  It's neither
generic (IP-block-specific) nor simple.


 I can remove it,
> but I don't see why the name 'simple'
> doesn't work for > this driver.

It's not simple; I've seen far simpler GPIO
controllers.

Just rename it to match the IP block used.
That removes all confusion.  Engineers will
 be able to know more easily whether this
driver is even relevant to them.

An example might be "amba-9999 (a made-up
number, likely not matching a GPIO controller).
DesignWare drivers tend to use dw_* prefixes.

- Dave
 

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[Alan Cox]

> Just rename it to match the IP block used.

There are zillions of IP blocks that have that interface, how exactly do
you propose to rename it.

> That removes all confusion.  Engineers will
>  be able to know more easily whether this
> driver is even relevant to them.

Not unless its got a name about 1000 characters long listing all the
devices it might drive - and changes name weekly.
 
> An example might be "amba-9999 (a made-up
> number, likely not matching a GPIO controller).

What's wrong with a sensible generic name like gpio-mmio-table ?

Alan

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[David Brownell]

--- On Thu, 8/26/10, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > Just rename it to match the IP block used.
> 
> There are zillions of IP blocks that have that interface,

If there are "zillions" that suggests the HW
engineers have version/naming issues just like
certain software engineers.  Only goes to show
how close Verilog and VHDL are to software! :)


> how exactly do
> you propose to rename it.

My suggestion was to use the name provided/used
by the hardware engineers.  (E.g. whatever the
Verilog or VHDL equivalent of a module name is.)

So -- NO RENAME -- except for the driver.

If I understand you correctly, those engineers
are not reusing a named module; they are at best
just copying/pasting some Verilog/VHDL and adding
ASIC/SoC/.../FPGA-specific hacks.  (Which calls into
question just how much assurance there can be that
one driver will work reliably for all instances...


> > That removes all confusion.  Engineers will
> >  be able to know more easily whether this
> > driver is even relevant to them.
> 
> Not unless its got a name about 1000 characters long
> listing all the
> devices it might drive - and changes name weekly.

Someone should have a few words with those hardware
engineers about module naming and consistency, so
software doesn't need to work around such issues.

Such problems have been solved for decades, and
there's no reason to create confusion higher up
the system stack.  (or force workarounds.)


> > An example might be "amba-9999 (a made-up
> > number, likely not matching a GPIO controller).

Note that such names are already used in Linux
with IP blocks from AMBA; Designware blocks, as
I noted, aren't dissimilar (dw_foo.c in at least
a few cases).

> What's wrong with a sensible generic name like
> gpio-mmio-table ?


That's a much better name.  I'ts got technical
content, even!  (vs "simple" being "marketing",
including easy confusion with other entities).

All I'll say is that it might
 be *too* generic a name.  I know I can come up
with current examples of GPIO done via MMIO
(tables) that this driver won't fit at all.
That can probably be worked around via docs and
conventions.

There'd need to be good docs on what this
gpio-mmio-table interface expects.  Some of that
belongs in Kconfig, not much; mostly I'd think
it belongs in driver header comments, but also
some in the patch comment itself.  I hear you
strongly implying that there's no generic hardware
doc to reference.  (As theree would be if this
block came from the AMBA or DesignWare families.)

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[Mark Brown]

On Thu, Aug 26, 2010 at 10:34:58AM -0700, David Brownell wrote:
> --- On Thu, 8/26/10, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > > Just rename it to match the IP block used.

> > There are zillions of IP blocks that have that interface,

> If there are "zillions" that suggests the HW
> engineers have version/naming issues just like
> certain software engineers.  Only goes to show
> how close Verilog and VHDL are to software! :)

Half the thing here is that it's barely IP - it's the sort of thing
that's so trivial to implement that it'd take more time to locate a
suitable IP than to just hook up the output pins to the register map
directly.

> > how exactly do
> > you propose to rename it.

> My suggestion was to use the name provided/used
> by the hardware engineers.  (E.g. whatever the
> Verilog or VHDL equivalent of a module name is.)

That's not going to happen, nobody's sharing any IP for this.

> If I understand you correctly, those engineers
> are not reusing a named module; they are at best
> just copying/pasting some Verilog/VHDL and adding
> ASIC/SoC/.../FPGA-specific hacks.  (Which calls into
> question just how much assurance there can be that
> one driver will work reliably for all instances...

Essentially all this sort of GPIO controller is is a straight wire
through of a set of register bits to an output signal (especially if you
don't have a separate clear register).  It's the first thing a hardware
engineer would think of for implementing this - it's not even copy and
paste really, my understanding is that normally it's just basic plumbing
to wire the relevant signals together.

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[Alan Cox]

O> If there are "zillions" that suggests the HW
> engineers have version/naming issues just like
> certain software engineers.  Only goes to show
> how close Verilog and VHDL are to software! :)

Verilog ? Its far far more primitive than that. You'll find exactly that
arrangement in prehistoric 8bit micro support chips - eg the 6522. In
fact the only reason I can see for not using it on primitive 8bit micro
devices is that they *already* had more features.

> are not reusing a named module; they are at best
> just copying/pasting some Verilog/VHDL and adding
> ASIC/SoC/.../FPGA-specific hacks.  (Which calls into
> question just how much assurance there can be that
> one driver will work reliably for all instances...

No they are implementing trivial common sense logic. It's a bit like
complaining we have multiple drivers that use addition.

I think you need a reality check. Its a VLSI undergraduate project level
device, or was back when they taught undergraduates a sampling of chip
design by drawing the transistors. This is "my first logic design" stuff.

Alan

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[David Brownell]

--- On Thu, 8/26/10, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
 (Which calls
> into addition.
> 
> I think you need a reality check.

I know you don't say that every time someone makes
different assumptions about context than you have.
So for now I guess I'll just feel special...

And I expect you'd have to admit that if indeed there
were some IP reuse happening, then everything I said
which assumed reuse would be true.  Likewise, that
such reuse does happen even for simple stuff like
GPIOs (to facilitate reuse of program code ... even
on  the 8-bit micros you mentioned, I've seen it).

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[Alan Cox]

On Thu, 26 Aug 2010 15:58:51 -0700 (PDT)
David Brownell <david-b@pacbell.net> wrote:

> 
> 
> --- On Thu, 8/26/10, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>  (Which calls
> > into addition.
> > 
> > I think you need a reality check.
> 
> I know you don't say that every time someone makes
> different assumptions about context than you have.

I don't think its assumptions. In this case it's a case of actually
looking at what these devices are in the real world. Hence 'reality
check'.

Sure there are plenty of 're-usable' components that appear everywhere -
Designware etch turns up in everything but this isn't one of them.

[PATCH v2] gpio: Add driver for Anton GPIO controllers

[Anton Vorontsov]

The Anton GPIO controllers are not so complex controllers that may
be found in various on-board FPGAs that are used to control board's
switches, LEDs, chip-selects, Ethernet/USB PHY power, etc.

Usually these controllers do not privide any means of pin setup
(in/out/open drain).

The driver provides:
- Support for 8/16/32/64 bits registers;
- Support for GPIO controllers with clear/set registers;
- Support for GPIO controllers with a single "data" register;
- Support for big endian bits/GPIOs ordering (mostly used on PowerPC).

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---

In v2:

- Hopefully addressed David's comments regarding driver name;
- Now the driver fills gpio_chip.label.

 drivers/gpio/Kconfig      |    5 +
 drivers/gpio/Makefile     |    1 +
 drivers/gpio/anton_gpio.c |  244 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 250 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/anton_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f623953..4658ca6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -70,6 +70,11 @@ config GPIO_MAX730X
 
 comment "Memory mapped GPIO expanders:"
 
+config GPIO_ANTON
+	tristate "Anton GPIO controllers support"
+	help
+	  Say yes here to support Anton GPIO controllers.
+
 config GPIO_IT8761E
 	tristate "IT8761E GPIO support"
 	depends on GPIOLIB
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a69e060..a31ddae 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 
 obj-$(CONFIG_GPIO_ADP5520)	+= adp5520-gpio.o
 obj-$(CONFIG_GPIO_ADP5588)	+= adp5588-gpio.o
+obj-$(CONFIG_GPIO_ANTON)	+= anton_gpio.o
 obj-$(CONFIG_GPIO_LANGWELL)	+= langwell_gpio.o
 obj-$(CONFIG_GPIO_MAX730X)	+= max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= max7300.o
diff --git a/drivers/gpio/anton_gpio.c b/drivers/gpio/anton_gpio.c
new file mode 100644
index 0000000..d044385
--- /dev/null
+++ b/drivers/gpio/anton_gpio.c
@@ -0,0 +1,244 @@
+/*
+ * Anton GPIO driver
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/init.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/log2.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+
+struct agpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_dat;
+	void __iomem *reg_set;
+	void __iomem *reg_clr;
+	spinlock_t lock;
+
+	int bits;
+	int big_endian_bits;
+
+	/* shadowed data register to clear/set bits safely */
+	unsigned long data;
+};
+
+static struct agpio_chip *to_agpio_chip(struct gpio_chip *gc)
+{
+	return container_of(gc, struct agpio_chip, gc);
+}
+
+static unsigned long agpio_in(struct agpio_chip *agc)
+{
+	switch (agc->bits) {
+	case 8:
+		return __raw_readb(agc->reg_dat);
+	case 16:
+		return __raw_readw(agc->reg_dat);
+	case 32:
+		return __raw_readl(agc->reg_dat);
+#if BITS_PER_LONG >= 64
+	case 64:
+		return __raw_readq(agc->reg_dat);
+#endif
+	}
+	BUG();
+}
+
+static void agpio_out(struct agpio_chip *agc, void __iomem *reg,
+		      unsigned long data)
+{
+	switch (agc->bits) {
+	case 8:
+		__raw_writeb(data, reg);
+		return;
+	case 16:
+		__raw_writew(data, reg);
+		return;
+	case 32:
+		__raw_writel(data, reg);
+		return;
+#if BITS_PER_LONG >= 64
+	case 64:
+		__raw_writeq(data, reg);
+		return;
+#endif
+	}
+	BUG();
+}
+
+static unsigned long agpio_pin2mask(struct agpio_chip *agc, unsigned int pin)
+{
+	if (agc->big_endian_bits)
+		return 1 << (agc->bits - 1 - pin);
+	else
+		return 1 << pin;
+}
+
+static int agpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct agpio_chip *agc = to_agpio_chip(gc);
+
+	return agpio_in(agc) & agpio_pin2mask(agc, gpio);
+}
+
+static void agpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct agpio_chip *agc = to_agpio_chip(gc);
+	unsigned long mask = agpio_pin2mask(agc, gpio);
+	unsigned long flags;
+
+	if (agc->reg_set) {
+		if (val)
+			agpio_out(agc, agc->reg_set, mask);
+		else
+			agpio_out(agc, agc->reg_clr, mask);
+		return;
+	}
+
+	spin_lock_irqsave(&agc->lock, flags);
+
+	if (val)
+		agc->data |= mask;
+	else
+		agc->data &= ~mask;
+
+	agpio_out(agc, agc->reg_dat, agc->data);
+
+	spin_unlock_irqrestore(&agc->lock, flags);
+}
+
+static int agpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int agpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	agpio_set(gc, gpio, val);
+	return 0;
+}
+
+static int __devinit agpio_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	struct device *dev = &pdev->dev;
+	struct agpio_chip *agc;
+	struct resource *res_dat;
+	struct resource *res_set;
+	struct resource *res_clr;
+	resource_size_t dat_sz;
+	int bits;
+
+	res_dat = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res_dat)
+		return -EINVAL;
+
+	dat_sz = resource_size(res_dat);
+	if (!is_power_of_2(dat_sz))
+		return -EINVAL;
+
+	bits = dat_sz * 8;
+	if (bits > BITS_PER_LONG)
+		return -EINVAL;
+
+	agc = devm_kzalloc(dev, sizeof(*agc), GFP_KERNEL);
+	if (!agc)
+		return -ENOMEM;
+
+	agc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+	if (!agc->reg_dat)
+		return -ENOMEM;
+
+	res_set = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	res_clr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (res_set && res_clr) {
+		if (resource_size(res_set) != resource_size(res_clr) ||
+				resource_size(res_set) != dat_sz)
+			return -EINVAL;
+
+		agc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
+		agc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+		if (!agc->reg_set || !agc->reg_clr)
+			return -ENOMEM;
+	} else if (res_set || res_clr) {
+		return -EINVAL;
+	}
+
+	spin_lock_init(&agc->lock);
+
+	agc->bits = bits;
+	agc->big_endian_bits = !strcmp(platid->name, "anton-gpio-be");
+	agc->data = agpio_in(agc);
+
+	agc->gc.ngpio = bits;
+	agc->gc.direction_input = agpio_dir_in;
+	agc->gc.direction_output = agpio_dir_out;
+	agc->gc.get = agpio_get;
+	agc->gc.set = agpio_set;
+	agc->gc.label = dev_name(dev);
+
+	if (dev->platform_data)
+		agc->gc.base = (unsigned long)dev->platform_data;
+	else
+		agc->gc.base = -1;
+
+	dev_set_drvdata(dev, agc);
+
+	return gpiochip_add(&agc->gc);
+}
+
+static int __devexit agpio_remove(struct platform_device *pdev)
+{
+	struct agpio_chip *agc = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&agc->gc);
+}
+
+static const struct platform_device_id agpio_id_table[] = {
+	{ "anton-gpio", },
+	{ "anton-gpio-be", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, agpio_id_table);
+
+static struct platform_driver agpio_driver = {
+	.driver = {
+		.name = "anton-gpio",
+	},
+	.id_table = agpio_id_table,
+	.probe = agpio_probe,
+	.remove = __devexit_p(agpio_remove),
+};
+
+static int __init agpio_init(void)
+{
+	return platform_driver_register(&agpio_driver);
+}
+module_init(agpio_init);
+
+static void __exit agpio_exit(void)
+{
+	platform_driver_unregister(&agpio_driver);
+}
+module_exit(agpio_exit);
+
+MODULE_DESCRIPTION("Driver for Anton GPIO controllers");
+MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.5

Re: [PATCH v2] gpio: Add driver for Anton GPIO controllers

[David Brownell]

--- On Thu, 8/26/10, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
>
> - Hopefully addressed David's comments
> regarding driver name;

I'm not sure about the precedent of putting
developer names in source.c and driver names...
start that, and everyone will want to follow!!

OK, so Linus started that one, arguably...  :)

Not that I want to be naming this, but how
about a tweak to Alan's suggestion, keeping
the name technical?

   "gpio_basic_mmio.c" ... or similar?

Re: [PATCH v2] gpio: Add driver for Anton GPIO controllers

[Anton Vorontsov]

On Thu, Aug 26, 2010 at 10:57:33AM -0700, David Brownell wrote:
[...]
> Not that I want to be naming this, but how
> about a tweak to Alan's suggestion, keeping
> the name technical?
> 
>    "gpio_basic_mmio.c" ... or similar?

I would agree yesterday, but now I tend to like
anton_gpio.c. :-D

You seemed to be fine with meaningless amba-9999
or dw_abc names, but now, when I propose such a
marvellous name, you seem to lean to a synonym of
an old one, the one that I used in v1? Wouldn't
it be a step backwards? :-)

Just kiding, of course. basic_mmio.c works for me.

I also think it makes sense to drop gpio_ prefix,
as the driver is already in gpio/ directory.

Thanks!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH v2] gpio: Add driver for Anton GPIO controllers

[David Brownell]

--- On Thu, 8/26/10, Anton Vorontsov <cbouatmailru@gmail.com> wrote:

> From: Anton Vorontsov <cbouatmailru@gmail.com>
> Subject: Re: [PATCH v2] gpio: Add driver for Anton GPIO controllers
> To: "David Brownell" <david-b@pacbell.net>
> Cc: "Andrew Morton" <akpm@linux-foundation.org>, "Samuel Ortiz" <sameo@linux.intel.com>, "Mark Brown" <broonie@opensource.wolfsonmicro.com>, "David Brownell" <dbrownell@users.sourceforge.net>, "Alan Cox" <alan@lxorguk.ukuu.org.uk>, linux-kernel@vger.kernel.org
> Date: Thursday, August 26, 2010, 2:20 PM
> On Thu, Aug 26, 2010 at 10:57:33AM
> -0700, David Brownell wrote:
> [...]
>  
> You seemed to be fine with meaningless amba-9999
> or dw_abc names, but 

NOT meaningless ... when there's actual IP behind
the controller!  Which, at first, is what I thought
you were talking about.


> Just kiding, of course. basic_mmio.c works for me.

OK, how about resubmitting?  With that name?

Be sure to add a bit of documentation about 
the controller requirements  ... basically that
it has a register from which all values can be
read, and that either that register can be written,
or there's a pair of set-bit/clear-bit registers
affecting that register and the output pins.

Might be worth mentioning how trivial it is to do
that in hardware like CPLDs/FPGAS/etc, which is
why this handles different word sizes/endianness.
and the expectation that in at least some cases
this will be used with roll-your-own ASIC/FPGA
logic in Verilog or VHDL (yes?).

> I also think it makes sense to drop gpio_ prefix,
> as the driver is already in gpio/ directory.

No, keep it; files move around sometimes, and it's
best if that loses no information.

[PATCH v3] gpio: Add driver for basic memory-mapped GPIO controllers

[Anton Vorontsov]

The basic GPIO controllers may be found in various on-board FPGA
and ASIC solutions that are used to control board's switches, LEDs,
chip-selects, Ethernet/USB PHY power, etc.

Usually these controllers do not privide any means of pin setup
(in/out/open drain).

The driver provides:
- Support for 8/16/32/64 bits registers;
- Support for GPIO controllers with clear/set registers;
- Support for GPIO controllers with a single "data" register;
- Support for big endian bits/GPIOs ordering (mostly used on PowerPC).

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---

On Thu, Aug 26, 2010 at 03:48:31PM -0700, David Brownell wrote:
[...]
> > Just kiding, of course. basic_mmio.c works for me.
> 
> OK, how about resubmitting?  With that name?

Sure thing.

> Be sure to add a bit of documentation about 
> the controller requirements  ... basically that
> it has a register from which all values can be
> read, and that either that register can be written,
> or there's a pair of set-bit/clear-bit registers
> affecting that register and the output pins.
> 
> Might be worth mentioning how trivial it is to do
> that in hardware like CPLDs/FPGAS/etc, which is
> why this handles different word sizes/endianness.
> and the expectation that in at least some cases
> this will be used with roll-your-own ASIC/FPGA
> logic in Verilog or VHDL (yes?).

Yep. Done.

> > I also think it makes sense to drop gpio_ prefix,
> > as the driver is already in gpio/ directory.
> 
> No, keep it; files move around sometimes, and it's
> best if that loses no information.

OK.

Also addressed Mark's comments wrt getting resources
by name. Plus, it appears that there's also gc.dev field
in gpio_chip, fill it too.

I hope it's perfect now. :-)

Thanks!

 drivers/gpio/Kconfig           |    5 +
 drivers/gpio/Makefile          |    1 +
 drivers/gpio/basic_mmio_gpio.c |  278 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 284 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/basic_mmio_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f623953..d1d1c24 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -70,6 +70,11 @@ config GPIO_MAX730X
 
 comment "Memory mapped GPIO expanders:"
 
+config GPIO_BASIC_MMIO
+	tristate "Basic memory-mapped GPIO controllers support"
+	help
+	  Say yes here to support basic memory-mapped GPIO controllers.
+
 config GPIO_IT8761E
 	tristate "IT8761E GPIO support"
 	depends on GPIOLIB
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a69e060..dfb571b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 
 obj-$(CONFIG_GPIO_ADP5520)	+= adp5520-gpio.o
 obj-$(CONFIG_GPIO_ADP5588)	+= adp5588-gpio.o
+obj-$(CONFIG_GPIO_BASIC_MMIO)	+= basic_mmio_gpio.o
 obj-$(CONFIG_GPIO_LANGWELL)	+= langwell_gpio.o
 obj-$(CONFIG_GPIO_MAX730X)	+= max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= max7300.o
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
new file mode 100644
index 0000000..6526959
--- /dev/null
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -0,0 +1,278 @@
+/*
+ * Driver for basic memory-mapped GPIO controllers.
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * ....``.```~~~~````.`.`.`.`.```````'',,,.........`````......`.......
+ * ...``                                                         ```````..
+ * ..The simplest form of a GPIO controller that the driver supports is``
+ *  `.just a single "data" register, where GPIO state can be read and/or `
+ *      ..written. ,,..``~~~~ .....``.`.`.~~.```.`.........``````.```````
+ *        ```````
+ *             __/~~|___/~|   . ```~~~~~~       ,~.`.`.`.`````.~~...,,,,...
+ *            |___________|~$@~~~        %~~   .. Implementing such a GPIO .
+ *              o        `                     . ` controller in FPGA is,`.
+ *                                              `....trivial. '~`.```.````
+ *                                                   ```````
+ *  .```````~~~~`..`.``.``.
+ * .  The driver supports  `...       ,..```.`~~~```````````````....````.``,,.
+ * .   big-endian notation, just`.  ... A bit more sophisticated controllers ,
+ *  . register the device with -be`. . with a pair of set/clear-bit registers ,
+ *   `.. suffix.  ```~~`````....`.`   . affecting the data register and the .`
+ *     ``.`.``...```                  ```.. output pins are also supported.`
+ *                        ^^             `````.`````````.,``~``~``~~``````
+ *                                                   .                  ^^
+ *   ,..`.`.`...````````````......`.`.`.`.`.`..`.`.`..
+ * .. The expectation is that in at least some cases .    ,-~~~-,
+ *  .this will be used with roll-your-own ASIC/FPGA .`     \   /
+ *  .logic in Verilog or VHDL. ~~~`````````..`````~~`       \ /
+ *  ..````````......```````````                             \o_
+ *                                                           |
+ *                              ^^                          / \
+ *
+ *           ...`````~~`.....``.`..........``````.`.``.```........``.
+ *            `  8, 16, 32 and 64 bits registers are supported, and``.
+ *            . the number of GPIOs is determined by the width of   ~
+ *             .. the registers. ,............```.`.`..`.`.~~~.`.`.`~
+ *               `.......````.```
+ */
+
+#include <linux/init.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/log2.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+
+struct bgpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_dat;
+	void __iomem *reg_set;
+	void __iomem *reg_clr;
+	spinlock_t lock;
+
+	int bits;
+	int big_endian_bits;
+
+	/* shadowed data register to clear/set bits safely */
+	unsigned long data;
+};
+
+static struct bgpio_chip *to_bgpio_chip(struct gpio_chip *gc)
+{
+	return container_of(gc, struct bgpio_chip, gc);
+}
+
+static unsigned long bgpio_in(struct bgpio_chip *bgc)
+{
+	switch (bgc->bits) {
+	case 8:
+		return __raw_readb(bgc->reg_dat);
+	case 16:
+		return __raw_readw(bgc->reg_dat);
+	case 32:
+		return __raw_readl(bgc->reg_dat);
+#if BITS_PER_LONG >= 64
+	case 64:
+		return __raw_readq(bgc->reg_dat);
+#endif
+	}
+	BUG();
+}
+
+static void bgpio_out(struct bgpio_chip *bgc, void __iomem *reg,
+		      unsigned long data)
+{
+	switch (bgc->bits) {
+	case 8:
+		__raw_writeb(data, reg);
+		return;
+	case 16:
+		__raw_writew(data, reg);
+		return;
+	case 32:
+		__raw_writel(data, reg);
+		return;
+#if BITS_PER_LONG >= 64
+	case 64:
+		__raw_writeq(data, reg);
+		return;
+#endif
+	}
+	BUG();
+}
+
+static unsigned long bgpio_pin2mask(struct bgpio_chip *bgc, unsigned int pin)
+{
+	if (bgc->big_endian_bits)
+		return 1 << (bgc->bits - 1 - pin);
+	else
+		return 1 << pin;
+}
+
+static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+	return bgpio_in(bgc) & bgpio_pin2mask(bgc, gpio);
+}
+
+static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+	unsigned long mask = bgpio_pin2mask(bgc, gpio);
+	unsigned long flags;
+
+	if (bgc->reg_set) {
+		if (val)
+			bgpio_out(bgc, bgc->reg_set, mask);
+		else
+			bgpio_out(bgc, bgc->reg_clr, mask);
+		return;
+	}
+
+	spin_lock_irqsave(&bgc->lock, flags);
+
+	if (val)
+		bgc->data |= mask;
+	else
+		bgc->data &= ~mask;
+
+	bgpio_out(bgc, bgc->reg_dat, bgc->data);
+
+	spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	bgpio_set(gc, gpio, val);
+	return 0;
+}
+
+static int __devinit bgpio_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	struct device *dev = &pdev->dev;
+	struct bgpio_chip *bgc;
+	struct resource *res_dat;
+	struct resource *res_set;
+	struct resource *res_clr;
+	resource_size_t dat_sz;
+	int bits;
+
+	res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
+	if (!res_dat)
+		return -EINVAL;
+
+	dat_sz = resource_size(res_dat);
+	if (!is_power_of_2(dat_sz))
+		return -EINVAL;
+
+	bits = dat_sz * 8;
+	if (bits > BITS_PER_LONG)
+		return -EINVAL;
+
+	bgc = devm_kzalloc(dev, sizeof(*bgc), GFP_KERNEL);
+	if (!bgc)
+		return -ENOMEM;
+
+	bgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+	if (!bgc->reg_dat)
+		return -ENOMEM;
+
+	res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
+	res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
+	if (res_set && res_clr) {
+		if (resource_size(res_set) != resource_size(res_clr) ||
+				resource_size(res_set) != dat_sz)
+			return -EINVAL;
+
+		bgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
+		bgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+		if (!bgc->reg_set || !bgc->reg_clr)
+			return -ENOMEM;
+	} else if (res_set || res_clr) {
+		return -EINVAL;
+	}
+
+	spin_lock_init(&bgc->lock);
+
+	bgc->bits = bits;
+	bgc->big_endian_bits = !strcmp(platid->name, "basic-mmio-gpio-be");
+	bgc->data = bgpio_in(bgc);
+
+	bgc->gc.ngpio = bits;
+	bgc->gc.direction_input = bgpio_dir_in;
+	bgc->gc.direction_output = bgpio_dir_out;
+	bgc->gc.get = bgpio_get;
+	bgc->gc.set = bgpio_set;
+	bgc->gc.dev = dev;
+	bgc->gc.label = dev_name(dev);
+
+	if (dev->platform_data)
+		bgc->gc.base = (unsigned long)dev->platform_data;
+	else
+		bgc->gc.base = -1;
+
+	dev_set_drvdata(dev, bgc);
+
+	return gpiochip_add(&bgc->gc);
+}
+
+static int __devexit bgpio_remove(struct platform_device *pdev)
+{
+	struct bgpio_chip *bgc = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&bgc->gc);
+}
+
+static const struct platform_device_id bgpio_id_table[] = {
+	{ "basic-mmio-gpio", },
+	{ "basic-mmio-gpio-be", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, bgpio_id_table);
+
+static struct platform_driver bgpio_driver = {
+	.driver = {
+		.name = "basic-mmio-gpio",
+	},
+	.id_table = bgpio_id_table,
+	.probe = bgpio_probe,
+	.remove = __devexit_p(bgpio_remove),
+};
+
+static int __init bgpio_init(void)
+{
+	return platform_driver_register(&bgpio_driver);
+}
+module_init(bgpio_init);
+
+static void __exit bgpio_exit(void)
+{
+	platform_driver_unregister(&bgpio_driver);
+}
+module_exit(bgpio_exit);
+
+MODULE_DESCRIPTION("Driver for basic memory-mapped GPIO controllers");
+MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.5

Re: [PATCH v3] gpio: Add driver for basic memory-mapped GPIO controllers

[David Brownell]

Much better, thanks!

But  do it without BUG()... assume that all
uses of BUG() are errors, unless the OS
really has *NO* way to continue operating.
In this case, it can clearly limp along.


- Dave

[PATCH v4] gpio: Add driver for basic memory-mapped GPIO controllers

[Anton Vorontsov]

The basic GPIO controllers may be found in various on-board FPGA
and ASIC solutions that are used to control board's switches, LEDs,
chip-selects, Ethernet/USB PHY power, etc.

Usually these controllers do not privide any means of pin setup
(in/out/open drain).

The driver provides:
- Support for 8/16/32/64 bits registers;
- Support for GPIO controllers with clear/set registers;
- Support for GPIO controllers with a single "data" register;
- Support for big endian bits/GPIOs ordering (mostly used on PowerPC).

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---

On Sat, Aug 28, 2010 at 12:08:20PM -0700, David Brownell wrote:
> But  do it without BUG()... assume that all
> uses of BUG() are errors, unless the OS
> really has *NO* way to continue operating.
> In this case, it can clearly limp along.

Yeah, good idea. Let's turn these BUG() into WARN_ON_ONCE()?

Thanks!

 drivers/gpio/Kconfig           |    5 +
 drivers/gpio/Makefile          |    1 +
 drivers/gpio/basic_mmio_gpio.c |  280 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 286 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/basic_mmio_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f623953..d1d1c24 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -70,6 +70,11 @@ config GPIO_MAX730X
 
 comment "Memory mapped GPIO expanders:"
 
+config GPIO_BASIC_MMIO
+	tristate "Basic memory-mapped GPIO controllers support"
+	help
+	  Say yes here to support basic memory-mapped GPIO controllers.
+
 config GPIO_IT8761E
 	tristate "IT8761E GPIO support"
 	depends on GPIOLIB
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a69e060..dfb571b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 
 obj-$(CONFIG_GPIO_ADP5520)	+= adp5520-gpio.o
 obj-$(CONFIG_GPIO_ADP5588)	+= adp5588-gpio.o
+obj-$(CONFIG_GPIO_BASIC_MMIO)	+= basic_mmio_gpio.o
 obj-$(CONFIG_GPIO_LANGWELL)	+= langwell_gpio.o
 obj-$(CONFIG_GPIO_MAX730X)	+= max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= max7300.o
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
new file mode 100644
index 0000000..7117f1d
--- /dev/null
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -0,0 +1,280 @@
+/*
+ * Driver for basic memory-mapped GPIO controllers.
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * ....``.```~~~~````.`.`.`.`.```````'',,,.........`````......`.......
+ * ...``                                                         ```````..
+ * ..The simplest form of a GPIO controller that the driver supports is``
+ *  `.just a single "data" register, where GPIO state can be read and/or `
+ *    `,..written. ,,..``~~~~ .....``.`.`.~~.```.`.........``````.```````
+ *        `````````
+                                    ___
+_/~~|___/~|   . ```~~~~~~       ___/___\___     ,~.`.`.`.`````.~~...,,,,...
+__________|~$@~~~        %~    /o*o*o*o*o*o\   .. Implementing such a GPIO .
+o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
+                                                 `....trivial..'~`.```.```
+ *                                                    ```````
+ *  .```````~~~~`..`.``.``.
+ * .  The driver supports  `...       ,..```.`~~~```````````````....````.``,,
+ * .   big-endian notation, just`.  .. A bit more sophisticated controllers ,
+ *  . register the device with -be`. .with a pair of set/clear-bit registers ,
+ *   `.. suffix.  ```~~`````....`.`   . affecting the data register and the .`
+ *     ``.`.``...```                  ```.. output pins are also supported.`
+ *                        ^^             `````.`````````.,``~``~``~~``````
+ *                                                   .                  ^^
+ *   ,..`.`.`...````````````......`.`.`.`.`.`..`.`.`..
+ * .. The expectation is that in at least some cases .    ,-~~~-,
+ *  .this will be used with roll-your-own ASIC/FPGA .`     \   /
+ *  .logic in Verilog or VHDL. ~~~`````````..`````~~`       \ /
+ *  ..````````......```````````                             \o_
+ *                                                           |
+ *                              ^^                          / \
+ *
+ *           ...`````~~`.....``.`..........``````.`.``.```........``.
+ *            `  8, 16, 32 and 64 bits registers are supported, and``.
+ *            . the number of GPIOs is determined by the width of   ~
+ *             .. the registers. ,............```.`.`..`.`.~~~.`.`.`~
+ *               `.......````.```
+ */
+
+#include <linux/init.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/log2.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+
+struct bgpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_dat;
+	void __iomem *reg_set;
+	void __iomem *reg_clr;
+	spinlock_t lock;
+
+	int bits;
+	int big_endian_bits;
+
+	/* shadowed data register to clear/set bits safely */
+	unsigned long data;
+};
+
+static struct bgpio_chip *to_bgpio_chip(struct gpio_chip *gc)
+{
+	return container_of(gc, struct bgpio_chip, gc);
+}
+
+static unsigned long bgpio_in(struct bgpio_chip *bgc)
+{
+	switch (bgc->bits) {
+	case 8:
+		return __raw_readb(bgc->reg_dat);
+	case 16:
+		return __raw_readw(bgc->reg_dat);
+	case 32:
+		return __raw_readl(bgc->reg_dat);
+#if BITS_PER_LONG >= 64
+	case 64:
+		return __raw_readq(bgc->reg_dat);
+#endif
+	}
+	WARN_ON_ONCE(1);
+	return 0;
+}
+
+static void bgpio_out(struct bgpio_chip *bgc, void __iomem *reg,
+		      unsigned long data)
+{
+	switch (bgc->bits) {
+	case 8:
+		__raw_writeb(data, reg);
+		return;
+	case 16:
+		__raw_writew(data, reg);
+		return;
+	case 32:
+		__raw_writel(data, reg);
+		return;
+#if BITS_PER_LONG >= 64
+	case 64:
+		__raw_writeq(data, reg);
+		return;
+#endif
+	}
+	WARN_ON_ONCE(1);
+}
+
+static unsigned long bgpio_pin2mask(struct bgpio_chip *bgc, unsigned int pin)
+{
+	if (bgc->big_endian_bits)
+		return 1 << (bgc->bits - 1 - pin);
+	else
+		return 1 << pin;
+}
+
+static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+	return bgpio_in(bgc) & bgpio_pin2mask(bgc, gpio);
+}
+
+static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+	unsigned long mask = bgpio_pin2mask(bgc, gpio);
+	unsigned long flags;
+
+	if (bgc->reg_set) {
+		if (val)
+			bgpio_out(bgc, bgc->reg_set, mask);
+		else
+			bgpio_out(bgc, bgc->reg_clr, mask);
+		return;
+	}
+
+	spin_lock_irqsave(&bgc->lock, flags);
+
+	if (val)
+		bgc->data |= mask;
+	else
+		bgc->data &= ~mask;
+
+	bgpio_out(bgc, bgc->reg_dat, bgc->data);
+
+	spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	bgpio_set(gc, gpio, val);
+	return 0;
+}
+
+static int __devinit bgpio_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	struct device *dev = &pdev->dev;
+	struct bgpio_chip *bgc;
+	struct resource *res_dat;
+	struct resource *res_set;
+	struct resource *res_clr;
+	resource_size_t dat_sz;
+	int bits;
+
+	res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
+	if (!res_dat)
+		return -EINVAL;
+
+	dat_sz = resource_size(res_dat);
+	if (!is_power_of_2(dat_sz))
+		return -EINVAL;
+
+	bits = dat_sz * 8;
+	if (bits > BITS_PER_LONG)
+		return -EINVAL;
+
+	bgc = devm_kzalloc(dev, sizeof(*bgc), GFP_KERNEL);
+	if (!bgc)
+		return -ENOMEM;
+
+	bgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+	if (!bgc->reg_dat)
+		return -ENOMEM;
+
+	res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
+	res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
+	if (res_set && res_clr) {
+		if (resource_size(res_set) != resource_size(res_clr) ||
+				resource_size(res_set) != dat_sz)
+			return -EINVAL;
+
+		bgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
+		bgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+		if (!bgc->reg_set || !bgc->reg_clr)
+			return -ENOMEM;
+	} else if (res_set || res_clr) {
+		return -EINVAL;
+	}
+
+	spin_lock_init(&bgc->lock);
+
+	bgc->bits = bits;
+	bgc->big_endian_bits = !strcmp(platid->name, "basic-mmio-gpio-be");
+	bgc->data = bgpio_in(bgc);
+
+	bgc->gc.ngpio = bits;
+	bgc->gc.direction_input = bgpio_dir_in;
+	bgc->gc.direction_output = bgpio_dir_out;
+	bgc->gc.get = bgpio_get;
+	bgc->gc.set = bgpio_set;
+	bgc->gc.dev = dev;
+	bgc->gc.label = dev_name(dev);
+
+	if (dev->platform_data)
+		bgc->gc.base = (unsigned long)dev->platform_data;
+	else
+		bgc->gc.base = -1;
+
+	dev_set_drvdata(dev, bgc);
+
+	return gpiochip_add(&bgc->gc);
+}
+
+static int __devexit bgpio_remove(struct platform_device *pdev)
+{
+	struct bgpio_chip *bgc = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&bgc->gc);
+}
+
+static const struct platform_device_id bgpio_id_table[] = {
+	{ "basic-mmio-gpio", },
+	{ "basic-mmio-gpio-be", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, bgpio_id_table);
+
+static struct platform_driver bgpio_driver = {
+	.driver = {
+		.name = "basic-mmio-gpio",
+	},
+	.id_table = bgpio_id_table,
+	.probe = bgpio_probe,
+	.remove = __devexit_p(bgpio_remove),
+};
+
+static int __init bgpio_init(void)
+{
+	return platform_driver_register(&bgpio_driver);
+}
+module_init(bgpio_init);
+
+static void __exit bgpio_exit(void)
+{
+	platform_driver_unregister(&bgpio_driver);
+}
+module_exit(bgpio_exit);
+
+MODULE_DESCRIPTION("Driver for basic memory-mapped GPIO controllers");
+MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.5

Re: [PATCH v4] gpio: Add driver for basic memory-mapped GPIO controllers

[David Brownell]

--- On Sun, 8/29/10, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
Brownell
> wrote:
> > But  do it without BUG()... 
> Yeah, good idea. Let's turn these BUG() into
> WARN_ON_ONCE()?

I'm not a big fan of redundant -- and
 noisy! error reports like
that:  recall that errors were already reported
(and ignored) on those paths, indicating invalid
GPIOs (requested, or direction set, etc).

The read paths should probably just return zero;
that's required in some cases, if you read the
interface spec.

[PATCH v5] gpio: Add driver for basic memory-mapped GPIO controllers

[Anton Vorontsov]

The basic GPIO controllers may be found in various on-board FPGA
and ASIC solutions that are used to control board's switches, LEDs,
chip-selects, Ethernet/USB PHY power, etc.

Usually these controllers do not privide any means of pin setup
(in/out/open drain).

The driver provides:
- Support for 8/16/32/64 bits registers;
- Support for GPIO controllers with clear/set registers;
- Support for GPIO controllers with a single "data" register;
- Support for big endian bits/GPIOs ordering (mostly used on PowerPC).

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---

On Mon, Aug 30, 2010 at 01:23:18PM -0700, David Brownell wrote:
> --- On Sun, 8/29/10, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
> > > But  do it without BUG()... 
> > Yeah, good idea. Let's turn these BUG() into
> > WARN_ON_ONCE()?
> 
> I'm not a big fan of redundant -- and
>  noisy! error reports like
> that:  recall that errors were already reported
> (and ignored) on those paths, indicating invalid
> GPIOs (requested, or direction set, etc).

OK, here's another version. Looks good?

Thanks,

 drivers/gpio/Kconfig           |    5 +
 drivers/gpio/Makefile          |    1 +
 drivers/gpio/basic_mmio_gpio.c |  278 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 284 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/basic_mmio_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f623953..d1d1c24 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -70,6 +70,11 @@ config GPIO_MAX730X
 
 comment "Memory mapped GPIO expanders:"
 
+config GPIO_BASIC_MMIO
+	tristate "Basic memory-mapped GPIO controllers support"
+	help
+	  Say yes here to support basic memory-mapped GPIO controllers.
+
 config GPIO_IT8761E
 	tristate "IT8761E GPIO support"
 	depends on GPIOLIB
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a69e060..dfb571b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 
 obj-$(CONFIG_GPIO_ADP5520)	+= adp5520-gpio.o
 obj-$(CONFIG_GPIO_ADP5588)	+= adp5588-gpio.o
+obj-$(CONFIG_GPIO_BASIC_MMIO)	+= basic_mmio_gpio.o
 obj-$(CONFIG_GPIO_LANGWELL)	+= langwell_gpio.o
 obj-$(CONFIG_GPIO_MAX730X)	+= max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= max7300.o
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
new file mode 100644
index 0000000..ff5bc06
--- /dev/null
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -0,0 +1,278 @@
+/*
+ * Driver for basic memory-mapped GPIO controllers.
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * ....``.```~~~~````.`.`.`.`.```````'',,,.........`````......`.......
+ * ...``                                                         ```````..
+ * ..The simplest form of a GPIO controller that the driver supports is``
+ *  `.just a single "data" register, where GPIO state can be read and/or `
+ *    `,..written. ,,..``~~~~ .....``.`.`.~~.```.`.........``````.```````
+ *        `````````
+                                    ___
+_/~~|___/~|   . ```~~~~~~       ___/___\___     ,~.`.`.`.`````.~~...,,,,...
+__________|~$@~~~        %~    /o*o*o*o*o*o\   .. Implementing such a GPIO .
+o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
+                                                 `....trivial..'~`.```.```
+ *                                                    ```````
+ *  .```````~~~~`..`.``.``.
+ * .  The driver supports  `...       ,..```.`~~~```````````````....````.``,,
+ * .   big-endian notation, just`.  .. A bit more sophisticated controllers ,
+ *  . register the device with -be`. .with a pair of set/clear-bit registers ,
+ *   `.. suffix.  ```~~`````....`.`   . affecting the data register and the .`
+ *     ``.`.``...```                  ```.. output pins are also supported.`
+ *                        ^^             `````.`````````.,``~``~``~~``````
+ *                                                   .                  ^^
+ *   ,..`.`.`...````````````......`.`.`.`.`.`..`.`.`..
+ * .. The expectation is that in at least some cases .    ,-~~~-,
+ *  .this will be used with roll-your-own ASIC/FPGA .`     \   /
+ *  .logic in Verilog or VHDL. ~~~`````````..`````~~`       \ /
+ *  ..````````......```````````                             \o_
+ *                                                           |
+ *                              ^^                          / \
+ *
+ *           ...`````~~`.....``.`..........``````.`.``.```........``.
+ *            `  8, 16, 32 and 64 bits registers are supported, and``.
+ *            . the number of GPIOs is determined by the width of   ~
+ *             .. the registers. ,............```.`.`..`.`.~~~.`.`.`~
+ *               `.......````.```
+ */
+
+#include <linux/init.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/log2.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+
+struct bgpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_dat;
+	void __iomem *reg_set;
+	void __iomem *reg_clr;
+	spinlock_t lock;
+
+	int bits;
+	int big_endian_bits;
+
+	/* shadowed data register to clear/set bits safely */
+	unsigned long data;
+};
+
+static struct bgpio_chip *to_bgpio_chip(struct gpio_chip *gc)
+{
+	return container_of(gc, struct bgpio_chip, gc);
+}
+
+static unsigned long bgpio_in(struct bgpio_chip *bgc)
+{
+	switch (bgc->bits) {
+	case 8:
+		return __raw_readb(bgc->reg_dat);
+	case 16:
+		return __raw_readw(bgc->reg_dat);
+	case 32:
+		return __raw_readl(bgc->reg_dat);
+#if BITS_PER_LONG >= 64
+	case 64:
+		return __raw_readq(bgc->reg_dat);
+#endif
+	}
+	return 0;
+}
+
+static void bgpio_out(struct bgpio_chip *bgc, void __iomem *reg,
+		      unsigned long data)
+{
+	switch (bgc->bits) {
+	case 8:
+		__raw_writeb(data, reg);
+		return;
+	case 16:
+		__raw_writew(data, reg);
+		return;
+	case 32:
+		__raw_writel(data, reg);
+		return;
+#if BITS_PER_LONG >= 64
+	case 64:
+		__raw_writeq(data, reg);
+		return;
+#endif
+	}
+}
+
+static unsigned long bgpio_pin2mask(struct bgpio_chip *bgc, unsigned int pin)
+{
+	if (bgc->big_endian_bits)
+		return 1 << (bgc->bits - 1 - pin);
+	else
+		return 1 << pin;
+}
+
+static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+	return bgpio_in(bgc) & bgpio_pin2mask(bgc, gpio);
+}
+
+static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+	unsigned long mask = bgpio_pin2mask(bgc, gpio);
+	unsigned long flags;
+
+	if (bgc->reg_set) {
+		if (val)
+			bgpio_out(bgc, bgc->reg_set, mask);
+		else
+			bgpio_out(bgc, bgc->reg_clr, mask);
+		return;
+	}
+
+	spin_lock_irqsave(&bgc->lock, flags);
+
+	if (val)
+		bgc->data |= mask;
+	else
+		bgc->data &= ~mask;
+
+	bgpio_out(bgc, bgc->reg_dat, bgc->data);
+
+	spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	bgpio_set(gc, gpio, val);
+	return 0;
+}
+
+static int __devinit bgpio_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	struct device *dev = &pdev->dev;
+	struct bgpio_chip *bgc;
+	struct resource *res_dat;
+	struct resource *res_set;
+	struct resource *res_clr;
+	resource_size_t dat_sz;
+	int bits;
+
+	res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
+	if (!res_dat)
+		return -EINVAL;
+
+	dat_sz = resource_size(res_dat);
+	if (!is_power_of_2(dat_sz))
+		return -EINVAL;
+
+	bits = dat_sz * 8;
+	if (bits > BITS_PER_LONG)
+		return -EINVAL;
+
+	bgc = devm_kzalloc(dev, sizeof(*bgc), GFP_KERNEL);
+	if (!bgc)
+		return -ENOMEM;
+
+	bgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+	if (!bgc->reg_dat)
+		return -ENOMEM;
+
+	res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
+	res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
+	if (res_set && res_clr) {
+		if (resource_size(res_set) != resource_size(res_clr) ||
+				resource_size(res_set) != dat_sz)
+			return -EINVAL;
+
+		bgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
+		bgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+		if (!bgc->reg_set || !bgc->reg_clr)
+			return -ENOMEM;
+	} else if (res_set || res_clr) {
+		return -EINVAL;
+	}
+
+	spin_lock_init(&bgc->lock);
+
+	bgc->bits = bits;
+	bgc->big_endian_bits = !strcmp(platid->name, "basic-mmio-gpio-be");
+	bgc->data = bgpio_in(bgc);
+
+	bgc->gc.ngpio = bits;
+	bgc->gc.direction_input = bgpio_dir_in;
+	bgc->gc.direction_output = bgpio_dir_out;
+	bgc->gc.get = bgpio_get;
+	bgc->gc.set = bgpio_set;
+	bgc->gc.dev = dev;
+	bgc->gc.label = dev_name(dev);
+
+	if (dev->platform_data)
+		bgc->gc.base = (unsigned long)dev->platform_data;
+	else
+		bgc->gc.base = -1;
+
+	dev_set_drvdata(dev, bgc);
+
+	return gpiochip_add(&bgc->gc);
+}
+
+static int __devexit bgpio_remove(struct platform_device *pdev)
+{
+	struct bgpio_chip *bgc = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&bgc->gc);
+}
+
+static const struct platform_device_id bgpio_id_table[] = {
+	{ "basic-mmio-gpio", },
+	{ "basic-mmio-gpio-be", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, bgpio_id_table);
+
+static struct platform_driver bgpio_driver = {
+	.driver = {
+		.name = "basic-mmio-gpio",
+	},
+	.id_table = bgpio_id_table,
+	.probe = bgpio_probe,
+	.remove = __devexit_p(bgpio_remove),
+};
+
+static int __init bgpio_init(void)
+{
+	return platform_driver_register(&bgpio_driver);
+}
+module_init(bgpio_init);
+
+static void __exit bgpio_exit(void)
+{
+	platform_driver_unregister(&bgpio_driver);
+}
+module_exit(bgpio_exit);
+
+MODULE_DESCRIPTION("Driver for basic memory-mapped GPIO controllers");
+MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.5

Re: [PATCH v5] gpio: Add driver for basic memory-mapped GPIO controllers

[Mark Brown]

On Tue, Aug 31, 2010 at 09:58:39PM +0400, Anton Vorontsov wrote:
> The basic GPIO controllers may be found in various on-board FPGA
> and ASIC solutions that are used to control board's switches, LEDs,
> chip-selects, Ethernet/USB PHY power, etc.
> 
> Usually these controllers do not privide any means of pin setup
> (in/out/open drain).
> 
> The driver provides:
> - Support for 8/16/32/64 bits registers;
> - Support for GPIO controllers with clear/set registers;
> - Support for GPIO controllers with a single "data" register;
> - Support for big endian bits/GPIOs ordering (mostly used on PowerPC).
> 
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH v5] gpio: Add driver for basic memory-mapped GPIO controllers

[David Brownell]

> 
> +static int bgpio_dir_in(struct gpio_chip *gc, unsigned int
> gpio)
> +{
> +    return 0;


Here -EINVAL is more correct there than 0 ...

[PATCH v6] gpio: Add driver for basic memory-mapped GPIO controllers

[Anton Vorontsov]

The basic GPIO controllers may be found in various on-board FPGA
and ASIC solutions that are used to control board's switches, LEDs,
chip-selects, Ethernet/USB PHY power, etc.

These controllers may not provide any means of pin setup
(in/out/open drain).

The driver supports:
- 8/16/32/64 bits registers;
- GPIO controllers with clear/set registers;
- GPIO controllers with a single "data" register;
- Big endian bits/GPIOs ordering (mostly used on PowerPC).

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---

On Tue, Aug 31, 2010 at 01:32:08PM -0700, David Brownell wrote:
> > +static int bgpio_dir_in(struct gpio_chip *gc, unsigned int
> > gpio)
> > +{
> > +    return 0;
> 
> Here -EINVAL is more correct there than 0 ...

IMHO, it doesn't really matter as we don't (and should
not) check that value. But OK, done. + some changelog
improvements as you suggested.

Thanks.

p.s.
On the next resend the cartoon guy will land and [most
probably] will break an arm (or a leg).

Either you give an ack, or have nightmares because of
that poor fellow.

 drivers/gpio/Kconfig           |    5 +
 drivers/gpio/Makefile          |    1 +
 drivers/gpio/basic_mmio_gpio.c |  279 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 285 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/basic_mmio_gpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f623953..d1d1c24 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -70,6 +70,11 @@ config GPIO_MAX730X
 
 comment "Memory mapped GPIO expanders:"
 
+config GPIO_BASIC_MMIO
+	tristate "Basic memory-mapped GPIO controllers support"
+	help
+	  Say yes here to support basic memory-mapped GPIO controllers.
+
 config GPIO_IT8761E
 	tristate "IT8761E GPIO support"
 	depends on GPIOLIB
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a69e060..dfb571b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 
 obj-$(CONFIG_GPIO_ADP5520)	+= adp5520-gpio.o
 obj-$(CONFIG_GPIO_ADP5588)	+= adp5588-gpio.o
+obj-$(CONFIG_GPIO_BASIC_MMIO)	+= basic_mmio_gpio.o
 obj-$(CONFIG_GPIO_LANGWELL)	+= langwell_gpio.o
 obj-$(CONFIG_GPIO_MAX730X)	+= max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= max7300.o
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
new file mode 100644
index 0000000..e6071bf
--- /dev/null
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -0,0 +1,279 @@
+/*
+ * Driver for basic memory-mapped GPIO controllers.
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * ....``.```~~~~````.`.`.`.`.```````'',,,.........`````......`.......
+ * ...``                                                         ```````..
+ * ..The simplest form of a GPIO controller that the driver supports is``
+ *  `.just a single "data" register, where GPIO state can be read and/or `
+ *    `,..written. ,,..``~~~~ .....``.`.`.~~.```.`.........``````.```````
+ *        `````````
+                                    ___
+_/~~|___/~|   . ```~~~~~~       ___/___\___     ,~.`.`.`.`````.~~...,,,,...
+__________|~$@~~~        %~    /o*o*o*o*o*o\   .. Implementing such a GPIO .
+o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
+                                                 `....trivial..'~`.```.```
+ *                                                    ```````
+ *  .```````~~~~`..`.``.``.
+ * .  The driver supports  `...       ,..```.`~~~```````````````....````.``,,
+ * .   big-endian notation, just`.  .. A bit more sophisticated controllers ,
+ *  . register the device with -be`. .with a pair of set/clear-bit registers ,
+ *   `.. suffix.  ```~~`````....`.`   . affecting the data register and the .`
+ *     ``.`.``...```                  ```.. output pins are also supported.`
+ *                        ^^             `````.`````````.,``~``~``~~``````
+ *                                                   .                  ^^
+ *   ,..`.`.`...````````````......`.`.`.`.`.`..`.`.`..
+ * .. The expectation is that in at least some cases .    ,-~~~-,
+ *  .this will be used with roll-your-own ASIC/FPGA .`     \   /
+ *  .logic in Verilog or VHDL. ~~~`````````..`````~~`       \ /
+ *  ..````````......```````````                             \o_
+ *                                                           |
+ *                              ^^                          / \
+ *
+ *           ...`````~~`.....``.`..........``````.`.``.```........``.
+ *            `  8, 16, 32 and 64 bits registers are supported, and``.
+ *            . the number of GPIOs is determined by the width of   ~
+ *             .. the registers. ,............```.`.`..`.`.~~~.`.`.`~
+ *               `.......````.```
+ */
+
+#include <linux/init.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/log2.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+
+struct bgpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_dat;
+	void __iomem *reg_set;
+	void __iomem *reg_clr;
+	spinlock_t lock;
+
+	int bits;
+	int big_endian_bits;
+
+	/* shadowed data register to clear/set bits safely */
+	unsigned long data;
+};
+
+static struct bgpio_chip *to_bgpio_chip(struct gpio_chip *gc)
+{
+	return container_of(gc, struct bgpio_chip, gc);
+}
+
+static unsigned long bgpio_in(struct bgpio_chip *bgc)
+{
+	switch (bgc->bits) {
+	case 8:
+		return __raw_readb(bgc->reg_dat);
+	case 16:
+		return __raw_readw(bgc->reg_dat);
+	case 32:
+		return __raw_readl(bgc->reg_dat);
+#if BITS_PER_LONG >= 64
+	case 64:
+		return __raw_readq(bgc->reg_dat);
+#endif
+	}
+	return -EINVAL;
+}
+
+static void bgpio_out(struct bgpio_chip *bgc, void __iomem *reg,
+		      unsigned long data)
+{
+	switch (bgc->bits) {
+	case 8:
+		__raw_writeb(data, reg);
+		return;
+	case 16:
+		__raw_writew(data, reg);
+		return;
+	case 32:
+		__raw_writel(data, reg);
+		return;
+#if BITS_PER_LONG >= 64
+	case 64:
+		__raw_writeq(data, reg);
+		return;
+#endif
+	}
+}
+
+static unsigned long bgpio_pin2mask(struct bgpio_chip *bgc, unsigned int pin)
+{
+	if (bgc->big_endian_bits)
+		return 1 << (bgc->bits - 1 - pin);
+	else
+		return 1 << pin;
+}
+
+static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+	return bgpio_in(bgc) & bgpio_pin2mask(bgc, gpio);
+}
+
+static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+	unsigned long mask = bgpio_pin2mask(bgc, gpio);
+	unsigned long flags;
+
+	if (bgc->reg_set) {
+		if (val)
+			bgpio_out(bgc, bgc->reg_set, mask);
+		else
+			bgpio_out(bgc, bgc->reg_clr, mask);
+		return;
+	}
+
+	spin_lock_irqsave(&bgc->lock, flags);
+
+	if (val)
+		bgc->data |= mask;
+	else
+		bgc->data &= ~mask;
+
+	bgpio_out(bgc, bgc->reg_dat, bgc->data);
+
+	spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	bgpio_set(gc, gpio, val);
+	return 0;
+}
+
+static int __devinit bgpio_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	struct device *dev = &pdev->dev;
+	struct bgpio_chip *bgc;
+	struct resource *res_dat;
+	struct resource *res_set;
+	struct resource *res_clr;
+	resource_size_t dat_sz;
+	int bits;
+
+	res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
+	if (!res_dat)
+		return -EINVAL;
+
+	dat_sz = resource_size(res_dat);
+	if (!is_power_of_2(dat_sz))
+		return -EINVAL;
+
+	bits = dat_sz * 8;
+	if (bits > BITS_PER_LONG)
+		return -EINVAL;
+
+	bgc = devm_kzalloc(dev, sizeof(*bgc), GFP_KERNEL);
+	if (!bgc)
+		return -ENOMEM;
+
+	bgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+	if (!bgc->reg_dat)
+		return -ENOMEM;
+
+	res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
+	res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
+	if (res_set && res_clr) {
+		if (resource_size(res_set) != resource_size(res_clr) ||
+				resource_size(res_set) != dat_sz)
+			return -EINVAL;
+
+		bgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
+		bgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+		if (!bgc->reg_set || !bgc->reg_clr)
+			return -ENOMEM;
+	} else if (res_set || res_clr) {
+		return -EINVAL;
+	}
+
+	spin_lock_init(&bgc->lock);
+
+	bgc->bits = bits;
+	bgc->big_endian_bits = !strcmp(platid->name, "basic-mmio-gpio-be");
+	bgc->data = bgpio_in(bgc);
+
+	bgc->gc.ngpio = bits;
+	bgc->gc.direction_input = bgpio_dir_in;
+	bgc->gc.direction_output = bgpio_dir_out;
+	bgc->gc.get = bgpio_get;
+	bgc->gc.set = bgpio_set;
+	bgc->gc.dev = dev;
+	bgc->gc.label = dev_name(dev);
+
+	if (dev->platform_data)
+		bgc->gc.base = (unsigned long)dev->platform_data;
+	else
+		bgc->gc.base = -1;
+
+	dev_set_drvdata(dev, bgc);
+
+	return gpiochip_add(&bgc->gc);
+}
+
+static int __devexit bgpio_remove(struct platform_device *pdev)
+{
+	struct bgpio_chip *bgc = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&bgc->gc);
+}
+
+static const struct platform_device_id bgpio_id_table[] = {
+	{ "basic-mmio-gpio", },
+	{ "basic-mmio-gpio-be", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, bgpio_id_table);
+
+static struct platform_driver bgpio_driver = {
+	.driver = {
+		.name = "basic-mmio-gpio",
+	},
+	.id_table = bgpio_id_table,
+	.probe = bgpio_probe,
+	.remove = __devexit_p(bgpio_remove),
+};
+
+static int __init bgpio_init(void)
+{
+	return platform_driver_register(&bgpio_driver);
+}
+module_init(bgpio_init);
+
+static void __exit bgpio_exit(void)
+{
+	platform_driver_unregister(&bgpio_driver);
+}
+module_exit(bgpio_exit);
+
+MODULE_DESCRIPTION("Driver for basic memory-mapped GPIO controllers");
+MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.5

[PATCH v7] gpio: Add driver for basic memory-mapped GPIO controllers

[Anton Vorontsov]

The basic GPIO controllers may be found in various on-board FPGA
and ASIC solutions that are used to control board's switches, LEDs,
chip-selects, Ethernet/USB PHY power, etc.

These controllers may not provide any means of pin setup
(in/out/open drain).

The driver supports:
- 8/16/32/64 bits registers;
- GPIO controllers with clear/set registers;
- GPIO controllers with a single "data" register;
- Big endian bits/GPIOs ordering (mostly used on PowerPC).

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---

A lucky v7.

In v7 I only changed platform data handling. It appears that
when used with MFD core, we can't just pass integers via
platform_data poiner, as platform_device_add_data() would try
to copy the data pointed by mfd_cell.platform_data.

This is now fixed by introducing a proper platform data
struct.

 drivers/gpio/Kconfig            |    5 +
 drivers/gpio/Makefile           |    1 +
 drivers/gpio/basic_mmio_gpio.c  |  281 +++++++++++++++++++++++++++++++++++++++
 include/linux/basic_mmio_gpio.h |   20 +++
 4 files changed, 307 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/basic_mmio_gpio.c
 create mode 100644 include/linux/basic_mmio_gpio.h

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 510aa20..e47ef94 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -70,6 +70,11 @@ config GPIO_MAX730X
 
 comment "Memory mapped GPIO expanders:"
 
+config GPIO_BASIC_MMIO
+	tristate "Basic memory-mapped GPIO controllers support"
+	help
+	  Say yes here to support basic memory-mapped GPIO controllers.
+
 config GPIO_IT8761E
 	tristate "IT8761E GPIO support"
 	depends on GPIOLIB
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index fc6019d..3ff0651 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 
 obj-$(CONFIG_GPIO_ADP5520)	+= adp5520-gpio.o
 obj-$(CONFIG_GPIO_ADP5588)	+= adp5588-gpio.o
+obj-$(CONFIG_GPIO_BASIC_MMIO)	+= basic_mmio_gpio.o
 obj-$(CONFIG_GPIO_LANGWELL)	+= langwell_gpio.o
 obj-$(CONFIG_GPIO_MAX730X)	+= max730x.o
 obj-$(CONFIG_GPIO_MAX7300)	+= max7300.o
diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
new file mode 100644
index 0000000..c2c11e7
--- /dev/null
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -0,0 +1,281 @@
+/*
+ * Driver for basic memory-mapped GPIO controllers.
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * ....``.```~~~~````.`.`.`.`.```````'',,,.........`````......`.......
+ * ...``                                                         ```````..
+ * ..The simplest form of a GPIO controller that the driver supports is``
+ *  `.just a single "data" register, where GPIO state can be read and/or `
+ *    `,..written. ,,..``~~~~ .....``.`.`.~~.```.`.........``````.```````
+ *        `````````
+                                    ___
+_/~~|___/~|   . ```~~~~~~       ___/___\___     ,~.`.`.`.`````.~~...,,,,...
+__________|~$@~~~        %~    /o*o*o*o*o*o\   .. Implementing such a GPIO .
+o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
+                                                 `....trivial..'~`.```.```
+ *                                                    ```````
+ *  .```````~~~~`..`.``.``.
+ * .  The driver supports  `...       ,..```.`~~~```````````````....````.``,,
+ * .   big-endian notation, just`.  .. A bit more sophisticated controllers ,
+ *  . register the device with -be`. .with a pair of set/clear-bit registers ,
+ *   `.. suffix.  ```~~`````....`.`   . affecting the data register and the .`
+ *     ``.`.``...```                  ```.. output pins are also supported.`
+ *                        ^^             `````.`````````.,``~``~``~~``````
+ *                                                   .                  ^^
+ *   ,..`.`.`...````````````......`.`.`.`.`.`..`.`.`..
+ * .. The expectation is that in at least some cases .    ,-~~~-,
+ *  .this will be used with roll-your-own ASIC/FPGA .`     \   /
+ *  .logic in Verilog or VHDL. ~~~`````````..`````~~`       \ /
+ *  ..````````......```````````                             \o_
+ *                                                           |
+ *                              ^^                          / \
+ *
+ *           ...`````~~`.....``.`..........``````.`.``.```........``.
+ *            `  8, 16, 32 and 64 bits registers are supported, and``.
+ *            . the number of GPIOs is determined by the width of   ~
+ *             .. the registers. ,............```.`.`..`.`.~~~.`.`.`~
+ *               `.......````.```
+ */
+
+#include <linux/init.h>
+#include <linux/bug.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/compiler.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/log2.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/basic_mmio_gpio.h>
+
+struct bgpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_dat;
+	void __iomem *reg_set;
+	void __iomem *reg_clr;
+	spinlock_t lock;
+
+	int bits;
+	int big_endian_bits;
+
+	/* shadowed data register to clear/set bits safely */
+	unsigned long data;
+};
+
+static struct bgpio_chip *to_bgpio_chip(struct gpio_chip *gc)
+{
+	return container_of(gc, struct bgpio_chip, gc);
+}
+
+static unsigned long bgpio_in(struct bgpio_chip *bgc)
+{
+	switch (bgc->bits) {
+	case 8:
+		return __raw_readb(bgc->reg_dat);
+	case 16:
+		return __raw_readw(bgc->reg_dat);
+	case 32:
+		return __raw_readl(bgc->reg_dat);
+#if BITS_PER_LONG >= 64
+	case 64:
+		return __raw_readq(bgc->reg_dat);
+#endif
+	}
+	return -EINVAL;
+}
+
+static void bgpio_out(struct bgpio_chip *bgc, void __iomem *reg,
+		      unsigned long data)
+{
+	switch (bgc->bits) {
+	case 8:
+		__raw_writeb(data, reg);
+		return;
+	case 16:
+		__raw_writew(data, reg);
+		return;
+	case 32:
+		__raw_writel(data, reg);
+		return;
+#if BITS_PER_LONG >= 64
+	case 64:
+		__raw_writeq(data, reg);
+		return;
+#endif
+	}
+}
+
+static unsigned long bgpio_pin2mask(struct bgpio_chip *bgc, unsigned int pin)
+{
+	if (bgc->big_endian_bits)
+		return 1 << (bgc->bits - 1 - pin);
+	else
+		return 1 << pin;
+}
+
+static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+	return bgpio_in(bgc) & bgpio_pin2mask(bgc, gpio);
+}
+
+static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+	unsigned long mask = bgpio_pin2mask(bgc, gpio);
+	unsigned long flags;
+
+	if (bgc->reg_set) {
+		if (val)
+			bgpio_out(bgc, bgc->reg_set, mask);
+		else
+			bgpio_out(bgc, bgc->reg_clr, mask);
+		return;
+	}
+
+	spin_lock_irqsave(&bgc->lock, flags);
+
+	if (val)
+		bgc->data |= mask;
+	else
+		bgc->data &= ~mask;
+
+	bgpio_out(bgc, bgc->reg_dat, bgc->data);
+
+	spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int bgpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	bgpio_set(gc, gpio, val);
+	return 0;
+}
+
+static int __devinit bgpio_probe(struct platform_device *pdev)
+{
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	struct device *dev = &pdev->dev;
+	struct bgpio_pdata *pdata = dev_get_platdata(dev);
+	struct bgpio_chip *bgc;
+	struct resource *res_dat;
+	struct resource *res_set;
+	struct resource *res_clr;
+	resource_size_t dat_sz;
+	int bits;
+
+	res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
+	if (!res_dat)
+		return -EINVAL;
+
+	dat_sz = resource_size(res_dat);
+	if (!is_power_of_2(dat_sz))
+		return -EINVAL;
+
+	bits = dat_sz * 8;
+	if (bits > BITS_PER_LONG)
+		return -EINVAL;
+
+	bgc = devm_kzalloc(dev, sizeof(*bgc), GFP_KERNEL);
+	if (!bgc)
+		return -ENOMEM;
+
+	bgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
+	if (!bgc->reg_dat)
+		return -ENOMEM;
+
+	res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
+	res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
+	if (res_set && res_clr) {
+		if (resource_size(res_set) != resource_size(res_clr) ||
+				resource_size(res_set) != dat_sz)
+			return -EINVAL;
+
+		bgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
+		bgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
+		if (!bgc->reg_set || !bgc->reg_clr)
+			return -ENOMEM;
+	} else if (res_set || res_clr) {
+		return -EINVAL;
+	}
+
+	spin_lock_init(&bgc->lock);
+
+	bgc->bits = bits;
+	bgc->big_endian_bits = !strcmp(platid->name, "basic-mmio-gpio-be");
+	bgc->data = bgpio_in(bgc);
+
+	bgc->gc.ngpio = bits;
+	bgc->gc.direction_input = bgpio_dir_in;
+	bgc->gc.direction_output = bgpio_dir_out;
+	bgc->gc.get = bgpio_get;
+	bgc->gc.set = bgpio_set;
+	bgc->gc.dev = dev;
+	bgc->gc.label = dev_name(dev);
+
+	if (pdata)
+		bgc->gc.base = pdata->base;
+	else
+		bgc->gc.base = -1;
+
+	dev_set_drvdata(dev, bgc);
+
+	return gpiochip_add(&bgc->gc);
+}
+
+static int __devexit bgpio_remove(struct platform_device *pdev)
+{
+	struct bgpio_chip *bgc = dev_get_drvdata(&pdev->dev);
+
+	return gpiochip_remove(&bgc->gc);
+}
+
+static const struct platform_device_id bgpio_id_table[] = {
+	{ "basic-mmio-gpio", },
+	{ "basic-mmio-gpio-be", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, bgpio_id_table);
+
+static struct platform_driver bgpio_driver = {
+	.driver = {
+		.name = "basic-mmio-gpio",
+	},
+	.id_table = bgpio_id_table,
+	.probe = bgpio_probe,
+	.remove = __devexit_p(bgpio_remove),
+};
+
+static int __init bgpio_init(void)
+{
+	return platform_driver_register(&bgpio_driver);
+}
+module_init(bgpio_init);
+
+static void __exit bgpio_exit(void)
+{
+	platform_driver_unregister(&bgpio_driver);
+}
+module_exit(bgpio_exit);
+
+MODULE_DESCRIPTION("Driver for basic memory-mapped GPIO controllers");
+MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/basic_mmio_gpio.h b/include/linux/basic_mmio_gpio.h
new file mode 100644
index 0000000..198087a
--- /dev/null
+++ b/include/linux/basic_mmio_gpio.h
@@ -0,0 +1,20 @@
+/*
+ * Basic memory-mapped GPIO controllers.
+ *
+ * Copyright 2008 MontaVista Software, Inc.
+ * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#ifndef __BASIC_MMIO_GPIO_H
+#define __BASIC_MMIO_GPIO_H
+
+struct bgpio_pdata {
+	int base;
+};
+
+#endif /* __BASIC_MMIO_GPIO_H */
-- 
1.7.0.5

Re: [PATCH v7] gpio: Add driver for basic memory-mapped GPIO controllers

[Anton Vorontsov]

On Tue, Sep 07, 2010 at 06:01:32PM +0400, Anton Vorontsov wrote:
> The basic GPIO controllers may be found in various on-board FPGA
> and ASIC solutions that are used to control board's switches, LEDs,
> chip-selects, Ethernet/USB PHY power, etc.
> 
> These controllers may not provide any means of pin setup
> (in/out/open drain).
> 
> The driver supports:
> - 8/16/32/64 bits registers;
> - GPIO controllers with clear/set registers;
> - GPIO controllers with a single "data" register;
> - Big endian bits/GPIOs ordering (mostly used on PowerPC).
> 
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> 
> A lucky v7.
> 
> In v7 I only changed platform data handling. It appears that
> when used with MFD core, we can't just pass integers via
> platform_data poiner, as platform_device_add_data() would try
> to copy the data pointed by mfd_cell.platform_data.
> 
> This is now fixed by introducing a proper platform data
> struct.

Ping? Anyone?

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH v7] gpio: Add driver for basic memory-mapped GPIO controllers

[Andrew Morton]

On Tue, 7 Sep 2010 18:01:32 +0400
Anton Vorontsov <cbouatmailru@gmail.com> wrote:

> The basic GPIO controllers may be found in various on-board FPGA
> and ASIC solutions that are used to control board's switches, LEDs,
> chip-selects, Ethernet/USB PHY power, etc.
> 
> These controllers may not provide any means of pin setup
> (in/out/open drain).
> 
> The driver supports:
> - 8/16/32/64 bits registers;
> - GPIO controllers with clear/set registers;
> - GPIO controllers with a single "data" register;
> - Big endian bits/GPIOs ordering (mostly used on PowerPC).
> 
>
> ...
>
> --- /dev/null
> +++ b/drivers/gpio/basic_mmio_gpio.c
> @@ -0,0 +1,281 @@
> +/*
> + * Driver for basic memory-mapped GPIO controllers.
> + *
> + * Copyright 2008 MontaVista Software, Inc.
> + * Copyright 2008,2010 Anton Vorontsov <cbouatmailru@gmail.com>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + *
> + * ....``.```~~~~````.`.`.`.`.```````'',,,.........`````......`.......
> + * ...``                                                         ```````..
> + * ..The simplest form of a GPIO controller that the driver supports is``
> + *  `.just a single "data" register, where GPIO state can be read and/or `
> + *    `,..written. ,,..``~~~~ .....``.`.`.~~.```.`.........``````.```````
> + *        `````````
> +                                    ___
> +_/~~|___/~|   . ```~~~~~~       ___/___\___     ,~.`.`.`.`````.~~...,,,,...
> +__________|~$@~~~        %~    /o*o*o*o*o*o\   .. Implementing such a GPIO .
> +o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
> +                                                 `....trivial..'~`.```.```
> + *                                                    ```````
> + *  .```````~~~~`..`.``.``.
> + * .  The driver supports  `...       ,..```.`~~~```````````````....````.``,,
> + * .   big-endian notation, just`.  .. A bit more sophisticated controllers ,
> + *  . register the device with -be`. .with a pair of set/clear-bit registers ,
> + *   `.. suffix.  ```~~`````....`.`   . affecting the data register and the .`
> + *     ``.`.``...```                  ```.. output pins are also supported.`
> + *                        ^^             `````.`````````.,``~``~``~~``````
> + *                                                   .                  ^^
> + *   ,..`.`.`...````````````......`.`.`.`.`.`..`.`.`..
> + * .. The expectation is that in at least some cases .    ,-~~~-,
> + *  .this will be used with roll-your-own ASIC/FPGA .`     \   /
> + *  .logic in Verilog or VHDL. ~~~`````````..`````~~`       \ /
> + *  ..````````......```````````                             \o_
> + *                                                           |
> + *                              ^^                          / \
> + *
> + *           ...`````~~`.....``.`..........``````.`.``.```........``.
> + *            `  8, 16, 32 and 64 bits registers are supported, and``.
> + *            . the number of GPIOs is determined by the width of   ~
> + *             .. the registers. ,............```.`.`..`.`.~~~.`.`.`~
> + *               `.......````.```
> + */

ooh, sparkly!

>
> ...
>
> +struct bgpio_chip {
> +	struct gpio_chip gc;
> +	void __iomem *reg_dat;
> +	void __iomem *reg_set;
> +	void __iomem *reg_clr;
> +	spinlock_t lock;
> +
> +	int bits;
> +	int big_endian_bits;
> +
> +	/* shadowed data register to clear/set bits safely */
> +	unsigned long data;
> +};

It would be good to document `bits' and `big_endian_bits', and to
describe what `lock' locks.

>
> ...
>
> +static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> +	return 0;
> +}

hm, what does this mean.  The hardware cannot set pin directions to
"in"?  If so, shouldn't we tell someone that their attempt to do this
failed?

>
> ...
>
> +static int __devinit bgpio_probe(struct platform_device *pdev)
> +{
> +	const struct platform_device_id *platid = platform_get_device_id(pdev);
> +	struct device *dev = &pdev->dev;
> +	struct bgpio_pdata *pdata = dev_get_platdata(dev);
> +	struct bgpio_chip *bgc;
> +	struct resource *res_dat;
> +	struct resource *res_set;
> +	struct resource *res_clr;
> +	resource_size_t dat_sz;
> +	int bits;
> +
> +	res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
> +	if (!res_dat)
> +		return -EINVAL;
> +
> +	dat_sz = resource_size(res_dat);
> +	if (!is_power_of_2(dat_sz))
> +		return -EINVAL;
> +
> +	bits = dat_sz * 8;
> +	if (bits > BITS_PER_LONG)
> +		return -EINVAL;
> +
> +	bgc = devm_kzalloc(dev, sizeof(*bgc), GFP_KERNEL);
> +	if (!bgc)
> +		return -ENOMEM;
> +
> +	bgc->reg_dat = devm_ioremap(dev, res_dat->start, dat_sz);
> +	if (!bgc->reg_dat)
> +		return -ENOMEM;
> +
> +	res_set = platform_get_resource_byname(pdev, IORESOURCE_MEM, "set");
> +	res_clr = platform_get_resource_byname(pdev, IORESOURCE_MEM, "clr");
> +	if (res_set && res_clr) {
> +		if (resource_size(res_set) != resource_size(res_clr) ||
> +				resource_size(res_set) != dat_sz)
> +			return -EINVAL;
> +
> +		bgc->reg_set = devm_ioremap(dev, res_set->start, dat_sz);
> +		bgc->reg_clr = devm_ioremap(dev, res_clr->start, dat_sz);
> +		if (!bgc->reg_set || !bgc->reg_clr)
> +			return -ENOMEM;
> +	} else if (res_set || res_clr) {
> +		return -EINVAL;
> +	}
> +
> +	spin_lock_init(&bgc->lock);
> +
> +	bgc->bits = bits;
> +	bgc->big_endian_bits = !strcmp(platid->name, "basic-mmio-gpio-be");
> +	bgc->data = bgpio_in(bgc);
> +
> +	bgc->gc.ngpio = bits;
> +	bgc->gc.direction_input = bgpio_dir_in;
> +	bgc->gc.direction_output = bgpio_dir_out;
> +	bgc->gc.get = bgpio_get;
> +	bgc->gc.set = bgpio_set;
> +	bgc->gc.dev = dev;
> +	bgc->gc.label = dev_name(dev);
> +
> +	if (pdata)
> +		bgc->gc.base = pdata->base;
> +	else
> +		bgc->gc.base = -1;
> +
> +	dev_set_drvdata(dev, bgc);
> +
> +	return gpiochip_add(&bgc->gc);
> +}

If this function returns -EINVAL then much head-scratching will ensue. 
It might make your life easier to emit a diagnostic just before the
failure so you can work out why it failed.

[PATCH v7-fix] gpio: Add driver for basic memory-mapped GPIO controllers (fix)

[Anton Vorontsov]

On Fri, Sep 24, 2010 at 02:45:35PM -0700, Andrew Morton wrote:
> It would be good to document `bits' and `big_endian_bits', and to
> describe what `lock' locks.

Done.

> > +static int bgpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
> > +{
> > +	return 0;
> > +}
> 
> hm, what does this mean.  The hardware cannot set pin directions to
> "in"?

Nope, 0 is the success. The hardware cannot set pin directions
at all (well, some hw can, but we don't support these yet).

> > +	return gpiochip_add(&bgc->gc);
> > +}
> 
> If this function returns -EINVAL then much head-scratching will ensue. 
> It might make your life easier to emit a diagnostic just before the
> failure so you can work out why it failed.

OK.

Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---

Thanks!

 drivers/gpio/basic_mmio_gpio.c |   22 +++++++++++++++++++---
 1 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/basic_mmio_gpio.c b/drivers/gpio/basic_mmio_gpio.c
index c2c11e7..3addea6 100644
--- a/drivers/gpio/basic_mmio_gpio.c
+++ b/drivers/gpio/basic_mmio_gpio.c
@@ -66,12 +66,23 @@ struct bgpio_chip {
 	void __iomem *reg_dat;
 	void __iomem *reg_set;
 	void __iomem *reg_clr;
-	spinlock_t lock;
 
+	/* Number of bits (GPIOs): <register width> * 8. */
 	int bits;
+
+	/*
+	 * Some GPIO controllers work with the big-endian bits notation,
+	 * e.g. in a 8-bits register, GPIO7 is the least significant bit.
+	 */
 	int big_endian_bits;
 
-	/* shadowed data register to clear/set bits safely */
+	/*
+	 * Used to lock bgpio_chip->data. Also, this is needed to keep
+	 * shadowed and real data registers writes together.
+	 */
+	spinlock_t lock;
+
+	/* Shadowed data register to clear/set bits safely. */
 	unsigned long data;
 };
 
@@ -181,6 +192,7 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
 	struct resource *res_clr;
 	resource_size_t dat_sz;
 	int bits;
+	int ret;
 
 	res_dat = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
 	if (!res_dat)
@@ -238,7 +250,11 @@ static int __devinit bgpio_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, bgc);
 
-	return gpiochip_add(&bgc->gc);
+	ret = gpiochip_add(&bgc->gc);
+	if (ret)
+		dev_err(dev, "gpiochip_add() failed: %d\n", ret);
+
+	return ret;
 }
 
 static int __devexit bgpio_remove(struct platform_device *pdev)
-- 
1.7.0.5

Re: [PATCH] gpio: Add generic driver for simple memory mapped controllers

[Mark Brown]

On Wed, Aug 25, 2010 at 11:42:49PM +0400, Anton Vorontsov wrote:

> +	res_dat = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res_dat)
> +		return -EINVAL;

I'd suggest naming these resources rather than grabbing by number,
that'll make it easier to plumb the machine driver side in since you
don't have to remember which number corresponds to which function.