Hello Haibo Chen! Setting GPIOD_ACTIVE_LOW has no effect. It filtered out by dm_gpio_set_dir_flags(). > > > if (bit) > - dm_gpio_set_dir_flags(pin, GPIOD_IS_IN); > + dm_gpio_set_dir_flags(pin, GPIOD_IS_IN | > + GPIOD_ACTIVE_LOW);
Here in original code GPIOD_ACTIVE_LOW has not effect. else dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE); This code actually fix problem with your DTS-settings: > + return !dm_gpio_get_value(pin); Can you debug and check using oscilloscope the code like this: static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit) { ulong flags; ulong active; if (bit) { dm_gpio_set_dir_flags(pin, GPIOD_IS_IN); } else { dm_gpio_get_flags(pin, &flags); active = (flags & GPIOD_ACTIVE_LOW) ? GPIOD_IS_OUT_ACTIVE : 0; dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | active); } } static int i2c_gpio_get_pin(struct gpio_desc *pin) { ulong flags; int value; value = dm_gpio_get_value(pin); return (flags & GPIOD_ACTIVE_LOW) ? !value : value; } > 10 февр. 2023 г., в 12:27, haibo.c...@nxp.com написал(а): > > From: Haibo Chen <haibo.c...@nxp.com> > > Current code use dm_gpio_get_value() to get SDA and SCL value, and the > value depends on the flag GPIOD_ACTIVE_LOW. When toggle SCL to wait > slave release SDA, the SDA are config as GPIOD_IS_IN, and whether contain > the GPIOD_ACTIVE_LOW depends on the DTS setting. Usually, for I2C GPIO, > we use GPIOD_ACTIVE_LOW flag in DTS, so if the SDA is in low level, then > dm_gpio_get_value() will return 1, current code logic will stop the SCL > toggle wrongly, cause the I2C deblock not work as expect. > > This patch force set the GPIOD_ACTIVE_LOW for both GPIOD_IS_IN and > GPIOD_IS_OUT, and make the return value of i2c_gpio_get_pin() eaqual to > the physical voltage logic level. > > Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock") > Signed-off-by: Haibo Chen <haibo.c...@nxp.com> > --- > drivers/i2c/i2c-uclass.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c > index 8d9a89ed89..4dc707c659 100644 > --- a/drivers/i2c/i2c-uclass.c > +++ b/drivers/i2c/i2c-uclass.c > @@ -505,7 +505,8 @@ uint i2c_get_chip_addr_offset_mask(struct udevice *dev) > static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit) > { > if (bit) > - dm_gpio_set_dir_flags(pin, GPIOD_IS_IN); > + dm_gpio_set_dir_flags(pin, GPIOD_IS_IN | > + GPIOD_ACTIVE_LOW); > else > dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | > GPIOD_ACTIVE_LOW | > @@ -514,7 +515,7 @@ static void i2c_gpio_set_pin(struct gpio_desc *pin, int > bit) > > static int i2c_gpio_get_pin(struct gpio_desc *pin) > { > - return dm_gpio_get_value(pin); > + return !dm_gpio_get_value(pin); > } > > int i2c_deblock_gpio_loop(struct gpio_desc *sda_pin, > -- > 2.34.1 >