From: Roland Stigge <hidden> Date: 2012-10-17 12:37:40
This set of patches adds:
* Block GPIO API to gpiolib
* Sysfs support for GPIO API, to provide userland access
* Devicetree support to instantiate GPIO blocks via DT
* Example implementations in several gpio drivers since they need
special accessor functions for block wise GPIO access
* Fix for race condition in gpiolib on device creation
Signed-off-by: Roland Stigge <redacted>
--
Changes since v4:
* Documented word width
* Bugfix: export/unexport on register/unregister
* Using default dev_attrs for gpio_block_class
* Fix gpiolib: race condition on device creation
* Added driver support for ucb14500, vt8500, xilinx
Changes since v3:
* Added driver support for pca953x, em, pl061, max732x, pcf857x
* Coding style improvements
* Fixed krealloc memory leak in error case
* sysfs: values in hex
* Register blocks in a list
* Narrowing lock scope
* Use S_IWUSR and S_IRUGO instead of direct octal values
* Use for_each_set_bit()
* Change from unsigned to unsigned long for masks and values
Changes since v2:
* Added sysfs support
* Added devicetree support
* Added support for lpc32xx, generic
* Added functions for GPIO block registration
* Added more error checking
* Bit remapping bugfix
Changes since v1:
* API change to 32/64 bit word, bit masks
Thanks to Ryan Mallon, Linus Walleij, Stijn Devriendt, Jean-Christophe
Plagniol-Villard, Mark Brown and Greg Kroah-Hartman for reviewing!
Roland Stigge (15):
gpio: Add a block GPIO API to gpiolib
gpio: Add sysfs support to block GPIO API
gpiolib: Fix default attributes for class
gpio: Add device tree support to block GPIO API
gpio-max730x: Add block GPIO API
gpio-lpc32xx: Add block GPIO API
gpio-generic: Add block GPIO API
gpio-pca953x: Add block GPIO API
gpio-em: Add block GPIO API
gpio-pl061: Add block GPIO API
gpio-max732x: Add block GPIO API
gpio-pcf857x: Add block GPIO API
gpio-xilinx: Add block GPIO API
gpio-vt8500: Add block GPIO API
gpio-ucb1400: Add block GPIO API
Documentation/ABI/testing/sysfs-gpio | 6
Documentation/devicetree/bindings/gpio/gpio-block.txt | 36 +
Documentation/gpio.txt | 47 +
drivers/gpio/Makefile | 1
drivers/gpio/gpio-em.c | 23
drivers/gpio/gpio-generic.c | 56 +
drivers/gpio/gpio-lpc32xx.c | 82 ++
drivers/gpio/gpio-max730x.c | 61 ++
drivers/gpio/gpio-max732x.c | 59 ++
drivers/gpio/gpio-pca953x.c | 64 ++
drivers/gpio/gpio-pcf857x.c | 24
drivers/gpio/gpio-pl061.c | 17
drivers/gpio/gpio-ucb1400.c | 23
drivers/gpio/gpio-vt8500.c | 24
drivers/gpio/gpio-xilinx.c | 44 +
drivers/gpio/gpioblock-of.c | 84 ++
drivers/gpio/gpiolib.c | 510 ++++++++++++++++--
include/asm-generic/gpio.h | 26
include/linux/gpio.h | 87 +++
19 files changed, 1228 insertions(+), 46 deletions(-)
@@ -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
@@ -972,6 +972,214 @@ 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 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, sizeof(unsigned long) == 4 ? "0x%08lx\n" :+ "0x%016lx\n", gpio_block_get(block));+}++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;+ 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;++ status = kstrtoul(buf, 0, &value);+ if (status == 0) {+ mutex_lock(&sysfs_lock);+ 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, S_IWUSR | S_IRUGO, gpio_block_value_show,+ gpio_block_value_store);++static struct class gpio_block_class;++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_block_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;+}++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_block_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)+ gpio_free(block->gpio[i]);++ 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_block_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 struct device_attribute gpio_block_attrs[] = {+ __ATTR(exported, S_IWUSR | S_IRUGO, gpio_block_exported_show,+ gpio_block_exported_store),+ __ATTR(ngpio, S_IRUGO, gpio_block_ngpio_show, NULL),+ __ATTR_NULL,+};++static struct class gpio_block_class = {+ .name = "gpioblock",+ .owner = THIS_MODULE,++ .dev_attrs = gpio_block_attrs,+};++int gpio_block_export(struct gpio_block *block)+{+ int status = 0;+ struct device *dev;++ /* can't export until sysfs is available ... */+ if (!gpio_block_class.p) {+ pr_debug("%s: called too early!\n", __func__);+ return -ENOENT;+ }++ mutex_lock(&sysfs_lock);+ dev = device_create(&gpio_block_class, NULL, MKDEV(0, 0), block,+ block->name);+ if (IS_ERR(dev))+ 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_block_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;
@@ -982,6 +1190,10 @@ static int __init gpiolib_sysfs_init(voi if (status < 0) return status;+ status = class_register(&gpio_block_class);+ if (status < 0)+ return status;+ /* Scan and register the gpio_chips which registered very * early (e.g. before the class_register above was called). *
@@ -291,6 +291,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 */
From: Roland Stigge <hidden> Date: 2012-10-17 12:37:45
This patch adds block GPIO API support to the gpio-generic driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-generic.c | 56 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:37:47
There is a race condition between creating a gpio or gpiochip device and adding
default attribues. This patch fixes this by defining the default attributes as
dev_attrs of the class. For this, it was necessary to create a separate
gpiochip_class besides gpio_class.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpiolib.c | 79 +++++++++++++++++++++----------------------------
1 file changed, 34 insertions(+), 45 deletions(-)
@@ -738,10 +729,7 @@ int gpio_export(unsigned gpio, bool diredev=device_create(&gpio_class,desc->chip->dev,MKDEV(0,0),desc,ioname?ioname:"gpio%u",gpio);if(!IS_ERR(dev)){-status=sysfs_create_group(&dev->kobj,-&gpio_attr_group);--if(!status&&direction_may_change)+if(direction_may_change)status=device_create_file(dev,&dev_attr_direction);
@@ -911,25 +899,22 @@ EXPORT_SYMBOL_GPL(gpio_unexport);staticintgpiochip_export(structgpio_chip*chip){-intstatus;+intstatus=0;structdevice*dev;/* Many systems register gpio chips for SOC support very early,*beforedrivermodelsupportisavailable.Inthosecaseswe*exportthislater,ingpiolib_sysfs_init()...herewejust-*verifythat_some_fieldofgpio_classgotinitialized.+*verifythat_some_fieldofgpiochip_classgotinitialized.*/-if(!gpio_class.p)+if(!gpiochip_class.p)return0;/* use chip->base for the ID; it's already known to be unique */mutex_lock(&sysfs_lock);-dev=device_create(&gpio_class,chip->dev,MKDEV(0,0),chip,-"gpiochip%d",chip->base);-if(!IS_ERR(dev)){-status=sysfs_create_group(&dev->kobj,-&gpiochip_attr_group);-}else+dev=device_create(&gpiochip_class,chip->dev,MKDEV(0,0),chip,+"gpiochip%d",chip->base);+if(IS_ERR(dev))status=PTR_ERR(dev);chip->exported=(status==0);mutex_unlock(&sysfs_lock);
From: Roland Stigge <hidden> Date: 2012-10-17 12:37:49
This patch adds block GPIO API support to the gpio-pl061 driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-pl061.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:37:52
This patch adds block GPIO API support to the gpio-xilinx driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-xilinx.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:37:58
This patch adds block GPIO API support to the gpio-vt8500 driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-vt8500.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:38:31
This patch adds block GPIO API support to the gpio-ucb1400 driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-ucb1400.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:38:48
This patch adds block GPIO API support to the gpio-pcf857x driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-pcf857x.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:39:09
This patch adds block GPIO API support to the gpio-max732x driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-max732x.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:39:36
This patch adds block GPIO API support to the gpio-em driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-em.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:40:08
This patch adds block GPIO API support to the LPC32xx driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-lpc32xx.c | 82 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
@@ -297,6 +297,22 @@ static int lpc32xx_gpio_get_value_p3(strreturn__get_gpio_state_p3(group,pin);}+staticunsignedlonglpc32xx_gpio_get_block_p3(structgpio_chip*chip,+unsignedlongmask)+{+structlpc32xx_gpio_chip*group=to_lpc32xx_gpio(chip);+u32bits=__raw_readl(group->gpio_grp->inp_state);++/* In P3_INP_STATE, the 6 GPIOs are scattered over the register,+*rearrangingtobits0-5+*/+bits>>=10;+bits&=0x401F;+bits|=(bits&0x4000)>>9;++returnbits&mask&0x3F;+}+staticintlpc32xx_gpi_get_value(structgpio_chip*chip,unsignedpin){structlpc32xx_gpio_chip*group=to_lpc32xx_gpio(chip);
@@ -304,6 +320,15 @@ static int lpc32xx_gpi_get_value(structreturn__get_gpi_state_p3(group,pin);}+staticunsignedlonglpc32xx_gpi_get_block(structgpio_chip*chip,+unsignedlongmask)+{+structlpc32xx_gpio_chip*group=to_lpc32xx_gpio(chip);+u32bits=__raw_readl(group->gpio_grp->inp_state);++returnbits&mask;+}+staticintlpc32xx_gpio_dir_output_p012(structgpio_chip*chip,unsignedpin,intvalue){
@@ -351,6 +376,27 @@ static void lpc32xx_gpio_set_value_p3(st__set_gpio_level_p3(group,pin,value);}+staticvoidlpc32xx_gpio_set_block_p3(structgpio_chip*chip,+unsignedlongmask,+unsignedlongvalues)+{+structlpc32xx_gpio_chip*group=to_lpc32xx_gpio(chip);+u32set_bits=values&mask;+u32clr_bits=~values&mask;++/* States of GPIO 0-5 start at bit 25 */+set_bits<<=25;+clr_bits<<=25;++/* Note: On LPC32xx, GPOs can only be set at once or cleared at once,+*butnotsetandcleared@once+*/+if(set_bits)+__raw_writel(set_bits,group->gpio_grp->outp_set);+if(clr_bits)+__raw_writel(clr_bits,group->gpio_grp->outp_clr);+}+staticvoidlpc32xx_gpo_set_value(structgpio_chip*chip,unsignedpin,intvalue){
@@ -366,6 +412,31 @@ static int lpc32xx_gpo_get_value(structreturn__get_gpo_state_p3(group,pin);}+staticvoidlpc32xx_gpo_set_block(structgpio_chip*chip,unsignedlongmask,+unsignedlongvalues)+{+structlpc32xx_gpio_chip*group=to_lpc32xx_gpio(chip);+u32set_bits=values&mask;+u32clr_bits=~values&mask;++/* Note: On LPC32xx, GPOs can only be set at once or cleared at once,+*butnotsetandclearedatonce+*/+if(set_bits)+__raw_writel(set_bits,group->gpio_grp->outp_set);+if(clr_bits)+__raw_writel(clr_bits,group->gpio_grp->outp_clr);+}++staticunsignedlonglpc32xx_gpo_get_block(structgpio_chip*chip,+unsignedlongmask)+{+structlpc32xx_gpio_chip*group=to_lpc32xx_gpio(chip);+u32bits=__raw_readl(group->gpio_grp->outp_state);++returnbits&mask;+}+staticintlpc32xx_gpio_request(structgpio_chip*chip,unsignedpin){if(pin<chip->ngpio)
From: Roland Stigge <hidden> Date: 2012-10-17 12:40:49
This patch adds block GPIO API support to the gpio-pca953x driver.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-pca953x.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
From: Roland Stigge <hidden> Date: 2012-10-17 12:41:04
The recurring task of providing simultaneous access to GPIO lines (especially
for bit banging protocols) needs an appropriate API.
This patch adds a kernel internal "Block GPIO" API that enables simultaneous
access to several GPIOs. This is done by abstracting GPIOs to an n-bit word:
Once requested, it provides access to a group of GPIOs which can range over
multiple GPIO chips.
Signed-off-by: Roland Stigge <redacted>
---
Documentation/gpio.txt | 47 +++++++++
drivers/gpio/gpiolib.c | 217 +++++++++++++++++++++++++++++++++++++++++++++
include/asm-generic/gpio.h | 15 +++
include/linux/gpio.h | 74 +++++++++++++++
4 files changed, 353 insertions(+)
@@ -439,6 +439,53 @@ slower clock delays the rising edge of S signaling rate accordingly.+Block GPIO+----------++The above described interface concentrates on handling single GPIOs. However,+in applications where it is critical to set several GPIOs at once, this+interface doesn't work well, e.g. bit-banging protocols via grouped GPIO lines.+Consider a GPIO controller that is connected via a slow I2C line. When+switching two or more GPIOs one after another, there can be considerable time+between those events. This is solved by an interface called Block GPIO:++struct gpio_block *gpio_block_create(unsigned int *gpios, size_t size);++This creates a new block of GPIOs as a list of GPIO numbers with the specified+size which are accessible via the returned struct gpio_block and the accessor+functions described below. Please note that you need to request the GPIOs+separately via gpio_request(). An arbitrary list of globally valid GPIOs can be+specified, even ranging over several gpio_chips. Actual handling of I/O+operations will be done on a best effort base, i.e. simultaneous I/O only where+possible by hardware and implemented in the respective GPIO driver. The number+of GPIOs in one block is limited to the number of bits in an unsigned long, or+BITS_PER_LONG, of the respective platform, i.e. typically at least 32 on a 32+bit system, and at least 64 on a 64 bit system. However, several blocks can be+defined at once.++unsigned gpio_block_get(struct gpio_block *block);+void gpio_block_set(struct gpio_block *block, unsigned value);++With those accessor functions, setting and getting the GPIO values is possible,+analogous to gpio_get_value() and gpio_set_value(). Each bit in the return+value of gpio_block_get() and in the value argument of gpio_block_set()+corresponds to a bit specified on gpio_block_create(). Block operations in+hardware are only possible where the respective GPIO driver implements it,+falling back to using single GPIO operations where the driver only implements+single GPIO access.++void gpio_block_free(struct gpio_block *block);++After the GPIO block isn't used anymore, it should be free'd via+gpio_block_free().++int gpio_block_register(struct gpio_block *block);+void gpio_block_unregister(struct gpio_block *block);++These functions can be used to register a GPIO block. Blocks registered this+way will be available via sysfs.++ What do these conventions omit? =============================== One of the biggest things these conventions omit is pin multiplexing, since--- linux-2.6.orig/drivers/gpio/gpiolib.c+++ linux-2.6/drivers/gpio/gpiolib.c
@@ -83,6 +83,8 @@ static inline void desc_set_label(struct #endif }+static LIST_HEAD(gpio_block_list);+ /* Warn when drivers omit gpio_request() calls -- legal but ill-advised * when setting direction, and otherwise illegal. Until board setup code * and drivers use explicit requests everywhere (which won't happen when
@@ -1676,6 +1678,221 @@ void __gpio_set_value(unsigned gpio, int } EXPORT_SYMBOL_GPL(__gpio_set_value);+static int gpio_block_chip_index(struct gpio_block *block, struct gpio_chip *gc)+{+ int i;++ for (i = 0; i < block->nchip; i++) {+ if (block->gbc[i].gc == gc)+ return i;+ }+ return -1;+}++struct gpio_block *gpio_block_create(unsigned *gpios, size_t size,+ const char *name)+{+ struct gpio_block *block;+ struct gpio_block_chip *gbc;+ struct gpio_remap *remap;+ void *tmp;+ int i;++ if (size < 1 || size > sizeof(unsigned long) * 8)+ return ERR_PTR(-EINVAL);++ for (i = 0; i < size; i++)+ if (!gpio_is_valid(gpios[i]))+ return ERR_PTR(-EINVAL);++ block = kzalloc(sizeof(struct gpio_block), GFP_KERNEL);+ if (!block)+ return ERR_PTR(-ENOMEM);++ block->name = name;+ block->ngpio = size;+ block->gpio = kzalloc(sizeof(*block->gpio) * size, GFP_KERNEL);+ if (!block->gpio)+ goto err1;++ memcpy(block->gpio, gpios, sizeof(*block->gpio) * size);++ for (i = 0; i < size; i++) {+ struct gpio_chip *gc = gpio_to_chip(gpios[i]);+ int bit = gpios[i] - gc->base;+ int index = gpio_block_chip_index(block, gc);++ if (index < 0) {+ block->nchip++;+ tmp = krealloc(block->gbc,+ sizeof(struct gpio_block_chip) *+ block->nchip, GFP_KERNEL);+ if (!tmp) {+ kfree(block->gbc);+ goto err2;+ }+ block->gbc = tmp;+ gbc = &block->gbc[block->nchip - 1];+ gbc->gc = gc;+ gbc->remap = NULL;+ gbc->nremap = 0;+ gbc->mask = 0;+ } else {+ gbc = &block->gbc[index];+ }+ /* represents the mask necessary on calls to the driver's+ * .get_block() and .set_block()+ */+ gbc->mask |= BIT(bit);++ /* collect gpios that are specified together, represented by+ * neighboring bits+ *+ * Note that even though in setting remap is given a negative+ * index, the next lines guard that the potential out-of-bounds+ * pointer is not dereferenced when out of bounds.+ */+ remap = &gbc->remap[gbc->nremap - 1];+ if (!gbc->nremap || (bit - i != remap->offset)) {+ gbc->nremap++;+ tmp = krealloc(gbc->remap,+ sizeof(struct gpio_remap) *+ gbc->nremap, GFP_KERNEL);+ if (!tmp) {+ kfree(gbc->remap);+ goto err3;+ }+ gbc->remap = tmp;+ remap = &gbc->remap[gbc->nremap - 1];+ remap->offset = bit - i;+ remap->mask = 0;+ }++ /* represents the mask necessary for bit reordering between+ * gpio_block (i.e. as specified on gpio_block_get() and+ * gpio_block_set()) and gpio_chip domain (i.e. as specified on+ * the driver's .set_block() and .get_block())+ */+ remap->mask |= BIT(i);+ }++ return block;+err3:+ for (i = 0; i < block->nchip - 1; i++)+ kfree(block->gbc[i].remap);+ kfree(block->gbc);+err2:+ kfree(block->gpio);+err1:+ kfree(block);+ return ERR_PTR(-ENOMEM);+}+EXPORT_SYMBOL_GPL(gpio_block_create);++void gpio_block_free(struct gpio_block *block)+{+ int i;++ for (i = 0; i < block->nchip; i++)+ kfree(block->gbc[i].remap);+ kfree(block->gpio);+ kfree(block->gbc);+ kfree(block);+}+EXPORT_SYMBOL_GPL(gpio_block_free);++unsigned long gpio_block_get(const struct gpio_block *block)+{+ struct gpio_block_chip *gbc;+ int i, j;+ unsigned long values = 0;++ for (i = 0; i < block->nchip; i++) {+ unsigned long remapped = 0;++ gbc = &block->gbc[i];++ if (gbc->gc->get_block) {+ remapped = gbc->gc->get_block(gbc->gc, gbc->mask);+ } else {+ /* emulate */+ for_each_set_bit(j, &gbc->mask, BITS_PER_LONG)+ remapped |= gbc->gc->get(gbc->gc,+ gbc->gc->base + j) << j;+ }++ for (j = 0; j < gbc->nremap; j++) {+ struct gpio_remap *gr = &gbc->remap[j];++ values |= (remapped >> gr->offset) & gr->mask;+ }+ }++ return values;+}+EXPORT_SYMBOL_GPL(gpio_block_get);++void gpio_block_set(struct gpio_block *block, unsigned long values)+{+ struct gpio_block_chip *gbc;+ int i, j;++ for (i = 0; i < block->nchip; i++) {+ unsigned long remapped = 0;++ gbc = &block->gbc[i];++ for (j = 0; j < gbc->nremap; j++) {+ struct gpio_remap *gr = &gbc->remap[j];++ remapped |= (values & gr->mask) << gr->offset;+ }+ if (gbc->gc->set_block) {+ gbc->gc->set_block(gbc->gc, gbc->mask, remapped);+ } else {+ /* emulate */+ for_each_set_bit(j, &gbc->mask, BITS_PER_LONG)+ gbc->gc->set(gbc->gc, gbc->gc->base + j,+ (remapped >> j) & 1);+ }+ }+}+EXPORT_SYMBOL_GPL(gpio_block_set);++struct gpio_block *gpio_block_find_by_name(const char *name)+{+ struct gpio_block *i;++ list_for_each_entry(i, &gpio_block_list, list)+ if (!strcmp(i->name, name))+ return i;+ return NULL;+}+EXPORT_SYMBOL_GPL(gpio_block_find_by_name);++int gpio_block_register(struct gpio_block *block)+{+ if (gpio_block_find_by_name(block->name))+ return -EBUSY;++ list_add(&block->list, &gpio_block_list);++ return 0;+}+EXPORT_SYMBOL_GPL(gpio_block_register);++void gpio_block_unregister(struct gpio_block *block)+{+ struct gpio_block *i;++ list_for_each_entry(i, &gpio_block_list, list)+ if (i == block) {+ list_del(&i->list);+ break;+ }+}+EXPORT_SYMBOL_GPL(gpio_block_unregister);+ /** * __gpio_cansleep() - report whether gpio value access will sleep * @gpio: gpio in question--- linux-2.6.orig/include/asm-generic/gpio.h+++ linux-2.6/include/asm-generic/gpio.h
@@ -39,6 +41,43 @@ struct gpio { const char *label; };+/*+ * struct gpio_remap - a structure for describing a bit mapping+ * @mask: a bit mask+ * @offset: how many bits to shift to the left (negative: to the right)+ *+ * When we are mapping bit values from one word to another (here: from GPIO+ * block domain to GPIO driver domain) we first mask them out with mask and+ * shift them as specified with offset. More complicated mappings are done by+ * grouping several of those structs and adding the results together.+ */+struct gpio_remap {+ unsigned long mask;+ int offset;+};++struct gpio_block_chip {+ struct gpio_chip *gc;+ struct gpio_remap *remap;+ int nremap;+ unsigned long mask;+};++/**+ * struct gpio_block - a structure describing a list of GPIOs for simultaneous+ * operations+ */+struct gpio_block {+ struct gpio_block_chip *gbc;+ size_t nchip;+ const char *name;++ int ngpio;+ unsigned *gpio;++ struct list_head list;+};+ #ifdef CONFIG_GENERIC_GPIO #ifdef CONFIG_ARCH_HAVE_CUSTOM_GPIO_H
@@ -169,6 +208,41 @@ static inline void gpio_set_value(unsign WARN_ON(1); }+static inline+struct gpio_block *gpio_block_create(unsigned *gpios, size_t size,+ const char *name)+{+ WARN_ON(1);+ return NULL;+}++static inline void gpio_block_free(struct gpio_block *block)+{+ WARN_ON(1);+}++static inline unsigned long gpio_block_get(const struct gpio_block *block)+{+ WARN_ON(1);+ return 0;+}++static inline void gpio_block_set(struct gpio_block *block, unsigned long value)+{+ WARN_ON(1);+}++static inline int gpio_block_register(struct gpio_block *block)+{+ WARN_ON(1);+ return 0;+}++static inline void gpio_block_unregister(struct gpio_block *block)+{+ WARN_ON(1);+}+ static inline int gpio_cansleep(unsigned gpio) { /* GPIO can never have been requested or set as {in,out}put */
From: Roland Stigge <hidden> Date: 2012-10-17 12:41:31
This patch adds block GPIO API support to the MAX730x driver.
Due to hardware constraints in this chip, simultaneous access to GPIO lines can
only be done in groups of 8: GPIOs 0-7, 8-15, 16-23, 24-27. However, setting
and clearing will be done at once.
Signed-off-by: Roland Stigge <redacted>
---
drivers/gpio/gpio-max730x.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
@@ -146,6 +146,44 @@ static int max7301_get(struct gpio_chipreturnlevel;}+staticunsignedlongmax7301_get_block(structgpio_chip*chip,+unsignedlongmask)+{+structmax7301*ts=container_of(chip,structmax7301,chip);+inti,j;+unsignedlongresult=0;++for(i=0;i<4;i++){+if((mask>>(i*8))&0xFF){/* i/o only if necessary */+u8in_level=ts->read(ts->dev,0x44+i*8);+u8in_mask=0;+u8out_level=(ts->out_level>>(i*8+4))&0xFF;+u8out_mask=0;++for(j=0;j<8;j++){+intoffset=4+i*8+j;+intconfig=(ts->port_config[offset>>2]>>+((offset&3)<<1))&+PIN_CONFIG_MASK;++switch(config){+casePIN_CONFIG_OUT:+out_mask|=BIT(j);+break;+casePIN_CONFIG_IN_WO_PULLUP:+casePIN_CONFIG_IN_PULLUP:+in_mask|=BIT(j);+}+}++result|=((unsignedlong)(in_level&in_mask)|+(out_level&out_mask))<<(i*8);+}+}++returnresult&mask;+}+staticvoidmax7301_set(structgpio_chip*chip,unsignedoffset,intvalue){structmax7301*ts=container_of(chip,structmax7301,chip);
@@ -160,6 +198,27 @@ static void max7301_set(struct gpio_chipmutex_unlock(&ts->lock);}+staticvoidmax7301_set_block(structgpio_chip*chip,unsignedlongmask,+unsignedlongvalues)+{+structmax7301*ts=container_of(chip,structmax7301,chip);+unsignedlongchanges;+inti;++mutex_lock(&ts->lock);++changes=(ts->out_level^(values<<4))&(mask<<4);+ts->out_level^=changes;++for(i=0;i<4;i++){+if((changes>>(i*8+4))&0xFF)/* i/o only on change */+ts->write(ts->dev,0x44+i*8,+(ts->out_level>>(i*8+4))&0xFF);+}++mutex_unlock(&ts->lock);+}+int__devinit__max730x_probe(structmax7301*ts){structdevice*dev=ts->dev;
@@ -183,8 +242,10 @@ int __devinit __max730x_probe(struct maxts->chip.direction_input=max7301_direction_input;ts->chip.get=max7301_get;+ts->chip.get_block=max7301_get_block;ts->chip.direction_output=max7301_direction_output;ts->chip.set=max7301_set;+ts->chip.set_block=max7301_set_block;ts->chip.base=pdata->base;ts->chip.ngpio=PIN_NUMBER;
@@ -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)
I think you need some more documentation here, as I just noticed
something "odd":
+static int gpio_block_value_unexport(struct gpio_block *block)
+{
+ struct device *dev;
+ int i;
+
+ dev = class_find_device(&gpio_block_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;
+}
Wait, what? You are removing a sysfs file in this function, from within
a sysfs write:
+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);
That looks like a recipie for disaster. Why do you allow userspace to
do this?
Anyway, the other fixups for how you create/destroy the attribute files
looks great, thanks for making those changes.
greg k-h
From: Roland Stigge <hidden> Date: 2012-10-18 10:07:47
On 10/17/2012 09:05 PM, Greg KH wrote:
quoted
+static int gpio_block_value_unexport(struct gpio_block *block)
+{
+ struct device *dev;
+ int i;
+
+ dev = class_find_device(&gpio_block_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;
+}
Wait, what? You are removing a sysfs file in this function, from within
a sysfs write:
Yes, exactly:
quoted
+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);
That looks like a recipie for disaster. Why do you allow userspace to
do this?
Exporting for gpio blocks is done as follows: writing "1" to the
"exported" _device_ attribute of the gpio block creates the "values"
attribute and at the same time requests the whole block (including all
of its gpios) as "sysfs".
This admittedly deviates from the exporting of gpios (with the "export"
and "unexport" _class_ attributes) because blocks are not numbered. In
contrast, they are registered in a list (as was requested). Now, I
already had the idea of also having an "export" and "unexport" _class_
attribute for blocks also, but from a userspace perspective you only see
the presence and name of the gpio block if it is already being in sysfs
(even if not exported yet). If it wouldn't be this way, a user couldn't
guess how the required gpio block is called (or numbered), wouldn't even
know about its presence.
Just for understanding your strong desire for the device attribute
("value") being always present (in contrast to being created and removed
dynamically) - can you please give me any hint how the dynamic approach
would lead to disaster?
One possibility would be to always have "value" as a default device
attribute, but then it wouldn't be so obvious that it's useless until
"exported" is "1".
What do you think?
For now, I would elaborate about my terse "only available if /exported
is 1" like this:
"
Block GPIO devices are visible in sysfs as soon as they are registered
(e.g. via devicetree definition). For actual I/O use, their "exported"
boolean attribute must be set to "1". Then, the attribute "values" is
created and at the same time, the GPIOs in the block are requested for
exclusive use by sysfs.
"
Thanks in advance,
Roland
On Thu, Oct 18, 2012 at 12:07 PM, Roland Stigge [off-list ref] wrote:
On 10/17/2012 09:05 PM, Greg KH wrote:
quoted
quoted
+ if (value != exported) {
+ if (value)
+ status = gpio_block_value_export(block);
+ else
+ status = gpio_block_value_unexport(block);
That looks like a recipie for disaster. Why do you allow userspace to
do this?
Exporting for gpio blocks is done as follows: writing "1" to the
"exported" _device_ attribute of the gpio block creates the "values"
attribute and at the same time requests the whole block (including all
of its gpios) as "sysfs".
To me it reads like Greg's comment is basically pinpointing a flaw
in Brownell's initial design of gpio sysfs: that new sysfs files are
created and destroyed by writing into sysfs */export files from
userspace?
See commit: d8f388d8dc8d4f36539dd37c1fff62cc404ea0fc
The block GPIO stuff is just following that design pattern.
Yours,
Linus Walleij
From: Roland Stigge <hidden> Date: 2012-10-19 11:51:17
On 10/19/2012 12:35 PM, Linus Walleij wrote:
On Thu, Oct 18, 2012 at 12:07 PM, Roland Stigge [off-list ref] wrote:
quoted
On 10/17/2012 09:05 PM, Greg KH wrote:
quoted
quoted
+ if (value != exported) {
+ if (value)
+ status = gpio_block_value_export(block);
+ else
+ status = gpio_block_value_unexport(block);
That looks like a recipie for disaster. Why do you allow userspace to
do this?
Exporting for gpio blocks is done as follows: writing "1" to the
"exported" _device_ attribute of the gpio block creates the "values"
attribute and at the same time requests the whole block (including all
of its gpios) as "sysfs".
To me it reads like Greg's comment is basically pinpointing a flaw
in Brownell's initial design of gpio sysfs: that new sysfs files are
created and destroyed by writing into sysfs */export files from
userspace?
See commit: d8f388d8dc8d4f36539dd37c1fff62cc404ea0fc
The block GPIO stuff is just following that design pattern.
So what do you think about my just proposed idea of always having the
"value" argument present, but only useable when "exported" is "1"? Now
only talking about the block gpios, but later maybe also for gpios?
But I would only do this if you and Greg consider it reasonable.
Thanks in advance,
Roland
On Thu, Oct 18, 2012 at 12:07:39PM +0200, Roland Stigge wrote:
On 10/17/2012 09:05 PM, Greg KH wrote:
quoted
quoted
+static int gpio_block_value_unexport(struct gpio_block *block)
+{
+ struct device *dev;
+ int i;
+
+ dev = class_find_device(&gpio_block_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;
+}
Wait, what? You are removing a sysfs file in this function, from within
a sysfs write:
Yes, exactly:
quoted
quoted
+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);
That looks like a recipie for disaster. Why do you allow userspace to
do this?
Exporting for gpio blocks is done as follows: writing "1" to the
"exported" _device_ attribute of the gpio block creates the "values"
attribute and at the same time requests the whole block (including all
of its gpios) as "sysfs".
This admittedly deviates from the exporting of gpios (with the "export"
and "unexport" _class_ attributes) because blocks are not numbered. In
contrast, they are registered in a list (as was requested). Now, I
already had the idea of also having an "export" and "unexport" _class_
attribute for blocks also, but from a userspace perspective you only see
the presence and name of the gpio block if it is already being in sysfs
(even if not exported yet). If it wouldn't be this way, a user couldn't
guess how the required gpio block is called (or numbered), wouldn't even
know about its presence.
Just for understanding your strong desire for the device attribute
("value") being always present (in contrast to being created and removed
dynamically) - can you please give me any hint how the dynamic approach
would lead to disaster?
One possibility would be to always have "value" as a default device
attribute, but then it wouldn't be so obvious that it's useless until
"exported" is "1".
What do you think?
For now, I would elaborate about my terse "only available if /exported
is 1" like this:
"
Block GPIO devices are visible in sysfs as soon as they are registered
(e.g. via devicetree definition). For actual I/O use, their "exported"
boolean attribute must be set to "1". Then, the attribute "values" is
created and at the same time, the GPIOs in the block are requested for
exclusive use by sysfs.
"
Yes, that explanation makes it more obvious as to what is going on, it
caught me by supprise.
We used to have problems with attributes removing/adding devices or
attributes from their callbacks, but that has been fixed up now, and the
fact that the other gpio code works this way is good enough precedence
to keep me from objecting to it.
thanks,
greg k-h
On Fri, Oct 19, 2012 at 8:02 PM, Greg KH [off-list ref] wrote:
We used to have problems with attributes removing/adding devices or
attributes from their callbacks, but that has been fixed up now, and the
fact that the other gpio code works this way is good enough precedence
to keep me from objecting to it.
OK thanks I get this now too...
Yours,
Linus Walleij
On Fri, Oct 19, 2012 at 1:51 PM, Roland Stigge [off-list ref] wrote:
[Me]
quoted
The block GPIO stuff is just following that design pattern.
So what do you think about my just proposed idea of always having the
"value" argument present, but only useable when "exported" is "1"? Now
only talking about the block gpios, but later maybe also for gpios?
We should atleast be consistent and do it the same way for
blocks as individual gpios.
But I would only do this if you and Greg consider it reasonable.
I'm still afraid of this whole thing and would seek Grant's consent.
Yours,
Linus Walleij
From: Roland Stigge <hidden> Date: 2012-10-22 09:05:56
On 10/22/2012 10:55 AM, Linus Walleij wrote:
On Fri, Oct 19, 2012 at 1:51 PM, Roland Stigge [off-list ref] wrote:
quoted
So what do you think about my just proposed idea of always having the
"value" argument present, but only useable when "exported" is "1"? Now
only talking about the block gpios, but later maybe also for gpios?
We should atleast be consistent and do it the same way for
blocks as individual gpios.
OK, I'll leave it as creating-attribute-on-export.
quoted
But I would only do this if you and Greg consider it reasonable.
I'm still afraid of this whole thing and would seek Grant's consent.