Re: [PATCH] i2c: correct I2C deblock logic
> If use this code, must make sure dts use GPIO_ACTIVE_LOW, otherwise
> dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE) will config
> gpio output high, this is not what we want.
Patrik told that if we don’t have GPIO_ACTIVE_LOW in the DTS, this is the case
where hardware has some sort of
signal inversion inside. May be there is no such hardware now.
When we set gpio output active, it became high inside hardware, hardware set
i2c line low.
If you think that previous version of my today’s patch is more correct, try it.
>
> And here use !dm_gpio_get_value(pin), this already hit use GPIO_ACTIVE_LOW,
> why not directly use GPIOD_ACTIVE_LOW
> when call dm_gpio_set_dir_flags, this make code more readable, and make sure
> the code logic can work and do not depends
> on dts setting.
Patric told, that we must not use GPIOD_ACTIVE_LOW in client code. So you must
drop it. From any patch you send.
GPIOD_ACTIVE_LOW is not applied to gpio flags when you call
dm_gpio_set_dir_flags(). It is configured from DTS.
You think it is applied, but actually not. It’s your finding. Here your words:
> i2c_gpio_set_pin-> dm_gpio_set_dir_flags-> dm_gpio_clrset_flags, it only
> clear GPIOD_MASK_DIR, this has no impact with GPIOD_ACTIVE_LOW. So this
> GPIOD_ACTIVE_LOW keep in flags.
> 23 марта 2023 г., в 13:41, Bough Chen написал(а):
>
>> -Original Message-
>> From: Alexander Kochetkov mailto:al.koc...@gmail.com>>
>> Sent: 2023年3月23日 17:34
>> To: Bough Chen mailto:haibo.c...@nxp.com>>
>> Cc: h...@denx.de <mailto:h...@denx.de>; ma...@denx.de
>> <mailto:ma...@denx.de>; u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>;
>> dl-uboot-imx
>> mailto:uboot-...@nxp.com>>; xypron.g...@gmx.de
>> <mailto:xypron.g...@gmx.de>
>> Subject: Re: [PATCH] i2c: correct I2C deblock logic
>>
>> Or even simpler. Like your original patch. If we take into accout Patrik’s
>> comment from another message:
>>
>>> but if I assume that GPIO_ACTIVE_HIGH is NOT activated in DT
>>
>> 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_IS_OUT_ACTIVE);
>> }
>> }
>>
>> static int i2c_gpio_get_pin(struct gpio_desc *pin) {
>>return !dm_gpio_get_value(pin);
>> }
>
>
>
> And here use !dm_gpio_get_value(pin), this already hit use GPIO_ACTIVE_LOW,
> why not directly use GPIOD_ACTIVE_LOW
> when call dm_gpio_set_dir_flags, this make code more readable, and make sure
> the code logic can work and do not depends
> on dts setting.
>
> Best Regards
> Haibo Chen
>>
>>
>>> 23 марта 2023 г., в 11:43, Alexander Kochetkov
>> написал(а):
>>>
>>> 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
>>>>
>>>> Current code use dm_gpio_get_value() to get SDA and SCL value, and
>>>
RE: [PATCH] i2c: correct I2C deblock logic
> -Original Message-
> From: Alexander Kochetkov
> Sent: 2023年3月23日 17:34
> To: Bough Chen
> Cc: h...@denx.de; ma...@denx.de; u-boot@lists.denx.de; dl-uboot-imx
> ; xypron.g...@gmx.de
> Subject: Re: [PATCH] i2c: correct I2C deblock logic
>
> Or even simpler. Like your original patch. If we take into accout Patrik’s
> comment from another message:
>
> > but if I assume that GPIO_ACTIVE_HIGH is NOT activated in DT
>
> 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_IS_OUT_ACTIVE);
>}
> }
>
> static int i2c_gpio_get_pin(struct gpio_desc *pin) {
> return !dm_gpio_get_value(pin);
> }
If use this code, must make sure dts use GPIO_ACTIVE_LOW, otherwise
dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE) will config gpio
output high, this is not what we want.
And here use !dm_gpio_get_value(pin), this already hit use GPIO_ACTIVE_LOW,
why not directly use GPIOD_ACTIVE_LOW
when call dm_gpio_set_dir_flags, this make code more readable, and make sure
the code logic can work and do not depends
on dts setting.
Best Regards
Haibo Chen
>
>
> > 23 марта 2023 г., в 11:43, Alexander Kochetkov
> написал(а):
> >
> > 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
> >>
> >> 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
> >> ---
> >> 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
> >>
> >
Re: [PATCH] i2c: correct I2C deblock logic
Or even simpler. Like your original patch. If we take into accout Patrik’s
comment from another message:
> but if I assume that GPIO_ACTIVE_HIGH is NOT activated in DT
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_IS_OUT_ACTIVE);
}
}
static int i2c_gpio_get_pin(struct gpio_desc *pin)
{
return !dm_gpio_get_value(pin);
}
> 23 марта 2023 г., в 11:43, Alexander Kochetkov
> написал(а):
>
> 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
>>
>> 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
>> ---
>> 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
>>
>
Re: [PATCH] i2c: correct I2C deblock logic
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
>
> 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
> ---
> 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
>
RE: [PATCH] i2c: correct I2C deblock logic
> -Original Message- > From: Alexander Kochetkov > Sent: 2023年3月21日 17:50 > To: Bough Chen > Cc: h...@denx.de; ma...@denx.de; u-boot@lists.denx.de; dl-uboot-imx > ; xypron.g...@gmx.de; Simon Glass > > Subject: Re: [PATCH] i2c: correct I2C deblock logic > > Hi Haibo. > > Nice catch! Agree with you patch. But may be fix problem in general? > I suggest to add GPIOD_ACTIVE_LOW to the GPIOD_MASK_DIR mask to fix > problems 1, 2, 4. > Not sure if that logically correct. What do you think? Hi Alexander I will have a try, and test on my side. Best Regards Haibo Chen > > I grepped the code for ‘dm_gpio_set_dir_flags’ and found another places > affected by the bug. > > 1. u-boot/drivers/spi/mxc_spi.c: > ret = dm_gpio_set_dir_flags(&mxcs->cs_gpios[i], > GPIOD_IS_OUT | > GPIOD_ACTIVE_LOW); > > Here GPIOD_ACTIVE_LOW is passed to dm_gpio_set_dir_flags() and filtered out > by dm_gpio_set_dir_flags(). > GPIOD_ACTIVE_LOW can be added to GPIOD_MASK_DIR mask to fix that. > > 2. u-boot/drivers/spi/mvebu_a3700_spi.c: > ret = dm_gpio_set_dir_flags(&plat->cs_gpios[i], > GPIOD_IS_OUT | > GPIOD_ACTIVE_LOW); Same as 1. > > 3. u-boot/drivers/phy/allwinner/phy-sun4i-usb.c: > ret = dm_gpio_set_dir_flags(&phy->gpio_id_det, > GPIOD_IS_IN | > GPIOD_PULL_UP); > > Here GPIOD_PULL_UP is passed to dm_gpio_set_dir_flags() and filtered out by > dm_gpio_set_dir_flags(). > Looks, like we have to introduce dm_gpio_set_pull_flags() and use it together > with dm_gpio_set_dir_flags(). > > 4. u-boot/drivers/i2c/i2c-uclass.c > dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | > > GPIOD_ACTIVE_LOW | > > GPIOD_IS_OUT_ACTIVE); Same as 1. Your patch cover that bug. > > > > > > 21 марта 2023 г., в 11:37, Bough Chen > написал(а): > > > >> -Original Message- > >> From: Alexander Kochetkov > >> Sent: 2023年3月20日 16:03 > >> To: h...@denx.de > >> Cc: Bough Chen ; ma...@denx.de; > >> u-boot@lists.denx.de; dl-uboot-imx ; > >> xypron.g...@gmx.de; Simon Glass > >> Subject: Re: [PATCH] i2c: correct I2C deblock logic > >> > >> Hello! > >> > >> The patch doesn’t add new functionality to the code. > >> May be it makes code more readable. But in later case the patch > >> description should be corrected and Fixes tag removed. > > > > Hi Alexander, > > > > This patch not only make the code more readable, but also really fix a bug. > > In our current code, we will call i2c_gpio_set_pin(scl_pin, 1) and then > i2c_gpio_set_pin(scl_pin, 0). > > After the i2c_gpio_set_pin(scl_pin, 0), the flags contain GPIOD_ACTIVE_LOW. > > Though for SDA, it will first call i2c_gpio_set_pin(sda_pin, 0) then > > i2c_gpio_set_pin(sda_pin, 1), but i2c_gpio_set_pin-> dm_gpio_set_dir_flags-> > dm_gpio_clrset_flags, it only clear GPIOD_MASK_DIR, this has no impact with > GPIOD_ACTIVE_LOW. So this GPIOD_ACTIVE_LOW keep in flags. > > Then call i2c_gpio_get_pin by using dm_gpio_get_value(pin) will get > > the wrong data, let the function i2c_deblock_gpio_loop always return > > -EREMOTEIO > > > > Best Regards > > Haibo Chen > > > >> > >> The flag GPIOD_ACTIVE_LOW affects return value dm_gpio_get_value(). > >> And return value doesn’t depends on the DTS settings. It depends on > >> the flag passed to dm_gpio_set_dir_flags(). Usually the code parses > >> DTS and provide the correct flag to the dm_gpio_set_dir_flags(). > >> > >> So the patch adds flag GPIOD_ACTIVE_LOW to the > >> dm_gpio_set_dir_flags() and as expected this negates the return value > >> of dm_gpio_get_value(). So in order to keep the code correct the > >> patch also fixes adds negotiation to dm_gpio_get_value(). > >> > >> Alexander. > >> > >> > >> > >>> 20 марта 2023 г., в 10:44, Heiko Schocher написал(а): > >>> > >>> Hi! > >>> > >>> On 13.03.23 03:55, Bough Chen wrote: > >>>>> -Original Message- > >>>>> From: Bough Chen > >>>>> Sent: 2023年2月10日 17:27 > >>>>> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de > >>>>> Cc: u-boot@lists.denx.de; dl-uboot-imx ; > >>>>> xypron.g...@gmx.de; Bough Chen > >>>>> Subject: [PATCH] i2c: correct I2C deblock logic > >>>>> > >>>>>
Re: [PATCH] i2c: correct I2C deblock logic
Hi Haibo. Nice catch! Agree with you patch. But may be fix problem in general? I suggest to add GPIOD_ACTIVE_LOW to the GPIOD_MASK_DIR mask to fix problems 1, 2, 4. Not sure if that logically correct. What do you think? I grepped the code for ‘dm_gpio_set_dir_flags’ and found another places affected by the bug. 1. u-boot/drivers/spi/mxc_spi.c: ret = dm_gpio_set_dir_flags(&mxcs->cs_gpios[i], GPIOD_IS_OUT | GPIOD_ACTIVE_LOW); Here GPIOD_ACTIVE_LOW is passed to dm_gpio_set_dir_flags() and filtered out by dm_gpio_set_dir_flags(). GPIOD_ACTIVE_LOW can be added to GPIOD_MASK_DIR mask to fix that. 2. u-boot/drivers/spi/mvebu_a3700_spi.c: ret = dm_gpio_set_dir_flags(&plat->cs_gpios[i], GPIOD_IS_OUT | GPIOD_ACTIVE_LOW); Same as 1. 3. u-boot/drivers/phy/allwinner/phy-sun4i-usb.c: ret = dm_gpio_set_dir_flags(&phy->gpio_id_det, GPIOD_IS_IN | GPIOD_PULL_UP); Here GPIOD_PULL_UP is passed to dm_gpio_set_dir_flags() and filtered out by dm_gpio_set_dir_flags(). Looks, like we have to introduce dm_gpio_set_pull_flags() and use it together with dm_gpio_set_dir_flags(). 4. u-boot/drivers/i2c/i2c-uclass.c dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT | GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE); Same as 1. Your patch cover that bug. > 21 марта 2023 г., в 11:37, Bough Chen написал(а): > >> -Original Message- >> From: Alexander Kochetkov >> Sent: 2023年3月20日 16:03 >> To: h...@denx.de >> Cc: Bough Chen ; ma...@denx.de; >> u-boot@lists.denx.de; dl-uboot-imx ; >> xypron.g...@gmx.de; Simon Glass >> Subject: Re: [PATCH] i2c: correct I2C deblock logic >> >> Hello! >> >> The patch doesn’t add new functionality to the code. >> May be it makes code more readable. But in later case the patch description >> should be corrected and Fixes tag removed. > > Hi Alexander, > > This patch not only make the code more readable, but also really fix a bug. > In our current code, we will call i2c_gpio_set_pin(scl_pin, 1) and then > i2c_gpio_set_pin(scl_pin, 0). > After the i2c_gpio_set_pin(scl_pin, 0), the flags contain GPIOD_ACTIVE_LOW. > Though for SDA, it will first call i2c_gpio_set_pin(sda_pin, 0) then > i2c_gpio_set_pin(sda_pin, 1), but > i2c_gpio_set_pin-> dm_gpio_set_dir_flags-> dm_gpio_clrset_flags, it only > clear GPIOD_MASK_DIR, this has no impact with GPIOD_ACTIVE_LOW. So this > GPIOD_ACTIVE_LOW keep in flags. > Then call i2c_gpio_get_pin by using dm_gpio_get_value(pin) will get the wrong > data, let the function i2c_deblock_gpio_loop always return -EREMOTEIO > > Best Regards > Haibo Chen > >> >> The flag GPIOD_ACTIVE_LOW affects return value dm_gpio_get_value(). >> And return value doesn’t depends on the DTS settings. It depends on the flag >> passed to dm_gpio_set_dir_flags(). Usually the code parses DTS and provide >> the >> correct flag to the dm_gpio_set_dir_flags(). >> >> So the patch adds flag GPIOD_ACTIVE_LOW to the dm_gpio_set_dir_flags() and >> as expected this negates the return value of dm_gpio_get_value(). So in >> order to >> keep the code correct the patch also fixes adds negotiation to >> dm_gpio_get_value(). >> >> Alexander. >> >> >> >>> 20 марта 2023 г., в 10:44, Heiko Schocher написал(а): >>> >>> Hi! >>> >>> On 13.03.23 03:55, Bough Chen wrote: >>>>> -Original Message- >>>>> From: Bough Chen >>>>> Sent: 2023年2月10日 17:27 >>>>> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de >>>>> Cc: u-boot@lists.denx.de; dl-uboot-imx ; >>>>> xypron.g...@gmx.de; Bough Chen >>>>> Subject: [PATCH] i2c: correct I2C deblock logic >>>>> >>>>> From: Haibo Chen >>>>> >>>>> 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 GP
RE: [PATCH] i2c: correct I2C deblock logic
> -Original Message-
> From: Alexander Kochetkov
> Sent: 2023年3月20日 16:03
> To: h...@denx.de
> Cc: Bough Chen ; ma...@denx.de;
> u-boot@lists.denx.de; dl-uboot-imx ;
> xypron.g...@gmx.de; Simon Glass
> Subject: Re: [PATCH] i2c: correct I2C deblock logic
>
> Hello!
>
> The patch doesn’t add new functionality to the code.
> May be it makes code more readable. But in later case the patch description
> should be corrected and Fixes tag removed.
Hi Alexander,
This patch not only make the code more readable, but also really fix a bug.
In our current code, we will call i2c_gpio_set_pin(scl_pin, 1) and then
i2c_gpio_set_pin(scl_pin, 0).
After the i2c_gpio_set_pin(scl_pin, 0), the flags contain GPIOD_ACTIVE_LOW.
Though for SDA, it will first call i2c_gpio_set_pin(sda_pin, 0) then
i2c_gpio_set_pin(sda_pin, 1), but
i2c_gpio_set_pin-> dm_gpio_set_dir_flags-> dm_gpio_clrset_flags, it only clear
GPIOD_MASK_DIR, this has no impact with GPIOD_ACTIVE_LOW. So this
GPIOD_ACTIVE_LOW keep in flags.
Then call i2c_gpio_get_pin by using dm_gpio_get_value(pin) will get the wrong
data, let the function i2c_deblock_gpio_loop always return -EREMOTEIO
Best Regards
Haibo Chen
>
> The flag GPIOD_ACTIVE_LOW affects return value dm_gpio_get_value().
> And return value doesn’t depends on the DTS settings. It depends on the flag
> passed to dm_gpio_set_dir_flags(). Usually the code parses DTS and provide the
> correct flag to the dm_gpio_set_dir_flags().
>
> So the patch adds flag GPIOD_ACTIVE_LOW to the dm_gpio_set_dir_flags() and
> as expected this negates the return value of dm_gpio_get_value(). So in order
> to
> keep the code correct the patch also fixes adds negotiation to
> dm_gpio_get_value().
>
> Alexander.
>
>
>
> > 20 марта 2023 г., в 10:44, Heiko Schocher написал(а):
> >
> > Hi!
> >
> > On 13.03.23 03:55, Bough Chen wrote:
> >>> -Original Message-
> >>> From: Bough Chen
> >>> Sent: 2023年2月10日 17:27
> >>> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de
> >>> Cc: u-boot@lists.denx.de; dl-uboot-imx ;
> >>> xypron.g...@gmx.de; Bough Chen
> >>> Subject: [PATCH] i2c: correct I2C deblock logic
> >>>
> >>> From: Haibo Chen
> >>>
> >>> 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.
> >>>
> >>
> >> Hi,
> >>
> >> Any comments for this patch, just a reminder in case you miss it.
> >
> > Indeed, I missed this patch, sorry!
> > Your explanation sounds reasonable to me, but I wonder if nobody
> > tapped into this problem... it would be good to have some test reports
> > from others! Also added Simon, as he did a lot of work in this space.
> >
> > Thanks!
> >
> > bye,
> > Heiko
> >>
> >> Best Regards
> >> Haibo Chen
> >>
> >>> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
> >>> Signed-off-by: Haibo Chen
> >>> ---
> >>> 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
> >>
> >
> > --
> > DENX Software Engineering GmbH, Managing Director: Erika Unter
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [PATCH] i2c: correct I2C deblock logic
Hello!
The patch doesn’t add new functionality to the code.
May be it makes code more readable. But in later case the patch description
should be corrected and
Fixes tag removed.
The flag GPIOD_ACTIVE_LOW affects return value dm_gpio_get_value().
And return value doesn’t depends on the DTS settings. It depends on
the flag passed to dm_gpio_set_dir_flags(). Usually the code parses DTS and
provide the correct flag to the dm_gpio_set_dir_flags().
So the patch adds flag GPIOD_ACTIVE_LOW to the dm_gpio_set_dir_flags() and
as expected this negates the return value of dm_gpio_get_value(). So in
order to keep the code correct the patch also fixes adds negotiation to
dm_gpio_get_value().
Alexander.
> 20 марта 2023 г., в 10:44, Heiko Schocher написал(а):
>
> Hi!
>
> On 13.03.23 03:55, Bough Chen wrote:
>>> -Original Message-
>>> From: Bough Chen
>>> Sent: 2023年2月10日 17:27
>>> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de
>>> Cc: u-boot@lists.denx.de; dl-uboot-imx ;
>>> xypron.g...@gmx.de; Bough Chen
>>> Subject: [PATCH] i2c: correct I2C deblock logic
>>>
>>> From: Haibo Chen
>>>
>>> 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.
>>>
>>
>> Hi,
>>
>> Any comments for this patch, just a reminder in case you miss it.
>
> Indeed, I missed this patch, sorry!
> Your explanation sounds reasonable to me, but I wonder if nobody tapped
> into this problem... it would be good to have some test reports
> from others! Also added Simon, as he did a lot of work in this space.
>
> Thanks!
>
> bye,
> Heiko
>>
>> Best Regards
>> Haibo Chen
>>
>>> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
>>> Signed-off-by: Haibo Chen
>>> ---
>>> 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
>>
>
> --
> DENX Software Engineering GmbH, Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [PATCH] i2c: correct I2C deblock logic
Hi!
On 13.03.23 03:55, Bough Chen wrote:
>> -Original Message-
>> From: Bough Chen
>> Sent: 2023年2月10日 17:27
>> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de
>> Cc: u-boot@lists.denx.de; dl-uboot-imx ;
>> xypron.g...@gmx.de; Bough Chen
>> Subject: [PATCH] i2c: correct I2C deblock logic
>>
>> From: Haibo Chen
>>
>> 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.
>>
>
> Hi,
>
> Any comments for this patch, just a reminder in case you miss it.
Indeed, I missed this patch, sorry!
Your explanation sounds reasonable to me, but I wonder if nobody tapped
into this problem... it would be good to have some test reports
from others! Also added Simon, as he did a lot of work in this space.
Thanks!
bye,
Heiko
>
> Best Regards
> Haibo Chen
>
>> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
>> Signed-off-by: Haibo Chen
>> ---
>> 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
>
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
RE: [PATCH] i2c: correct I2C deblock logic
> -Original Message-
> From: Bough Chen
> Sent: 2023年2月10日 17:27
> To: h...@denx.de; al.koc...@gmail.com; ma...@denx.de
> Cc: u-boot@lists.denx.de; dl-uboot-imx ;
> xypron.g...@gmx.de; Bough Chen
> Subject: [PATCH] i2c: correct I2C deblock logic
>
> From: Haibo Chen
>
> 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.
>
Hi,
Any comments for this patch, just a reminder in case you miss it.
Best Regards
Haibo Chen
> Fixes: aa54192d4a87 ("dm: i2c: implement gpio-based I2C deblock")
> Signed-off-by: Haibo Chen
> ---
> 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
[PATCH] i2c: correct I2C deblock logic
From: Haibo Chen
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
---
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
