* [PATCH RESEND 0/6 v10] gpio: Add block GPIO
@ 2012-12-14 14:26 Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 1/6 v10] gpio: Add a block GPIO API to gpiolib Roland Stigge
` (6 more replies)
0 siblings, 7 replies; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 14:26 UTC (permalink / raw)
To: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Cc: Roland Stigge
This set of patches adds:
* Block GPIO API to gpiolib
* Sysfs support for GPIO API, to provide userland access
* Device interface for userland access (alternative to sysfs)
* 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 <stigge@antcom.de>
Tested by: Wolfgang Grandegger <wg@grandegger.com>
--
Changed since v9:
* Changed IRQ flags for device interface's poll() from IRQF_TRIGGER_FALLING
to IRQF_SHARED
* Use block name on gpio_request
* Added spinlock for ISR manipulated data (dev interface / poll())
Changed since v8:
* Added poll() function to dev interface for IRQ handling
* Fixed gpio_block_register() in case of missing CONFIG_GPIO_SYSFS
* Fixed mask handling in sysfs interface
* Fixed get_user/put_user pointer (cast)
Changes since v7:
* Fixed error in API documentation (name arg missing in prototype)
* Fixed compile error if sysfs is missing: Moved gpio_block_is_output()
out of sysfs-ifdef
* Added error handling to gpio_block_register()
* Added mask attribute to sysfs (although dev interface exists now)
Changes since v6:
* Changed realloc to list handling
* Added mask to kernel API
* Added device interface as userspace API
* Changed sizeof() to BITS_PER_LONG
* Fixed multiline comment style
* Documented direction setting by the user
* Documented order of handled chips in case of multiple chips in one block
Changes since v5:
* Documented sysfs: elaborated on "exported" and "values" attributes
* Documented sysfs: gpiochip is a separate class now
* Aggregated driver support patches for block gpio into one single patch
* Added gpio block driver support for twl6040 and pch
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, Greg Kroah-Hartman, Grant Likely, Stefan
Roese and Wolfgang Grandegger for reviewing!
Roland Stigge (6):
gpio: Add a block GPIO API to gpiolib
gpio: Add sysfs support to block GPIO API
gpio: Add userland device interface to block GPIO
gpiolib: Fix default attributes for class
gpio: Add device tree support to block GPIO API
gpio: Add block gpio to several gpio drivers
Documentation/ABI/testing/dev-gpioblock | 34
Documentation/ABI/testing/sysfs-gpio | 31
Documentation/devicetree/bindings/gpio/gpio-block.txt | 36
Documentation/gpio.txt | 58 +
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-pch.c | 27
drivers/gpio/gpio-pl061.c | 17
drivers/gpio/gpio-twl6040.c | 32
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 | 780 ++++++++++++++++--
include/asm-generic/gpio.h | 28
include/linux/gpio.h | 125 ++
22 files changed, 1666 insertions(+), 47 deletions(-)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RESEND 1/6 v10] gpio: Add a block GPIO API to gpiolib
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
@ 2012-12-14 14:26 ` Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 2/6 v10] gpio: Add sysfs support to block GPIO API Roland Stigge
` (5 subsequent siblings)
6 siblings, 0 replies; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 14:26 UTC (permalink / raw)
To: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Cc: Roland Stigge
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 <stigge@antcom.de>
---
Documentation/gpio.txt | 58 +++++++++++
drivers/gpio/gpiolib.c | 227 +++++++++++++++++++++++++++++++++++++++++++++
include/asm-generic/gpio.h | 17 +++
include/linux/gpio.h | 97 +++++++++++++++++++
4 files changed, 399 insertions(+)
--- linux-2.6.orig/Documentation/gpio.txt
+++ linux-2.6/Documentation/gpio.txt
@@ -439,6 +439,64 @@ 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,
+ const char *name);
+
+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(). Similarly, the direction of the used GPIOs need
+to be set by the user before using the block. 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 long gpio_block_get(struct gpio_block *block, unsigned long mask);
+void gpio_block_set(struct gpio_block *block, unsigned long mask,
+ unsigned long values);
+
+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(). The mask parameters
+specify which bits in the block are acted upon. This is useful to let some bits
+untouched when doing a set operation on the block.
+
+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.
+
+If a GPIO block includes GPIOs from several chips, the chips are handled one
+after another in the order of first specification in the list of GPIOs defined
+in the GPIO block, starting with bit 0. Note that in this case, you typically
+can't assume simultaneous 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 userspace API.
+
+
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
@@ -191,6 +193,16 @@ err:
return ret;
}
+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;
+}
+
#ifdef CONFIG_GPIO_SYSFS
/* lock protects against unexport_gpio() being called while
@@ -1680,6 +1692,221 @@ void __gpio_set_value(unsigned gpio, int
}
EXPORT_SYMBOL_GPL(__gpio_set_value);
+static struct gpio_block_chip *gpio_block_chip_get(struct gpio_block *block,
+ struct gpio_chip *gc)
+{
+ struct gpio_block_chip *i;
+
+ list_for_each_entry(i, &block->gbc_list, list)
+ if (i->gc == gc)
+ return i;
+ return NULL;
+}
+
+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;
+ int i;
+
+ if (size < 1 || size > BITS_PER_LONG)
+ 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;
+ INIT_LIST_HEAD(&block->gbc_list);
+ block->gpio = kzalloc(sizeof(*block->gpio) * size, GFP_KERNEL);
+ if (!block->gpio)
+ goto err;
+
+ 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;
+ gbc = gpio_block_chip_get(block, gc);
+
+ if (!gbc) {
+ gbc = kzalloc(sizeof(struct gpio_block_chip),
+ GFP_KERNEL);
+ if (!gbc)
+ goto err;
+ list_add_tail(&gbc->list, &block->gbc_list);
+ gbc->gc = gc;
+ INIT_LIST_HEAD(&gbc->remap_list);
+ }
+
+ /*
+ * 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
+ *
+ * get last element and create new element if list empty or
+ * new element necessary
+ */
+ remap = list_entry(gbc->remap_list.prev, struct gpio_remap,
+ list);
+ if (list_empty(&gbc->remap_list) ||
+ (bit - i != remap->offset)) {
+ remap = kzalloc(sizeof(struct gpio_remap), GFP_KERNEL);
+ if (!remap)
+ goto err;
+ list_add_tail(&remap->list, &gbc->remap_list);
+ remap->offset = bit - i;
+ }
+
+ /*
+ * 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;
+err:
+ gpio_block_free(block);
+ return ERR_PTR(-ENOMEM);
+}
+EXPORT_SYMBOL_GPL(gpio_block_create);
+
+void gpio_block_free(struct gpio_block *block)
+{
+ struct list_head *i, *itmp, *j, *jtmp;
+ struct gpio_block_chip *gbc;
+ struct gpio_remap *remap;
+
+ list_for_each_safe(i, itmp, &block->gbc_list) {
+ gbc = list_entry(i, struct gpio_block_chip, list);
+ list_del(i);
+ list_for_each_safe(j, jtmp, &gbc->remap_list) {
+ remap = list_entry(j, struct gpio_remap, list);
+ list_del(j);
+ kfree(remap);
+ }
+ kfree(gbc);
+ }
+
+ kfree(block->gpio);
+ kfree(block);
+}
+EXPORT_SYMBOL_GPL(gpio_block_free);
+
+unsigned long gpio_block_get(const struct gpio_block *block, unsigned long mask)
+{
+ struct gpio_block_chip *gbc;
+ unsigned long values = 0;
+
+ list_for_each_entry(gbc, &block->gbc_list, list) {
+ struct gpio_remap *remap;
+ int i;
+ unsigned long remapped = 0;
+ unsigned long remapped_mask = 0;
+
+ list_for_each_entry(remap, &gbc->remap_list, list)
+ remapped_mask |= (mask & remap->mask) << remap->offset;
+ remapped_mask &= gbc->mask;
+
+ if (!remapped_mask)
+ continue;
+
+ if (gbc->gc->get_block)
+ remapped = gbc->gc->get_block(gbc->gc, remapped_mask);
+ else
+ /* emulate */
+ for_each_set_bit(i, &remapped_mask, BITS_PER_LONG)
+ remapped |= gbc->gc->get(gbc->gc,
+ gbc->gc->base + i) << i;
+
+ list_for_each_entry(remap, &gbc->remap_list, list)
+ values |= (remapped >> remap->offset) & remap->mask;
+ }
+
+ return values;
+}
+EXPORT_SYMBOL_GPL(gpio_block_get);
+
+void gpio_block_set(struct gpio_block *block, unsigned long mask,
+ unsigned long values)
+{
+ struct gpio_block_chip *gbc;
+
+ list_for_each_entry(gbc, &block->gbc_list, list) {
+ struct gpio_remap *remap;
+ int i;
+ unsigned long remapped = 0;
+ unsigned long remapped_mask = 0;
+
+ list_for_each_entry(remap, &gbc->remap_list, list) {
+ remapped |= (values & remap->mask) << remap->offset;
+ remapped_mask |= (mask & remap->mask) << remap->offset;
+ }
+ remapped_mask &= gbc->mask;
+
+ if (!remapped_mask)
+ continue;
+
+ if (gbc->gc->set_block)
+ gbc->gc->set_block(gbc->gc, remapped_mask, remapped);
+ else
+ /* emulate */
+ for_each_set_bit(i, &remapped_mask, BITS_PER_LONG)
+ gbc->gc->set(gbc->gc, gbc->gc->base + i,
+ (remapped >> i) & 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
@@ -43,6 +43,7 @@ static inline bool gpio_is_valid(int num
struct device;
struct gpio;
+struct gpio_block;
struct seq_file;
struct module;
struct device_node;
@@ -105,6 +106,8 @@ struct gpio_chip {
unsigned offset);
int (*get)(struct gpio_chip *chip,
unsigned offset);
+ unsigned long (*get_block)(struct gpio_chip *chip,
+ unsigned long mask);
int (*direction_output)(struct gpio_chip *chip,
unsigned offset, int value);
int (*set_debounce)(struct gpio_chip *chip,
@@ -112,6 +115,9 @@ struct gpio_chip {
void (*set)(struct gpio_chip *chip,
unsigned offset, int value);
+ void (*set_block)(struct gpio_chip *chip,
+ unsigned long mask,
+ unsigned long values);
int (*to_irq)(struct gpio_chip *chip,
unsigned offset);
@@ -171,6 +177,17 @@ extern void gpio_set_value_cansleep(unsi
extern int __gpio_get_value(unsigned gpio);
extern void __gpio_set_value(unsigned gpio, int value);
+extern struct gpio_block *gpio_block_create(unsigned *gpio, size_t size,
+ const char *name);
+extern void gpio_block_free(struct gpio_block *block);
+extern unsigned long gpio_block_get(const struct gpio_block *block,
+ unsigned long mask);
+extern void gpio_block_set(struct gpio_block *block, unsigned long mask,
+ unsigned long values);
+extern struct gpio_block *gpio_block_find_by_name(const char *name);
+extern int gpio_block_register(struct gpio_block *block);
+extern void gpio_block_unregister(struct gpio_block *block);
+
extern int __gpio_cansleep(unsigned gpio);
extern int __gpio_to_irq(unsigned gpio);
--- linux-2.6.orig/include/linux/gpio.h
+++ linux-2.6/include/linux/gpio.h
@@ -2,6 +2,8 @@
#define __LINUX_GPIO_H
#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/list.h>
/* see Documentation/gpio.txt */
@@ -39,6 +41,64 @@ struct gpio {
const char *label;
};
+/**
+ * struct gpio_remap - a structure for describing a bit mapping
+ * @mask: a bit mask, relevant for a (partial) mapping
+ * @offset: how many bits to shift to the left (negative: to the right)
+ * @list: several remappings are internally collected in a list
+ *
+ * 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 list_head list;
+};
+
+/**
+ * struct gpio_block_chip - a structure representing chip specific data in a
+ * gpio block
+ * @gc: the chip
+ * @remap_list: list of remappings, there are several necessary if the bits
+ * are not consecutive
+ * @mask: chip specific mask, used for propagating to the driver's
+ * get_block() and set_block() functions
+ * @list: list collecting potentially multiple chips in one block
+ *
+ * This structure holds information about remapping and masking of gpios within
+ * one chip. There can be several of those in one block.
+ */
+struct gpio_block_chip {
+ struct gpio_chip *gc;
+ struct list_head remap_list;
+ unsigned long mask;
+
+ struct list_head list;
+};
+
+/**
+ * struct gpio_block - a structure describing a list of GPIOs for simultaneous
+ * operations
+ * @gbc_list: list of chips in this block, typically just one
+ * @name: the name of this block
+ * @ngpio: number of gpios in this block
+ * @gpio: list of gpios in this block
+ * @list: global list of blocks, maintained by gpiolib
+ */
+struct gpio_block {
+ struct list_head gbc_list;
+ const char *name;
+
+ int ngpio;
+ unsigned *gpio;
+
+ struct list_head list;
+};
+
#ifdef CONFIG_GENERIC_GPIO
#ifdef CONFIG_ARCH_HAVE_CUSTOM_GPIO_H
@@ -169,6 +229,43 @@ 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,
+ unsigned long mask)
+{
+ WARN_ON(1);
+ return 0;
+}
+
+static inline void gpio_block_set(struct gpio_block *block, unsigned long mask,
+ unsigned long values)
+{
+ 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 */
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RESEND 2/6 v10] gpio: Add sysfs support to block GPIO API
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 1/6 v10] gpio: Add a block GPIO API to gpiolib Roland Stigge
@ 2012-12-14 14:26 ` Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 3/6 v10] gpio: Add userland device interface to block GPIO Roland Stigge
` (4 subsequent siblings)
6 siblings, 0 replies; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 14:26 UTC (permalink / raw)
To: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Cc: Roland Stigge
This patch adds sysfs support to the block GPIO API.
Signed-off-by: Roland Stigge <stigge@antcom.de>
---
Documentation/ABI/testing/sysfs-gpio | 20 ++
drivers/gpio/gpiolib.c | 250 ++++++++++++++++++++++++++++++++++-
include/asm-generic/gpio.h | 11 +
include/linux/gpio.h | 15 ++
4 files changed, 295 insertions(+), 1 deletion(-)
--- linux-2.6.orig/Documentation/ABI/testing/sysfs-gpio
+++ linux-2.6/Documentation/ABI/testing/sysfs-gpio
@@ -25,3 +25,23 @@ Description:
/label ... (r/o) descriptive, not necessarily unique
/ngpio ... (r/o) number of GPIOs; numbered N to N + (ngpio - 1)
+What: /sys/class/gpioblock/
+Date: October 2012
+KernelVersion: 3.7
+Contact: Roland Stigge <stigge@antcom.de>
+Description:
+
+ 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.
+
+ /sys/class/gpioblock
+ /BLOCKNAME ... for each GPIO block name
+ /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 hex
+ (only available if /exported is 1)
+ /mask ... current mask as 32 or 64 bit integer in hex
+ (only available if /exported is 1)
--- linux-2.6.orig/drivers/gpio/gpiolib.c
+++ linux-2.6/drivers/gpio/gpiolib.c
@@ -198,7 +198,8 @@ static bool gpio_block_is_output(struct
int i;
for (i = 0; i < block->ngpio; i++)
- if (!test_bit(FLAG_IS_OUT, &gpio_desc[block->gpio[i]].flags))
+ if ((block->cur_mask & BIT(i)) &&
+ !test_bit(FLAG_IS_OUT, &gpio_desc[block->gpio[i]].flags))
return false;
return true;
}
@@ -984,6 +985,238 @@ 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, block->cur_mask));
+}
+
+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, block->cur_mask, 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 ssize_t gpio_block_mask_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", block->cur_mask);
+}
+
+static ssize_t gpio_block_mask_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 mask;
+
+ status = kstrtoul(buf, 0, &mask);
+ if (status == 0)
+ block->cur_mask = mask;
+ return status;
+}
+
+static struct device_attribute
+dev_attr_block_mask = __ATTR(mask, S_IWUSR | S_IRUGO, gpio_block_mask_show,
+ gpio_block_mask_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 out1;
+ }
+
+ dev = class_find_device(&gpio_block_class, NULL, block, match_export);
+ if (!dev) {
+ status = -ENODEV;
+ goto out1;
+ }
+
+ status = device_create_file(dev, &dev_attr_block_value);
+ if (status)
+ goto out1;
+
+ status = device_create_file(dev, &dev_attr_block_mask);
+ if (status)
+ goto out2;
+
+ mutex_unlock(&sysfs_lock);
+ return 0;
+
+out2:
+ device_remove_file(dev, &dev_attr_block_value);
+out1:
+ 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);
+ device_remove_file(dev, &dev_attr_block_mask);
+
+ 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;
@@ -994,6 +1227,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).
*
@@ -1724,6 +1961,7 @@ struct gpio_block *gpio_block_create(uns
block->name = name;
block->ngpio = size;
+ block->cur_mask = ~0;
INIT_LIST_HEAD(&block->gbc_list);
block->gpio = kzalloc(sizeof(*block->gpio) * size, GFP_KERNEL);
if (!block->gpio)
@@ -1886,12 +2124,21 @@ EXPORT_SYMBOL_GPL(gpio_block_find_by_nam
int gpio_block_register(struct gpio_block *block)
{
+ int ret;
+
if (gpio_block_find_by_name(block->name))
return -EBUSY;
list_add(&block->list, &gpio_block_list);
+ ret = gpio_block_export(block);
+ if (ret)
+ goto err1;
+
return 0;
+err1:
+ list_del(&block->list);
+ return ret;
}
EXPORT_SYMBOL_GPL(gpio_block_register);
@@ -1902,6 +2149,7 @@ void gpio_block_unregister(struct gpio_b
list_for_each_entry(i, &gpio_block_list, list)
if (i == block) {
list_del(&i->list);
+ gpio_block_unexport(block);
break;
}
}
--- linux-2.6.orig/include/asm-generic/gpio.h
+++ linux-2.6/include/asm-generic/gpio.h
@@ -213,6 +213,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 */
@@ -272,6 +274,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
@@ -88,6 +88,7 @@ struct gpio_block_chip {
* @ngpio: number of gpios in this block
* @gpio: list of gpios in this block
* @list: global list of blocks, maintained by gpiolib
+ * @cur_mask: currently used gpio mask used by userspace API
*/
struct gpio_block {
struct list_head gbc_list;
@@ -97,6 +98,7 @@ struct gpio_block {
unsigned *gpio;
struct list_head list;
+ unsigned long cur_mask;
};
#ifdef CONFIG_GENERIC_GPIO
@@ -314,6 +316,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 */
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RESEND 3/6 v10] gpio: Add userland device interface to block GPIO
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 1/6 v10] gpio: Add a block GPIO API to gpiolib Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 2/6 v10] gpio: Add sysfs support to block GPIO API Roland Stigge
@ 2012-12-14 14:26 ` Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 4/6 v10] gpiolib: Fix default attributes for class Roland Stigge
` (3 subsequent siblings)
6 siblings, 0 replies; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 14:26 UTC (permalink / raw)
To: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Cc: Roland Stigge
This patch adds a character device interface to the block GPIO system.
Signed-off-by: Roland Stigge <stigge@antcom.de>
---
Documentation/ABI/testing/dev-gpioblock | 34 ++++
drivers/gpio/gpiolib.c | 224 +++++++++++++++++++++++++++++++-
include/linux/gpio.h | 13 +
3 files changed, 270 insertions(+), 1 deletion(-)
--- /dev/null
+++ linux-2.6/Documentation/ABI/testing/dev-gpioblock
@@ -0,0 +1,34 @@
+What: /dev/<gpioblock>
+Date: Nov 2012
+KernelVersion: 3.7
+Contact: Roland Stigge <stigge@antcom.de>
+Description: The /dev/<gpioblock> character device node provides userspace
+ access to GPIO blocks, named exactly as the block, e.g.
+ /dev/block0.
+
+ Reading:
+ When reading sizeof(unsigned long) bytes from the device, the
+ current state of the block, masked by the current mask (see
+ below) can be obtained as a word. When the device is opened
+ with O_NONBLOCK, read() always returns with data immediately,
+ otherwise it blocks until data is available, see IRQ handling
+ below.
+
+ Writing:
+ By writing sizeof(unsigned long) bytes to the device, the
+ current state of the block can be set. This operation is
+ masked by the current mask (see below).
+
+ IRQ handling:
+ When one or more IRQs in the block are IRQ capable, you can
+ poll() on the device to check/wait for this IRQ. If no IRQ
+ is available, poll() returns -ENOSYS and userspace needs to
+ (busy) poll itself if necessary.
+
+ Setting the mask (default: all bits set):
+ By doing an ioctl(fd, 0, &mask) with an unsigned long mask, the
+ current mask for read and write operations on this gpio block
+ can be set.
+
+ See also Documentation/gpio.txt for an explanation of block
+ GPIO.
--- linux-2.6.orig/drivers/gpio/gpiolib.c
+++ linux-2.6/drivers/gpio/gpiolib.c
@@ -11,6 +11,8 @@
#include <linux/of_gpio.h>
#include <linux/idr.h>
#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/poll.h>
#define CREATE_TRACE_POINTS
#include <trace/events/gpio.h>
@@ -2122,6 +2124,206 @@ struct gpio_block *gpio_block_find_by_na
}
EXPORT_SYMBOL_GPL(gpio_block_find_by_name);
+static struct gpio_block *gpio_block_find_by_minor(int minor)
+{
+ struct gpio_block *i;
+
+ list_for_each_entry(i, &gpio_block_list, list)
+ if (i->miscdev.minor == minor)
+ return i;
+ return NULL;
+}
+
+static bool gpio_block_is_irq_duplicate(struct gpio_block *block, int index)
+{
+ int irq = gpio_to_irq(block->gpio[index]);
+ int i;
+
+ for (i = 0; i < index; i++)
+ if (gpio_to_irq(block->gpio[i]) == irq)
+ return true;
+ return false;
+}
+
+static irqreturn_t gpio_block_irq_handler(int irq, void *dev)
+{
+ struct gpio_block *block = dev;
+
+ wake_up_interruptible(&block->wait_queue);
+ block->got_int = true;
+
+ return IRQ_HANDLED;
+}
+
+static int gpio_block_fop_open(struct inode *in, struct file *f)
+{
+ int i;
+ struct gpio_block *block = gpio_block_find_by_minor(MINOR(in->i_rdev));
+ int status;
+ int irq;
+
+ if (!block)
+ return -ENOENT;
+
+ block->irq_controlled = false;
+ block->got_int = false;
+ spin_lock_init(&block->lock);
+ init_waitqueue_head(&block->wait_queue);
+ f->private_data = block;
+
+ for (i = 0; i < block->ngpio; i++) {
+ status = gpio_request(block->gpio[i], block->name);
+ if (status)
+ goto err1;
+
+ irq = gpio_to_irq(block->gpio[i]);
+ if (irq >= 0 &&
+ !test_bit(FLAG_IS_OUT, &gpio_desc[block->gpio[i]].flags) &&
+ !gpio_block_is_irq_duplicate(block, i)) {
+ status = request_irq(irq, gpio_block_irq_handler,
+ IRQF_SHARED,
+ block->name, block);
+ if (status)
+ goto err2;
+
+ block->irq_controlled = true;
+ }
+ }
+
+ return 0;
+
+err1:
+ while (i > 0) {
+ i--;
+
+ irq = gpio_to_irq(block->gpio[i]);
+ if (irq >= 0 &&
+ !test_bit(FLAG_IS_OUT, &gpio_desc[block->gpio[i]].flags) &&
+ !gpio_block_is_irq_duplicate(block, i))
+ free_irq(irq, block);
+err2:
+ gpio_free(block->gpio[i]);
+ }
+ return status;
+}
+
+static int gpio_block_fop_release(struct inode *in, struct file *f)
+{
+ int i;
+ struct gpio_block *block = (struct gpio_block *)f->private_data;
+
+ for (i = 0; i < block->ngpio; i++) {
+ int irq = gpio_to_irq(block->gpio[i]);
+
+ if (irq >= 0 &&
+ !test_bit(FLAG_IS_OUT, &gpio_desc[block->gpio[i]].flags) &&
+ !gpio_block_is_irq_duplicate(block, i))
+ free_irq(irq, block);
+
+ gpio_free(block->gpio[i]);
+ }
+
+ return 0;
+}
+
+static int got_int(struct gpio_block *block)
+{
+ unsigned long flags;
+ int result;
+
+ spin_lock_irqsave(&block->lock, flags);
+ result = block->got_int;
+ spin_unlock_irqrestore(&block->lock, flags);
+
+ return result;
+}
+
+static ssize_t gpio_block_fop_read(struct file *f, char __user *buf, size_t n,
+ loff_t *offset)
+{
+ struct gpio_block *block = (struct gpio_block *)f->private_data;
+ int err;
+ unsigned long flags;
+
+ if (block->irq_controlled) {
+ if (!(f->f_flags & O_NONBLOCK))
+ wait_event_interruptible(block->wait_queue,
+ got_int(block));
+ spin_lock_irqsave(&block->lock, flags);
+ block->got_int = 0;
+ spin_unlock_irqrestore(&block->lock, flags);
+ }
+
+ if (n >= sizeof(unsigned long)) {
+ unsigned long values = gpio_block_get(block, block->cur_mask);
+
+ err = put_user(values, (unsigned long __user *)buf);
+ if (err)
+ return err;
+
+ return sizeof(unsigned long);
+ }
+ return 0;
+}
+
+static ssize_t gpio_block_fop_write(struct file *f, const char __user *buf,
+ size_t n, loff_t *offset)
+{
+ struct gpio_block *block = (struct gpio_block *)f->private_data;
+ int err;
+
+ if (n >= sizeof(unsigned long)) {
+ unsigned long values;
+
+ err = get_user(values, (unsigned long __user *)buf);
+ if (err)
+ return err;
+ if (gpio_block_is_output(block))
+ gpio_block_set(block, block->cur_mask, values);
+ else
+ return -EPERM;
+ return sizeof(unsigned long);
+ }
+ return 0;
+}
+
+static long gpio_block_fop_ioctl(struct file *f, unsigned int cmd,
+ unsigned long arg)
+{
+ struct gpio_block *block = (struct gpio_block *)f->private_data;
+ unsigned long __user *x = (unsigned long __user *)arg;
+
+ if (cmd == 0)
+ return get_user(block->cur_mask, x);
+
+ return -EINVAL;
+}
+
+unsigned int gpio_block_fop_poll(struct file *f, struct poll_table_struct *pt)
+{
+ struct gpio_block *block = (struct gpio_block *)f->private_data;
+
+ if (!block->irq_controlled)
+ return -ENOSYS;
+
+ if (!got_int(block))
+ poll_wait(f, &block->wait_queue, pt);
+
+ if (got_int(block))
+ return POLLIN;
+
+ return 0;
+}
+
+static const struct file_operations gpio_block_fops = {
+ .open = gpio_block_fop_open,
+ .release = gpio_block_fop_release,
+ .read = gpio_block_fop_read,
+ .write = gpio_block_fop_write,
+ .unlocked_ioctl = gpio_block_fop_ioctl,
+ .poll = gpio_block_fop_poll,
+};
+
int gpio_block_register(struct gpio_block *block)
{
int ret;
@@ -2132,12 +2334,31 @@ int gpio_block_register(struct gpio_bloc
list_add(&block->list, &gpio_block_list);
ret = gpio_block_export(block);
- if (ret)
+
+ /*
+ * ret == ENOSYS is the case where GPIO_SYSFS is deactivated. In this
+ * case, we can continue safely anyway, since we can provide the device
+ * interface.
+ */
+ if (ret && ret != -ENOSYS)
goto err1;
+ block->miscdev.name = block->name;
+ block->miscdev.nodename = block->name;
+ block->miscdev.minor = MISC_DYNAMIC_MINOR;
+ block->miscdev.fops = &gpio_block_fops;
+ block->miscdev.mode = S_IWUSR | S_IRUGO;
+
+ ret = misc_register(&block->miscdev);
+ if (ret)
+ goto err2;
+
return 0;
+err2:
+ gpio_block_unexport(block);
err1:
list_del(&block->list);
+
return ret;
}
EXPORT_SYMBOL_GPL(gpio_block_register);
@@ -2150,6 +2371,7 @@ void gpio_block_unregister(struct gpio_b
if (i == block) {
list_del(&i->list);
gpio_block_unexport(block);
+ misc_deregister(&block->miscdev);
break;
}
}
--- linux-2.6.orig/include/linux/gpio.h
+++ linux-2.6/include/linux/gpio.h
@@ -4,6 +4,9 @@
#include <linux/errno.h>
#include <linux/types.h>
#include <linux/list.h>
+#include <linux/sched.h>
+#include <linux/miscdevice.h>
+#include <linux/spinlock.h>
/* see Documentation/gpio.txt */
@@ -89,6 +92,11 @@ struct gpio_block_chip {
* @gpio: list of gpios in this block
* @list: global list of blocks, maintained by gpiolib
* @cur_mask: currently used gpio mask used by userspace API
+ * @miscdev: userspace API: device
+ * @wait_queue: userspace API: wait queue waiting for IRQ
+ * @irq_controlled: userspace API: flag: using IRQ or not
+ * @got_int: userspace API: change detection via IRQ
+ * @lock: userspace API: spinlock for IRQ manipulated data
*/
struct gpio_block {
struct list_head gbc_list;
@@ -99,6 +107,11 @@ struct gpio_block {
struct list_head list;
unsigned long cur_mask;
+ struct miscdevice miscdev;
+ wait_queue_head_t wait_queue;
+ bool irq_controlled;
+ bool got_int;
+ spinlock_t lock;
};
#ifdef CONFIG_GENERIC_GPIO
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RESEND 4/6 v10] gpiolib: Fix default attributes for class
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
` (2 preceding siblings ...)
2012-12-14 14:26 ` [PATCH RESEND 3/6 v10] gpio: Add userland device interface to block GPIO Roland Stigge
@ 2012-12-14 14:26 ` Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API Roland Stigge
` (2 subsequent siblings)
6 siblings, 0 replies; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 14:26 UTC (permalink / raw)
To: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Cc: Roland Stigge
There is a race condition between creating a gpio or gpiochip device and adding
default attributes. 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 <stigge@antcom.de>
---
Documentation/ABI/testing/sysfs-gpio | 11 ++++
drivers/gpio/gpiolib.c | 79 +++++++++++++++--------------------
2 files changed, 45 insertions(+), 45 deletions(-)
--- linux-2.6.orig/Documentation/ABI/testing/sysfs-gpio
+++ linux-2.6/Documentation/ABI/testing/sysfs-gpio
@@ -20,6 +20,17 @@ Description:
/value ... always readable, writes fail for input GPIOs
/direction ... r/w as: in, out (default low); write: high, low
/edge ... r/w as: none, falling, rising, both
+
+What: /sys/class/gpiochip/
+Date: October 2012
+KernelVersion: 3.7
+Contact: Roland Stigge <stigge@antcom.de>
+Description:
+
+ Each gpiochip is represented by a separate device having the following
+ attributes:
+
+ /sys/class/gpiochip
/gpiochipN ... for each gpiochip; #N is its first GPIO
/base ... (r/o) same as N
/label ... (r/o) descriptive, not necessarily unique
--- linux-2.6.orig/drivers/gpio/gpiolib.c
+++ linux-2.6/drivers/gpio/gpiolib.c
@@ -334,9 +334,6 @@ static ssize_t gpio_value_store(struct d
return status;
}
-static const DEVICE_ATTR(value, 0644,
- gpio_value_show, gpio_value_store);
-
static irqreturn_t gpio_sysfs_irq(int irq, void *priv)
{
struct sysfs_dirent *value_sd = priv;
@@ -557,19 +554,6 @@ static ssize_t gpio_active_low_store(str
return status ? : size;
}
-static const DEVICE_ATTR(active_low, 0644,
- gpio_active_low_show, gpio_active_low_store);
-
-static const struct attribute *gpio_attrs[] = {
- &dev_attr_value.attr,
- &dev_attr_active_low.attr,
- NULL,
-};
-
-static const struct attribute_group gpio_attr_group = {
- .attrs = (struct attribute **) gpio_attrs,
-};
-
/*
* /sys/class/gpio/gpiochipN/
* /base ... matching gpio_chip.base (N)
@@ -584,7 +568,6 @@ static ssize_t chip_base_show(struct dev
return sprintf(buf, "%d\n", chip->base);
}
-static DEVICE_ATTR(base, 0444, chip_base_show, NULL);
static ssize_t chip_label_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -593,7 +576,6 @@ static ssize_t chip_label_show(struct de
return sprintf(buf, "%s\n", chip->label ? : "");
}
-static DEVICE_ATTR(label, 0444, chip_label_show, NULL);
static ssize_t chip_ngpio_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -602,18 +584,6 @@ static ssize_t chip_ngpio_show(struct de
return sprintf(buf, "%u\n", chip->ngpio);
}
-static DEVICE_ATTR(ngpio, 0444, chip_ngpio_show, NULL);
-
-static const struct attribute *gpiochip_attrs[] = {
- &dev_attr_base.attr,
- &dev_attr_label.attr,
- &dev_attr_ngpio.attr,
- NULL,
-};
-
-static const struct attribute_group gpiochip_attr_group = {
- .attrs = (struct attribute **) gpiochip_attrs,
-};
/*
* /sys/class/gpio/export ... write-only
@@ -692,11 +662,32 @@ static struct class_attribute gpio_class
__ATTR_NULL,
};
+static struct device_attribute gpio_attrs[] = {
+ __ATTR(active_low, 0644, gpio_active_low_show, gpio_active_low_store),
+ __ATTR(value, 0644, gpio_value_show, gpio_value_store),
+ __ATTR_NULL,
+};
+
static struct class gpio_class = {
.name = "gpio",
.owner = THIS_MODULE,
- .class_attrs = gpio_class_attrs,
+ .class_attrs = gpio_class_attrs,
+ .dev_attrs = gpio_attrs,
+};
+
+static struct device_attribute gpiochip_attrs[] = {
+ __ATTR(label, 0444, chip_label_show, NULL),
+ __ATTR(base, 0444, chip_base_show, NULL),
+ __ATTR(ngpio, 0444, chip_ngpio_show, NULL),
+ __ATTR_NULL,
+};
+
+static struct class gpiochip_class = {
+ .name = "gpiochip",
+ .owner = THIS_MODULE,
+
+ .dev_attrs = gpiochip_attrs,
};
@@ -753,10 +744,7 @@ int gpio_export(unsigned gpio, bool dire
dev = 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);
@@ -926,25 +914,22 @@ EXPORT_SYMBOL_GPL(gpio_unexport);
static int gpiochip_export(struct gpio_chip *chip)
{
- int status;
+ int status = 0;
struct device *dev;
/* Many systems register gpio chips for SOC support very early,
* before driver model support is available. In those cases we
* export this later, in gpiolib_sysfs_init() ... here we just
- * verify that _some_ field of gpio_class got initialized.
+ * verify that _some_ field of gpiochip_class got initialized.
*/
- if (!gpio_class.p)
+ if (!gpiochip_class.p)
return 0;
/* 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);
@@ -972,7 +957,7 @@ static void gpiochip_unexport(struct gpi
struct device *dev;
mutex_lock(&sysfs_lock);
- dev = class_find_device(&gpio_class, NULL, chip, match_export);
+ dev = class_find_device(&gpiochip_class, NULL, chip, match_export);
if (dev) {
put_device(dev);
device_unregister(dev);
@@ -1229,6 +1214,10 @@ static int __init gpiolib_sysfs_init(voi
if (status < 0)
return status;
+ status = class_register(&gpiochip_class);
+ if (status < 0)
+ return status;
+
status = class_register(&gpio_block_class);
if (status < 0)
return status;
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
` (3 preceding siblings ...)
2012-12-14 14:26 ` [PATCH RESEND 4/6 v10] gpiolib: Fix default attributes for class Roland Stigge
@ 2012-12-14 14:26 ` Roland Stigge
2012-12-17 15:51 ` Mark Rutland
2012-12-14 14:26 ` [PATCH RESEND 6/6 v10] gpio: Add block gpio to several gpio drivers Roland Stigge
2012-12-14 17:58 ` [PATCH RESEND 0/6 v10] gpio: Add block GPIO Wolfgang Grandegger
6 siblings, 1 reply; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 14:26 UTC (permalink / raw)
To: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Cc: Roland Stigge
This patch adds device tree support to the block GPIO API.
Signed-off-by: Roland Stigge <stigge@antcom.de>
---
Documentation/devicetree/bindings/gpio/gpio-block.txt | 36 +++++++
drivers/gpio/Makefile | 1
drivers/gpio/gpioblock-of.c | 84 ++++++++++++++++++
3 files changed, 121 insertions(+)
--- /dev/null
+++ linux-2.6/Documentation/devicetree/bindings/gpio/gpio-block.txt
@@ -0,0 +1,36 @@
+Block GPIO definition
+=====================
+
+This binding specifies arbitrary blocks of gpios, combining gpios from one or
+more GPIO controllers together, to form a word for r/w access.
+
+Required property:
+- compatible: must be "linux,gpio-block"
+
+Required subnodes:
+- the name will be the registered name of the block
+- property "gpios" is a list of gpios for the respective block
+
+Example:
+
+ blockgpio {
+ compatible = "linux,gpio-block";
+
+ block0 {
+ gpios = <&gpio 3 0 0>,
+ <&gpio 3 1 0>;
+ };
+ block1 {
+ gpios = <&gpio 4 1 0>,
+ <&gpio 4 3 0>,
+ <&gpio 4 2 0>,
+ <&gpio 4 4 0>,
+ <&gpio 4 5 0>,
+ <&gpio 4 6 0>,
+ <&gpio 4 7 0>,
+ <&gpio 4 8 0>,
+ <&gpio 4 9 0>,
+ <&gpio 4 10 0>,
+ <&gpio 4 19 0>;
+ };
+ };
--- linux-2.6.orig/drivers/gpio/Makefile
+++ linux-2.6/drivers/gpio/Makefile
@@ -4,6 +4,7 @@ ccflags-$(CONFIG_DEBUG_GPIO) += -DDEBUG
obj-$(CONFIG_GPIOLIB) += gpiolib.o devres.o
obj-$(CONFIG_OF_GPIO) += gpiolib-of.o
+obj-$(CONFIG_OF_GPIO) += gpioblock-of.o
# Device drivers. Generally keep list sorted alphabetically
obj-$(CONFIG_GPIO_GENERIC) += gpio-generic.o
--- /dev/null
+++ linux-2.6/drivers/gpio/gpioblock-of.c
@@ -0,0 +1,84 @@
+/*
+ * OF implementation for Block GPIO
+ *
+ * Copyright (C) 2012 Roland Stigge <stigge@antcom.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+
+static int __devinit gpioblock_of_probe(struct platform_device *pdev)
+{
+ struct device_node *block;
+ unsigned *gpios;
+ int ngpio;
+ int ret;
+ struct gpio_block *gb;
+
+ for_each_available_child_of_node(pdev->dev.of_node, block) {
+ int i;
+
+ ngpio = of_gpio_count(block);
+ gpios = kzalloc(ngpio * sizeof(*gpios), GFP_KERNEL);
+ if (!gpios)
+ return -ENOMEM;
+ for (i = 0; i < ngpio; i++) {
+ ret = of_get_gpio(block, i);
+ if (ret < 0)
+ return ret; /* expect -EPROBE_DEFER */
+ gpios[i] = ret;
+ }
+ gb = gpio_block_create(gpios, ngpio, block->name);
+ if (IS_ERR(gb)) {
+ dev_err(&pdev->dev,
+ "Error creating GPIO block from device tree\n");
+ return PTR_ERR(gb);
+ }
+ ret = gpio_block_register(gb);
+ if (ret < 0) {
+ gpio_block_free(gb);
+ return ret;
+ }
+ kfree(gpios);
+ dev_info(&pdev->dev, "Registered gpio block %s: %d gpios\n",
+ block->name, ngpio);
+ }
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static struct of_device_id gpioblock_of_match[] __devinitdata = {
+ { .compatible = "linux,gpio-block", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, gpioblock_of_match);
+#endif
+
+static struct platform_driver gpioblock_of_driver = {
+ .driver = {
+ .name = "gpio-block",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(gpioblock_of_match),
+
+ },
+ .probe = gpioblock_of_probe,
+};
+
+module_platform_driver(gpioblock_of_driver);
+
+MODULE_DESCRIPTION("GPIO Block driver");
+MODULE_AUTHOR("Roland Stigge <stigge@antcom.de>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpioblock-of");
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH RESEND 6/6 v10] gpio: Add block gpio to several gpio drivers
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
` (4 preceding siblings ...)
2012-12-14 14:26 ` [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API Roland Stigge
@ 2012-12-14 14:26 ` Roland Stigge
2012-12-14 17:58 ` [PATCH RESEND 0/6 v10] gpio: Add block GPIO Wolfgang Grandegger
6 siblings, 0 replies; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 14:26 UTC (permalink / raw)
To: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Cc: Roland Stigge
This patch adds block GPIO support to several gpio drivers.
Signed-off-by: Roland Stigge <stigge@antcom.de>
---
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-pch.c | 27 ++++++++++++++
drivers/gpio/gpio-pl061.c | 17 +++++++++
drivers/gpio/gpio-twl6040.c | 32 +++++++++++++++++
drivers/gpio/gpio-ucb1400.c | 23 ++++++++++++
drivers/gpio/gpio-vt8500.c | 24 ++++++++++++
drivers/gpio/gpio-xilinx.c | 44 +++++++++++++++++++++++
13 files changed, 536 insertions(+)
--- linux-2.6.orig/drivers/gpio/gpio-em.c
+++ linux-2.6/drivers/gpio/gpio-em.c
@@ -203,6 +203,27 @@ static void em_gio_set(struct gpio_chip
__em_gio_set(chip, GIO_OH, offset - 16, value);
}
+static unsigned long em_gio_get_block(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ return (int)(em_gio_read(gpio_to_priv(chip), GIO_I) & mask);
+}
+
+static void em_gio_set_block(struct gpio_chip *chip, unsigned long mask,
+ unsigned long values)
+{
+ unsigned long mask_ol = mask & 0xFFFF;
+ unsigned long mask_oh = mask >> 16;
+
+ unsigned long values_ol = values & mask_ol;
+ unsigned long values_oh = (values >> 16) & mask_oh;
+
+ em_gio_write(gpio_to_priv(chip), GIO_OL,
+ mask_ol << 16 | values_ol);
+ em_gio_write(gpio_to_priv(chip), GIO_OH,
+ mask_oh << 16 | values_oh);
+}
+
static int em_gio_direction_output(struct gpio_chip *chip, unsigned offset,
int value)
{
@@ -317,8 +338,10 @@ static int __devinit em_gio_probe(struct
gpio_chip = &p->gpio_chip;
gpio_chip->direction_input = em_gio_direction_input;
gpio_chip->get = em_gio_get;
+ gpio_chip->get_block = em_gio_get_block;
gpio_chip->direction_output = em_gio_direction_output;
gpio_chip->set = em_gio_set;
+ gpio_chip->set_block = em_gio_set_block;
gpio_chip->to_irq = em_gio_to_irq;
gpio_chip->label = name;
gpio_chip->owner = THIS_MODULE;
--- linux-2.6.orig/drivers/gpio/gpio-generic.c
+++ linux-2.6/drivers/gpio/gpio-generic.c
@@ -122,6 +122,13 @@ static int bgpio_get(struct gpio_chip *g
return bgc->read_reg(bgc->reg_dat) & bgc->pin2mask(bgc, gpio);
}
+static unsigned long bgpio_get_block(struct gpio_chip *gc, unsigned long mask)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+ return bgc->read_reg(bgc->reg_dat) & mask;
+}
+
static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
{
struct bgpio_chip *bgc = to_bgpio_chip(gc);
@@ -170,6 +177,51 @@ static void bgpio_set_set(struct gpio_ch
spin_unlock_irqrestore(&bgc->lock, flags);
}
+static void bgpio_set_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+
+ bgc->data &= ~mask;
+ bgc->data |= values & mask;
+
+ bgc->write_reg(bgc->reg_dat, bgc->data);
+
+ spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static void bgpio_set_with_clear_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long set_bits = values & mask;
+ unsigned long clr_bits = ~values & mask;
+
+ if (set_bits)
+ bgc->write_reg(bgc->reg_set, set_bits);
+ if (clr_bits)
+ bgc->write_reg(bgc->reg_set, clr_bits);
+}
+
+static void bgpio_set_set_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ struct bgpio_chip *bgc = to_bgpio_chip(gc);
+ unsigned long flags;
+
+ spin_lock_irqsave(&bgc->lock, flags);
+
+ bgc->data &= ~mask;
+ bgc->data |= values & mask;
+
+ bgc->write_reg(bgc->reg_set, bgc->data);
+
+ spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
static int bgpio_simple_dir_in(struct gpio_chip *gc, unsigned int gpio)
{
return 0;
@@ -317,14 +369,18 @@ static int bgpio_setup_io(struct bgpio_c
bgc->reg_set = set;
bgc->reg_clr = clr;
bgc->gc.set = bgpio_set_with_clear;
+ bgc->gc.set_block = bgpio_set_with_clear_block;
} else if (set && !clr) {
bgc->reg_set = set;
bgc->gc.set = bgpio_set_set;
+ bgc->gc.set_block = bgpio_set_set_block;
} else {
bgc->gc.set = bgpio_set;
+ bgc->gc.set_block = bgpio_set_block;
}
bgc->gc.get = bgpio_get;
+ bgc->gc.get_block = bgpio_get_block;
return 0;
}
--- linux-2.6.orig/drivers/gpio/gpio-lpc32xx.c
+++ linux-2.6/drivers/gpio/gpio-lpc32xx.c
@@ -297,6 +297,22 @@ static int lpc32xx_gpio_get_value_p3(str
return __get_gpio_state_p3(group, pin);
}
+static unsigned long lpc32xx_gpio_get_block_p3(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
+ u32 bits = __raw_readl(group->gpio_grp->inp_state);
+
+ /* In P3_INP_STATE, the 6 GPIOs are scattered over the register,
+ * rearranging to bits 0-5
+ */
+ bits >>= 10;
+ bits &= 0x401F;
+ bits |= (bits & 0x4000) >> 9;
+
+ return bits & mask & 0x3F;
+}
+
static int lpc32xx_gpi_get_value(struct gpio_chip *chip, unsigned pin)
{
struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
@@ -304,6 +320,15 @@ static int lpc32xx_gpi_get_value(struct
return __get_gpi_state_p3(group, pin);
}
+static unsigned long lpc32xx_gpi_get_block(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
+ u32 bits = __raw_readl(group->gpio_grp->inp_state);
+
+ return bits & mask;
+}
+
static int lpc32xx_gpio_dir_output_p012(struct gpio_chip *chip, unsigned pin,
int value)
{
@@ -351,6 +376,27 @@ static void lpc32xx_gpio_set_value_p3(st
__set_gpio_level_p3(group, pin, value);
}
+static void lpc32xx_gpio_set_block_p3(struct gpio_chip *chip,
+ unsigned long mask,
+ unsigned long values)
+{
+ struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
+ u32 set_bits = values & mask;
+ u32 clr_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,
+ * but not set and cleared at 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);
+}
+
static void lpc32xx_gpo_set_value(struct gpio_chip *chip, unsigned pin,
int value)
{
@@ -366,6 +412,31 @@ static int lpc32xx_gpo_get_value(struct
return __get_gpo_state_p3(group, pin);
}
+static void lpc32xx_gpo_set_block(struct gpio_chip *chip, unsigned long mask,
+ unsigned long values)
+{
+ struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
+ u32 set_bits = values & mask;
+ u32 clr_bits = ~values & mask;
+
+ /* Note: On LPC32xx, GPOs can only be set at once or cleared at once,
+ * but not set and cleared at 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);
+}
+
+static unsigned long lpc32xx_gpo_get_block(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
+ u32 bits = __raw_readl(group->gpio_grp->outp_state);
+
+ return bits & mask;
+}
+
static int lpc32xx_gpio_request(struct gpio_chip *chip, unsigned pin)
{
if (pin < chip->ngpio)
@@ -440,8 +511,10 @@ static struct lpc32xx_gpio_chip lpc32xx_
.label = "gpio_p0",
.direction_input = lpc32xx_gpio_dir_input_p012,
.get = lpc32xx_gpio_get_value_p012,
+ .get_block = lpc32xx_gpi_get_block,
.direction_output = lpc32xx_gpio_dir_output_p012,
.set = lpc32xx_gpio_set_value_p012,
+ .set_block = lpc32xx_gpo_set_block,
.request = lpc32xx_gpio_request,
.to_irq = lpc32xx_gpio_to_irq_p01,
.base = LPC32XX_GPIO_P0_GRP,
@@ -456,8 +529,10 @@ static struct lpc32xx_gpio_chip lpc32xx_
.label = "gpio_p1",
.direction_input = lpc32xx_gpio_dir_input_p012,
.get = lpc32xx_gpio_get_value_p012,
+ .get_block = lpc32xx_gpi_get_block,
.direction_output = lpc32xx_gpio_dir_output_p012,
.set = lpc32xx_gpio_set_value_p012,
+ .set_block = lpc32xx_gpo_set_block,
.request = lpc32xx_gpio_request,
.to_irq = lpc32xx_gpio_to_irq_p01,
.base = LPC32XX_GPIO_P1_GRP,
@@ -472,8 +547,10 @@ static struct lpc32xx_gpio_chip lpc32xx_
.label = "gpio_p2",
.direction_input = lpc32xx_gpio_dir_input_p012,
.get = lpc32xx_gpio_get_value_p012,
+ .get_block = lpc32xx_gpi_get_block,
.direction_output = lpc32xx_gpio_dir_output_p012,
.set = lpc32xx_gpio_set_value_p012,
+ .set_block = lpc32xx_gpo_set_block,
.request = lpc32xx_gpio_request,
.base = LPC32XX_GPIO_P2_GRP,
.ngpio = LPC32XX_GPIO_P2_MAX,
@@ -487,8 +564,10 @@ static struct lpc32xx_gpio_chip lpc32xx_
.label = "gpio_p3",
.direction_input = lpc32xx_gpio_dir_input_p3,
.get = lpc32xx_gpio_get_value_p3,
+ .get_block = lpc32xx_gpio_get_block_p3,
.direction_output = lpc32xx_gpio_dir_output_p3,
.set = lpc32xx_gpio_set_value_p3,
+ .set_block = lpc32xx_gpio_set_block_p3,
.request = lpc32xx_gpio_request,
.to_irq = lpc32xx_gpio_to_irq_gpio_p3,
.base = LPC32XX_GPIO_P3_GRP,
@@ -503,6 +582,7 @@ static struct lpc32xx_gpio_chip lpc32xx_
.label = "gpi_p3",
.direction_input = lpc32xx_gpio_dir_in_always,
.get = lpc32xx_gpi_get_value,
+ .get_block = lpc32xx_gpi_get_block,
.request = lpc32xx_gpio_request,
.to_irq = lpc32xx_gpio_to_irq_gpi_p3,
.base = LPC32XX_GPI_P3_GRP,
@@ -517,7 +597,9 @@ static struct lpc32xx_gpio_chip lpc32xx_
.label = "gpo_p3",
.direction_output = lpc32xx_gpio_dir_out_always,
.set = lpc32xx_gpo_set_value,
+ .set_block = lpc32xx_gpo_set_block,
.get = lpc32xx_gpo_get_value,
+ .get_block = lpc32xx_gpo_get_block,
.request = lpc32xx_gpio_request,
.base = LPC32XX_GPO_P3_GRP,
.ngpio = LPC32XX_GPO_P3_MAX,
--- linux-2.6.orig/drivers/gpio/gpio-max730x.c
+++ linux-2.6/drivers/gpio/gpio-max730x.c
@@ -146,6 +146,44 @@ static int max7301_get(struct gpio_chip
return level;
}
+static unsigned long max7301_get_block(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ struct max7301 *ts = container_of(chip, struct max7301, chip);
+ int i, j;
+ unsigned long result = 0;
+
+ for (i = 0; i < 4; i++) {
+ if ((mask >> (i * 8)) & 0xFF) { /* i/o only if necessary */
+ u8 in_level = ts->read(ts->dev, 0x44 + i * 8);
+ u8 in_mask = 0;
+ u8 out_level = (ts->out_level >> (i * 8 + 4)) & 0xFF;
+ u8 out_mask = 0;
+
+ for (j = 0; j < 8; j++) {
+ int offset = 4 + i * 8 + j;
+ int config = (ts->port_config[offset >> 2] >>
+ ((offset & 3) << 1)) &
+ PIN_CONFIG_MASK;
+
+ switch (config) {
+ case PIN_CONFIG_OUT:
+ out_mask |= BIT(j);
+ break;
+ case PIN_CONFIG_IN_WO_PULLUP:
+ case PIN_CONFIG_IN_PULLUP:
+ in_mask |= BIT(j);
+ }
+ }
+
+ result |= ((unsigned long)(in_level & in_mask) |
+ (out_level & out_mask)) << (i * 8);
+ }
+ }
+
+ return result & mask;
+}
+
static void max7301_set(struct gpio_chip *chip, unsigned offset, int value)
{
struct max7301 *ts = container_of(chip, struct max7301, chip);
@@ -160,6 +198,27 @@ static void max7301_set(struct gpio_chip
mutex_unlock(&ts->lock);
}
+static void max7301_set_block(struct gpio_chip *chip, unsigned long mask,
+ unsigned long values)
+{
+ struct max7301 *ts = container_of(chip, struct max7301, chip);
+ unsigned long changes;
+ int i;
+
+ 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(struct max7301 *ts)
{
struct device *dev = ts->dev;
@@ -184,8 +243,10 @@ int __devinit __max730x_probe(struct max
ts->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.ngpio = PIN_NUMBER;
ts->chip.can_sleep = 1;
--- linux-2.6.orig/drivers/gpio/gpio-max732x.c
+++ linux-2.6/drivers/gpio/gpio-max732x.c
@@ -219,6 +219,63 @@ out:
mutex_unlock(&chip->lock);
}
+static unsigned long max732x_gpio_get_block(struct gpio_chip *gc,
+ unsigned long mask)
+{
+ struct max732x_chip *chip;
+ uint8_t lo = 0;
+ uint8_t hi = 0;
+ int ret;
+
+ chip = container_of(gc, struct max732x_chip, gpio_chip);
+
+ if (mask & 0xFF) {
+ ret = max732x_readb(chip, is_group_a(chip, 0), &lo);
+ if (ret < 0)
+ return 0;
+ }
+ if (mask & 0xFF00) {
+ ret = max732x_readb(chip, is_group_a(chip, 8), &hi);
+ if (ret < 0)
+ return 0;
+ }
+
+ return (((unsigned long)hi << 8) | lo) & mask;
+}
+
+static void max732x_gpio_set_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ struct max732x_chip *chip;
+ uint8_t reg_out;
+ uint8_t lo_mask = mask & 0xFF;
+ uint8_t hi_mask = (mask >> 8) & 0xFF;
+ int ret;
+
+ chip = container_of(gc, struct max732x_chip, gpio_chip);
+
+ mutex_lock(&chip->lock);
+
+ if (lo_mask) {
+ reg_out = (chip->reg_out[0] & ~lo_mask) | (values & lo_mask);
+ ret = max732x_writeb(chip, is_group_a(chip, 0), reg_out);
+ if (ret < 0)
+ goto out;
+ chip->reg_out[0] = reg_out;
+ }
+ if (hi_mask) {
+ reg_out = (chip->reg_out[1] & ~hi_mask) |
+ ((values >> 8) & hi_mask);
+ ret = max732x_writeb(chip, is_group_a(chip, 8), reg_out);
+ if (ret < 0)
+ goto out;
+ chip->reg_out[1] = reg_out;
+ }
+
+out:
+ mutex_unlock(&chip->lock);
+}
+
static int max732x_gpio_direction_input(struct gpio_chip *gc, unsigned off)
{
struct max732x_chip *chip;
@@ -562,8 +619,10 @@ static int __devinit max732x_setup_gpio(
if (chip->dir_output) {
gc->direction_output = max732x_gpio_direction_output;
gc->set = max732x_gpio_set_value;
+ gc->set_block = max732x_gpio_set_block;
}
gc->get = max732x_gpio_get_value;
+ gc->get_block = max732x_gpio_get_block;
gc->can_sleep = 1;
gc->base = gpio_start;
--- linux-2.6.orig/drivers/gpio/gpio-pca953x.c
+++ linux-2.6/drivers/gpio/gpio-pca953x.c
@@ -300,6 +300,68 @@ exit:
mutex_unlock(&chip->i2c_lock);
}
+static unsigned long pca953x_gpio_get_block(struct gpio_chip *gc,
+ unsigned long mask)
+{
+ struct pca953x_chip *chip;
+ u32 reg_val;
+ int ret, offset = 0;
+
+ chip = container_of(gc, struct pca953x_chip, gpio_chip);
+
+ mutex_lock(&chip->i2c_lock);
+ switch (chip->chip_type) {
+ case PCA953X_TYPE:
+ offset = PCA953X_INPUT;
+ break;
+ case PCA957X_TYPE:
+ offset = PCA957X_IN;
+ break;
+ }
+ ret = pca953x_read_reg(chip, offset, ®_val);
+ mutex_unlock(&chip->i2c_lock);
+ if (ret < 0) {
+ /* NOTE: diagnostic already emitted; that's all we should
+ * do unless gpio_*_value_cansleep() calls become different
+ * from their nonsleeping siblings (and report faults).
+ */
+ return 0;
+ }
+
+ return reg_val & mask;
+}
+
+static void pca953x_gpio_set_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ struct pca953x_chip *chip;
+ u32 reg_val;
+ int ret, offset = 0;
+
+ chip = container_of(gc, struct pca953x_chip, gpio_chip);
+
+ mutex_lock(&chip->i2c_lock);
+
+ reg_val = chip->reg_output & ~mask;
+ reg_val |= values & mask;
+
+ switch (chip->chip_type) {
+ case PCA953X_TYPE:
+ offset = PCA953X_OUTPUT;
+ break;
+ case PCA957X_TYPE:
+ offset = PCA957X_OUT;
+ break;
+ }
+ ret = pca953x_write_reg(chip, offset, reg_val);
+ if (ret)
+ goto exit;
+
+ chip->reg_output = reg_val;
+exit:
+ mutex_unlock(&chip->i2c_lock);
+}
+
static void pca953x_setup_gpio(struct pca953x_chip *chip, int gpios)
{
struct gpio_chip *gc;
@@ -310,6 +372,8 @@ static void pca953x_setup_gpio(struct pc
gc->direction_output = pca953x_gpio_direction_output;
gc->get = pca953x_gpio_get_value;
gc->set = pca953x_gpio_set_value;
+ gc->get_block = pca953x_gpio_get_block;
+ gc->set_block = pca953x_gpio_set_block;
gc->can_sleep = 1;
gc->base = chip->gpio_start;
--- linux-2.6.orig/drivers/gpio/gpio-pcf857x.c
+++ linux-2.6/drivers/gpio/gpio-pcf857x.c
@@ -158,6 +158,28 @@ static void pcf857x_set(struct gpio_chip
pcf857x_output(chip, offset, value);
}
+static unsigned long pcf857x_get_block(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ struct pcf857x *gpio = container_of(chip, struct pcf857x, chip);
+ int value;
+
+ value = gpio->read(gpio->client);
+ return (value < 0) ? 0 : (value & mask);
+}
+
+static void pcf857x_set_block(struct gpio_chip *chip, unsigned long mask,
+ unsigned long values)
+{
+ struct pcf857x *gpio = container_of(chip, struct pcf857x, chip);
+
+ mutex_lock(&gpio->lock);
+ gpio->out &= ~mask;
+ gpio->out |= values & mask;
+ gpio->write(gpio->client, gpio->out);
+ mutex_unlock(&gpio->lock);
+}
+
/*-------------------------------------------------------------------------*/
static int pcf857x_to_irq(struct gpio_chip *chip, unsigned offset)
@@ -280,6 +302,8 @@ static int pcf857x_probe(struct i2c_clie
gpio->chip.owner = THIS_MODULE;
gpio->chip.get = pcf857x_get;
gpio->chip.set = pcf857x_set;
+ gpio->chip.get_block = pcf857x_get_block;
+ gpio->chip.set_block = pcf857x_set_block;
gpio->chip.direction_input = pcf857x_input;
gpio->chip.direction_output = pcf857x_output;
gpio->chip.ngpio = id->driver_data;
--- linux-2.6.orig/drivers/gpio/gpio-pch.c
+++ linux-2.6/drivers/gpio/gpio-pch.c
@@ -122,6 +122,23 @@ static void pch_gpio_set(struct gpio_chi
spin_unlock_irqrestore(&chip->spinlock, flags);
}
+static void pch_gpio_set_block(struct gpio_chip *gpio, unsigned long mask,
+ unsigned long values)
+{
+ u32 reg_val;
+ struct pch_gpio *chip = container_of(gpio, struct pch_gpio, gpio);
+ unsigned long flags;
+
+ spin_lock_irqsave(&chip->spinlock, flags);
+ reg_val = ioread32(&chip->reg->po);
+
+ reg_val &= ~mask;
+ reg_val |= values & mask;
+
+ iowrite32(reg_val, &chip->reg->po);
+ spin_unlock_irqrestore(&chip->spinlock, flags);
+}
+
static int pch_gpio_get(struct gpio_chip *gpio, unsigned nr)
{
struct pch_gpio *chip = container_of(gpio, struct pch_gpio, gpio);
@@ -129,6 +146,14 @@ static int pch_gpio_get(struct gpio_chip
return ioread32(&chip->reg->pi) & (1 << nr);
}
+static unsigned long pch_gpio_get_block(struct gpio_chip *gpio,
+ unsigned long mask)
+{
+ struct pch_gpio *chip = container_of(gpio, struct pch_gpio, gpio);
+
+ return ioread32(&chip->reg->pi) & mask;
+}
+
static int pch_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
int val)
{
@@ -218,8 +243,10 @@ static void pch_gpio_setup(struct pch_gp
gpio->owner = THIS_MODULE;
gpio->direction_input = pch_gpio_direction_input;
gpio->get = pch_gpio_get;
+ gpio->get_block = pch_gpio_get_block;
gpio->direction_output = pch_gpio_direction_output;
gpio->set = pch_gpio_set;
+ gpio->set_block = pch_gpio_set_block;
gpio->dbg_show = NULL;
gpio->base = -1;
gpio->ngpio = gpio_pins[chip->ioh];
--- linux-2.6.orig/drivers/gpio/gpio-pl061.c
+++ linux-2.6/drivers/gpio/gpio-pl061.c
@@ -123,6 +123,21 @@ static void pl061_set_value(struct gpio_
writeb(!!value << offset, chip->base + (1 << (offset + 2)));
}
+static unsigned long pl061_get_block(struct gpio_chip *gc, unsigned long mask)
+{
+ struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
+
+ return !!readb(chip->base + (mask << 2));
+}
+
+static void pl061_set_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
+
+ writeb(values & mask, chip->base + (mask << 2));
+}
+
static int pl061_to_irq(struct gpio_chip *gc, unsigned offset)
{
struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
@@ -256,6 +271,8 @@ static int pl061_probe(struct amba_devic
chip->gc.direction_output = pl061_direction_output;
chip->gc.get = pl061_get_value;
chip->gc.set = pl061_set_value;
+ chip->gc.get_block = pl061_get_block;
+ chip->gc.set_block = pl061_set_block;
chip->gc.to_irq = pl061_to_irq;
chip->gc.ngpio = PL061_GPIO_NR;
chip->gc.label = dev_name(&dev->dev);
--- linux-2.6.orig/drivers/gpio/gpio-twl6040.c
+++ linux-2.6/drivers/gpio/gpio-twl6040.c
@@ -46,6 +46,19 @@ static int twl6040gpo_get(struct gpio_ch
return (ret >> offset) & 1;
}
+static unsigned long twl6040gpo_get_block(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ struct twl6040 *twl6040 = dev_get_drvdata(chip->dev->parent);
+ int ret = 0;
+
+ ret = twl6040_reg_read(twl6040, TWL6040_REG_GPOCTL);
+ if (ret < 0)
+ return 0;
+
+ return ret & mask;
+}
+
static int twl6040gpo_direction_out(struct gpio_chip *chip, unsigned offset,
int value)
{
@@ -71,12 +84,31 @@ static void twl6040gpo_set(struct gpio_c
twl6040_reg_write(twl6040, TWL6040_REG_GPOCTL, gpoctl);
}
+static void twl6040gpo_set_block(struct gpio_chip *chip, unsigned long mask,
+ unsigned long values)
+{
+ struct twl6040 *twl6040 = dev_get_drvdata(chip->dev->parent);
+ int ret;
+ u8 gpoctl;
+
+ ret = twl6040_reg_read(twl6040, TWL6040_REG_GPOCTL);
+ if (ret < 0)
+ return;
+
+ gpoctl = ret & ~mask;
+ gpoctl |= values & mask;
+
+ twl6040_reg_write(twl6040, TWL6040_REG_GPOCTL, gpoctl);
+}
+
static struct gpio_chip twl6040gpo_chip = {
.label = "twl6040",
.owner = THIS_MODULE,
.get = twl6040gpo_get,
+ .get_block = twl6040gpo_get_block,
.direction_output = twl6040gpo_direction_out,
.set = twl6040gpo_set,
+ .set_block = twl6040gpo_set_block,
.can_sleep = 1,
};
--- linux-2.6.orig/drivers/gpio/gpio-ucb1400.c
+++ linux-2.6/drivers/gpio/gpio-ucb1400.c
@@ -45,6 +45,27 @@ static void ucb1400_gpio_set(struct gpio
ucb1400_gpio_set_value(gpio->ac97, off, val);
}
+static unsigned long ucb1400_gpio_get_block(struct gpio_chip *gc,
+ unsigned long mask)
+{
+ struct ucb1400_gpio *gpio;
+ gpio = container_of(gc, struct ucb1400_gpio, gc);
+ return ucb1400_reg_read(gpio->ac97, UCB_IO_DATA) & mask;
+}
+
+static void ucb1400_gpio_set_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ struct ucb1400_gpio *gpio;
+ u16 tmp;
+ gpio = container_of(gc, struct ucb1400_gpio, gc);
+
+ tmp = ucb1400_reg_read(gpio->ac97, UCB_IO_DATA) & ~mask;
+ tmp |= values & mask;
+
+ ucb1400_reg_write(gpio->ac97, UCB_IO_DATA, tmp);
+}
+
static int ucb1400_gpio_probe(struct platform_device *dev)
{
struct ucb1400_gpio *ucb = dev->dev.platform_data;
@@ -66,6 +87,8 @@ static int ucb1400_gpio_probe(struct pla
ucb->gc.direction_output = ucb1400_gpio_dir_out;
ucb->gc.get = ucb1400_gpio_get;
ucb->gc.set = ucb1400_gpio_set;
+ ucb->gc.get_block = ucb1400_gpio_get_block;
+ ucb->gc.set_block = ucb1400_gpio_set_block;
ucb->gc.can_sleep = 1;
err = gpiochip_add(&ucb->gc);
--- linux-2.6.orig/drivers/gpio/gpio-vt8500.c
+++ linux-2.6/drivers/gpio/gpio-vt8500.c
@@ -209,6 +209,28 @@ static void vt8500_gpio_set_value(struct
writel_relaxed(val, vt8500_chip->base + vt8500_chip->regs->data_out);
}
+static unsigned long vt8500_gpio_get_block(struct gpio_chip *chip,
+ unsigned long mask)
+{
+ struct vt8500_gpio_chip *vt8500_chip = to_vt8500(chip);
+
+ return readl_relaxed(vt8500_chip->base + vt8500_chip->regs->data_in) &
+ mask;
+}
+
+static void vt8500_gpio_set_block(struct gpio_chip *chip, unsigned long mask,
+ unsigned long values)
+{
+ struct vt8500_gpio_chip *vt8500_chip = to_vt8500(chip);
+
+ u32 val = readl_relaxed(vt8500_chip->base +
+ vt8500_chip->regs->data_out);
+ val &= ~mask;
+ val |= values & mask;
+
+ writel_relaxed(val, vt8500_chip->base + vt8500_chip->regs->data_out);
+}
+
static int vt8500_of_xlate(struct gpio_chip *gc,
const struct of_phandle_args *gpiospec, u32 *flags)
{
@@ -251,6 +273,8 @@ static int vt8500_add_chips(struct platf
chip->direction_output = vt8500_gpio_direction_output;
chip->get = vt8500_gpio_get_value;
chip->set = vt8500_gpio_set_value;
+ chip->get_block = vt8500_gpio_get_block;
+ chip->set_block = vt8500_gpio_set_block;
chip->can_sleep = 0;
chip->base = pin_cnt;
chip->ngpio = data->banks[i].ngpio;
--- linux-2.6.orig/drivers/gpio/gpio-xilinx.c
+++ linux-2.6/drivers/gpio/gpio-xilinx.c
@@ -49,6 +49,21 @@ static int xgpio_get(struct gpio_chip *g
}
/**
+ * xgpio_get_block - Read a block of specified signals of the GPIO device.
+ * @gc: Pointer to gpio_chip device structure.
+ * @mask: Bit mask (bit 0 == 0th GPIO) for GPIOs to get
+ *
+ * This function reads a block of specified signal of the GPIO device, returned
+ * as a bit mask, each bit representing a GPIO
+ */
+static unsigned long xgpio_get_block(struct gpio_chip *gc, unsigned long mask)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+
+ return in_be32(mm_gc->regs + XGPIO_DATA_OFFSET) & mask;
+}
+
+/**
* xgpio_set - Write the specified signal of the GPIO device.
* @gc: Pointer to gpio_chip device structure.
* @gpio: GPIO signal number.
@@ -77,6 +92,33 @@ static void xgpio_set(struct gpio_chip *
}
/**
+ * xgpio_set_block - Write a block of specified signals of the GPIO device.
+ * @gc: Pointer to gpio_chip device structure.
+ * @mask: Bit mask (bit 0 == 0th GPIO) for GPIOs to set
+ * @values: Bit mapped values
+ *
+ * This function writes the specified values in to the specified signals of the
+ * GPIO device.
+ */
+static void xgpio_set_block(struct gpio_chip *gc, unsigned long mask,
+ unsigned long values)
+{
+ unsigned long flags;
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct xgpio_instance *chip =
+ container_of(mm_gc, struct xgpio_instance, mmchip);
+
+ spin_lock_irqsave(&chip->gpio_lock, flags);
+
+ chip->gpio_state &= ~mask;
+ chip->gpio_state |= mask & values;
+
+ out_be32(mm_gc->regs + XGPIO_DATA_OFFSET, chip->gpio_state);
+
+ spin_unlock_irqrestore(&chip->gpio_lock, flags);
+}
+
+/**
* xgpio_dir_in - Set the direction of the specified GPIO signal as input.
* @gc: Pointer to gpio_chip device structure.
* @gpio: GPIO signal number.
@@ -195,6 +237,8 @@ static int __devinit xgpio_of_probe(stru
chip->mmchip.gc.direction_output = xgpio_dir_out;
chip->mmchip.gc.get = xgpio_get;
chip->mmchip.gc.set = xgpio_set;
+ chip->mmchip.gc.get_block = xgpio_get_block;
+ chip->mmchip.gc.set_block = xgpio_set_block;
chip->mmchip.save_regs = xgpio_save_regs;
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
` (5 preceding siblings ...)
2012-12-14 14:26 ` [PATCH RESEND 6/6 v10] gpio: Add block gpio to several gpio drivers Roland Stigge
@ 2012-12-14 17:58 ` Wolfgang Grandegger
2012-12-14 23:49 ` Roland Stigge
6 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-14 17:58 UTC (permalink / raw)
To: Roland Stigge
Cc: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]
On 12/14/2012 03:26 PM, Roland Stigge wrote:
> This set of patches adds:
>
> * Block GPIO API to gpiolib
> * Sysfs support for GPIO API, to provide userland access
> * Device interface for userland access (alternative to sysfs)
> * 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 <stigge@antcom.de>
> Tested by: Wolfgang Grandegger <wg@grandegger.com>
I'm going to re-test this version next week. Attached you will find my
GPIO block adoptions for the AT91.
Wolfgang
[-- Attachment #2: 0001-gpio-add-GPIO-block-callback-functions-for-AT91.patch --]
[-- Type: text/x-diff, Size: 2721 bytes --]
>From 16c0e4b933dcdb3859184190b62655f4ac90949f Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
arch/arm/mach-at91/gpio.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..dd0853d 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block = at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block = at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,14 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+
+ __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-14 17:58 ` [PATCH RESEND 0/6 v10] gpio: Add block GPIO Wolfgang Grandegger
@ 2012-12-14 23:49 ` Roland Stigge
2012-12-15 10:51 ` Russell King - ARM Linux
2012-12-17 11:37 ` Wolfgang Grandegger
0 siblings, 2 replies; 29+ messages in thread
From: Roland Stigge @ 2012-12-14 23:49 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr
Hi Wolfgang,
thank you for the patch!
On 14/12/12 18:58, Wolfgang Grandegger wrote:
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> +}
> +
Without having an AT91 available right now, I guess the hardware
interface of this GPIO chip is different from the GPIO block API. While
the hardware has clear and set registers, the val parameter of
at91_gpiolib_set_block() should be interpreted as the actual output
values. See lpc32xx_gpo_set_block() for an example for handling set and
clear registers like this: First, set_bits and clear_bits words are
calculated from mask and val parameters, and finally written to the
respective hardware registers.
Note that one .set_block() can result in writing both the set and clear
registers of the hardware when val contains both 0s and 1s in
respectively masked positions.
Roland
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-14 23:49 ` Roland Stigge
@ 2012-12-15 10:51 ` Russell King - ARM Linux
2012-12-17 11:37 ` Wolfgang Grandegger
1 sibling, 0 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2012-12-15 10:51 UTC (permalink / raw)
To: Roland Stigge
Cc: Wolfgang Grandegger, rmallon, gregkh, linus.walleij, broonie,
linux-kernel, w.sang, grant.likely, daniel-gl, sr, plagnioj,
linux-arm-kernel, highguy
On Sat, Dec 15, 2012 at 12:49:57AM +0100, Roland Stigge wrote:
> Without having an AT91 available right now, I guess the hardware
> interface of this GPIO chip is different from the GPIO block API. While
> the hardware has clear and set registers, the val parameter of
> at91_gpiolib_set_block() should be interpreted as the actual output
> values. See lpc32xx_gpo_set_block() for an example for handling set and
> clear registers like this: First, set_bits and clear_bits words are
> calculated from mask and val parameters, and finally written to the
> respective hardware registers.
>
> Note that one .set_block() can result in writing both the set and clear
> registers of the hardware when val contains both 0s and 1s in
> respectively masked positions.
Note also that if this is the same IP as found in SAM3N devices, that it's
possible to write the bit values directly through the OWER/OWDR (output
write enable register/disable register) plus the ODSR (output data status
register) which will synchronously change the state of all the
write-enabled output pins. That may be important for some applications.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-14 23:49 ` Roland Stigge
2012-12-15 10:51 ` Russell King - ARM Linux
@ 2012-12-17 11:37 ` Wolfgang Grandegger
2012-12-17 11:51 ` Wolfgang Grandegger
1 sibling, 1 reply; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-17 11:37 UTC (permalink / raw)
To: Roland Stigge
Cc: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr
Hi Roland,
On 12/15/2012 12:49 AM, Roland Stigge wrote:
> Hi Wolfgang,
>
> thank you for the patch!
>
> On 14/12/12 18:58, Wolfgang Grandegger wrote:
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
>> +{
>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> + void __iomem *pio = at91_gpio->regbase;
>> +
>> + __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
>> +}
>> +
>
> Without having an AT91 available right now, I guess the hardware
> interface of this GPIO chip is different from the GPIO block API. While
> the hardware has clear and set registers, the val parameter of
> at91_gpiolib_set_block() should be interpreted as the actual output
> values. See lpc32xx_gpo_set_block() for an example for handling set and
> clear registers like this: First, set_bits and clear_bits words are
> calculated from mask and val parameters, and finally written to the
> respective hardware registers.
>
> Note that one .set_block() can result in writing both the set and clear
> registers of the hardware when val contains both 0s and 1s in
> respectively masked positions.
Oops, I obviously did not test GPIO block write. The patch below does
work now. Feel free to add it to the next version of your series.
I tested with a block having both, inputs and outputs. The handling
of such a mixed block is clumsy because both, read and write do depend
on "block->cur_mask". It needs to be re-set when switching from read to
write or vice versa. Defining two blocks, one for input and the other
for output seems to be the better solution.
Hope this block gpio support will show up in mainline soon.
Thanks.
Wolfgang.
>From 6249995d129b290704cacb2c0114782414abeba7 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
arch/arm/mach-at91/gpio.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..0998854 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block = at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block = at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 set_bits = values & mask;
+ u32 clr_bits = ~values & mask;
+
+ /* GPIO outputs can only be set at once or cleared at once */
+ if (set_bits)
+ __raw_writel(set_bits, pio + PIO_SODR);
+ if (clr_bits)
+ __raw_writel(clr_bits, pio + PIO_CODR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 11:37 ` Wolfgang Grandegger
@ 2012-12-17 11:51 ` Wolfgang Grandegger
2012-12-17 12:10 ` Russell King - ARM Linux
2012-12-17 13:32 ` Roland Stigge
0 siblings, 2 replies; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-17 11:51 UTC (permalink / raw)
To: Roland Stigge
Cc: rmallon, gregkh, linus.walleij, broonie, linux-kernel, w.sang,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 12:37 PM, Wolfgang Grandegger wrote:
> Hi Roland,
>
> On 12/15/2012 12:49 AM, Roland Stigge wrote:
>> Hi Wolfgang,
>>
>> thank you for the patch!
>>
>> On 14/12/12 18:58, Wolfgang Grandegger wrote:
>>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
>>> +{
>>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>>> + void __iomem *pio = at91_gpio->regbase;
>>> +
>>> + __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
>>> +}
>>> +
>>
>> Without having an AT91 available right now, I guess the hardware
>> interface of this GPIO chip is different from the GPIO block API. While
>> the hardware has clear and set registers, the val parameter of
>> at91_gpiolib_set_block() should be interpreted as the actual output
>> values. See lpc32xx_gpo_set_block() for an example for handling set and
>> clear registers like this: First, set_bits and clear_bits words are
>> calculated from mask and val parameters, and finally written to the
>> respective hardware registers.
>>
>> Note that one .set_block() can result in writing both the set and clear
>> registers of the hardware when val contains both 0s and 1s in
>> respectively masked positions.
>
> Oops, I obviously did not test GPIO block write. The patch below does
> work now. Feel free to add it to the next version of your series.
The patch lacks an important fix, sorry. Please consider the updated
patch below.
Wolfgang.
>From a1f93ddea9c6c9c6a80d7a02d3c8d9902823fe47 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
arch/arm/mach-at91/gpio.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..77c6f91 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -48,7 +48,9 @@ struct at91_gpio_chip {
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +64,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block = at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block = at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
@@ -896,6 +900,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +919,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 set_bits = val & mask;
+ u32 clr_bits = ~val & mask;
+
+ /* GPIO outputs can only be set at once or cleared at once */
+ if (set_bits)
+ __raw_writel(set_bits, pio + PIO_SODR);
+ if (clr_bits)
+ __raw_writel(clr_bits, pio + PIO_CODR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 11:51 ` Wolfgang Grandegger
@ 2012-12-17 12:10 ` Russell King - ARM Linux
2012-12-17 14:57 ` Wolfgang Grandegger
2012-12-17 13:32 ` Roland Stigge
1 sibling, 1 reply; 29+ messages in thread
From: Russell King - ARM Linux @ 2012-12-17 12:10 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Roland Stigge, rmallon, gregkh, linus.walleij, broonie, w.sang,
linux-kernel, grant.likely, daniel-gl, sr, plagnioj,
linux-arm-kernel, highguy
On Mon, Dec 17, 2012 at 12:51:32PM +0100, Wolfgang Grandegger wrote:
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> + u32 set_bits = val & mask;
> + u32 clr_bits = ~val & mask;
> +
> + /* GPIO outputs can only be set at once or cleared at once */
> + if (set_bits)
> + __raw_writel(set_bits, pio + PIO_SODR);
> + if (clr_bits)
> + __raw_writel(clr_bits, pio + PIO_CODR);
> +}
You obviously didn't see my email...
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 11:51 ` Wolfgang Grandegger
2012-12-17 12:10 ` Russell King - ARM Linux
@ 2012-12-17 13:32 ` Roland Stigge
2012-12-17 13:51 ` Roland Stigge
1 sibling, 1 reply; 29+ messages in thread
From: Roland Stigge @ 2012-12-17 13:32 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: rmallon, gregkh, linus.walleij, broonie, linux-kernel, w.sang,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 12:51 PM, Wolfgang Grandegger wrote:
>>> Without having an AT91 available right now, I guess the hardware
>>> interface of this GPIO chip is different from the GPIO block API. While
>>> the hardware has clear and set registers, the val parameter of
>>> at91_gpiolib_set_block() should be interpreted as the actual output
>>> values. See lpc32xx_gpo_set_block() for an example for handling set and
>>> clear registers like this: First, set_bits and clear_bits words are
>>> calculated from mask and val parameters, and finally written to the
>>> respective hardware registers.
>>>
>>> Note that one .set_block() can result in writing both the set and clear
>>> registers of the hardware when val contains both 0s and 1s in
>>> respectively masked positions.
>>
>> Oops, I obviously did not test GPIO block write. The patch below does
>> work now. Feel free to add it to the next version of your series.
>
> The patch lacks an important fix, sorry. Please consider the updated
> patch below.
Thanks!
And I guess Russell is right: If possible, we should write outputs
simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
I wonder if we need to save/restore the state of OWSR at every write
operation or if we need/can cache it. Assuming that block GPIO are the
only code in the kernel that manipulates ODSR.
Further: Can we include this patch for arch/arm/mach-at91 via the gpio
subsystem or does it need to go separately via arm-soc.git?
Thanks,
Roland
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 13:32 ` Roland Stigge
@ 2012-12-17 13:51 ` Roland Stigge
2012-12-17 16:28 ` Wolfgang Grandegger
0 siblings, 1 reply; 29+ messages in thread
From: Roland Stigge @ 2012-12-17 13:51 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: rmallon, gregkh, linus.walleij, broonie, linux-kernel, w.sang,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
Hi Wolfgang,
On 12/17/2012 02:32 PM, Roland Stigge wrote:
> And I guess Russell is right: If possible, we should write outputs
> simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
>
> I wonder if we need to save/restore the state of OWSR at every write
> operation or if we need/can cache it. Assuming that block GPIO are the
> only code in the kernel that manipulates ODSR.
Can you please test the following:
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ __raw_writel(val, pio + PIO_ODSR);
+}
Would caching OWSR be a significant speedup here?
Thanks in advance,
Roland
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 12:10 ` Russell King - ARM Linux
@ 2012-12-17 14:57 ` Wolfgang Grandegger
0 siblings, 0 replies; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-17 14:57 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Roland Stigge, rmallon, gregkh, linus.walleij, broonie, w.sang,
linux-kernel, grant.likely, daniel-gl, sr, plagnioj,
linux-arm-kernel, highguy
On 12/17/2012 01:10 PM, Russell King - ARM Linux wrote:
> On Mon, Dec 17, 2012 at 12:51:32PM +0100, Wolfgang Grandegger wrote:
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
>> +{
>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> + void __iomem *pio = at91_gpio->regbase;
>> + u32 set_bits = val & mask;
>> + u32 clr_bits = ~val & mask;
>> +
>> + /* GPIO outputs can only be set at once or cleared at once */
>> + if (set_bits)
>> + __raw_writel(set_bits, pio + PIO_SODR);
>> + if (clr_bits)
>> + __raw_writel(clr_bits, pio + PIO_CODR);
>> +}
>
> You obviously didn't see my email...
Well, I did read you mail but it was not obvious to me how to maintain
compatibility with the existing "set" method... at a first glance.
But the synchronous data output is important and it's even simpler than
I thought. The block-set callback can directly write to ODSR because
"at91_set_gpio_output()" already has set OWER. Will give that a try
later today.
Wolfgang.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API
2012-12-14 14:26 ` [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API Roland Stigge
@ 2012-12-17 15:51 ` Mark Rutland
2012-12-18 14:30 ` Roland Stigge
0 siblings, 1 reply; 29+ messages in thread
From: Mark Rutland @ 2012-12-17 15:51 UTC (permalink / raw)
To: Roland Stigge
Cc: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Hi,
I have a few comments on the parsing code.
On Fri, Dec 14, 2012 at 02:26:24PM +0000, Roland Stigge wrote:
> This patch adds device tree support to the block GPIO API.
>
> Signed-off-by: Roland Stigge <stigge@antcom.de>
>
> ---
> Documentation/devicetree/bindings/gpio/gpio-block.txt | 36 +++++++
> drivers/gpio/Makefile | 1
> drivers/gpio/gpioblock-of.c | 84 ++++++++++++++++++
> 3 files changed, 121 insertions(+)
>
> --- /dev/null
> +++ linux-2.6/Documentation/devicetree/bindings/gpio/gpio-block.txt
> @@ -0,0 +1,36 @@
> +Block GPIO definition
> +=====================
> +
> +This binding specifies arbitrary blocks of gpios, combining gpios from one or
> +more GPIO controllers together, to form a word for r/w access.
> +
> +Required property:
> +- compatible: must be "linux,gpio-block"
> +
> +Required subnodes:
> +- the name will be the registered name of the block
> +- property "gpios" is a list of gpios for the respective block
> +
> +Example:
> +
> + blockgpio {
> + compatible = "linux,gpio-block";
> +
> + block0 {
> + gpios = <&gpio 3 0 0>,
> + <&gpio 3 1 0>;
> + };
> + block1 {
> + gpios = <&gpio 4 1 0>,
> + <&gpio 4 3 0>,
> + <&gpio 4 2 0>,
> + <&gpio 4 4 0>,
> + <&gpio 4 5 0>,
> + <&gpio 4 6 0>,
> + <&gpio 4 7 0>,
> + <&gpio 4 8 0>,
> + <&gpio 4 9 0>,
> + <&gpio 4 10 0>,
> + <&gpio 4 19 0>;
> + };
> + };
> --- linux-2.6.orig/drivers/gpio/Makefile
> +++ linux-2.6/drivers/gpio/Makefile
> @@ -4,6 +4,7 @@ ccflags-$(CONFIG_DEBUG_GPIO) += -DDEBUG
>
> obj-$(CONFIG_GPIOLIB) += gpiolib.o devres.o
> obj-$(CONFIG_OF_GPIO) += gpiolib-of.o
> +obj-$(CONFIG_OF_GPIO) += gpioblock-of.o
>
> # Device drivers. Generally keep list sorted alphabetically
> obj-$(CONFIG_GPIO_GENERIC) += gpio-generic.o
> --- /dev/null
> +++ linux-2.6/drivers/gpio/gpioblock-of.c
> @@ -0,0 +1,84 @@
> +/*
> + * OF implementation for Block GPIO
> + *
> + * Copyright (C) 2012 Roland Stigge <stigge@antcom.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +
> +static int __devinit gpioblock_of_probe(struct platform_device *pdev)
> +{
> + struct device_node *block;
> + unsigned *gpios;
> + int ngpio;
> + int ret;
> + struct gpio_block *gb;
> +
> + for_each_available_child_of_node(pdev->dev.of_node, block) {
> + int i;
> +
> + ngpio = of_gpio_count(block);
> + gpios = kzalloc(ngpio * sizeof(*gpios), GFP_KERNEL);
What if the block node is malformed? ngpio might be -ENOENT or -EINVAL.
> + if (!gpios)
> + return -ENOMEM;
> + for (i = 0; i < ngpio; i++) {
> + ret = of_get_gpio(block, i);
> + if (ret < 0)
> + return ret; /* expect -EPROBE_DEFER */
> + gpios[i] = ret;
> + }
> + gb = gpio_block_create(gpios, ngpio, block->name);
> + if (IS_ERR(gb)) {
> + dev_err(&pdev->dev,
> + "Error creating GPIO block from device tree\n");
> + return PTR_ERR(gb);
Won't this leak the memory for the gpios object we kzalloc'd earlier?
> + }
> + ret = gpio_block_register(gb);
> + if (ret < 0) {
> + gpio_block_free(gb);
> + return ret;
Same here.
> + }
> + kfree(gpios);
> + dev_info(&pdev->dev, "Registered gpio block %s: %d gpios\n",
> + block->name, ngpio);
Any of the returns in this block will leave the block node's refcount
incremented.
> + }
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static struct of_device_id gpioblock_of_match[] __devinitdata = {
> + { .compatible = "linux,gpio-block", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, gpioblock_of_match);
> +#endif
> +
> +static struct platform_driver gpioblock_of_driver = {
> + .driver = {
> + .name = "gpio-block",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(gpioblock_of_match),
> +
> + },
> + .probe = gpioblock_of_probe,
> +};
> +
> +module_platform_driver(gpioblock_of_driver);
> +
> +MODULE_DESCRIPTION("GPIO Block driver");
> +MODULE_AUTHOR("Roland Stigge <stigge@antcom.de>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:gpioblock-of");
>
Thanks,
Mark.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 13:51 ` Roland Stigge
@ 2012-12-17 16:28 ` Wolfgang Grandegger
2012-12-17 17:15 ` Roland Stigge
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-17 16:28 UTC (permalink / raw)
To: Roland Stigge
Cc: rmallon, gregkh, linus.walleij, broonie, linux-kernel, w.sang,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 02:51 PM, Roland Stigge wrote:
> Hi Wolfgang,
>
> On 12/17/2012 02:32 PM, Roland Stigge wrote:
>> And I guess Russell is right: If possible, we should write outputs
>> simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
>>
>> I wonder if we need to save/restore the state of OWSR at every write
>> operation or if we need/can cache it. Assuming that block GPIO are the
>> only code in the kernel that manipulates ODSR.
>
> Can you please test the following:
>
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + __raw_writel(~mask, pio + PIO_OWDR);
This would also disable normal GPIOs configured for output! From the
manual I understand that if the pin is configured for output, we could
either use PIO_SODR/PIO_CODR to set/clear the bits individually or
PIO_ODSR for synchronous data output. But than we need to care about the
non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
Wolfgang.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 16:28 ` Wolfgang Grandegger
@ 2012-12-17 17:15 ` Roland Stigge
2012-12-17 17:37 ` Wolfgang Grandegger
0 siblings, 1 reply; 29+ messages in thread
From: Roland Stigge @ 2012-12-17 17:15 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: rmallon, gregkh, linus.walleij, broonie, linux-kernel, w.sang,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 05:28 PM, Wolfgang Grandegger wrote:
> On 12/17/2012 02:51 PM, Roland Stigge wrote:
>> Hi Wolfgang,
>>> And I guess Russell is right: If possible, we should write outputs
>>> simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
>>>
>>> I wonder if we need to save/restore the state of OWSR at every write
>>> operation or if we need/can cache it. Assuming that block GPIO are the
>>> only code in the kernel that manipulates ODSR.
>>
>> Can you please test the following:
>>
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
>> +{
>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> + void __iomem *pio = at91_gpio->regbase;
>> +
>> + __raw_writel(~mask, pio + PIO_OWDR);
>
> This would also disable normal GPIOs configured for output! From the
> manual I understand that if the pin is configured for output, we could
> either use PIO_SODR/PIO_CODR to set/clear the bits individually or
> PIO_ODSR for synchronous data output. But than we need to care about the
> non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
>From the manual, I read about OWER: "Enables writing PIO_ODSR for the
I/O line" (analogous for OWDR). Would interpret this as affecting ODSR
(for block GPIO) but not SODR/CODR (as currently with single GPIOs).
Have you tried? ;-)
Thanks in advance,
Roland
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 17:15 ` Roland Stigge
@ 2012-12-17 17:37 ` Wolfgang Grandegger
2012-12-17 18:02 ` Roland Stigge
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-17 17:37 UTC (permalink / raw)
To: Roland Stigge
Cc: rmallon, gregkh, linus.walleij, broonie, w.sang, linux-kernel,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 06:15 PM, Roland Stigge wrote:
> On 12/17/2012 05:28 PM, Wolfgang Grandegger wrote:
>> On 12/17/2012 02:51 PM, Roland Stigge wrote:
>>> Hi Wolfgang,
>>>> And I guess Russell is right: If possible, we should write outputs
>>>> simultaneously via ODSR (plus OWER/OWDR/OWSR) instead of separate set/clear.
>>>>
>>>> I wonder if we need to save/restore the state of OWSR at every write
>>>> operation or if we need/can cache it. Assuming that block GPIO are the
>>>> only code in the kernel that manipulates ODSR.
>>>
>>> Can you please test the following:
>>>
>>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
>>> +{
>>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>>> + void __iomem *pio = at91_gpio->regbase;
>>> +
>>> + __raw_writel(~mask, pio + PIO_OWDR);
>>
>> This would also disable normal GPIOs configured for output! From the
>> manual I understand that if the pin is configured for output, we could
>> either use PIO_SODR/PIO_CODR to set/clear the bits individually or
>> PIO_ODSR for synchronous data output. But than we need to care about the
>> non-block GPIO outputs as well... requiring a read-modify-write cycle :(.
>
>>From the manual, I read about OWER: "Enables writing PIO_ODSR for the
> I/O line" (analogous for OWDR). Would interpret this as affecting ODSR
> (for block GPIO) but not SODR/CODR (as currently with single GPIOs).
>
> Have you tried? ;-)
Grrr, I mixed OER with OWER, sorry for the noise. Back to your approach,
which works.
/* Do synchronous data output with a single write access */
__raw_writel(~mask, pio + PIO_OWDR);
__raw_writel(mask, pio + PIO_OWER);
__raw_writel(val, pio + PIO_ODSR);
For caching we would need a storage. Not sure if it's worth compared to
a context switch into the kernel.
Wolfgang.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 17:37 ` Wolfgang Grandegger
@ 2012-12-17 18:02 ` Roland Stigge
2012-12-17 19:47 ` Wolfgang Grandegger
0 siblings, 1 reply; 29+ messages in thread
From: Roland Stigge @ 2012-12-17 18:02 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: rmallon, gregkh, linus.walleij, broonie, w.sang, linux-kernel,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
> /* Do synchronous data output with a single write access */
> __raw_writel(~mask, pio + PIO_OWDR);
> __raw_writel(mask, pio + PIO_OWER);
> __raw_writel(val, pio + PIO_ODSR);
>
> For caching we would need a storage. Not sure if it's worth compared to
> a context switch into the kernel.
Block GPIO is not only for you in userspace. ;-) You can also implement
efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
always context switches involved.
Roland
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 18:02 ` Roland Stigge
@ 2012-12-17 19:47 ` Wolfgang Grandegger
2012-12-17 21:33 ` Roland Stigge
2012-12-18 5:55 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 2 replies; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-17 19:47 UTC (permalink / raw)
To: Roland Stigge
Cc: rmallon, gregkh, linus.walleij, broonie, w.sang, linux-kernel,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 07:02 PM, Roland Stigge wrote:
> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
>> /* Do synchronous data output with a single write access */
>> __raw_writel(~mask, pio + PIO_OWDR);
>> __raw_writel(mask, pio + PIO_OWER);
>> __raw_writel(val, pio + PIO_ODSR);
>>
>> For caching we would need a storage. Not sure if it's worth compared to
>> a context switch into the kernel.
>
> Block GPIO is not only for you in userspace. ;-) You can also implement
> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
> always context switches involved.
OK, what do you think about the following untested patch:
>From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Mon, 3 Dec 2012 08:31:55 +0100
Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
arch/arm/mach-at91/gpio.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index be42cf0..cf6bd45 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -42,13 +42,16 @@ struct at91_gpio_chip {
void __iomem *regbase; /* PIO bank virtual address */
struct clk *clock; /* associated clock */
struct irq_domain *domain; /* associated irq domain */
+ unsigned long mask_shadow; /* synchronous data output */
};
#define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
static int at91_gpiolib_direction_output(struct gpio_chip *chip,
unsigned offset, int val);
static int at91_gpiolib_direction_input(struct gpio_chip *chip,
@@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
.direction_input = at91_gpiolib_direction_input, \
.direction_output = at91_gpiolib_direction_output, \
.get = at91_gpiolib_get, \
+ .get_block = at91_gpiolib_get_block, \
.set = at91_gpiolib_set, \
+ .set_block = at91_gpiolib_set_block, \
.dbg_show = at91_gpiolib_dbg_show, \
.to_irq = at91_gpiolib_to_irq, \
.ngpio = nr_gpio, \
@@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
return (pdsr & mask) != 0;
}
+static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+ u32 pdsr;
+
+ pdsr = __raw_readl(pio + PIO_PDSR);
+ return pdsr & mask;
+}
+
static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
{
struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
@@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
__raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
}
+static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
+{
+ struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+ void __iomem *pio = at91_gpio->regbase;
+
+ /* Do synchronous data output with a single write access */
+ if (mask != at91_gpio->mask_shadow) {
+ at91_gpio->mask_shadow = mask;
+ __raw_writel(~mask, pio + PIO_OWDR);
+ __raw_writel(mask, pio + PIO_OWER);
+ }
+ __raw_writel(val, pio + PIO_ODSR);
+}
+
static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
int i;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 19:47 ` Wolfgang Grandegger
@ 2012-12-17 21:33 ` Roland Stigge
2012-12-18 6:51 ` Wolfgang Grandegger
2012-12-18 5:55 ` Jean-Christophe PLAGNIOL-VILLARD
1 sibling, 1 reply; 29+ messages in thread
From: Roland Stigge @ 2012-12-17 21:33 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: rmallon, gregkh, linus.walleij, broonie, w.sang, linux-kernel,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 17/12/12 20:47, Wolfgang Grandegger wrote:
> On 12/17/2012 07:02 PM, Roland Stigge wrote:
>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
>>> /* Do synchronous data output with a single write access */
>>> __raw_writel(~mask, pio + PIO_OWDR);
>>> __raw_writel(mask, pio + PIO_OWER);
>>> __raw_writel(val, pio + PIO_ODSR);
>>>
>>> For caching we would need a storage. Not sure if it's worth compared to
>>> a context switch into the kernel.
>>
>> Block GPIO is not only for you in userspace. ;-) You can also implement
>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
>> always context switches involved.
>
> OK, what do you think about the following untested patch:
Looks good!
Why "untested"? ;-)
Thanks,
Roland
>>From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
> From: Wolfgang Grandegger <wg@grandegger.com>
> Date: Mon, 3 Dec 2012 08:31:55 +0100
> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
> arch/arm/mach-at91/gpio.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> index be42cf0..cf6bd45 100644
> --- a/arch/arm/mach-at91/gpio.c
> +++ b/arch/arm/mach-at91/gpio.c
> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
> void __iomem *regbase; /* PIO bank virtual address */
> struct clk *clock; /* associated clock */
> struct irq_domain *domain; /* associated irq domain */
> + unsigned long mask_shadow; /* synchronous data output */
> };
>
> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
>
> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
> unsigned offset, int val);
> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
> .direction_input = at91_gpiolib_direction_input, \
> .direction_output = at91_gpiolib_direction_output, \
> .get = at91_gpiolib_get, \
> + .get_block = at91_gpiolib_get_block, \
> .set = at91_gpiolib_set, \
> + .set_block = at91_gpiolib_set_block, \
> .dbg_show = at91_gpiolib_dbg_show, \
> .to_irq = at91_gpiolib_to_irq, \
> .ngpio = nr_gpio, \
> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
> return (pdsr & mask) != 0;
> }
>
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> + u32 pdsr;
> +
> + pdsr = __raw_readl(pio + PIO_PDSR);
> + return pdsr & mask;
> +}
> +
> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
> {
> struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
> __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> }
>
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + /* Do synchronous data output with a single write access */
> + if (mask != at91_gpio->mask_shadow) {
> + at91_gpio->mask_shadow = mask;
> + __raw_writel(~mask, pio + PIO_OWDR);
> + __raw_writel(mask, pio + PIO_OWER);
> + }
> + __raw_writel(val, pio + PIO_ODSR);
> +}
> +
> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> {
> int i;
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 19:47 ` Wolfgang Grandegger
2012-12-17 21:33 ` Roland Stigge
@ 2012-12-18 5:55 ` Jean-Christophe PLAGNIOL-VILLARD
2012-12-18 6:58 ` Wolfgang Grandegger
1 sibling, 1 reply; 29+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-12-18 5:55 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Roland Stigge, rmallon, gregkh, linus.walleij, broonie, w.sang,
linux-kernel, grant.likely, daniel-gl, sr, linux-arm-kernel,
highguy
On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
> On 12/17/2012 07:02 PM, Roland Stigge wrote:
> > On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
> >> /* Do synchronous data output with a single write access */
> >> __raw_writel(~mask, pio + PIO_OWDR);
> >> __raw_writel(mask, pio + PIO_OWER);
> >> __raw_writel(val, pio + PIO_ODSR);
> >>
> >> For caching we would need a storage. Not sure if it's worth compared to
> >> a context switch into the kernel.
> >
> > Block GPIO is not only for you in userspace. ;-) You can also implement
> > efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
> > always context switches involved.
>
> OK, what do you think about the following untested patch:
>
> From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
> From: Wolfgang Grandegger <wg@grandegger.com>
> Date: Mon, 3 Dec 2012 08:31:55 +0100
> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> ---
> arch/arm/mach-at91/gpio.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> index be42cf0..cf6bd45 100644
> --- a/arch/arm/mach-at91/gpio.c
> +++ b/arch/arm/mach-at91/gpio.c
> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
> void __iomem *regbase; /* PIO bank virtual address */
> struct clk *clock; /* associated clock */
> struct irq_domain *domain; /* associated irq domain */
> + unsigned long mask_shadow; /* synchronous data output */
> };
>
> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
>
> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
> unsigned offset, int val);
> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
> .direction_input = at91_gpiolib_direction_input, \
> .direction_output = at91_gpiolib_direction_output, \
> .get = at91_gpiolib_get, \
> + .get_block = at91_gpiolib_get_block, \
> .set = at91_gpiolib_set, \
> + .set_block = at91_gpiolib_set_block, \
> .dbg_show = at91_gpiolib_dbg_show, \
> .to_irq = at91_gpiolib_to_irq, \
> .ngpio = nr_gpio, \
> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
> return (pdsr & mask) != 0;
> }
>
> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> + u32 pdsr;
> +
> + pdsr = __raw_readl(pio + PIO_PDSR);
> + return pdsr & mask;
> +}
> +
> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
> {
> struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
> __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> }
>
> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
> +{
> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> + void __iomem *pio = at91_gpio->regbase;
> +
> + /* Do synchronous data output with a single write access */
> + if (mask != at91_gpio->mask_shadow) {
> + at91_gpio->mask_shadow = mask;
> + __raw_writel(~mask, pio + PIO_OWDR);
> + __raw_writel(mask, pio + PIO_OWER);
> + }
> + __raw_writel(val, pio + PIO_ODSR);
> +}
this driver is only for old at91 platfrom if you touch at91 you need to update
the pinctrl too
Best Regards,
J.
> +
> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> {
> int i;
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-17 21:33 ` Roland Stigge
@ 2012-12-18 6:51 ` Wolfgang Grandegger
0 siblings, 0 replies; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-18 6:51 UTC (permalink / raw)
To: Roland Stigge
Cc: rmallon, gregkh, linus.walleij, broonie, w.sang, linux-kernel,
grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel, highguy
On 12/17/2012 10:33 PM, Roland Stigge wrote:
> On 17/12/12 20:47, Wolfgang Grandegger wrote:
>> On 12/17/2012 07:02 PM, Roland Stigge wrote:
>>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
>>>> /* Do synchronous data output with a single write access */
>>>> __raw_writel(~mask, pio + PIO_OWDR);
>>>> __raw_writel(mask, pio + PIO_OWER);
>>>> __raw_writel(val, pio + PIO_ODSR);
>>>>
>>>> For caching we would need a storage. Not sure if it's worth compared to
>>>> a context switch into the kernel.
>>>
>>> Block GPIO is not only for you in userspace. ;-) You can also implement
>>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
>>> always context switches involved.
>>
>> OK, what do you think about the following untested patch:
>
> Looks good!
>
> Why "untested"? ;-)
Because I didn't have a chance to test it yet. Will do tomorrow.
Wolfgang.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-18 5:55 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-12-18 6:58 ` Wolfgang Grandegger
2012-12-18 7:54 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Grandegger @ 2012-12-18 6:58 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Roland Stigge, rmallon, gregkh, linus.walleij, broonie, w.sang,
linux-kernel, grant.likely, daniel-gl, sr, linux-arm-kernel,
highguy
On 12/18/2012 06:55 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
>> On 12/17/2012 07:02 PM, Roland Stigge wrote:
>>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
>>>> /* Do synchronous data output with a single write access */
>>>> __raw_writel(~mask, pio + PIO_OWDR);
>>>> __raw_writel(mask, pio + PIO_OWER);
>>>> __raw_writel(val, pio + PIO_ODSR);
>>>>
>>>> For caching we would need a storage. Not sure if it's worth compared to
>>>> a context switch into the kernel.
>>>
>>> Block GPIO is not only for you in userspace. ;-) You can also implement
>>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
>>> always context switches involved.
>>
>> OK, what do you think about the following untested patch:
>>
>> From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
>> From: Wolfgang Grandegger <wg@grandegger.com>
>> Date: Mon, 3 Dec 2012 08:31:55 +0100
>> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
>>
>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> ---
>> arch/arm/mach-at91/gpio.c | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
>> index be42cf0..cf6bd45 100644
>> --- a/arch/arm/mach-at91/gpio.c
>> +++ b/arch/arm/mach-at91/gpio.c
>> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
>> void __iomem *regbase; /* PIO bank virtual address */
>> struct clk *clock; /* associated clock */
>> struct irq_domain *domain; /* associated irq domain */
>> + unsigned long mask_shadow; /* synchronous data output */
>> };
>>
>> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
>>
>> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
>> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
>> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
>> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
>> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
>> unsigned offset, int val);
>> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
>> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
>> .direction_input = at91_gpiolib_direction_input, \
>> .direction_output = at91_gpiolib_direction_output, \
>> .get = at91_gpiolib_get, \
>> + .get_block = at91_gpiolib_get_block, \
>> .set = at91_gpiolib_set, \
>> + .set_block = at91_gpiolib_set_block, \
>> .dbg_show = at91_gpiolib_dbg_show, \
>> .to_irq = at91_gpiolib_to_irq, \
>> .ngpio = nr_gpio, \
>> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
>> return (pdsr & mask) != 0;
>> }
>>
>> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
>> +{
>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> + void __iomem *pio = at91_gpio->regbase;
>> + u32 pdsr;
>> +
>> + pdsr = __raw_readl(pio + PIO_PDSR);
>> + return pdsr & mask;
>> +}
>> +
>> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
>> {
>> struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
>> __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
>> }
>>
>> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
>> +{
>> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> + void __iomem *pio = at91_gpio->regbase;
>> +
>> + /* Do synchronous data output with a single write access */
>> + if (mask != at91_gpio->mask_shadow) {
>> + at91_gpio->mask_shadow = mask;
>> + __raw_writel(~mask, pio + PIO_OWDR);
>> + __raw_writel(mask, pio + PIO_OWER);
>> + }
>> + __raw_writel(val, pio + PIO_ODSR);
>> +}
> this driver is only for old at91 platfrom if you touch at91 you need to update
> the pinctrl too
Well, the patch is for the hardware I have at hand and I can test. There
are many other GPIO hardware interfaces which could be enhanced with
block gpio. Roland only did it for the interfaces in driver/gpio. Also,
I think, an ACK for this patch series would be nice before we continue.
Wolfgang.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 0/6 v10] gpio: Add block GPIO
2012-12-18 6:58 ` Wolfgang Grandegger
@ 2012-12-18 7:54 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 29+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-12-18 7:54 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Roland Stigge, rmallon, gregkh, linus.walleij, broonie, w.sang,
linux-kernel, grant.likely, daniel-gl, sr, linux-arm-kernel,
highguy
On 07:58 Tue 18 Dec , Wolfgang Grandegger wrote:
> On 12/18/2012 06:55 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 20:47 Mon 17 Dec , Wolfgang Grandegger wrote:
> >> On 12/17/2012 07:02 PM, Roland Stigge wrote:
> >>> On 12/17/2012 06:37 PM, Wolfgang Grandegger wrote:
> >>>> /* Do synchronous data output with a single write access */
> >>>> __raw_writel(~mask, pio + PIO_OWDR);
> >>>> __raw_writel(mask, pio + PIO_OWER);
> >>>> __raw_writel(val, pio + PIO_ODSR);
> >>>>
> >>>> For caching we would need a storage. Not sure if it's worth compared to
> >>>> a context switch into the kernel.
> >>>
> >>> Block GPIO is not only for you in userspace. ;-) You can also implement
> >>> efficient n-bit bus I/O in kernel drivers, n-bit-banging. :-) So not
> >>> always context switches involved.
> >>
> >> OK, what do you think about the following untested patch:
> >>
> >> From b44cad16cbbca84715dffd4cb5268497216add25 Mon Sep 17 00:00:00 2001
> >> From: Wolfgang Grandegger <wg@grandegger.com>
> >> Date: Mon, 3 Dec 2012 08:31:55 +0100
> >> Subject: [PATCH 1/2] gpio: add GPIO block callback functions for AT91
> >>
> >> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
> >> ---
> >> arch/arm/mach-at91/gpio.c | 29 +++++++++++++++++++++++++++++
> >> 1 file changed, 29 insertions(+)
> >>
> >> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> >> index be42cf0..cf6bd45 100644
> >> --- a/arch/arm/mach-at91/gpio.c
> >> +++ b/arch/arm/mach-at91/gpio.c
> >> @@ -42,13 +42,16 @@ struct at91_gpio_chip {
> >> void __iomem *regbase; /* PIO bank virtual address */
> >> struct clk *clock; /* associated clock */
> >> struct irq_domain *domain; /* associated irq domain */
> >> + unsigned long mask_shadow; /* synchronous data output */
> >> };
> >>
> >> #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
> >>
> >> static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip);
> >> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val);
> >> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val);
> >> static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset);
> >> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask);
> >> static int at91_gpiolib_direction_output(struct gpio_chip *chip,
> >> unsigned offset, int val);
> >> static int at91_gpiolib_direction_input(struct gpio_chip *chip,
> >> @@ -62,7 +65,9 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
> >> .direction_input = at91_gpiolib_direction_input, \
> >> .direction_output = at91_gpiolib_direction_output, \
> >> .get = at91_gpiolib_get, \
> >> + .get_block = at91_gpiolib_get_block, \
> >> .set = at91_gpiolib_set, \
> >> + .set_block = at91_gpiolib_set_block, \
> >> .dbg_show = at91_gpiolib_dbg_show, \
> >> .to_irq = at91_gpiolib_to_irq, \
> >> .ngpio = nr_gpio, \
> >> @@ -896,6 +901,16 @@ static int at91_gpiolib_get(struct gpio_chip *chip, unsigned offset)
> >> return (pdsr & mask) != 0;
> >> }
> >>
> >> +static unsigned long at91_gpiolib_get_block(struct gpio_chip *chip, unsigned long mask)
> >> +{
> >> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> >> + void __iomem *pio = at91_gpio->regbase;
> >> + u32 pdsr;
> >> +
> >> + pdsr = __raw_readl(pio + PIO_PDSR);
> >> + return pdsr & mask;
> >> +}
> >> +
> >> static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
> >> {
> >> struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> >> @@ -905,6 +920,20 @@ static void at91_gpiolib_set(struct gpio_chip *chip, unsigned offset, int val)
> >> __raw_writel(mask, pio + (val ? PIO_SODR : PIO_CODR));
> >> }
> >>
> >> +static void at91_gpiolib_set_block(struct gpio_chip *chip, unsigned long mask, unsigned long val)
> >> +{
> >> + struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> >> + void __iomem *pio = at91_gpio->regbase;
> >> +
> >> + /* Do synchronous data output with a single write access */
> >> + if (mask != at91_gpio->mask_shadow) {
> >> + at91_gpio->mask_shadow = mask;
> >> + __raw_writel(~mask, pio + PIO_OWDR);
> >> + __raw_writel(mask, pio + PIO_OWER);
> >> + }
> >> + __raw_writel(val, pio + PIO_ODSR);
> >> +}
> > this driver is only for old at91 platfrom if you touch at91 you need to update
> > the pinctrl too
>
> Well, the patch is for the hardware I have at hand and I can test. There
> are many other GPIO hardware interfaces which could be enhanced with
> block gpio. Roland only did it for the interfaces in driver/gpio. Also,
> I think, an ACK for this patch series would be nice before we continue.
no sorry the support of the pinctrl is mandatory
check drivers/pinctrl/pinctrl-at91.c
as this new drivers include both pinctrl & gpio
Best Regards,
J.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API
2012-12-17 15:51 ` Mark Rutland
@ 2012-12-18 14:30 ` Roland Stigge
2012-12-18 16:35 ` Mark Rutland
0 siblings, 1 reply; 29+ messages in thread
From: Roland Stigge @ 2012-12-18 14:30 UTC (permalink / raw)
To: Mark Rutland
Cc: gregkh, grant.likely, linus.walleij, linux-kernel,
linux-arm-kernel, w.sang, jbe, plagnioj, highguy, broonie,
daniel-gl, rmallon, sr, wg
Hi Mark,
On 12/17/2012 04:51 PM, Mark Rutland wrote:
>> +static int __devinit gpioblock_of_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *block;
>> + unsigned *gpios;
>> + int ngpio;
>> + int ret;
>> + struct gpio_block *gb;
>> +
>> + for_each_available_child_of_node(pdev->dev.of_node, block) {
>> + int i;
>> +
>> + ngpio = of_gpio_count(block);
>> + gpios = kzalloc(ngpio * sizeof(*gpios), GFP_KERNEL);
>
> What if the block node is malformed? ngpio might be -ENOENT or -EINVAL.
AFAICS, of_gpio_count() always returns at least 0. Both if
CONFIG_OF_GPIO is y, m or n. And called of_gpio_named_count() also
currently doesn't return error values. Further, other drivers using
of_gpio_count() don't expect or catch <0.
However, it's reasonable to guard against of_gpio_count() < 1 since
probing without provided blocks should be void.
Will change this for the next patch update together with your leakage
findings.
Thanks for reporting!
Roland
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API
2012-12-18 14:30 ` Roland Stigge
@ 2012-12-18 16:35 ` Mark Rutland
0 siblings, 0 replies; 29+ messages in thread
From: Mark Rutland @ 2012-12-18 16:35 UTC (permalink / raw)
To: Roland Stigge
Cc: rmallon, gregkh, linus.walleij, broonie, linux-kernel, wg,
w.sang, grant.likely, daniel-gl, sr, plagnioj, linux-arm-kernel,
highguy
On Tue, Dec 18, 2012 at 02:30:23PM +0000, Roland Stigge wrote:
> Hi Mark,
>
> On 12/17/2012 04:51 PM, Mark Rutland wrote:
> >> +static int __devinit gpioblock_of_probe(struct platform_device *pdev)
> >> +{
> >> + struct device_node *block;
> >> + unsigned *gpios;
> >> + int ngpio;
> >> + int ret;
> >> + struct gpio_block *gb;
> >> +
> >> + for_each_available_child_of_node(pdev->dev.of_node, block) {
> >> + int i;
> >> +
> >> + ngpio = of_gpio_count(block);
> >> + gpios = kzalloc(ngpio * sizeof(*gpios), GFP_KERNEL);
> >
> > What if the block node is malformed? ngpio might be -ENOENT or -EINVAL.
>
> AFAICS, of_gpio_count() always returns at least 0. Both if
> CONFIG_OF_GPIO is y, m or n. And called of_gpio_named_count() also
> currently doesn't return error values. Further, other drivers using
> of_gpio_count() don't expect or catch <0.
Whoops. I'd managed to misread the logic in of_gpio_named_count, sorry.
> However, it's reasonable to guard against of_gpio_count() < 1 since
> probing without provided blocks should be void.
>
> Will change this for the next patch update together with your leakage
> findings.
>
> Thanks for reporting!
>
> Roland
Sounds good!
Thanks,
Mark.
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2012-12-18 16:35 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-14 14:26 [PATCH RESEND 0/6 v10] gpio: Add block GPIO Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 1/6 v10] gpio: Add a block GPIO API to gpiolib Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 2/6 v10] gpio: Add sysfs support to block GPIO API Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 3/6 v10] gpio: Add userland device interface to block GPIO Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 4/6 v10] gpiolib: Fix default attributes for class Roland Stigge
2012-12-14 14:26 ` [PATCH RESEND 5/6 v10] gpio: Add device tree support to block GPIO API Roland Stigge
2012-12-17 15:51 ` Mark Rutland
2012-12-18 14:30 ` Roland Stigge
2012-12-18 16:35 ` Mark Rutland
2012-12-14 14:26 ` [PATCH RESEND 6/6 v10] gpio: Add block gpio to several gpio drivers Roland Stigge
2012-12-14 17:58 ` [PATCH RESEND 0/6 v10] gpio: Add block GPIO Wolfgang Grandegger
2012-12-14 23:49 ` Roland Stigge
2012-12-15 10:51 ` Russell King - ARM Linux
2012-12-17 11:37 ` Wolfgang Grandegger
2012-12-17 11:51 ` Wolfgang Grandegger
2012-12-17 12:10 ` Russell King - ARM Linux
2012-12-17 14:57 ` Wolfgang Grandegger
2012-12-17 13:32 ` Roland Stigge
2012-12-17 13:51 ` Roland Stigge
2012-12-17 16:28 ` Wolfgang Grandegger
2012-12-17 17:15 ` Roland Stigge
2012-12-17 17:37 ` Wolfgang Grandegger
2012-12-17 18:02 ` Roland Stigge
2012-12-17 19:47 ` Wolfgang Grandegger
2012-12-17 21:33 ` Roland Stigge
2012-12-18 6:51 ` Wolfgang Grandegger
2012-12-18 5:55 ` Jean-Christophe PLAGNIOL-VILLARD
2012-12-18 6:58 ` Wolfgang Grandegger
2012-12-18 7:54 ` Jean-Christophe PLAGNIOL-VILLARD
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).