[RFC PATCH 0/3] GPIO switch framework

[RFC PATCH 0/3] GPIO switch framework

[Jani Nikula]

Hi -

This RFC patchset is a pretty straightforward adaptation of OMAP GPIO
switch framework for mainline integration.

The GPIO switch framework allows reporting and changing GPIO switches
via sysfs, with debouncing and sysfs/in-kernel notifications for input
switches.

The switches are added as /sys/class/gpio/switch-<switch name> under
the gpiolib sysfs structure.


BR,
Jani.


Jani Nikula (3):
      GPIOLIB: Add new gpio_device_create function
      GPIO-SWITCH: Adaptation of GPIO switch framework for mainline
      GPIO-SWITCH: Kconfig and Makefile

 drivers/gpio/Kconfig        |   10 +
 drivers/gpio/Makefile       |    2 +
 drivers/gpio/gpio-switch.c  |  534 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpiolib.c      |   52 +++++
 include/asm-generic/gpio.h  |   12 +
 include/linux/gpio-switch.h |   74 ++++++
 6 files changed, 684 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-switch.c
 create mode 100644 include/linux/gpio-switch.h

[RFC PATCH 1/3] GPIOLIB: Add new gpio_device_create function

[Jani Nikula]

Add support function for creating additional sysfs entries within the
gpiolib sysfs structure. Initially required for GPIO switch framework
entries.

Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>
---
 drivers/gpio/gpiolib.c     |   52 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |   12 ++++++++++
 2 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 42fb2fd..b531edd 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -643,6 +643,58 @@ static inline void gpiochip_unexport(struct gpio_chip *chip)
 
 #endif /* CONFIG_GPIO_SYSFS */
 
+#if defined(CONFIG_GPIO_SWITCH) || defined(CONFIG_GPIO_SWITCH_MODULE)
+/**
+ * gpio_device_create - Create a sysfs entry for GPIO
+ * @gpio:
+ * @drvdata:
+ * @fmt:
+ *
+ */
+struct device *gpio_device_create(unsigned gpio, void *drvdata,
+				const char *fmt, ...)
+{
+	unsigned long		flags;
+	struct gpio_desc	*desc;
+	int			status = -EINVAL;
+	struct device		*dev = NULL;
+
+	/* can't export until sysfs is available ... */
+	if (!gpio_class.p) {
+		pr_debug("%s: called too early!\n", __func__);
+		goto done;
+	}
+
+	if (!gpio_is_valid(gpio))
+		goto done;
+
+	mutex_lock(&sysfs_lock);
+
+	spin_lock_irqsave(&gpio_lock, flags);
+	desc = &gpio_desc[gpio];
+	if (test_bit(FLAG_REQUESTED, &desc->flags))
+		status = 0;
+	spin_unlock_irqrestore(&gpio_lock, flags);
+
+	if (status == 0) {
+		va_list vargs;
+
+		va_start(vargs, fmt);
+		dev = device_create_vargs(&gpio_class, desc->chip->dev,
+					MKDEV(0, 0), drvdata, fmt, vargs);
+		va_end(vargs);
+	}
+	mutex_unlock(&sysfs_lock);
+
+	return dev;
+done:
+	pr_debug("%s: gpio%d status %d\n", __func__, gpio, status);
+
+	return ERR_PTR(status);
+}
+EXPORT_SYMBOL_GPL(gpio_device_create);
+#endif /* defined(CONFIG_GPIO_SWITCH) || defined(CONFIG_GPIO_SWITCH_MODULE) */
+
 /**
  * gpiochip_add() - register a gpio_chip
  * @chip: the chip to register, with chip->base initialized
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 81797ec..c4380d7 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -7,6 +7,7 @@
 #ifdef CONFIG_GPIOLIB
 
 #include <linux/compiler.h>
+#include <linux/err.h>
 
 /* Platforms may implement their GPIO interface with library code,
  * at a small performance cost for non-inlined operations and some
@@ -185,4 +186,15 @@ static inline void gpio_unexport(unsigned gpio)
 }
 #endif	/* CONFIG_GPIO_SYSFS */
 
+#if defined(CONFIG_GPIO_SWITCH) || defined(CONFIG_GPIO_SWITCH_MODULE)
+struct device *gpio_device_create(unsigned gpio, void *drvdata,
+				const char *fmt, ...);
+#else
+static inline struct device *gpio_device_create(unsigned gpio, void *drvdata,
+				const char *fmt, ...)
+{
+	return ERR_PTR(-EINVAL);
+}
+#endif
+
 #endif /* _ASM_GENERIC_GPIO_H */
-- 
1.6.0.4

[RFC PATCH 2/3] GPIO-SWITCH: Adaptation of GPIO switch framework for mainline

[Jani Nikula]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 15075 bytes --]

Adapt OMAP GPIO switch framework for mainline integration. The purpose
of the framework is to support reporting and changing GPIO switches via
sysfs. The starting point for the adaptation is:

  $ cp arch/arm/plat-omap/gpio-switch.c drivers/gpio/.
  $ cp arch/arm/plat-omap/include/mach/gpio-switch.h include/linux/.

Changes from OMAP version:
  - Remove OMAP dependencies.
  - Add dynamic switch add/remove.
  - Use gpiolib sysfs structure for the switches.
  - Use dev_dbg and friends instead of printk.
  - Misc cleanup.

Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>
---
 drivers/gpio/gpio-switch.c  |  534 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/gpio-switch.h |   74 ++++++
 2 files changed, 608 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-switch.c
 create mode 100644 include/linux/gpio-switch.h

diff --git a/drivers/gpio/gpio-switch.c b/drivers/gpio/gpio-switch.c
new file mode 100644
index 0000000..141e4e2
--- /dev/null
+++ b/drivers/gpio/gpio-switch.c
@@ -0,0 +1,534 @@
+/*
+ *  gpio-switch.c - GPIO switch framework
+ *
+ *  Copyright (C) 2004-2006,2009 Nokia Corporation
+ *  Written by Juha Yrjölä <juha.yrjola@nokia.com>
+ *         and Paul Mundt <paul.mundt@nokia.com>
+ *  Adapted for mainline by Jani Nikula <ext-jani.1.nikula@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/sched.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/timer.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/gpio-switch.h>
+
+struct gpio_sw {
+	char		name[14];
+	u16		gpio;
+	unsigned	flags:4;
+	unsigned	type:4;
+	unsigned	state:1;
+
+	u16		debounce_rising;
+	u16		debounce_falling;
+
+	void (*notify)(void *data, int state);
+	void *notify_data;
+
+	struct work_struct	work;
+	struct timer_list	timer;
+	struct device		*dev;
+
+	struct list_head	node;
+};
+
+static LIST_HEAD(gpio_switches);
+/* FIXME: this lock protects gpio_switches list, but how about locking
+ * in sysfs access? */
+static DEFINE_MUTEX(gpio_switches_mutex);
+static struct device *dbg_dev;
+
+static const char *cover_str[2] = { "open", "closed" };
+static const char *connection_str[2] = { "disconnected", "connected" };
+static const char *activity_str[2] = { "inactive", "active" };
+
+/*
+ * GPIO switch state default debounce delay in ms
+ */
+#define GPIO_SW_DEFAULT_DEBOUNCE		10
+
+static const char **get_sw_str(struct gpio_sw *sw)
+{
+	switch (sw->type) {
+	case GPIO_SWITCH_TYPE_COVER:
+		return cover_str;
+	case GPIO_SWITCH_TYPE_CONNECTION:
+		return connection_str;
+	case GPIO_SWITCH_TYPE_ACTIVITY:
+		return activity_str;
+	default:
+		BUG();
+		return NULL;
+	}
+}
+
+static const char *get_sw_type(struct gpio_sw *sw)
+{
+	switch (sw->type) {
+	case GPIO_SWITCH_TYPE_COVER:
+		return "cover";
+	case GPIO_SWITCH_TYPE_CONNECTION:
+		return "connection";
+	case GPIO_SWITCH_TYPE_ACTIVITY:
+		return "activity";
+	default:
+		BUG();
+		return NULL;
+	}
+}
+
+static void print_sw_state(struct gpio_sw *sw, int state)
+{
+	const char **str;
+
+	str = get_sw_str(sw);
+	if (str != NULL)
+		dev_info(sw->dev, "(GPIO %d) is now %s\n", sw->gpio,
+			str[state]);
+}
+
+static int gpio_sw_get_state(struct gpio_sw *sw)
+{
+	int state;
+
+	state = gpio_get_value(sw->gpio);
+	if (sw->flags & GPIO_SWITCH_FLAG_INVERTED)
+		state = !state;
+
+	return state;
+}
+
+static ssize_t gpio_sw_state_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct gpio_sw *sw = dev_get_drvdata(dev);
+	const char **str;
+	char state[16];
+	int enable;
+
+	if (!(sw->flags & GPIO_SWITCH_FLAG_OUTPUT))
+		return -EPERM;
+
+	if (sscanf(buf, "%15s", state) != 1)
+		return -EINVAL;
+
+	str = get_sw_str(sw);
+	if (strcmp(state, str[0]) == 0)
+		sw->state = enable = 0;
+	else if (strcmp(state, str[1]) == 0)
+		sw->state = enable = 1;
+	else
+		return -EINVAL;
+
+	if (sw->flags & GPIO_SWITCH_FLAG_INVERTED)
+		enable = !enable;
+	gpio_set_value(sw->gpio, enable);
+
+	return count;
+}
+
+static ssize_t gpio_sw_state_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct gpio_sw *sw = dev_get_drvdata(dev);
+	const char **str;
+
+	str = get_sw_str(sw);
+
+	return sprintf(buf, "%s\n", str[sw->state]);
+}
+
+static DEVICE_ATTR(state, S_IRUGO | S_IWUSR, gpio_sw_state_show,
+		gpio_sw_state_store);
+
+static ssize_t gpio_sw_type_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct gpio_sw *sw = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", get_sw_type(sw));
+}
+
+static DEVICE_ATTR(type, S_IRUGO, gpio_sw_type_show, NULL);
+
+static ssize_t gpio_sw_direction_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct gpio_sw *sw = dev_get_drvdata(dev);
+	int is_output;
+
+	is_output = sw->flags & GPIO_SWITCH_FLAG_OUTPUT;
+
+	return sprintf(buf, "%s\n", is_output ? "output" : "input");
+}
+
+static DEVICE_ATTR(direction, S_IRUGO, gpio_sw_direction_show, NULL);
+
+static struct attribute *gpio_sw_attributes[] = {
+	&dev_attr_state.attr,
+	&dev_attr_type.attr,
+	&dev_attr_direction.attr,
+	NULL
+};
+
+static const struct attribute_group gpio_sw_attr_group = {
+	.attrs = gpio_sw_attributes,
+};
+
+static irqreturn_t gpio_sw_irq_handler(int irq, void *arg)
+{
+	struct gpio_sw *sw = arg;
+	unsigned long timeout;
+	int state;
+
+	state = gpio_sw_get_state(sw);
+	if (sw->state == state)
+		return IRQ_HANDLED;
+
+	if (state)
+		timeout = sw->debounce_rising;
+	else
+		timeout = sw->debounce_falling;
+
+	if (!timeout)
+		schedule_work(&sw->work);
+	else
+		mod_timer(&sw->timer, jiffies + msecs_to_jiffies(timeout));
+
+	return IRQ_HANDLED;
+}
+
+static void gpio_sw_timer(unsigned long arg)
+{
+	struct gpio_sw *sw = (struct gpio_sw *) arg;
+
+	schedule_work(&sw->work);
+}
+
+static void gpio_sw_handler(struct work_struct *work)
+{
+	struct gpio_sw *sw = container_of(work, struct gpio_sw, work);
+	int state;
+
+	state = gpio_sw_get_state(sw);
+	if (sw->state == state)
+		return;
+
+	sw->state = state;
+	if (sw->notify != NULL)
+		sw->notify(sw->notify_data, state);
+	sysfs_notify(&sw->dev->kobj, NULL, "state");
+	print_sw_state(sw, state);
+}
+
+static int new_switch(struct gpio_sw *sw)
+{
+	int r, direction;
+	struct device *dev;
+
+	switch (sw->type) {
+	case GPIO_SWITCH_TYPE_COVER:
+	case GPIO_SWITCH_TYPE_CONNECTION:
+	case GPIO_SWITCH_TYPE_ACTIVITY:
+		break;
+	default:
+		dev_err(dbg_dev, "invalid GPIO switch type: %d\n", sw->type);
+		return -EINVAL;
+	}
+
+	r = gpio_request(sw->gpio, sw->name);
+	if (r < 0) {
+		dev_err(dbg_dev, "gpio_request failed for %s", sw->name);
+		goto err_gpio;
+	}
+
+	/* input: 1, output: 0 */
+	direction = !(sw->flags & GPIO_SWITCH_FLAG_OUTPUT);
+	if (direction) {
+		gpio_direction_input(sw->gpio);
+		sw->state = gpio_sw_get_state(sw);
+	} else {
+		int state = sw->state =
+			!!(sw->flags & GPIO_SWITCH_FLAG_OUTPUT_INIT_HIGH);
+
+		if (sw->flags & GPIO_SWITCH_FLAG_INVERTED)
+			state = !state;
+		gpio_direction_output(sw->gpio, state);
+	}
+
+	/* switch device in gpiolib sysfs */
+	dev = gpio_device_create(sw->gpio, sw, "switch-%s", sw->name);
+	if (IS_ERR(dev)) {
+		r = PTR_ERR(dev);
+		goto err_dev;
+	}
+
+	sw->dev = dev;
+
+	r = sysfs_create_group(&dev->kobj, &gpio_sw_attr_group);
+	if (r)
+		dev_err(dbg_dev, "attribute group creation failed for %s\n",
+			sw->name);
+
+	if (direction) {
+		r = request_irq(gpio_to_irq(sw->gpio), gpio_sw_irq_handler,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
+				IRQF_SHARED, sw->name, sw);
+		if (r < 0) {
+			dev_err(dbg_dev, "request_irq() failed for GPIO %d\n",
+				sw->gpio);
+			goto err_irq;
+		}
+
+		INIT_WORK(&sw->work, gpio_sw_handler);
+		init_timer(&sw->timer);
+
+		sw->timer.function = gpio_sw_timer;
+		sw->timer.data = (unsigned long)sw;
+	}
+
+	list_add(&sw->node, &gpio_switches);
+
+	return 0;
+
+err_irq:
+	device_unregister(dev);
+err_dev:
+	gpio_free(sw->gpio);
+err_gpio:
+	return r;
+}
+
+static struct gpio_sw *find_switch(int gpio, const char *name)
+{
+	struct gpio_sw *sw;
+
+	list_for_each_entry(sw, &gpio_switches, node) {
+		if ((gpio < 0 || sw->gpio != gpio) &&
+			(name == NULL || strcmp(sw->name, name) != 0))
+			continue;
+
+		if (gpio < 0 || name == NULL)
+			goto no_check;
+
+		if (strcmp(sw->name, name) != 0)
+			dev_err(dbg_dev, "name mismatch for %d (%s, %s)\n",
+				gpio, name, sw->name);
+		else if (sw->gpio != gpio)
+			dev_err(dbg_dev, "GPIO mismatch for %s (%d, %d)\n",
+				name, gpio, sw->gpio);
+no_check:
+		return sw;
+	}
+
+	return NULL;
+}
+
+static void report_initial_state(struct gpio_sw *sw)
+{
+	int state;
+
+	if (sw->flags & GPIO_SWITCH_FLAG_OUTPUT)
+		return;
+
+	state = gpio_get_value(sw->gpio);
+	if (sw->flags & GPIO_SWITCH_FLAG_INVERTED)
+		state = !state;
+	if (sw->notify != NULL)
+		sw->notify(sw->notify_data, state);
+	print_sw_state(sw, state);
+}
+
+static void free_switch(struct gpio_sw *sw)
+{
+	if (!(sw->flags & GPIO_SWITCH_FLAG_OUTPUT)) {
+		free_irq(gpio_to_irq(sw->gpio), sw);
+		del_timer_sync(&sw->timer);
+		flush_scheduled_work();
+	}
+
+	sysfs_remove_group(&sw->dev->kobj, &gpio_sw_attr_group);
+	device_unregister(sw->dev);
+
+	gpio_free(sw->gpio);
+
+	kfree(sw);
+}
+
+static void free_switches(void)
+{
+	struct gpio_sw *sw;
+	struct gpio_sw *tmp;
+
+	mutex_lock(&gpio_switches_mutex);
+	list_for_each_entry_safe(sw, tmp, &gpio_switches, node) {
+		list_del(&sw->node);
+		free_switch(sw);
+	}
+	mutex_unlock(&gpio_switches_mutex);
+}
+
+/**
+ * gpio_switch_request - request a GPIO switch
+ * @sw: pointer to GPIO switch structure
+ *
+ */
+int gpio_switch_request(const struct gpio_switch *sw)
+{
+	int r = 0;
+	struct gpio_sw *drvdata;
+
+	mutex_lock(&gpio_switches_mutex);
+	if (find_switch(sw->gpio, sw->name) != NULL) {
+		r = -EINVAL;
+		goto done;
+	}
+
+	drvdata = kzalloc(sizeof(*drvdata), GFP_KERNEL);
+	if (drvdata == NULL) {
+		r = -ENOMEM;
+		goto done;
+	}
+
+	strlcpy(drvdata->name, sw->name, sizeof(drvdata->name));
+	drvdata->gpio = sw->gpio;
+	drvdata->flags = sw->flags;
+	drvdata->type = sw->type;
+	drvdata->debounce_rising = sw->debounce_rising;
+	drvdata->debounce_falling = sw->debounce_falling;
+	drvdata->notify = sw->notify;
+	drvdata->notify_data = sw->notify_data;
+
+	r = new_switch(drvdata);
+	if (r < 0) {
+		kfree(drvdata);
+		goto done;
+	}
+
+	report_initial_state(drvdata);
+
+done:
+	mutex_unlock(&gpio_switches_mutex);
+
+	return r;
+}
+EXPORT_SYMBOL_GPL(gpio_switch_request);
+
+/**
+ * gpio_switch_free - remove a GPIO switch
+ * @gpio: the GPIO switch to remove
+ */
+void gpio_switch_free(unsigned gpio)
+{
+	struct gpio_sw *sw;
+
+	mutex_lock(&gpio_switches_mutex);
+	sw = find_switch(gpio, NULL);
+	if (sw != NULL) {
+		list_del(&sw->node);
+		free_switch(sw);
+	}
+	mutex_unlock(&gpio_switches_mutex);
+}
+EXPORT_SYMBOL_GPL(gpio_switch_free);
+
+static int add_switches(const struct gpio_switch **switches, unsigned count)
+{
+	unsigned i;
+	int ret = 0;
+
+	for (i = 0; i < count; i++) {
+		ret = gpio_switch_request(switches[i]);
+		if (ret) {
+			while (--i >= 0)
+				gpio_switch_free(switches[i]->gpio);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+static int __init gpio_switch_probe(struct platform_device *pdev)
+{
+	struct gpio_switch_platform_data *pdata;
+	int ret;
+
+	dbg_dev = &pdev->dev;
+
+	dev_dbg(dbg_dev, "GPIO switch handler initializing\n");
+
+	pdata = pdev->dev.platform_data;
+	if (pdata != NULL) {
+		ret = add_switches(pdata->switches, pdata->count);
+		if (ret) {
+			dev_err(dbg_dev, "could not add GPIO switches\n");
+			goto err_addsw;
+		}
+	}
+
+	dev_info(dbg_dev, "GPIO switch handler initialized\n");
+
+	return 0;
+
+err_addsw:
+	return ret;
+}
+
+static int __exit gpio_switch_remove(struct platform_device *pdev)
+{
+	free_switches();
+
+	return 0;
+}
+
+static struct platform_device gpio_switch_device = {
+	.name	= "gpio-switch",
+	.id	= -1,
+};
+
+static struct platform_driver gpio_switch_driver = {
+	.driver		= {
+		.name	= "gpio-switch",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= gpio_switch_probe,
+	.remove		= __exit_p(gpio_switch_remove),
+};
+
+static int __init gpio_switch_init(void)
+{
+	/* FIXME: don't register device if it's been done in board
+	 * file - how to accomplish? now assuming failure means it's
+	 * been registered already, which isn't the way to go. */
+	platform_device_register(&gpio_switch_device);
+
+	return platform_driver_register(&gpio_switch_driver);
+}
+module_init(gpio_switch_init);
+
+static void __exit gpio_switch_exit(void)
+{
+	/* FIXME: platform_device_unregister if registered in init. */
+	platform_driver_unregister(&gpio_switch_driver);
+}
+module_exit(gpio_switch_exit);
+
+MODULE_AUTHOR("Juha Yrjölä <juha.yrjola@nokia.com>, "
+	"Paul Mundt <paul.mundt@nokia.com, "
+	"Jani Nikula <ext-jani.1.nikula@nokia.com>");
+MODULE_DESCRIPTION("GPIO switch driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/gpio-switch.h b/include/linux/gpio-switch.h
new file mode 100644
index 0000000..ac432cc
--- /dev/null
+++ b/include/linux/gpio-switch.h
@@ -0,0 +1,74 @@
+/*
+ * GPIO switch definitions
+ *
+ * Copyright (C) 2006,2009 Nokia Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ */
+
+#ifndef __GPIO_SWITCH_H
+#define __GPIO_SWITCH_H
+
+#include <linux/types.h>
+
+/* Cover:
+ *	high -> closed
+ *	low  -> open
+ * Connection:
+ *	high -> connected
+ *	low  -> disconnected
+ * Activity:
+ *	high -> active
+ *	low  -> inactive
+ *
+ */
+#define GPIO_SWITCH_TYPE_COVER			0x0000
+#define GPIO_SWITCH_TYPE_CONNECTION		0x0001
+#define GPIO_SWITCH_TYPE_ACTIVITY		0x0002
+#define GPIO_SWITCH_FLAG_INVERTED		0x0001
+#define GPIO_SWITCH_FLAG_OUTPUT			0x0002
+#define GPIO_SWITCH_FLAG_OUTPUT_INIT_HIGH	0x0004
+
+struct gpio_switch {
+	const char *name;
+	s16 gpio;
+	unsigned flags:4;
+	unsigned type:4;
+
+	/* Time in ms to debounce when transitioning from
+	 * inactive state to active state. */
+	u16 debounce_rising;
+	/* Same for transition from active to inactive state. */
+	u16 debounce_falling;
+
+	/* Callback for state changes */
+	void (*notify)(void *data, int state);
+	void *notify_data;
+};
+
+struct gpio_switch_platform_data {
+	const struct gpio_switch **switches;
+	unsigned count;
+};
+
+#if defined(CONFIG_GPIO_SWITCH) || defined(CONFIG_GPIO_SWITCH_MODULE)
+
+extern int gpio_switch_request(const struct gpio_switch *sw);
+extern void gpio_switch_free(unsigned gpio);
+
+#else
+
+static inline int gpio_switch_request(const struct gpio_switch *sw)
+{
+	return -EINVAL;
+}
+
+static inline void gpio_switch_free(unsigned gpio)
+{
+}
+
+#endif /* defined(CONFIG_GPIO_SWITCH) || defined(CONFIG_GPIO_SWITCH_MODULE) */
+
+#endif
-- 
1.6.0.4

[RFC PATCH 3/3] GPIO-SWITCH: Kconfig and Makefile

[Jani Nikula]

Add new CONFIG_GPIO_SWITCH.

Signed-off-by: Jani Nikula <ext-jani.1.nikula@nokia.com>
---
 drivers/gpio/Kconfig  |   10 ++++++++++
 drivers/gpio/Makefile |    2 ++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3d25654..a24b16e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -63,6 +63,16 @@ config GPIO_SYSFS
 	  Kernel drivers may also request that a particular GPIO be
 	  exported to userspace; this can be useful when debugging.
 
+config GPIO_SWITCH
+	tristate "GPIO switch support"
+	depends on GPIO_SYSFS
+        default n
+        help
+          Say Y, if you want to have support for reporting of GPIO
+          switches (e.g. cover switches) via sysfs.
+
+	  If unsure, say N.
+
 # put expanders in the right section, in alphabetical order
 
 comment "Memory mapped GPIO expanders:"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 49ac64e..02eaee8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -12,3 +12,5 @@ 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_SWITCH)	+= gpio-switch.o
-- 
1.6.0.4

Re: [RFC PATCH 0/3] GPIO switch framework

[pHilipp Zabel]

Hi,

On Fri, Mar 20, 2009 at 2:50 PM, Jani Nikula
<ext-jani.1.nikula@nokia.com> wrote:
>
> Hi -
>
> This RFC patchset is a pretty straightforward adaptation of OMAP GPIO
> switch framework for mainline integration.
>
> The GPIO switch framework allows reporting and changing GPIO switches
> via sysfs, with debouncing and sysfs/in-kernel notifications for input
> switches.

Doesn't this partially duplicate what the input layer is for?
There is also a gpio-keys driver in input/keyboard already that can
report switch events (EV_SW) to the input layer, but it doesn't have
the in-kernel notification.

> The switches are added as /sys/class/gpio/switch-<switch name> under
> the gpiolib sysfs structure.
>
> BR,
> Jani.
>
>
> Jani Nikula (3):
>      GPIOLIB: Add new gpio_device_create function
>      GPIO-SWITCH: Adaptation of GPIO switch framework for mainline
>      GPIO-SWITCH: Kconfig and Makefile
>
>  drivers/gpio/Kconfig        |   10 +
>  drivers/gpio/Makefile       |    2 +
>  drivers/gpio/gpio-switch.c  |  534 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/gpio/gpiolib.c      |   52 +++++
>  include/asm-generic/gpio.h  |   12 +
>  include/linux/gpio-switch.h |   74 ++++++
>  6 files changed, 684 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpio/gpio-switch.c
>  create mode 100644 include/linux/gpio-switch.h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

regards
Philipp

Re: [RFC PATCH 0/3] GPIO switch framework

[Ben Nizette]

On Fri, 2009-03-20 at 15:50 +0200, Jani Nikula wrote:
> Hi -
> 
> This RFC patchset is a pretty straightforward adaptation of OMAP GPIO
> switch framework for mainline integration.
> 
> The GPIO switch framework allows reporting and changing GPIO switches
> via sysfs, with debouncing and sysfs/in-kernel notifications for input
> switches.

OK, so what does this do that /sys/class/gpio/gpioN doesn't currently do
apart from debouncing?  And the output being a string rather than a
simple value (which IMO might be better suited to userspace
interpretation anyway).

I've got a patch, little abandoned at the bottom of my queue, which adds
poll(2) compatibility to the gpiolib sysfs entries [1] and extending
this patch to do debouncing as well would be almost trivial.

I guess what I'm getting at is that this seems like it solves a problem
which has been pretty much solved elsewhere since the OMAP boys wrote
this.

	--Ben.
 
[1] http://marc.info/?l=linux-kernel&m=122454305213792&w=2

Re: [RFC PATCH 0/3] GPIO switch framework

[Jani Nikula]

Hi Ben, and thanks for your feedback.

On Sun, 2009-03-22 at 07:25 +0100, ext Ben Nizette wrote:
> On Fri, 2009-03-20 at 15:50 +0200, Jani Nikula wrote:
> > Hi -
> > 
> > This RFC patchset is a pretty straightforward adaptation of OMAP GPIO
> > switch framework for mainline integration.
> > 
> > The GPIO switch framework allows reporting and changing GPIO switches
> > via sysfs, with debouncing and sysfs/in-kernel notifications for input
> > switches.
> 
> OK, so what does this do that /sys/class/gpio/gpioN doesn't currently do
> apart from debouncing?  And the output being a string rather than a
> simple value (which IMO might be better suited to userspace
> interpretation anyway).

In addition to debouncing, there's sysfs and in-kernel notifications.

To hide potential changes in hardware from userspace, there's naming of
the sysfs entries (e.g. /sys/class/gpio/switch-foo) and a flag for
inverting the value.

> I've got a patch, little abandoned at the bottom of my queue, which adds
> poll(2) compatibility to the gpiolib sysfs entries [1] and extending
> this patch to do debouncing as well would be almost trivial.

Your patch certainly looks promising, and indeed overlaps with my work.
Debouncing could be easily added, and perhaps symbolic links to provide
names for the switches.

However, if I didn't miss anything, your patch allows setting up the
notification through sysfs only, and there's no way of doing it from
kernel side. Also, I'd need debounced callback notifications.

> I guess what I'm getting at is that this seems like it solves a problem
> which has been pretty much solved elsewhere since the OMAP boys wrote
> this.

It's been only partially solved by your patch, and as you say yourself,
it's still at the bottom of your queue. There's also the gpio-keys
(pointed out by Philipp Zabel elsewhere in this thread), but that's
input only and lacks in-kernel notifications. AFAIK the input layer is
also pretty fixed in what can be reported. So I am not convinced this
has been solved elsewhere.

Do you have any plans to resume work on your patch?


BR,
Jani.


> 	--Ben.
>  
> [1] http://marc.info/?l=linux-kernel&m=122454305213792&w=2

Re: [RFC PATCH 0/3] GPIO switch framework

[David Brownell]

On Tuesday 31 March 2009, Jani Nikula wrote:
> Hi Ben, and thanks for your feedback.
> 
> On Sun, 2009-03-22 at 07:25 +0100, ext Ben Nizette wrote:
> > On Fri, 2009-03-20 at 15:50 +0200, Jani Nikula wrote:
> > > Hi -
> > > 
> > > This RFC patchset is a pretty straightforward adaptation of OMAP GPIO
> > > switch framework for mainline integration.
> > > 
> > > The GPIO switch framework allows reporting and changing GPIO switches
> > > via sysfs, with debouncing and sysfs/in-kernel notifications for input
> > > switches.
> > 
> > OK, so what does this do that /sys/class/gpio/gpioN doesn't currently do
> > apart from debouncing?  And the output being a string rather than a
> > simple value (which IMO might be better suited to userspace
> > interpretation anyway).
> 
> In addition to debouncing, there's sysfs and in-kernel notifications.

And you had said off-list you were looking at adding
some debouncing support to the sysfs hooks.


> To hide potential changes in hardware from userspace, there's naming of
> the sysfs entries (e.g. /sys/class/gpio/switch-foo) and a flag for
> inverting the value.

You've recently sent a patch to do that using the existing
gpio sysfs support ... it's in the MM tree, although an update
will remove the BUG_ON calls.

  http://marc.info/?l=linux-kernel&m=124471204121635&w=2

I agree with Ben that string names are better left for userspace
tools to handle.  (Why limit things to English?)  Same for all
interpretation -- "0" might mean "open" or "connected" or "on",
or maybe sometimes it's "1" which means that.


> > I've got a patch, little abandoned at the bottom of my queue, which adds
> > poll(2) compatibility to the gpiolib sysfs entries [1] and extending
> > this patch to do debouncing as well would be almost trivial.
> 
> Your patch certainly looks promising, and indeed overlaps with my work.
> Debouncing could be easily added, and perhaps symbolic links to provide
> names for the switches.

And as you know, Daniel Glöckner has an updated version:

  http://marc.info/?l=linux-kernel&m=124463747423885

Looks promising.

 
> However, if I didn't miss anything, your patch allows setting up the
> notification through sysfs only, and there's no way of doing it from
> kernel side. Also, I'd need debounced callback notifications.

That is, in-kernel notifications?  Normally that's what GPIO interrupts
handle.

Debouncing gets messy, what with hardware support varying so much,
but presumably some low-bitrate software debounce may serve.  Folk
have asked for such things in the not-so-distant past.


> > I guess what I'm getting at is that this seems like it solves a problem
> > which has been pretty much solved elsewhere since the OMAP boys wrote
> > this.
> 
> It's been only partially solved by your patch, and as you say yourself,
> it's still at the bottom of your queue. There's also the gpio-keys
> (pointed out by Philipp Zabel elsewhere in this thread), but that's
> input only and lacks in-kernel notifications.

Note that gpio-keys has some minimal debouncing already.
Maybe not the most effective solution for software debounce
of GPIOs, but if there's a generalized version of debounce
then maybe that driver could use it too.

As for the input framework ... stuff like MMC/SD and PCMCIA
card insert/remove events have not yet been handled through
that framework, and I think it's very appropriate that an
MMC host adapter be independent of the input subsystem.  :)

But maybe some of those cases should be handled through small
shims that can issue the event callbacks to MMC or PCMCIA
host adapter code based on input events.  The systems you
work with can assume the input subsystem, I think.


> AFAIK the input layer is 
> also pretty fixed in what can be reported. So I am not convinced this
> has been solved elsewhere.

Parts of it have been, and in more general ways.  I'm a lot more
comfortable with the notion of having a solution that's built of
reusable chunks than having something quite so focussed on what,
say, only a Nokia tablet needs.  Which seems to be what you're
thinking lately too, so all is fine.  ;)

- Dave

Re: [RFC PATCH 0/3] GPIO switch framework

[Jani Nikula]

On Wed, 2009-07-01 at 23:03 +0200, ext David Brownell wrote:
> On Tuesday 31 March 2009, Jani Nikula wrote:
> > Hi Ben, and thanks for your feedback.
> > 
> > On Sun, 2009-03-22 at 07:25 +0100, ext Ben Nizette wrote:
> > > On Fri, 2009-03-20 at 15:50 +0200, Jani Nikula wrote:
> > > > Hi -
> > > > 
> > > > This RFC patchset is a pretty straightforward adaptation of OMAP GPIO
> > > > switch framework for mainline integration.
> > > > 
> > > > The GPIO switch framework allows reporting and changing GPIO switches
> > > > via sysfs, with debouncing and sysfs/in-kernel notifications for input
> > > > switches.
> > > 
> > > OK, so what does this do that /sys/class/gpio/gpioN doesn't currently do
> > > apart from debouncing?  And the output being a string rather than a
> > > simple value (which IMO might be better suited to userspace
> > > interpretation anyway).
> > 
> > In addition to debouncing, there's sysfs and in-kernel notifications.
> 
> And you had said off-list you were looking at adding
> some debouncing support to the sysfs hooks.

Yes, Daniel Glöckner's patch you refer to below looks like a good base
for that. It came after this patch set, and I think it's a nice generic
building block that accomplishes a part of what the gpio-switch
framework does.

> > To hide potential changes in hardware from userspace, there's naming of
> > the sysfs entries (e.g. /sys/class/gpio/switch-foo) and a flag for
> > inverting the value.
> 
> You've recently sent a patch to do that using the existing
> gpio sysfs support ... it's in the MM tree, although an update
> will remove the BUG_ON calls.
> 
>   http://marc.info/?l=linux-kernel&m=124471204121635&w=2

The update is now in the MM tree as well. Another building block.

> I agree with Ben that string names are better left for userspace
> tools to handle.  (Why limit things to English?)  Same for all
> interpretation -- "0" might mean "open" or "connected" or "on",
> or maybe sometimes it's "1" which means that.

Agreed.

> > > I've got a patch, little abandoned at the bottom of my queue, which adds
> > > poll(2) compatibility to the gpiolib sysfs entries [1] and extending
> > > this patch to do debouncing as well would be almost trivial.
> > 
> > Your patch certainly looks promising, and indeed overlaps with my work.
> > Debouncing could be easily added, and perhaps symbolic links to provide
> > names for the switches.
> 
> And as you know, Daniel Glöckner has an updated version:
> 
>   http://marc.info/?l=linux-kernel&m=124463747423885
> 
> Looks promising.

Indeed.

> > However, if I didn't miss anything, your patch allows setting up the
> > notification through sysfs only, and there's no way of doing it from
> > kernel side. Also, I'd need debounced callback notifications.
> 
> That is, in-kernel notifications?  Normally that's what GPIO interrupts
> handle.

True, but then you'd need to have two debouncing mechanisms (I'm talking
about software debouncing), one for gpiolib sysfs (once someone
implements that) and one for the interrupts. Or the driver would need to
have its own sysfs.

Without debouncing, there's no problem of course in just using GPIO
interrupts - except maybe for gpio_request being done in gpiolib for the
sysfs, so it can't be done in the driver.

Maybe this is a corner case and a non-issue here.

> Debouncing gets messy, what with hardware support varying so much,
> but presumably some low-bitrate software debounce may serve.  Folk
> have asked for such things in the not-so-distant past.

Software debouncing is what I'm talking about, and in connection with
the gpiolib sysfs, a high-bitrate hardware debouncing would be overkill,
don't you think?

> > > I guess what I'm getting at is that this seems like it solves a problem
> > > which has been pretty much solved elsewhere since the OMAP boys wrote
> > > this.
> > 
> > It's been only partially solved by your patch, and as you say yourself,
> > it's still at the bottom of your queue. There's also the gpio-keys
> > (pointed out by Philipp Zabel elsewhere in this thread), but that's
> > input only and lacks in-kernel notifications.
> 
> Note that gpio-keys has some minimal debouncing already.
> Maybe not the most effective solution for software debounce
> of GPIOs, but if there's a generalized version of debounce
> then maybe that driver could use it too.

Something along the lines of what OMAP gpio-switch framework or
gpio-keys have for debouncing was what I had in mind. If you have better
ideas, I'm all ears. Anyway, implementing that as a generic debouncing
module that could be used elsewhere would be a good idea too.

> As for the input framework ... stuff like MMC/SD and PCMCIA
> card insert/remove events have not yet been handled through
> that framework, and I think it's very appropriate that an
> MMC host adapter be independent of the input subsystem.  :)
> 
> But maybe some of those cases should be handled through small
> shims that can issue the event callbacks to MMC or PCMCIA
> host adapter code based on input events.  The systems you
> work with can assume the input subsystem, I think.
> 
> 
> > AFAIK the input layer is 
> > also pretty fixed in what can be reported. So I am not convinced this
> > has been solved elsewhere.
> 
> Parts of it have been, and in more general ways.  I'm a lot more
> comfortable with the notion of having a solution that's built of
> reusable chunks than having something quite so focussed on what,
> say, only a Nokia tablet needs.  Which seems to be what you're
> thinking lately too, so all is fine.  ;)

Yes, well, I kind of had to change my thinking, since this patch set
wasn't going anywhere! :) And I do appreciate the reusable chunks as
well.

Thanks for your comments.


BR,
Jani.