[RFC PATCH 1/2] gpiolib: allow simultaneous setting of multiple GPIO outputs

* [RFC PATCH 1/2] gpiolib: allow simultaneous setting of multiple GPIO outputs
@ 2014-05-21 12:58 Rojhalat Ibrahim
  2014-05-21 13:00 ` [RFC PATCH 2/2] gpio-mpc8xxx: add mpc8xxx_gpio_set_multiple function Rojhalat Ibrahim
  2014-05-22 16:50 ` [RFC PATCH 1/2] gpiolib: allow simultaneous setting of multiple GPIO outputs Gerhard Sittig
  0 siblings, 2 replies; 6+ messages in thread
From: Rojhalat Ibrahim @ 2014-05-21 12:58 UTC (permalink / raw)
  To: linux-gpio; +Cc: Alexandre Courbot, Gerhard Sittig

This patch introduces a new function gpiod_set_raw_array to the consumer
interface which allows setting multiple outputs with just one function call.
It also adds an optional set_multiple function to the driver interface.

Multiple GPIOs are represented by an array of descriptors. The values to be
set are stored in an integer array. Example:

	struct gpio_desc *desc_array[10];
	int value_array[10];

	... acquire descriptors ...
	... set values in value_array ...

	gpiod_set_raw_array(10, desc_array, value_array);

Benefits:
	- Uses descriptor interface. GPIOs have to be acquired before use.
	- Allows arbitrary groups of GPIOs spanning multiple chips.
	- Works without adjustments in GPIO chip drivers.
	- Implementing the new set_multiple function in a chip driver results in
	  additional benefits:
		* Improved performance for certain use cases. The original motivation for
		  this was the task of configuring an FPGA. In that specific case, where
		  9 GPIO lines have to be set many times, configuration time goes down from
		  48 s to 19 s when using the new function.
		* Simultaneous glitch-free setting of multiple pins on any kind of parallel
		  bus attached to GPIOs.

Limitations:
	- Performance is only improved for normal high-low outputs. Open drain and
	  open source outputs are always set separately from each other. Those kinds
	  of outputs could probably be accelerated in a similar way if we could forgo
	  the error checking when setting GPIO directions.
	- There is no gpiod_set_array function that regards the ACTIVE_LOW bits.
	  The _raw_ function should be sufficient for many use cases. So I avoided
	  the code duplication the other functions would require.

Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
---
 drivers/gpio/gpiolib.c        | 116 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/gpio/consumer.h |  18 +++++++
 include/linux/gpio/driver.h   |   3 ++
 3 files changed, 137 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f48817d..ee67ef1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2345,6 +2345,77 @@ static void _gpiod_set_raw_value(struct gpio_desc *desc, bool value)
 		chip->set(chip, gpio_chip_hwgpio(desc), value);
 }
 
+static void _gpio_chip_set_multiple(struct gpio_chip *chip,
+				    u32 mask[ARCH_NR_GPIOS/32],
+				    u32 bits[ARCH_NR_GPIOS/32])
+{
+	if (chip->set_multiple) {
+		chip->set_multiple(chip, mask, bits);
+	} else {
+		int i;
+		for (i = 0; i < ARCH_NR_GPIOS; i++) {
+			if (i > chip->ngpio - 1)
+				break;
+			if (mask[i/32] == 0) {
+				/* skip ahead */
+				i = (i/32 + 1) * 32 - 1;
+				continue;
+			}
+			if (mask[i/32] & (1 << (i % 32))) {
+				chip->set(chip, i, (bits[i/32] >> (i % 32)) & 1);
+				mask[i/32] &= ~(1 << (i % 32));
+			}
+		}
+	}
+}
+
+static void _gpiod_set_raw_array(unsigned int array_size,
+				 struct gpio_desc **desc_array,
+				 int *value_array)
+{
+	struct gpio_chip *chip = desc_array[0]->chip;
+	u32 mask[ARCH_NR_GPIOS/32];
+	u32 bits[ARCH_NR_GPIOS/32];
+	int count = 0;
+	int i;
+
+	memset(mask, 0, sizeof(mask));
+	for (i = 0; i < array_size; i++) {
+		struct gpio_desc *desc = desc_array[i];
+		int hwgpio = gpio_chip_hwgpio(desc);
+		int value = value_array[i];
+
+		/* another chip; push collected bits to outputs */
+		if (desc->chip != chip) {
+			if (count != 0) {
+				_gpio_chip_set_multiple(chip, mask, bits);
+				memset(mask, 0, sizeof(mask));
+				count = 0;
+			}
+			chip = desc->chip;
+		}
+		/* collect all normal outputs belonging to the same chip */
+		/* open drain and open source outputs are set individually */
+		trace_gpio_value(desc_to_gpio(desc), 0, value);
+		if (test_bit(FLAG_OPEN_DRAIN, &desc->flags)) {
+			_gpio_set_open_drain_value(desc, value);
+		} else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) {
+			_gpio_set_open_source_value(desc, value);
+		} else {
+			mask[hwgpio/32] |= (1 << (hwgpio % 32));
+			if (value) {
+				bits[hwgpio/32] |= (1 << (hwgpio % 32));
+			} else {
+				bits[hwgpio/32] &= ~(1 << (hwgpio % 32));
+			}
+			count++;
+		}
+	}
+	if (count != 0) {
+		_gpio_chip_set_multiple(chip, mask, bits);
+	}
+}
+
 /**
  * gpiod_set_raw_value() - assign a gpio's raw value
  * @desc: gpio whose value will be assigned
@@ -2390,6 +2461,30 @@ void gpiod_set_value(struct gpio_desc *desc, int value)
 EXPORT_SYMBOL_GPL(gpiod_set_value);
 
 /**
+ * gpiod_set_raw_array() - assign values to an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be assigned
+ * @value_array: array of values to assign
+ *
+ * Set the raw values of the GPIOs, i.e. the values of the physical lines
+ * without regard for their ACTIVE_LOW status.
+ *
+ * This function should be called from contexts where we cannot sleep, and will
+ * complain if the GPIO chip functions potentially sleep.
+ */
+void gpiod_set_raw_array(unsigned int array_size,
+			 struct gpio_desc **desc_array, int *value_array)
+{
+	if (!desc_array)
+		return;
+	if (!desc_array[0])
+		return;
+	/* Should be using gpiod_set_raw_array_cansleep() */
+	WARN_ON(desc_array[0]->chip->can_sleep);
+	_gpiod_set_raw_array(array_size, desc_array, value_array);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_raw_array);
+
+/**
  * gpiod_cansleep() - report whether gpio value access may sleep
  * @desc: gpio to check
  *
@@ -2559,6 +2654,27 @@ void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
 
 /**
+ * gpiod_set_raw_array_cansleep() - assign values to an array of GPIOs
+ * @desc_array: array of GPIO descriptors whose values will be assigned
+ * @value_array: array of values to assign
+ *
+ * Set the raw values of the GPIOs, i.e. the values of the physical lines
+ * without regard for their ACTIVE_LOW status.
+ *
+ * This function is to be called from contexts that can sleep.
+ */
+void gpiod_set_raw_array_cansleep(unsigned int array_size,
+				  struct gpio_desc **desc_array,
+				  int *value_array)
+{
+	might_sleep_if(extra_checks);
+	if (!desc_array)
+		return;
+	_gpiod_set_raw_array(array_size, desc_array, value_array);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_raw_array_cansleep);
+
+/**
  * gpiod_add_lookup_table() - register GPIO device consumers
  * @table: table of consumers to register
  */
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index bed128e..1d0bab3 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -42,12 +42,17 @@ int gpiod_get_value(const struct gpio_desc *desc);
 void gpiod_set_value(struct gpio_desc *desc, int value);
 int gpiod_get_raw_value(const struct gpio_desc *desc);
 void gpiod_set_raw_value(struct gpio_desc *desc, int value);
+void gpiod_set_raw_array(unsigned int array_size,
+			 struct gpio_desc **desc_array, int *value_array);
 
 /* Value get/set from sleeping context */
 int gpiod_get_value_cansleep(const struct gpio_desc *desc);
 void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
 int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
 void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
+void gpiod_set_raw_array_cansleep(unsigned int array_size,
+				  struct gpio_desc **desc_array,
+				  int *value_array);
 
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
 
@@ -150,6 +155,12 @@ static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
+void gpiod_set_raw_array(unsigned int array_size,
+			 struct gpio_desc **desc_array, int *value_array)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+}
 
 static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc)
 {
@@ -174,6 +185,13 @@ static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
+void gpiod_set_raw_array_cansleep(unsigned int array_size,
+				  struct gpio_desc **desc_array,
+				  int *value_array)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(1);
+}
 
 static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
 {
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 1827b43..d7968c8 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -32,6 +32,7 @@ struct seq_file;
  * @get: returns value for signal "offset"; for output signals this
  *	returns either the value actually sensed, or zero
  * @set: assigns output value for signal "offset"
+ * @set_multiple: assigns output values for multiple signals defined by "mask"
  * @set_debounce: optional hook for setting debounce time for specified gpio in
  *      interrupt triggered gpio chips
  * @to_irq: optional hook supporting non-static gpio_to_irq() mappings;
@@ -84,6 +85,8 @@ struct gpio_chip {
 						unsigned offset);
 	void			(*set)(struct gpio_chip *chip,
 						unsigned offset, int value);
+	void			(*set_multiple)(struct gpio_chip *chip,
+						u32 *mask, u32 *bits);
 	int			(*set_debounce)(struct gpio_chip *chip,
 						unsigned offset,
 						unsigned debounce);

--
1.8.5.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread