2009-03-20 13:53:21

by Jani Nikula

[permalink] [raw]
Subject: [RFC PATCH 0/3] GPIO switch framework


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


2009-03-20 13:52:18

by Jani Nikula

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

Add new CONFIG_GPIO_SWITCH.

Signed-off-by: Jani Nikula <[email protected]>
---
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

2009-03-20 13:53:37

by Jani Nikula

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

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 <[email protected]>
---
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

2009-03-20 13:54:27

by Jani Nikula

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

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 <[email protected]>
---
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ä <[email protected]>
+ * and Paul Mundt <[email protected]>
+ * Adapted for mainline by Jani Nikula <[email protected]>
+ *
+ * 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ä <[email protected]>, "
+ "Paul Mundt <[email protected], "
+ "Jani Nikula <[email protected]>");
+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

2009-03-20 14:16:22

by Philipp Zabel

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

Hi,

On Fri, Mar 20, 2009 at 2:50 PM, Jani Nikula
<[email protected]> 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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

regards
Philipp

2009-03-22 06:25:42

by Ben Nizette

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

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


2009-03-31 09:21:20

by Jani Nikula

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

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

2009-07-01 21:04:03

by David Brownell

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

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

2009-07-02 09:08:50

by Jani Nikula

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

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.