Re: [PATCH v2 2/7] gpio: regmap: Add configurable dir/value order

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Am 2021-05-17 21:28, schrieb Sander Vanheule:
GPIO chips may not support setting the output value when a pin is
configured as an input, although the current implementation assumes this
is always possible.

Add support for setting pin direction before value. The order defaults
to setting the value first, but this can be reversed by setting the
regmap_config.no_set_on_input flag, similar to the corresponding flag in
the gpio-mmio driver.

Signed-off-by: Sander Vanheule <sander@xxxxxxxxxxxxx>
---
 drivers/gpio/gpio-regmap.c  | 20 +++++++++++++++++---
 include/linux/gpio/regmap.h |  3 +++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 134cedf151a7..1cdb20f8f8b4 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -170,14 +170,25 @@ static int gpio_regmap_direction_input(struct
gpio_chip *chip,
 	return gpio_regmap_set_direction(chip, offset, false);
 }

-static int gpio_regmap_direction_output(struct gpio_chip *chip,
-					unsigned int offset, int value)
+static int gpio_regmap_dir_out_val_first(struct gpio_chip *chip,
+					 unsigned int offset, int value)

Can we leave the name as is? TBH I find these two similar names
super confusing. Maybe its just me, though.

 {
 	gpio_regmap_set(chip, offset, value);

 	return gpio_regmap_set_direction(chip, offset, true);
 }

+static int gpio_regmap_dir_out_dir_first(struct gpio_chip *chip,
+					 unsigned int offset, int value)
+{
+	int err;

use ret for consistency here

+
+	err = gpio_regmap_set_direction(chip, offset, true);
+	gpio_regmap_set(chip, offset, value);
+
+	return err;
+}
+

Instead of adding a new one, we can also just check no_set_on_input
in gpio_regmap_direction_output(), which I'd prefer.

static int gpio_regmap_direction_output(struct gpio_chip *chip,
					unsigned int offset, int value)
{
	struct gpio_regmap *gpio = gpiochip_get_data(chip);
	int ret;

	if (gpio->no_set_on_input) {
		/* some smart comment here, also mention gliches */
		ret = gpio_regmap_set_direction(chip, offset, true);
		gpio_regmap_set(chip, offset, value);
	} else {
		gpio_regmap_set(chip, offset, value);
		ret = gpio_regmap_set_direction(chip, offset, true);
	}

	return ret;
}

 void gpio_regmap_set_drvdata(struct gpio_regmap *gpio, void *data)
 {
 	gpio->driver_data = data;
@@ -277,7 +288,10 @@ struct gpio_regmap *gpio_regmap_register(const
struct gpio_regmap_config *config
 	if (gpio->reg_dir_in_base || gpio->reg_dir_out_base) {
 		chip->get_direction = gio_regmap_get_direction;
 		chip->direction_input = gpio_regmap_direction_input;
-		chip->direction_output = gpio_regmap_direction_output;
+		if (config->no_set_on_input)
+			chip->direction_output = gpio_regmap_dir_out_dir_first;
+		else
+			chip->direction_output = gpio_regmap_dir_out_val_first;
 	}

 	ret = gpiochip_add_data(chip, gpio);
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index 334dd928042b..2a732f8f23be 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -30,6 +30,8 @@ struct regmap;
  * @reg_dir_out_base:	(Optional) out setting register base address
  * @reg_stride:		(Optional) May be set if the registers (of the
  *			same type, dat, set, etc) are not consecutive.
+ * @no_set_on_input: Set if output value can only be set when the direction
+ *			is configured as output.

set_direction_first ?

  * @ngpio_per_reg:	Number of GPIOs per register
  * @irq_domain:		(Optional) IRQ domain if the controller is
  *			interrupt-capable
@@ -73,6 +75,7 @@ struct gpio_regmap_config {
 	unsigned int reg_dir_out_base;
 	int reg_stride;
 	int ngpio_per_reg;
+	bool no_set_on_input;
 	struct irq_domain *irq_domain;

 	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,

-michael



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux