diff mbox series

[v3,1/6] gpio: regmap: Add quirk for output data register

Message ID be5ffefa007ee4ebd7d4cec88f5f2fb7cd5b689e.1621809029.git.sander@svanheule.net
State New
Headers show
Series [v3,1/6] gpio: regmap: Add quirk for output data register | expand

Commit Message

Sander Vanheule May 23, 2021, 10:33 p.m. UTC 
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
GPIO_REGMAP_QUIRK_SET_DIRECTION_FIRST flag in regmap_config.quirks.

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
 drivers/gpio/gpio-regmap.c  | 15 +++++++++++++--
 include/linux/gpio/regmap.h | 13 +++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

Comments

Michael Walle May 28, 2021, 6:40 a.m. UTC | #1 
Am 2021-05-24 00:33, 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
> GPIO_REGMAP_QUIRK_SET_DIRECTION_FIRST flag in regmap_config.quirks.

Nice! If this is really needed:

Reviewed-by: Michael Walle <michael@walle.cc>

-michael
Bartosz Golaszewski May 31, 2021, 7:25 a.m. UTC | #2 
On Mon, May 24, 2021 at 12:34 AM Sander Vanheule <sander@svanheule.net> wrote:
>
> 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
> GPIO_REGMAP_QUIRK_SET_DIRECTION_FIRST flag in regmap_config.quirks.
>
> Signed-off-by: Sander Vanheule <sander@svanheule.net>
> ---

Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Sander Vanheule June 3, 2021, 10:03 a.m. UTC | #3 
Hi Michael, Bartosz,

On Fri, 2021-05-28 at 08:40 +0200, Michael Walle wrote:
> Am 2021-05-24 00:33, 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
> > GPIO_REGMAP_QUIRK_SET_DIRECTION_FIRST flag in regmap_config.quirks.
> 
> Nice! If this is really needed:
> 
> Reviewed-by: Michael Walle <michael@walle.cc>

Looks like the quirk won't be needed for this series, but I can always resubmit
it separately if needed.


Best,
Sander
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 134cedf151a7..95553734e169 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -15,6 +15,7 @@  struct gpio_regmap {
 	struct device *parent;
 	struct regmap *regmap;
 	struct gpio_chip gpio_chip;
+	unsigned int quirks;
 
 	int reg_stride;
 	int ngpio_per_reg;
@@ -173,9 +174,18 @@  static int gpio_regmap_direction_input(struct gpio_chip *chip,
 static int gpio_regmap_direction_output(struct gpio_chip *chip,
 					unsigned int offset, int value)
 {
-	gpio_regmap_set(chip, offset, value);
+	struct gpio_regmap *gpio = gpiochip_get_data(chip);
+	int ret;
+
+	if (gpio->quirks & GPIO_REGMAP_QUIRK_SET_DIRECTION_FIRST) {
+		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 gpio_regmap_set_direction(chip, offset, true);
+	return ret;
 }
 
 void gpio_regmap_set_drvdata(struct gpio_regmap *gpio, void *data)
@@ -227,6 +237,7 @@  struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 
 	gpio->parent = config->parent;
 	gpio->regmap = config->regmap;
+	gpio->quirks = config->quirks;
 	gpio->ngpio_per_reg = config->ngpio_per_reg;
 	gpio->reg_stride = config->reg_stride;
 	gpio->reg_mask_xlate = config->reg_mask_xlate;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index 334dd928042b..cb609489903e 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -12,6 +12,17 @@  struct regmap;
 #define GPIO_REGMAP_ADDR_ZERO ((unsigned int)(-1))
 #define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
 
+enum gpio_regmap_quirk {
+	/*
+	 * For hardware where the pin output value cannot be set while the pin
+	 * is configured as an input. Resolve by setting the direction to
+	 * output first, and the new value second. Because the previous output
+	 * value is used immediately after the direction change, this may result
+	 * in glitches.
+	 */
+	GPIO_REGMAP_QUIRK_SET_DIRECTION_FIRST = BIT(0),
+};
+
 /**
  * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
  * @parent:		The parent device
@@ -31,6 +42,7 @@  struct regmap;
  * @reg_stride:		(Optional) May be set if the registers (of the
  *			same type, dat, set, etc) are not consecutive.
  * @ngpio_per_reg:	Number of GPIOs per register
+ * @quirks:		Flags indicating GPIO chip hardware issues
  * @irq_domain:		(Optional) IRQ domain if the controller is
  *			interrupt-capable
  * @reg_mask_xlate:     (Optional) Translates base address and GPIO
@@ -73,6 +85,7 @@  struct gpio_regmap_config {
 	unsigned int reg_dir_out_base;
 	int reg_stride;
 	int ngpio_per_reg;
+	unsigned int quirks;
 	struct irq_domain *irq_domain;
 
 	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,