Hi
On 3/22/23 12:26, haibo.chen@nxp.com wrote:
From: Haibo Chen haibo.chen@nxp.com
dm_gpio_set_dir_flags() will clear GPIOD_MASK_DIR and set new flags. But there are cases like i2c_deblock_gpio_loop() will do like this:
-first conifg GPIO(SDA) output with GPIOD_ACTIVE_LOW dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE);
-then config GPIO input dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
-then get the GPIO input value: dm_gpio_get_value(pin);
When config the GPIO input, only set GPIOD_IS_IN, but unfortunately since the previous GPIOD_ACTIVE_LOW is not cleared, still keep in flags, make the value from dm_gpio_get_value() not logic correct.
So add GPIOD_ACTIVE_LOW into GPIOD_MASK_DIR to avoid this issue.
Signed-off-by: Haibo Chen haibo.chen@nxp.com
include/asm-generic/gpio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index dd0bdf2315..903b237aac 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -131,7 +131,7 @@ struct gpio_desc {
/* Flags for updating the above */ #define GPIOD_MASK_DIR (GPIOD_IS_OUT | GPIOD_IS_IN | \
GPIOD_IS_OUT_ACTIVE)
GPIOD_IS_OUT_ACTIVE | GPIOD_ACTIVE_LOW)
I think you are breaking the management of GPIOD_ACTIVE_LOW, provided by device tree in the GPIO uclass:
because the modified GPIOD_MASK_DIR is used in other location....
normally GPIOD_ACTIVE_LOW is saved in desc->flags
it is the "desciptor flags" and must be not cleary by normal API
see gpio_xlate_offs_flags() => gpio_flags_xlate()
For example in gpio_request_tail(), in the line:
/* Keep any direction flags provided by the devicetree */ ret = dm_gpio_set_dir_flags(desc, flags | (desc->flags& GPIOD_MASK_DIR)); With your patch the descriptor flags is cleared / so DT information is lost. For me GPIOD_ACTIVE_LOW must be managed carefully to avoid side effect. and if you inverse the PIN logical in device tree (GPIOD_ACTIVE_LOW) it is normal to inverse it for INPUT and OUTPUT it is managed in GPIO U-Class => dm_gpio_set_dir_flagsshould not cleared this flag GPIOD_ACTIVE_LOW you can change the caller ? static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit) { if (bit) dm_gpio_set_dir_flags(pin, GPIOD_IS_IN); else dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE); } =>
static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit) { if (bit) dm_gpio_set_dir_flags(pin, GPIOD_IS_IN); else dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT); }
The output value is the same => GPIOD_ACTIVE_LOW and GPIOD_IS_OUT_ACTIVE not active but you don't need to modify GPIOD_ACTIVE_LOW outside the GPIO uclass. Patrick