[PATCH RFC 2/6 v3] gpio: Add sysfs support to block GPIO API
From: Ryan Mallon <hidden>
Date: 2012-10-15 05:36:04
Also in:
lkml
On 13/10/12 06:11, Roland Stigge wrote:
This patch adds sysfs support to the block GPIO API. Signed-off-by: Roland Stigge <redacted>
Hi Roland, Some comments below, ~Ryan
quoted hunk ↗ jump to hunk
--- Documentation/ABI/testing/sysfs-gpio | 6 drivers/gpio/gpiolib.c | 226 ++++++++++++++++++++++++++++++++++- include/asm-generic/gpio.h | 11 + include/linux/gpio.h | 13 ++ 4 files changed, 254 insertions(+), 2 deletions(-)--- linux-2.6.orig/Documentation/ABI/testing/sysfs-gpio +++ linux-2.6/Documentation/ABI/testing/sysfs-gpio@@ -24,4 +24,8 @@ Description: /base ... (r/o) same as N /label ... (r/o) descriptive, not necessarily unique /ngpio ... (r/o) number of GPIOs; numbered N to N + (ngpio - 1) - + /blockN ... for each GPIO block #N + /ngpio ... (r/o) number of GPIOs in this group + /exported ... sysfs export state of this group (0, 1) + /value ... current value as 32 or 64 bit integer in decimal + (only available if /exported is 1) --- linux-2.6.orig/drivers/gpio/gpiolib.c +++ linux-2.6/drivers/gpio/gpiolib.c@@ -974,6 +974,218 @@ static void gpiochip_unexport(struct gpi chip->label, status); } +static ssize_t gpio_block_ngpio_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + const struct gpio_block *block = dev_get_drvdata(dev); + + return sprintf(buf, "%u\n", block->ngpio); +} +static struct device_attribute +dev_attr_block_ngpio = __ATTR(ngpio, 0444, gpio_block_ngpio_show, NULL);
static DEVICE_ATTR(ngpio, S_IRUGO, gpio_block_ngpio_show, NULL);
+
+static ssize_t gpio_block_value_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ const struct gpio_block *block = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", gpio_block_get(block));Printing the value of a bunch of pins as a decimal is a bit odd. Hex, or a bitmap would be more appropriate.
+}
+
+static bool gpio_block_is_output(struct gpio_block *block)
+{
+ int i;
+
+ for (i = 0; i < block->ngpio; i++)
+ if (!test_bit(FLAG_IS_OUT, &gpio_desc[block->gpio[i]].flags))
+ return false;Shouldn't a block force all of the pins to be the same direction? Or at least have gpio_block_set skip pins which aren't outputs.
+ return true;
+}
+
+static ssize_t gpio_block_value_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ ssize_t status;
+ struct gpio_block *block = dev_get_drvdata(dev);
+ unsigned long value;
+
+ mutex_lock(&sysfs_lock);
+
+ status = kstrtoul(buf, 0, &value);
+ if (status == 0) {You don't need to do the kstrtoul under the lock: err = kstrtoul(buf, 0, &value); if (err) return err; mutex_lock(&sysfs_lock); ... Global lock is a bit lame, it serialises all of your bitbanged busses against each other. Why is it not part of the gpio_block structure?
+ if (gpio_block_is_output(block)) {
+ gpio_block_set(block, value);
+ status = size;
+ } else {
+ status = -EPERM;
+ }
+ }
+
+ mutex_unlock(&sysfs_lock);
+ return status;
+}
+
+static struct device_attribute
+dev_attr_block_value = __ATTR(value, 0644, gpio_block_value_show,
+ gpio_block_value_store);Use DEVICE_ATTR and S_IWUSR | S_IRUGO permission macros.
+
+static int gpio_block_value_is_exported(struct gpio_block *block)
+{
+ struct device *dev;
+ struct sysfs_dirent *sd = NULL;
+
+ mutex_lock(&sysfs_lock);
+ dev = class_find_device(&gpio_class, NULL, block, match_export);
+ if (!dev)
+ goto out;
+
+ sd = sysfs_get_dirent(dev->kobj.sd, NULL, "value");
+
+out:
+ mutex_unlock(&sysfs_lock);
+ return sd ? 1 : 0;return sd; or: return !!sd;
+}
+
+static ssize_t gpio_block_exported_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct gpio_block *block = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", gpio_block_value_is_exported(block));
+}
+
+static int gpio_block_value_export(struct gpio_block *block)
+{
+ struct device *dev;
+ int status;
+ int i;
+
+ mutex_lock(&sysfs_lock);
+
+ for (i = 0; i < block->ngpio; i++) {
+ status = gpio_request(block->gpio[i], "sysfs");
+ if (status)
+ goto out;
+ }
+
+ dev = class_find_device(&gpio_class, NULL, block, match_export);
+ if (!dev) {
+ status = -ENODEV;
+ goto out;
+ }
+
+ status = device_create_file(dev, &dev_attr_block_value);
+ if (status)
+ goto out;
+
+ mutex_unlock(&sysfs_lock);
+ return 0;
+
+out:
+ while (i > 0) {
+ i--;
+ gpio_free(block->gpio[i]);
+ }Nitpick: while (--i >= 0) gpio_free(block->gpio[i]);
quoted hunk ↗ jump to hunk
+ + mutex_unlock(&sysfs_lock); + return status; +} + +static int gpio_block_value_unexport(struct gpio_block *block) +{ + struct device *dev; + int i; + + dev = class_find_device(&gpio_class, NULL, block, match_export); + if (!dev) + return -ENODEV; + + for (i = 0; i < block->ngpio; i++) + gpio_free(block->gpio[i]); + + device_remove_file(dev, &dev_attr_block_value); + + return 0; +} + +static ssize_t gpio_block_exported_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + long value; + int status; + struct gpio_block *block = dev_get_drvdata(dev); + int exported = gpio_block_value_is_exported(block); + + status = kstrtoul(buf, 0, &value); + if (status < 0) + goto err; + + if (value != exported) { + if (value) + status = gpio_block_value_export(block); + else + status = gpio_block_value_unexport(block); + if (!status) + status = size; + } else { + status = size; + } +err: + return status; +} + +static DEVICE_ATTR(exported, 0644, gpio_block_exported_show, + gpio_block_exported_store); + +static const struct attribute *gpio_block_attrs[] = { + &dev_attr_block_ngpio.attr, + &dev_attr_exported.attr, + NULL, +}; + +static const struct attribute_group gpio_block_attr_group = { + .attrs = (struct attribute **) gpio_block_attrs, +}; + +int gpio_block_export(struct gpio_block *block) +{ + int status; + struct device *dev; + + /* can't export until sysfs is available ... */ + if (!gpio_class.p) { + pr_debug("%s: called too early!\n", __func__); + return -ENOENT; + } + + mutex_lock(&sysfs_lock); + dev = device_create(&gpio_class, NULL, MKDEV(0, 0), block, + block->name); + if (!IS_ERR(dev)) + status = sysfs_create_group(&dev->kobj, &gpio_block_attr_group); + else + status = PTR_ERR(dev); + mutex_unlock(&sysfs_lock); + + return status; +} +EXPORT_SYMBOL_GPL(gpio_block_export); + +void gpio_block_unexport(struct gpio_block *block) +{ + struct device *dev; + + mutex_lock(&sysfs_lock); + dev = class_find_device(&gpio_class, NULL, block, match_export); + if (dev) + device_unregister(dev); + mutex_unlock(&sysfs_lock); +} +EXPORT_SYMBOL_GPL(gpio_block_unexport); + static int __init gpiolib_sysfs_init(void) { int status;@@ -1882,7 +2094,14 @@ int gpio_block_register(struct gpio_bloc break; } } - return i == NR_GPIO_BLOCKS ? -EBUSY : 0; + if (i == NR_GPIO_BLOCKS) + goto err; + + gpio_block_export(block); + + return 0; +err: + return -EBUSY; } EXPORT_SYMBOL_GPL(gpio_block_register);@@ -1896,6 +2115,11 @@ void gpio_block_unregister(struct gpio_b break; } } + + if (i == NR_GPIO_BLOCKS) + return; + + gpio_block_unexport(block); } EXPORT_SYMBOL_GPL(gpio_block_unregister); --- linux-2.6.orig/include/asm-generic/gpio.h +++ linux-2.6/include/asm-generic/gpio.h@@ -210,6 +210,8 @@ extern int gpio_export_link(struct devic unsigned gpio); extern int gpio_sysfs_set_active_low(unsigned gpio, int value); extern void gpio_unexport(unsigned gpio); +extern int gpio_block_export(struct gpio_block *block); +extern void gpio_block_unexport(struct gpio_block *block); #endif /* CONFIG_GPIO_SYSFS */@@ -269,6 +271,15 @@ static inline int gpio_sysfs_set_active_ static inline void gpio_unexport(unsigned gpio) { } + +static inline int gpio_block_export(struct gpio_block *block) +{ + return -ENOSYS; +} + +static inline void gpio_block_unexport(struct gpio_block *block) +{ +} #endif /* CONFIG_GPIO_SYSFS */ #endif /* _ASM_GENERIC_GPIO_H */ --- linux-2.6.orig/include/linux/gpio.h +++ linux-2.6/include/linux/gpio.h@@ -278,6 +278,19 @@ static inline void gpio_unexport(unsigne WARN_ON(1); } +static inline int gpio_block_export(struct gpio_block *block) +{ + /* GPIO block can never have been requested or set as {in,out}put */ + WARN_ON(1); + return -EINVAL; +} + +static inline void gpio_block_unexport(struct gpio_block *block) +{ + /* GPIO block can never have been exported */ + WARN_ON(1); +} + static inline int gpio_to_irq(unsigned gpio) { /* GPIO can never have been requested or set as input */ --To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/