* [PATCH v2] gpio: exar: set value when external pull-up or pull-down is present [not found] <ZyjLSmtwyPiD9-r5@black.fi.intel.com> @ 2024-11-04 15:47 ` Sai Kumar Cholleti 2024-11-04 18:56 ` Andy Shevchenko 0 siblings, 1 reply; 9+ messages in thread From: Sai Kumar Cholleti @ 2024-11-04 15:47 UTC (permalink / raw) To: andriy.shevchenko, bgolaszewski, linux-gpio, mmcclain, skmr537; +Cc: stable 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 an input to an output, exar_set_value() is called with value = 1, but since the GPIO is an input when exar_set_value() is called, _regmap_update_bits() reads a 1 due to an external pull-up. regmap_set_bits() sets force_write = false, so the value (1) 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 Fixes: 36fb7218e878 ("gpio: exar: switch to using regmap") Signed-off-by: Sai Kumar Cholleti <skmr537@gmail.com> Signed-off-by: Matthew McClain <mmcclain@noprivs.com> Cc: <stable@vger.kernel.org> --- 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 5170fe7599cd..dfc7a9ca3e62 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, -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] gpio: exar: set value when external pull-up or pull-down is present 2024-11-04 15:47 ` [PATCH v2] gpio: exar: set value when external pull-up or pull-down is present Sai Kumar Cholleti @ 2024-11-04 18:56 ` Andy Shevchenko 2024-11-05 7:15 ` [PATCH v3] " Sai Kumar Cholleti 0 siblings, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2024-11-04 18:56 UTC (permalink / raw) To: Sai Kumar Cholleti; +Cc: bgolaszewski, linux-gpio, mmcclain, stable On Mon, Nov 04, 2024 at 09:17:57PM +0530, 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 an input to an output, > exar_set_value() is called with value = 1, but since the GPIO is an > input when exar_set_value() is called, _regmap_update_bits() reads a 1 > due to an external pull-up. regmap_set_bits() sets force_write = > false, so the value (1) 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 This... > $ cat /sys/class/gpio/gpio351/value > 1 > > $ echo in > /sys/class/gpio/gpio351/direction ...this... > $ cat /sys/class/gpio/gpio351/value > 0 > > $ echo low > /sys/class/gpio/gpio351/direction ...this... > $ cat /sys/class/gpio/gpio351/value > 1 > Fixes: 36fb7218e878 ("gpio: exar: switch to using regmap") ...and this lines have a trailing space. > Signed-off-by: Sai Kumar Cholleti <skmr537@gmail.com> > Signed-off-by: Matthew McClain <mmcclain@noprivs.com> Hmm... Missing Co-developed-by? This SoB chain puzzles me a bit, the committer should go last AFAIR. ... > + /* > + * regmap_write_bits forces value to be written when an external regmap_write_bits() > + * pull up/down might otherwise indicate value was already set Missing period at the end. > + */ ... Other than above LGTM. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] gpio: exar: set value when external pull-up or pull-down is present 2024-11-04 18:56 ` Andy Shevchenko @ 2024-11-05 7:15 ` Sai Kumar Cholleti 2024-11-05 14:39 ` Andy Shevchenko 2024-11-21 8:10 ` Bartosz Golaszewski 0 siblings, 2 replies; 9+ messages in thread From: Sai Kumar Cholleti @ 2024-11-05 7:15 UTC (permalink / raw) To: andriy.shevchenko, bgolaszewski, linux-gpio, mmcclain, skmr537; +Cc: stable 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 an input to an output, exar_set_value() is called with value = 1, but since the GPIO is an input when exar_set_value() is called, _regmap_update_bits() reads a 1 due to an external pull-up. regmap_set_bits() sets force_write = false, so the value (1) 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 Fixes: 36fb7218e878 ("gpio: exar: switch to using regmap") Co-developed-by: Matthew McClain <mmcclain@noprivs.com> Signed-off-by: Matthew McClain <mmcclain@noprivs.com> Signed-off-by: Sai Kumar Cholleti <skmr537@gmail.com> Cc: <stable@vger.kernel.org> --- 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 5170fe7599cd..d5909a4f0433 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, -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] gpio: exar: set value when external pull-up or pull-down is present 2024-11-05 7:15 ` [PATCH v3] " Sai Kumar Cholleti @ 2024-11-05 14:39 ` Andy Shevchenko 2024-11-12 15:30 ` Andy Shevchenko 2024-11-21 8:10 ` Bartosz Golaszewski 1 sibling, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2024-11-05 14:39 UTC (permalink / raw) To: Sai Kumar Cholleti; +Cc: bgolaszewski, linux-gpio, mmcclain, stable On Tue, Nov 05, 2024 at 12:45:23PM +0530, 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 an input to an output, > exar_set_value() is called with value = 1, but since the GPIO is an > input when exar_set_value() is called, _regmap_update_bits() reads a 1 > due to an external pull-up. regmap_set_bits() sets force_write = > false, so the value (1) 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 Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] gpio: exar: set value when external pull-up or pull-down is present 2024-11-05 14:39 ` Andy Shevchenko @ 2024-11-12 15:30 ` Andy Shevchenko 2024-11-18 11:00 ` Bartosz Golaszewski 0 siblings, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2024-11-12 15:30 UTC (permalink / raw) To: Sai Kumar Cholleti; +Cc: bgolaszewski, linux-gpio, mmcclain, stable On Tue, Nov 05, 2024 at 04:39:38PM +0200, Andy Shevchenko wrote: > On Tue, Nov 05, 2024 at 12:45:23PM +0530, 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 an input to an output, > > exar_set_value() is called with value = 1, but since the GPIO is an > > input when exar_set_value() is called, _regmap_update_bits() reads a 1 > > due to an external pull-up. regmap_set_bits() sets force_write = > > false, so the value (1) 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 > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Does this need to be applied, Bart? Seems it is missed in your branches... -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] gpio: exar: set value when external pull-up or pull-down is present 2024-11-12 15:30 ` Andy Shevchenko @ 2024-11-18 11:00 ` Bartosz Golaszewski 2024-11-18 12:34 ` Andy Shevchenko 0 siblings, 1 reply; 9+ messages in thread From: Bartosz Golaszewski @ 2024-11-18 11:00 UTC (permalink / raw) To: Andy Shevchenko Cc: Sai Kumar Cholleti, bgolaszewski, linux-gpio, mmcclain, stable On Tue, Nov 12, 2024 at 5:09 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Nov 05, 2024 at 04:39:38PM +0200, Andy Shevchenko wrote: > > On Tue, Nov 05, 2024 at 12:45:23PM +0530, 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 an input to an output, > > > exar_set_value() is called with value = 1, but since the GPIO is an > > > input when exar_set_value() is called, _regmap_update_bits() reads a 1 > > > due to an external pull-up. regmap_set_bits() sets force_write = > > > false, so the value (1) 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 > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Does this need to be applied, Bart? > Seems it is missed in your branches... > Maybe if the author used get_maintainers.pl as they should, I would have noticed this earlier? I have some other fixes to pick up so I'll send this later in the merge window. Bart ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] gpio: exar: set value when external pull-up or pull-down is present 2024-11-18 11:00 ` Bartosz Golaszewski @ 2024-11-18 12:34 ` Andy Shevchenko 2024-11-19 13:50 ` sai kumar 0 siblings, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2024-11-18 12:34 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Sai Kumar Cholleti, bgolaszewski, linux-gpio, mmcclain, stable On Mon, Nov 18, 2024 at 12:00:00PM +0100, Bartosz Golaszewski wrote: > On Tue, Nov 12, 2024 at 5:09 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Nov 05, 2024 at 04:39:38PM +0200, Andy Shevchenko wrote: > > > On Tue, Nov 05, 2024 at 12:45:23PM +0530, Sai Kumar Cholleti wrote: ... > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Does this need to be applied, Bart? > > Seems it is missed in your branches... > > Maybe if the author used get_maintainers.pl as they should, I would > have noticed this earlier? Ah good catch! Sai, FYI, I use my script [1] which does all required stuff for me. Feel free to use it, patch, comment, etc... > I have some other fixes to pick up so I'll send this later in the merge window. Thanks! [1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] gpio: exar: set value when external pull-up or pull-down is present 2024-11-18 12:34 ` Andy Shevchenko @ 2024-11-19 13:50 ` sai kumar 0 siblings, 0 replies; 9+ messages in thread From: sai kumar @ 2024-11-19 13:50 UTC (permalink / raw) To: Andy Shevchenko Cc: Bartosz Golaszewski, bgolaszewski, linux-gpio, mmcclain, stable Thanks Andy and Bart. On Mon, Nov 18, 2024 at 6:05 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Nov 18, 2024 at 12:00:00PM +0100, Bartosz Golaszewski wrote: > > On Tue, Nov 12, 2024 at 5:09 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Tue, Nov 05, 2024 at 04:39:38PM +0200, Andy Shevchenko wrote: > > > > On Tue, Nov 05, 2024 at 12:45:23PM +0530, Sai Kumar Cholleti wrote: > > ... > > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > > > Does this need to be applied, Bart? > > > Seems it is missed in your branches... > > > > Maybe if the author used get_maintainers.pl as they should, I would > > have noticed this earlier? > > Ah good catch! > > Sai, FYI, I use my script [1] which does all required stuff for me. > Feel free to use it, patch, comment, etc... > > > I have some other fixes to pick up so I'll send this later in the merge window. > > Thanks! > > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] gpio: exar: set value when external pull-up or pull-down is present 2024-11-05 7:15 ` [PATCH v3] " Sai Kumar Cholleti 2024-11-05 14:39 ` Andy Shevchenko @ 2024-11-21 8:10 ` Bartosz Golaszewski 1 sibling, 0 replies; 9+ messages in thread From: Bartosz Golaszewski @ 2024-11-21 8:10 UTC (permalink / raw) To: andriy.shevchenko, linux-gpio, mmcclain, Bartosz Golaszewski, Sai Kumar Cholleti Cc: Bartosz Golaszewski, stable From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Tue, 05 Nov 2024 12:45:23 +0530, 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 > > [...] Applied, thanks! [1/1] gpio: exar: set value when external pull-up or pull-down is present commit: 72cef64180de04a7b055b4773c138d78f4ebdb77 Best regards, -- Bartosz Golaszewski <bartosz.golaszewski@linaro.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-11-21 8:10 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <ZyjLSmtwyPiD9-r5@black.fi.intel.com>
2024-11-04 15:47 ` [PATCH v2] gpio: exar: set value when external pull-up or pull-down is present Sai Kumar Cholleti
2024-11-04 18:56 ` Andy Shevchenko
2024-11-05 7:15 ` [PATCH v3] " Sai Kumar Cholleti
2024-11-05 14:39 ` Andy Shevchenko
2024-11-12 15:30 ` Andy Shevchenko
2024-11-18 11:00 ` Bartosz Golaszewski
2024-11-18 12:34 ` Andy Shevchenko
2024-11-19 13:50 ` sai kumar
2024-11-21 8:10 ` Bartosz Golaszewski
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).