Re: [PATCH] gpio: exar set value handling for hw with gpio pull-up or pull-down

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

 



Hi all,

I worked with Sai on this.

We discovered this problem when we upgraded our kernel from 5.10 to 6.1.
The behavior changed with the switch to regmap in 5.11.

commit 36fb7218e87833b17e3042e77f3b102c75129e8f
Author: Bartosz Golaszewski <brgl@xxxxxxxx>
Date:   Mon Sep 28 17:00:26 2020 +0200

    gpio: exar: switch to using regmap

    We can simplify the code in gpio-exar by using regmap. This allows
    us to drop the mutex (regmap provides its own locking) and we can
    also reuse regmap's bit operations instead of implementing our own
    update function.

    Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
    Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>


We noticed because we had some gpios tied to reset pins and when
we set direction to high, value went to 0, and put devices in reset.

Thanks,
Matthew


On 7/30/24 08:46, Sai Kumar Cholleti wrote:
Setting gpio direction = high, sometimes results in gpio value = 0.

If a gpio is pulled high, the following construction results in the
value being 0 when the desired value is 1:

$ echo "high" > /sys/class/gpio/gpio336/direction
$ cat /sys/class/gpio/gpio336/value
0

Before the gpio direction is changed from input to output,
exar_set_value is set to 1, but since direction is input when
exar_set_value is called, _regmap_update_bits reads a 1 due to an
external pull-up.  When force_write is not set (regmap_set_bits has
force_write = false), the value is not written.  When the direction is
then changed, the gpio becomes an output with the value of 0 (the
hardware default).

regmap_write_bits sets force_write = true, so the value is always
written by exar_set_value and an external pull-up doesn't affect the
outcome of setting direction = high.


The same can happen when a gpio is pulled low, but the scenario is a
little more complicated.

$ echo high > /sys/class/gpio/gpio351/direction
$ cat /sys/class/gpio/gpio351/value
1

$ echo in > /sys/class/gpio/gpio351/direction
$ cat /sys/class/gpio/gpio351/value
0

$ echo low > /sys/class/gpio/gpio351/direction
$ cat /sys/class/gpio/gpio351/value
1

Signed-off-by: Sai Kumar Cholleti <skmr537@xxxxxxxxx>
---
  drivers/gpio/gpio-exar.c | 10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 482f678c893e..de5ce73159cb 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -99,11 +99,13 @@ static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
  	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
  	unsigned int addr = exar_offset_to_lvl_addr(exar_gpio, offset);
  	unsigned int bit = exar_offset_to_bit(exar_gpio, offset);
+	unsigned int bit_value = value ? BIT(bit) : 0;
- if (value)
-		regmap_set_bits(exar_gpio->regmap, addr, BIT(bit));
-	else
-		regmap_clear_bits(exar_gpio->regmap, addr, BIT(bit));
+	/*
+	 * regmap_write_bits forces value to be written when an external
+	 * pull up/down might otherwise indicate value was already set
+	 */
+	regmap_write_bits(exar_gpio->regmap, addr, BIT(bit), bit_value);
  }
static int exar_direction_output(struct gpio_chip *chip, unsigned int offset,





[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux