[PATCH 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[PATCH 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Tobias Müller]

From: Tobias Mueller <Tobias_Mueller@twam.info>

A GPIO driver for AMD Geode Companion Device CS5535/CS5536
using the GPIO framework as a replacement for old cs5535-gpio driver.
---
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index edb0253..0b7ef6f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -67,6 +67,14 @@ config GPIO_SYSFS

 comment "Memory mapped GPIO expanders:"

+config GPIO_CS5535
+	tristate "AMD CS5535/CS5536 (Geode Companion Device)"
+	depends on !CONFIG_CS5535_GPIO && X86 && EXPERIMENTAL
+	default N
+	help
+	  Say yes here to support GPIO pins of AMD CS5535/CS5536
+          (Geode Companion Device)
+
 config GPIO_XILINX
 	bool "Xilinx GPIO support"
 	depends on PPC_OF
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 49ac64e..949c723 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_GPIO_PCF857X)	+= pcf857x.o
 obj-$(CONFIG_GPIO_TWL4030)	+= twl4030-gpio.o
 obj-$(CONFIG_GPIO_XILINX)	+= xilinx_gpio.o
 obj-$(CONFIG_GPIO_BT8XX)	+= bt8xxgpio.o
+obj-$(CONFIG_GPIO_CS5535)	+= gpio_cs5535.o
diff --git a/drivers/gpio/gpio_cs5535.c b/drivers/gpio/gpio_cs5535.c
new file mode 100644
index 0000000..b0f7ef7
--- /dev/null
+++ b/drivers/gpio/gpio_cs5535.c
@@ -0,0 +1,321 @@
+/*
+    CS5535/CS5536 GPIO driver
+
+    Copyright (C) 2009 Tobias Mueller <Tobias_Mueller@twam.info>
+
+
+    Derived from the the cs5535_gpio (char) driver:
+
+    Copyright (c) 2005 Ben Gardner <bgardner@wabtec.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.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <asm/gpio.h>
+
+#define GPIO_CS5535_NAME	"gpio_cs5535"
+#define GPIO_CS5535_SIZE	256		/* size of I/O area */
+#define GPIO_CS5535_LBAR_GPIO	0x5140000C	/* p. 385 of datasheet */
+#define GPIO_CS5535_OUT_VAL	0x00		/* output value */
+#define GPIO_CS5535_OUT_EN	0x04		/* output enable */
+#define GPIO_CS5535_OD_EN	0x08		/* open-drain enable */
+#define GPIO_CS5535_OUT_AUX1	0x10		/* output aux 1 select */
+#define GPIO_CS5535_OUT_AUX2	0x14		/* output aux 2 select */
+#define GPIO_CS5535_OUT_PU_EN	0x18		/* pull-up enable */
+#define GPIO_CS5535_OUT_PD_EN	0x1C		/* poll-down eable */
+#define GPIO_CS5535_IN_EN	0x20		/* input enable */
+#define GPIO_CS5535_READ_BACK	0x30		/* read back */
+#define GPIO_CS5535_IN_AUX1	0x34		/* input aux 1 select */
+
+/**
+* Some GPIO pins
+*  31-29,23 : reserved (always mask out)
+*  28       : Power Button
+*  26       : PME#
+*  22-16    : LPC
+*  14,15    : SMBus
+*  9,8      : UART1
+*  7        : PCI INTB
+*  3,4      : UART2/DDC
+*  2        : IDE_IRQ0
+*  1        : AC_BEEP
+*  0        : PCI INTA
+*
+* If a mask was not specified, be conservative and only allow:
+*  1,2,5,6,10-13,24,25,27
+*/
+
+#ifndef GPIO_CS5535_DEF_MASK
+#define GPIO_CS5535_DEF_MASK	0x0B003C66
+#endif
+
+static struct pci_device_id gpio_cs5535_pci_tbl[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_NS,  PCI_DEVICE_ID_NS_CS5535_ISA) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA) },
+	{ } /* NULL entry */
+};
+
+/* pin names */
+static char* gpio_cs5535_names[] = {
+	"GPIO0", "GPIO1", "GPIO2", "GPIO3",
+	"GPIO4", "GPIO5", "GPIO6", "GPIO7",
+	"GPIO8", "GPIO9", "GPIO10", "GPIO11",
+	"GPIO12", "GPIO13", "GPIO14", "GPIO15",
+	"GPIO16", "GPIO17", "GPIO18", "GPIO19",
+	"GPIO20", "GPIO21", "GPIO22", NULL,
+	"GPIO24", "GPIO25", "GPIO26", "GPIO27",
+	"GPIO28", NULL, NULL, NULL,
+};
+
+struct gpio_cs5535 {
+	struct pci_dev *pdev;
+	struct gpio_chip gpio;
+};
+
+/* base address of gpio I/O*/
+static u32 iobase;
+
+static int gpiobase = 0;
+module_param_named(gpiobase, gpiobase, int, 0444);
+MODULE_PARM_DESC(gpiobase, "The GPIO number base. -1 means dynamic");
+
+static ulong mask = 0;
+module_param_named(mask, mask, ulong, 0444);
+MODULE_PARM_DESC(mask, "GPIO channel mask.");
+
+/* gets the regiter offset for the GPIO bank.
+ *      low (0-15) stats at 0x00, high (16-31) starts at 0x80 */
+static inline u32 gpio_cs5535_lowhigh_base(int offset)
+{
+	return (offset & 0x10) << 3;
+}
+
+/* optional hook for chip-specific activation, such as
+ *      enabling module power and clock; may sleep */
+static int gpio_cs5535_request(struct gpio_chip *chip, unsigned offset)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* check if this pin is available */
+	if ((mask & (1 << offset)) == 0) {
+		printk(KERN_INFO GPIO_CS5535_NAME "pin %u is not available\n",offset);
+		return -EINVAL;
+	}
+
+	/* disable output aux 1 & 2 on this pin */
+	outl(off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_OUT_AUX1);
+	outl(off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_OUT_AUX2);
+
+	/* disable input aux 1 on this pin */
+	outl(off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_IN_AUX1);
+
+	/* disable output */
+	outl(off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_OUT_EN);
+
+	/* enable input */
+	outl(on,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_IN_EN);
+
+	return 0;
+}
+
+/* configures signal "offset" as input, or returns error */
+static int gpio_cs5535_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* disable output */
+	outl(off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_OUT_EN);
+
+	/* enable input */
+	outl(on,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_IN_EN);
+
+	return 0;
+}
+
+/* returns value for signal "offset"; for output signals this
+ *      returns either the value actually sensed, or zero */
+static int gpio_cs5535_get(struct gpio_chip *chip, unsigned offset)
+{
+	u32 bit;
+
+	bit = 1 << (offset & 0x0F);
+
+	return inl(iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_READ_BACK)
& bit;
+}
+
+/* configures signal "offset" as output, or returns error */
+static int gpio_cs5535_direction_output(struct gpio_chip *chip,
unsigned offset, int value)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* disable input */
+	outl(off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_IN_EN);
+
+	/* set value */
+	outl(value ? on :
off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_OUT_VAL);
+
+	/* enable output */
+	outl(on,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_OUT_EN);
+
+	return 0;
+}
+
+/* assigns output value for signal "offset" */
+static void gpio_cs5535_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* set value */
+	outl(value ? on :
off,iobase+gpio_cs5535_lowhigh_base(offset)+GPIO_CS5535_OUT_VAL);
+}
+
+/* setups gpio_chip object */
+static void gpio_cs5535_setup(struct gpio_cs5535 *cs5535)
+{
+	struct gpio_chip *c = &cs5535->gpio;
+
+	c->label = dev_name(&cs5535->pdev->dev);
+	c->owner = THIS_MODULE;
+	c->request = gpio_cs5535_request;
+	c->direction_input = gpio_cs5535_direction_input;
+	c->get = gpio_cs5535_get;
+	c->direction_output = gpio_cs5535_direction_output;
+	c->set = gpio_cs5535_set;
+	c->dbg_show = NULL;
+	c->base = gpiobase;
+	c->ngpio = 32;
+	c->can_sleep = 0;
+	c->names = gpio_cs5535_names;
+}
+
+static int gpio_cs5535_probe(struct pci_dev *pdev, const struct
pci_device_id *pciid)
+{
+	struct gpio_cs5535 *cs5535;
+	u32 low, hi;
+	u32 mask_orig = mask;
+	int err;
+
+	cs5535 = kzalloc(sizeof(*cs5535), GFP_KERNEL);
+	if (!cs5535)
+		return -ENOMEM;
+
+	cs5535->pdev = pdev;
+
+	pci_set_drvdata(pdev, cs5535);
+
+	/* Grab the GPIO I/O range */
+	rdmsr(GPIO_CS5535_LBAR_GPIO, low, hi);
+
+	/* Check the mask and whether GPIO is enabled (sanity check) */
+	if (hi != 0x0000f001) {
+		printk(KERN_WARNING GPIO_CS5535_NAME ": GPIO not enabled\n");
+		err = -ENODEV;
+		goto err_release_mem;
+	}
+
+	iobase = low & 0x000ff00;
+
+	if (!request_region(iobase, GPIO_CS5535_SIZE, GPIO_CS5535_NAME)) {
+		printk(KERN_ERR GPIO_CS5535_NAME ": Can't allocate I/O for GPIO\n");
+		err = -ENODEV;
+		goto err_release_mem;
+	}
+
+	if (mask != 0)
+		mask &= 0x1F7FFFFF; /* mask out reserved */
+	else
+		mask = GPIO_CS5535_DEF_MASK;
+
+	/* do not allow pin 28, Power Button, as there's special handling
+         * in the PMC needed. (note 12, p. 48) */
+	mask &= ~(1 << 28);
+
+	if (mask_orig != mask)
+		printk(KERN_INFO GPIO_CS5535_NAME ": mask changed to 0x%08lX\n",mask);
+
+	gpio_cs5535_setup(cs5535);
+
+	err = gpiochip_add(&cs5535->gpio);
+	if (err) {
+		printk(KERN_ERR GPIO_CS5535_NAME ": Failed to register GPIOs\n");
+		goto err_release_mem;
+	}
+
+	return 0;
+
+err_release_mem:
+	pci_set_drvdata(pdev, NULL);
+	kfree(cs5535);
+
+	return err;
+}
+
+static void gpio_cs5535_remove(struct pci_dev *pdev)
+{
+	struct gpio_cs5535 *cs5535 = pci_get_drvdata(pdev);
+	int status = 0;
+
+	status = gpiochip_remove(&cs5535->gpio);
+
+	if (status == 0) {
+		pci_set_drvdata(pdev, NULL);
+		release_region(iobase, GPIO_CS5535_SIZE);
+		kfree(cs5535);
+	} else {
+                dev_err(&pdev->dev, "%s --> %d\n", "remove", status);
+	}
+}
+
+#define gpio_cs5535_suspend NULL
+#define gpio_cs5535_resume NULL
+
+static struct pci_driver gpio_cs5535_driver = {
+        .name           = "gpio_cs5535",
+        .id_table       = gpio_cs5535_pci_tbl,
+        .probe          = gpio_cs5535_probe,
+        .remove         = gpio_cs5535_remove,
+        .suspend        = gpio_cs5535_suspend,
+        .resume         = gpio_cs5535_resume,
+};
+
+static int __init gpio_cs5535_init(void)
+{
+	return pci_register_driver(&gpio_cs5535_driver);
+}
+module_init(gpio_cs5535_init)
+
+static void __exit gpio_cs5535_exit(void)
+{
+	pci_unregister_driver(&gpio_cs5535_driver);
+}
+module_exit(gpio_cs5535_exit);
+
+MODULE_AUTHOR("Tobias Mueller <Tobias_Mueller@twam.info>");
+MODULE_DESCRIPTION("AMD CS5535/CS5536 GPIO driver");
+MODULE_LICENSE("GPL");

Re: [PATCH 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Andrew Morton]

On Sat, 23 May 2009 13:53:58 +0200
Tobias M__ller <Tobias_Mueller@twam.info> wrote:

> From: Tobias Mueller <Tobias_Mueller@twam.info>
> 
> A GPIO driver for AMD Geode Companion Device CS5535/CS5536
> using the GPIO framework as a replacement for old cs5535-gpio driver.

It looks reasonsable from a quick scan.

The patch was somewhat wordwrapped by your email client.

The patch generates a large number of warnings when checked with
scripts/checkpatch.pl.  Please take a look at that and permanently
incorporate checkpatch into your patch generation tools.

Please cc the following individuals on future versions of this patch:

David Brownell <david-b@pacbell.net>
Jordan Crouse <jordan@cosmicpenguin.net>
Ben Gardner <gardner.ben@gmail.com>

> index edb0253..0b7ef6f 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -67,6 +67,14 @@ config GPIO_SYSFS
> 
>  comment "Memory mapped GPIO expanders:"
> 
> +config GPIO_CS5535
> +	tristate "AMD CS5535/CS5536 (Geode Companion Device)"
> +	depends on !CONFIG_CS5535_GPIO && X86 && EXPERIMENTAL
> +	default N
> +	help
> +	  Say yes here to support GPIO pins of AMD CS5535/CS5536
> +          (Geode Companion Device)

We'll need to discuss the migration plan - we don't want to maintain
two drivers for the same thing.

[PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Tobias Müller]

From: Tobias Mueller <Tobias_Mueller@twam.info>

A GPIO driver for AMD Geode Companion Device CS5535/CS5536
using the GPIO framework as a replacement for old cs5535-gpio driver.
Signed-off-by: Tobias Mueller <Tobias_Mueller@twam.info>
---
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index edb0253..0b7ef6f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -67,6 +67,14 @@ config GPIO_SYSFS

 comment "Memory mapped GPIO expanders:"

+config GPIO_CS5535
+	tristate "AMD CS5535/CS5536 (Geode Companion Device)"
+	depends on !CONFIG_CS5535_GPIO && X86 && EXPERIMENTAL
+	default N
+	help
+	  Say yes here to support GPIO pins of AMD CS5535/CS5536
+          (Geode Companion Device)
+
 config GPIO_XILINX
 	bool "Xilinx GPIO support"
 	depends on PPC_OF
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 49ac64e..949c723 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_GPIO_PCF857X)	+= pcf857x.o
 obj-$(CONFIG_GPIO_TWL4030)	+= twl4030-gpio.o
 obj-$(CONFIG_GPIO_XILINX)	+= xilinx_gpio.o
 obj-$(CONFIG_GPIO_BT8XX)	+= bt8xxgpio.o
+obj-$(CONFIG_GPIO_CS5535)	+= gpio_cs5535.o
diff --git a/drivers/gpio/gpio_cs5535.c b/drivers/gpio/gpio_cs5535.c
new file mode 100644
index 0000000..f7361e1
--- /dev/null
+++ b/drivers/gpio/gpio_cs5535.c
@@ -0,0 +1,342 @@
+/*
+    CS5535/CS5536 GPIO driver
+
+    Copyright (C) 2009 Tobias Mueller <Tobias_Mueller@twam.info>
+
+
+    Derived from the the cs5535_gpio (char) driver:
+
+    Copyright (c) 2005 Ben Gardner <bgardner@wabtec.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.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program; if not, write to the Free Software
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/gpio.h>
+
+#define GPIO_CS5535_NAME	"gpio_cs5535"
+#define GPIO_CS5535_SIZE	256		/* size of I/O area */
+#define GPIO_CS5535_LBAR_GPIO	0x5140000C	/* p. 385 of datasheet */
+#define GPIO_CS5535_OUT_VAL	0x00		/* output value */
+#define GPIO_CS5535_OUT_EN	0x04		/* output enable */
+#define GPIO_CS5535_OD_EN	0x08		/* open-drain enable */
+#define GPIO_CS5535_OUT_AUX1	0x10		/* output aux 1 select */
+#define GPIO_CS5535_OUT_AUX2	0x14		/* output aux 2 select */
+#define GPIO_CS5535_OUT_PU_EN	0x18		/* pull-up enable */
+#define GPIO_CS5535_OUT_PD_EN	0x1C		/* poll-down eable */
+#define GPIO_CS5535_IN_EN	0x20		/* input enable */
+#define GPIO_CS5535_READ_BACK	0x30		/* read back */
+#define GPIO_CS5535_IN_AUX1	0x34		/* input aux 1 select */
+
+/**
+* Some GPIO pins
+*  31-29,23 : reserved (always mask out)
+*  28       : Power Button
+*  26       : PME#
+*  22-16    : LPC
+*  14,15    : SMBus
+*  9,8      : UART1
+*  7        : PCI INTB
+*  3,4      : UART2/DDC
+*  2        : IDE_IRQ0
+*  1        : AC_BEEP
+*  0        : PCI INTA
+*
+* If a mask was not specified, be conservative and only allow:
+*  1,2,5,6,10-13,24,25,27
+*/
+
+#ifndef GPIO_CS5535_DEF_MASK
+#define GPIO_CS5535_DEF_MASK	0x0B003C66
+#endif
+
+static struct pci_device_id gpio_cs5535_pci_tbl[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_NS,  PCI_DEVICE_ID_NS_CS5535_ISA) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA) },
+	{ } /* NULL entry */
+};
+
+/* pin names */
+static char *gpio_cs5535_names[] = {
+	"GPIO0", "GPIO1", "GPIO2", "GPIO3",
+	"GPIO4", "GPIO5", "GPIO6", "GPIO7",
+	"GPIO8", "GPIO9", "GPIO10", "GPIO11",
+	"GPIO12", "GPIO13", "GPIO14", "GPIO15",
+	"GPIO16", "GPIO17", "GPIO18", "GPIO19",
+	"GPIO20", "GPIO21", "GPIO22", NULL,
+	"GPIO24", "GPIO25", "GPIO26", "GPIO27",
+	"GPIO28", NULL, NULL, NULL,
+};
+
+struct gpio_cs5535 {
+	struct pci_dev *pdev;
+	struct gpio_chip gpio;
+};
+
+/* base address of gpio I/O*/
+static u32 iobase;
+
+static int gpiobase;
+module_param_named(gpiobase, gpiobase, int, 0444);
+MODULE_PARM_DESC(gpiobase, "The GPIO number base. -1 means dynamic");
+
+static ulong mask;
+module_param_named(mask, mask, ulong, 0444);
+MODULE_PARM_DESC(mask, "GPIO channel mask.");
+
+/* gets the regiter offset for the GPIO bank.
+ *      low (0-15) stats at 0x00, high (16-31) starts at 0x80 */
+static inline u32 gpio_cs5535_lowhigh_base(int offset)
+{
+	return (offset & 0x10) << 3;
+}
+
+/* optional hook for chip-specific activation, such as
+ *      enabling module power and clock; may sleep */
+static int gpio_cs5535_request(struct gpio_chip *chip, unsigned offset)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* check if this pin is available */
+	if ((mask & (1 << offset)) == 0) {
+		printk(KERN_INFO GPIO_CS5535_NAME
+		       ": pin %u is not available\n", offset);
+		return -EINVAL;
+	}
+
+	/* disable output aux 1 & 2 on this pin */
+	outl(off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_OUT_AUX1);
+	outl(off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_OUT_AUX2);
+
+	/* disable input aux 1 on this pin */
+	outl(off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_IN_AUX1);
+
+	/* disable output */
+	outl(off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_OUT_EN);
+
+	/* enable input */
+	outl(on, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_IN_EN);
+
+	return 0;
+}
+
+/* configures signal "offset" as input, or returns error */
+static int gpio_cs5535_direction_input(struct gpio_chip *chip,
+				       unsigned offset)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* disable output */
+	outl(off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_OUT_EN);
+
+	/* enable input */
+	outl(on, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_IN_EN);
+
+	return 0;
+}
+
+/* returns value for signal "offset"; for output signals this
+ *      returns either the value actually sensed, or zero */
+static int gpio_cs5535_get(struct gpio_chip *chip, unsigned offset)
+{
+	u32 bit;
+
+	bit = 1 << (offset & 0x0F);
+
+	return inl(iobase+gpio_cs5535_lowhigh_base(offset)
+		   +GPIO_CS5535_READ_BACK) & bit;
+}
+
+/* configures signal "offset" as output, or returns error */
+static int gpio_cs5535_direction_output(struct gpio_chip *chip,
+					unsigned offset, int value)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* disable input */
+	outl(off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_IN_EN);
+
+	/* set value */
+	outl(value ? on : off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_OUT_VAL);
+
+	/* enable output */
+	outl(on, iobase+gpio_cs5535_lowhigh_base(offset)
+	    +GPIO_CS5535_OUT_EN);
+
+	return 0;
+}
+
+/* assigns output value for signal "offset" */
+static void gpio_cs5535_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	u32 on, off;
+
+	on = 1 << (offset & 0x0F);
+	off = on << 16;
+
+	/* set value */
+	outl(value ? on : off, iobase+gpio_cs5535_lowhigh_base(offset)
+	     +GPIO_CS5535_OUT_VAL);
+}
+
+/* setups gpio_chip object */
+static void gpio_cs5535_setup(struct gpio_cs5535 *cs5535)
+{
+	struct gpio_chip *c = &cs5535->gpio;
+
+	c->label = dev_name(&cs5535->pdev->dev);
+	c->owner = THIS_MODULE;
+	c->request = gpio_cs5535_request;
+	c->direction_input = gpio_cs5535_direction_input;
+	c->get = gpio_cs5535_get;
+	c->direction_output = gpio_cs5535_direction_output;
+	c->set = gpio_cs5535_set;
+	c->dbg_show = NULL;
+	c->base = gpiobase;
+	c->ngpio = 32;
+	c->can_sleep = 0;
+	c->names = gpio_cs5535_names;
+}
+
+static int gpio_cs5535_probe(struct pci_dev *pdev,
+			     const struct pci_device_id *pciid)
+{
+	struct gpio_cs5535 *cs5535;
+	u32 low, hi;
+	u32 mask_orig = mask;
+	int err;
+
+	cs5535 = kzalloc(sizeof(*cs5535), GFP_KERNEL);
+	if (!cs5535)
+		return -ENOMEM;
+
+	cs5535->pdev = pdev;
+
+	pci_set_drvdata(pdev, cs5535);
+
+	/* Grab the GPIO I/O range */
+	rdmsr(GPIO_CS5535_LBAR_GPIO, low, hi);
+
+	/* Check the mask and whether GPIO is enabled (sanity check) */
+	if (hi != 0x0000f001) {
+		printk(KERN_WARNING GPIO_CS5535_NAME
+		       ": GPIO not enabled\n");
+		err = -ENODEV;
+		goto err_release_mem;
+	}
+
+	iobase = low & 0x000ff00;
+
+	if (!request_region(iobase, GPIO_CS5535_SIZE, GPIO_CS5535_NAME)) {
+		printk(KERN_ERR GPIO_CS5535_NAME
+		       ": Can't allocate I/O for GPIO\n");
+		err = -ENODEV;
+		goto err_release_mem;
+	}
+
+	if (mask != 0)
+		mask &= 0x1F7FFFFF; /* mask out reserved */
+	else
+		mask = GPIO_CS5535_DEF_MASK;
+
+	/* do not allow pin 28, Power Button, as there's special handling
+	 * in the PMC needed. (note 12, p. 48) */
+	mask &= ~(1 << 28);
+
+	if (mask_orig != mask)
+		printk(KERN_INFO GPIO_CS5535_NAME
+		       ": mask changed from 0x%08lX to 0x%08lX\n",
+		       mask_orig, mask);
+
+	gpio_cs5535_setup(cs5535);
+
+	err = gpiochip_add(&cs5535->gpio);
+	if (err) {
+		printk(KERN_ERR GPIO_CS5535_NAME
+		       ": Failed to register GPIOs\n");
+		goto err_release_mem;
+	}
+
+	return 0;
+
+err_release_mem:
+	pci_set_drvdata(pdev, NULL);
+	kfree(cs5535);
+
+	return err;
+}
+
+static void gpio_cs5535_remove(struct pci_dev *pdev)
+{
+	struct gpio_cs5535 *cs5535 = pci_get_drvdata(pdev);
+	int status = 0;
+
+	status = gpiochip_remove(&cs5535->gpio);
+
+	if (status == 0) {
+		pci_set_drvdata(pdev, NULL);
+		release_region(iobase, GPIO_CS5535_SIZE);
+		kfree(cs5535);
+	} else {
+		dev_err(&pdev->dev, "%s --> %d\n", "remove", status);
+	}
+}
+
+#define gpio_cs5535_suspend NULL
+#define gpio_cs5535_resume NULL
+
+static struct pci_driver gpio_cs5535_driver = {
+	.name           = "gpio_cs5535",
+	.id_table       = gpio_cs5535_pci_tbl,
+	.probe          = gpio_cs5535_probe,
+	.remove         = gpio_cs5535_remove,
+	.suspend        = gpio_cs5535_suspend,
+	.resume         = gpio_cs5535_resume,
+};
+
+static int __init gpio_cs5535_init(void)
+{
+	return pci_register_driver(&gpio_cs5535_driver);
+}
+module_init(gpio_cs5535_init)
+
+static void __exit gpio_cs5535_exit(void)
+{
+	pci_unregister_driver(&gpio_cs5535_driver);
+}
+module_exit(gpio_cs5535_exit);
+
+MODULE_AUTHOR("Tobias Mueller <Tobias_Mueller@twam.info>");
+MODULE_DESCRIPTION("AMD CS5535/CS5536 GPIO driver");
+MODULE_LICENSE("GPL");

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Andrew Morton]

On Sat, 30 May 2009 20:05:52 +0200
Tobias M__ller <Tobias_Mueller@twam.info> wrote:

> From: Tobias Mueller <Tobias_Mueller@twam.info>
> 
> A GPIO driver for AMD Geode Companion Device CS5535/CS5536
> using the GPIO framework as a replacement for old cs5535-gpio driver.
> Signed-off-by: Tobias Mueller <Tobias_Mueller@twam.info>

Last time, I said "We'll need to discuss the migration plan - we don't
want to maintain two drivers for the same thing".

Back in February, Andres sent out a patch "cs553x-gpio: add AMD
CS5535/CS5536 GPIO driver support".  AFACIT that patch does the same
thing that this one does, but differently.  IIRC it had a great string
of weird-Kconfig-related build errors and I eventually dropped it.

What's the story here?

Thanks.

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Andrew Morton]

On Sat, 30 May 2009 20:05:52 +0200
Tobias M__ller <Tobias_Mueller@twam.info> wrote:

> From: Tobias Mueller <Tobias_Mueller@twam.info>
> 
> A GPIO driver for AMD Geode Companion Device CS5535/CS5536
> using the GPIO framework as a replacement for old cs5535-gpio driver.
>
> ...
>
> +/* base address of gpio I/O*/
> +static u32 iobase;

Formally this should have type resource_size_t, I think.  Or `void
__iomem *'.  But given the limited applicability of this driver
(geode-only), I guess u32 is OK.

> +static int gpiobase;
> +module_param_named(gpiobase, gpiobase, int, 0444);
> +MODULE_PARM_DESC(gpiobase, "The GPIO number base. -1 means dynamic");
> +
> +static ulong mask;

hm, ulong isn't a particularly popular "type" in the kernel.  But I
guess it's stable enough.

> +module_param_named(mask, mask, ulong, 0444);
> +MODULE_PARM_DESC(mask, "GPIO channel mask.");
> +
> +/* gets the regiter offset for the GPIO bank.
> + *      low (0-15) stats at 0x00, high (16-31) starts at 0x80 */
>
> ...
>
> +#define gpio_cs5535_suspend NULL
> +#define gpio_cs5535_resume NULL
> +
> +static struct pci_driver gpio_cs5535_driver = {
> +	.name           = "gpio_cs5535",
> +	.id_table       = gpio_cs5535_pci_tbl,
> +	.probe          = gpio_cs5535_probe,
> +	.remove         = gpio_cs5535_remove,
> +	.suspend        = gpio_cs5535_suspend,
> +	.resume         = gpio_cs5535_resume,
> +};

What are the implications of not having suspend/resume support?

> +static int __init gpio_cs5535_init(void)
> +{
> +	return pci_register_driver(&gpio_cs5535_driver);
> +}
> +module_init(gpio_cs5535_init)
> +
> +static void __exit gpio_cs5535_exit(void)
> +{
> +	pci_unregister_driver(&gpio_cs5535_driver);
> +}
> +module_exit(gpio_cs5535_exit);

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Andres Salomon]

On Mon, 1 Jun 2009 16:54:57 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
[...]
> 
> Back in February, Andres sent out a patch "cs553x-gpio: add AMD
> CS5535/CS5536 GPIO driver support".  AFACIT that patch does the same
> thing that this one does, but differently.  IIRC it had a great string
> of weird-Kconfig-related build errors and I eventually dropped it.
> 
> What's the story here?
> 
> Thanks.

My story is that I've since switched employers twice since Feb, and will
be resubmitting the patch now that sanity has returned to my life.
I'm planning to get to it within the next week.

As far as Tobias's driver, one major difference between our drivers is
that mine doesn't grab the entire PCI device.  This is because other
drivers (ie, the MFGPT driver) will want to share the PCI device.

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Jordan Crouse]

Andres Salomon wrote:
> On Mon, 1 Jun 2009 16:54:57 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> [...]
>> Back in February, Andres sent out a patch "cs553x-gpio: add AMD
>> CS5535/CS5536 GPIO driver support".  AFACIT that patch does the same
>> thing that this one does, but differently.  IIRC it had a great string
>> of weird-Kconfig-related build errors and I eventually dropped it.
>>
>> What's the story here?
>>
>> Thanks.
> 
> My story is that I've since switched employers twice since Feb, and will
> be resubmitting the patch now that sanity has returned to my life.
> I'm planning to get to it within the next week.
> 
> As far as Tobias's driver, one major difference between our drivers is
> that mine doesn't grab the entire PCI device.  This is because other
> drivers (ie, the MFGPT driver) will want to share the PCI device.

Also SMBUS. In fact, they *must* share the PCI device if they all want 
to play well together, so that alone would probably push us toward 
Andre's patch, once it gets clean.

Jordan

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Tobias Müller]

2009/6/2 Jordan Crouse <jordan@cosmicpenguin.net>:
> Andres Salomon wrote:
>> As far as Tobias's driver, one major difference between our drivers is
>> that mine doesn't grab the entire PCI device.  This is because other
>> drivers (ie, the MFGPT driver) will want to share the PCI device.
>
> Also SMBUS. In fact, they *must* share the PCI device if they all want to
> play well together, so that alone would probably push us toward Andre's
> patch, once it gets clean.

I had SMBUS and my driver tested in parallel and there I didn't found any
problems. But I would be fine if we would use Andre's patch. I haven't know
about, so that's why I tried my own. :)

I see some other problem's on Andre's patch:
- He allows the use of PIN 23 which is reserved
- No special handling is done for 28, which is a GPIO, but
  serves as Power Button as well
- OUT_AUX1/OUT_AUX2 should be disabled by default
  if pin is configured as in output
- IN_AUX should be disabled by default is pin is
  configured as in input

And, if I request a GPIO by userspace via sysfs, on my system it's an
input pin by default. So my driver initializes a GPIO as an input by default.
But I don't know if this is default or system specific. I didn't found anything
about this in the gpio docs.

So maybe we should decide whether we take Andre's or my approach
and then adjust it.

Tobias

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Andrew Morton]

On Tue, 2 Jun 2009 10:14:45 +0200
Tobias M__ller <Tobias_Mueller@twam.info> wrote:

> 2009/6/2 Jordan Crouse <jordan@cosmicpenguin.net>:
> > Andres Salomon wrote:
> >> As far as Tobias's driver, one major difference between our drivers is
> >> that mine doesn't grab the entire PCI device.  This is because other
> >> drivers (ie, the MFGPT driver) will want to share the PCI device.
> >
> > Also SMBUS. In fact, they *must* share the PCI device if they all want to
> > play well together, so that alone would probably push us toward Andre's
> > patch, once it gets clean.
> 
> I had SMBUS and my driver tested in parallel and there I didn't found any
> problems. But I would be fine if we would use Andre's patch. I haven't know
> about, so that's why I tried my own. :)
> 
> I see some other problem's on Andre's patch:
> - He allows the use of PIN 23 which is reserved
> - No special handling is done for 28, which is a GPIO, but
>   serves as Power Button as well
> - OUT_AUX1/OUT_AUX2 should be disabled by default
>   if pin is configured as in output
> - IN_AUX should be disabled by default is pin is
>   configured as in input
> 
> And, if I request a GPIO by userspace via sysfs, on my system it's an
> input pin by default. So my driver initializes a GPIO as an input by default.
> But I don't know if this is default or system specific. I didn't found anything
> about this in the gpio docs.
> 
> So maybe we should decide whether we take Andre's or my approach
> and then adjust it.
> 

I think I'll drop gpio-amd-cs5535-cs5536-gpio-driver.patch while you
guys sort all this out, please.

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Tobias Müller]

>> So maybe we should decide whether we take Andre's or my approach
>> and then adjust it.
>>
>
> I think I'll drop gpio-amd-cs5535-cs5536-gpio-driver.patch while you
> guys sort all this out, please.

So, how to carry on with this? Shall I adapt Andre's version to mine or
will Andre take care of this version?

A comment from Andre would be fine.

Re: [PATCH v2 001/001] gpio: AMD CS5535/CS5536 GPIO driver

[Andres Salomon]

On Tue, 9 Jun 2009 14:27:30 +0200
Tobias Müller <Tobias_Mueller@twam.info> wrote:

> >> So maybe we should decide whether we take Andre's or my approach
> >> and then adjust it.
> >>
> >
> > I think I'll drop gpio-amd-cs5535-cs5536-gpio-driver.patch while you
> > guys sort all this out, please.
> 
> So, how to carry on with this? Shall I adapt Andre's version to mine
> or will Andre take care of this version?
> 
> A comment from Andre would be fine.


My preference would be for you to base your work on the patches that
I'm about to (re)submit.  I now actually have time to get these through
the queue.