Greetings,

This series addresses a limitation in the I2C bus recovery mechanism when 
dealing with certain open-drain GPIO configurations where the direction 
cannot be automatically detected.

Jie Li (2):
  i2c: core: add "force-set-sda" flag for open-drain SDA recovery
  dt-bindings: i2c: add force-set-sda property

 .../devicetree/bindings/i2c/i2c-gpio.yaml     |  7 +++++++
 drivers/i2c/i2c-core-base.c                   | 21 ++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

-- 
2.43.0

Hi Jie,

thanks for your patch!

On Wed, Jan 14, 2026 at 3:13 PM Jie Li <lj29312931@gmail.com> wrote:

> This series addresses a limitation in the I2C bus recovery mechanism when
> dealing with certain open-drain GPIO configurations where the direction
> cannot be automatically detected.

I'm sorry but I don't understand the premise. How can we even get here?

So the mechanism is about I2C that is using a regular I2C block, and
the pins get re-muxed to GPIO to drive recovery using the I2C
core GPIO-mode recovery mechanism with bridge->sda_gpiod
which is retrieved in the core from "sda" which in DT is
sda-gpios = <....>; (calong with similarly named SCL) for
GPIO-mode recovery.

So if that is set in an input mode, such as during devm_gpiod_get()
reading the initial direction of the line,
so gpiod_get_direction(bri->sda_gpiod) == 1.
this patch set will go and write output values to the line
*anyway* because "it works".

This is how I understand the patch set.

In which scenario do you have a device tree where you can add
"force-set-sda" to a DT node, but you *can't* just fix up the
SCL/SDA flags like this:

#include <dt-bindings/gpio/gpio.h>

sda-gpios = <&gpio0 5 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
scl-gpios = <&gpio0 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

?

We should possibly also enforce it from the I2C recovery core,
for SDA we are currently doing:

gpiod = devm_gpiod_get(dev, "sda", GPIOD_IN);

what happens if you patch i2c-core-base.c to simply do:

gpiod = devm_gpiod_get(dev, "sda", GPIOD_OUT_HIGH_OPEN_DRAIN);

(Based on SDA resting polarity being high.)
I'm more uncertain about that one because I don't know exactly
how hardware behaves in response to this, but can you test this
first if you have to hack around in the core?

Yours,
Linus Walleij

Dear Linus,

Thank you for your feedback and the insightful suggestion regarding
GPIO_OPEN_DRAIN.

I have analyzed the current implementation of gpiod_get_direction() in
the kernel, and I believe that relying solely on standard GPIO flags
cannot resolve the "deadlock" on this specific hardware.

The issue lies in how gpiod_get_direction() interacts with certain
open-drain controllers. As seen in the source code:

Even if FLAG_OPEN_DRAIN is set, the function falls back to
gc->get_direction() if the FLAG_IS_OUT bit hasn't been established
yet. Crucially, some ASICs do not even implement a readable direction
bit in hardware.

In many true open-drain hardware implementations, a line driven "high"
(high-impedance) is physically reported as an Input by the hardware
register.

Consequently, gc->get_direction() returns 1 (Input), and the following
assign_bit(FLAG_IS_OUT, &desc->flags, !ret) explicitly clears the
output flag in the kernel's descriptor.

This creates a logic loop in i2c_init_recovery():

The I2C core queries the direction via gpiod_get_direction().

The function returns 1 because the line is currently high/floating or
the hardware lacks direction reporting.

The I2C core then assumes the pin is "Input-only" and skips the
assignment of bri->set_sda.

Bus recovery becomes impossible even though the hardware is fully
capable of driving the line low.

Regarding the suggestion to use GPIOD_OUT_HIGH_OPEN_DRAIN in the I2C
core: I am concerned that forcing the line to "Output" globally in the
core might be too aggressive for all platforms. My proposed
force-set-sda property provides a safe, explicit way for a specific
board to say: "I know this pin reports as Input, but it is safe to
treat it as an Output for recovery."

I believe this explicit opt-in mechanism is more robust than relying
on an automatic detection that is fundamentally tied to the
instantaneous state of a high-impedance line.

What do you think about this perspective?

Best regards,
Jie Li


On Thu, Jan 15, 2026 at 10:27 AM Linus Walleij <linusw@kernel.org> wrote:
>
> Hi Jie,
>
> thanks for your patch!
>
> On Wed, Jan 14, 2026 at 3:13 PM Jie Li <lj29312931@gmail.com> wrote:
>
> > This series addresses a limitation in the I2C bus recovery mechanism when
> > dealing with certain open-drain GPIO configurations where the direction
> > cannot be automatically detected.
>
> I'm sorry but I don't understand the premise. How can we even get here?
>
> So the mechanism is about I2C that is using a regular I2C block, and
> the pins get re-muxed to GPIO to drive recovery using the I2C
> core GPIO-mode recovery mechanism with bridge->sda_gpiod
> which is retrieved in the core from "sda" which in DT is
> sda-gpios = <....>; (calong with similarly named SCL) for
> GPIO-mode recovery.
>
> So if that is set in an input mode, such as during devm_gpiod_get()
> reading the initial direction of the line,
> so gpiod_get_direction(bri->sda_gpiod) == 1.
> this patch set will go and write output values to the line
> *anyway* because "it works".
>
> This is how I understand the patch set.
>
> In which scenario do you have a device tree where you can add
> "force-set-sda" to a DT node, but you *can't* just fix up the
> SCL/SDA flags like this:
>
> #include <dt-bindings/gpio/gpio.h>
>
> sda-gpios = <&gpio0 5 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> scl-gpios = <&gpio0 6 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
>
> ?
>
> We should possibly also enforce it from the I2C recovery core,
> for SDA we are currently doing:
>
> gpiod = devm_gpiod_get(dev, "sda", GPIOD_IN);
>
> what happens if you patch i2c-core-base.c to simply do:
>
> gpiod = devm_gpiod_get(dev, "sda", GPIOD_OUT_HIGH_OPEN_DRAIN);
>
> (Based on SDA resting polarity being high.)
> I'm more uncertain about that one because I don't know exactly
> how hardware behaves in response to this, but can you test this
> first if you have to hack around in the core?
>
> Yours,
> Linus Walleij

Hi Jie,

On Thu, Jan 15, 2026 at 2:13 PM 李杰 <lj29312931@gmail.com> wrote:

> Even if FLAG_OPEN_DRAIN is set, the function falls back to
> gc->get_direction() if the FLAG_IS_OUT bit hasn't been established
> yet. Crucially, some ASICs do not even implement a readable direction
> bit in hardware.
>
> In many true open-drain hardware implementations, a line driven "high"
> (high-impedance) is physically reported as an Input by the hardware
> register.

If this is the actual problem, then this is a Linux problem and not something
that should be solved by adding new flags to the OS-neutral device tree.
So we can immediately stop trying to add stuff to device tree for this.

What you would have to do is to augment the driver framework and
code in Linux to deal with open drain modes better.

> The function returns 1 because the line is currently high/floating or
> the hardware lacks direction reporting.
>
> The I2C core then assumes the pin is "Input-only" and skips the
> assignment of bri->set_sda.
>
> Bus recovery becomes impossible even though the hardware is fully
> capable of driving the line low.

So you need to think about what the framework needs to provide
for the I2C recovery code to realize this line is open drain and
can actually be driven high and low.

You can't just rely on gpiod_get_direction() to be the only thing
that will ever be provided just because it looks like that today.

For example: if <linux/gpio/consumer.h> would provide
gpiod_is_single_ended() (meaning it is open drain or open source)
I think you could use this instead of special "force" flags.

So implement something like that in the gpiolib code and
headers instead. This avoid hacks like random DT flags.

If the right open drain or open source flags are set on the line
in the device tree then the gpiolib knows this and then you know
you can also actively drive these lines and all you need is
a way to query it.

Yours,
Linus Walleij

Greetings,

Apologies for the late reply, as things have been a bit hectic at work lately.

Thank you very much for the guidance and the suggestion to move the logic into
gpiolib. This is a far better approach than my initial one. As this is my first
time submitting code to the Linux community, I am very grateful for your
mentorship and support.

This series (v2) addresses a limitation in the I2C bus recovery 
mechanism where certain open-drain GPIOs are incorrectly identified 
as input-only, preventing the recovery logic from functioning.

Following the suggestion from Linus Walleij, this version drops the 
previously proposed "force-set-sda" DT property. Instead, it 
introduces a generic helper in the GPIO subsystem to identify 
single-ended configurations. This allows the I2C core to reliably 
enable recovery for open-drain lines regardless of the 
instantaneous hardware direction reporting.

Changes in v2:
- Replaced DT-based "force-set-sda" with a gpiolib helper.
- Added gpiod_is_single_ended() to drivers/gpio/gpiolib.c.
- Updated i2c-core-base.c to use the new helper.

Jie Li (2):
  gpiolib: add gpiod_is_single_ended() helper
  i2c: core: support recovery for single-ended GPIOs

 drivers/gpio/gpiolib.c        | 22 ++++++++++++++++++++++
 drivers/i2c/i2c-core-base.c   |  3 ++-
 include/linux/gpio/consumer.h |  5 +++++
 3 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.43.0

The direction of a single-ended (open-drain or open-source) GPIO line
cannot always be reliably determined by reading hardware registers.
In true open-drain implementations, the "high" state is achieved by
entering a high-impedance mode, which many hardware controllers report
as "input" even if the software intends to use it as an output.

This creates issues for consumer drivers (like I2C) that rely on
gpiod_get_direction() to decide if a line can be driven.

Introduce gpiod_is_single_ended() to allow consumers to check the
software configuration (GPIO_FLAG_OPEN_DRAIN/GPIO_FLAG_OPEN_SOURCE) of
a descriptor. This provides a robust way to identify lines that are
capable of being driven, regardless of their instantaneous hardware state.

Signed-off-by: Jie Li <jie.i.li@nokia.com>
---
 drivers/gpio/gpiolib.c        | 22 ++++++++++++++++++++++
 include/linux/gpio/consumer.h |  5 +++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1578cf3a8c74..96c34bf65c7e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -486,6 +486,28 @@ int gpiod_get_direction(struct gpio_desc *desc)
 }
 EXPORT_SYMBOL_GPL(gpiod_get_direction);
 
+/**
+ * gpiod_is_single_ended - check if the GPIO is configured as single-ended
+ * @desc: the GPIO descriptor to check
+ *
+ * Returns true if the GPIO is configured as either Open Drain or Open Source.
+ * In these modes, the direction of the line cannot always be reliably
+ * determined by reading hardware registers, as the "off" state (High-Z)
+ * is physically indistinguishable from an input state.
+ */
+int gpiod_is_single_ended(struct gpio_desc *desc)
+{
+	if (!desc)
+		return 0;
+
+	if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &desc->flags) ||
+		test_bit(GPIOD_FLAG_OPEN_SOURCE, &desc->flags))
+		return 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpiod_is_single_ended);
+
 /*
  * Add a new chip to the global chips list, keeping the list of chips sorted
  * by range(means [base, base + ngpio - 1]) order.
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index cafeb7a40ad1..c5847c8f66fe 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -109,6 +109,7 @@ void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc);
 void devm_gpiod_put_array(struct device *dev, struct gpio_descs *descs);
 
 int gpiod_get_direction(struct gpio_desc *desc);
+int gpiod_is_single_ended(struct gpio_desc *desc);
 int gpiod_direction_input(struct gpio_desc *desc);
 int gpiod_direction_output(struct gpio_desc *desc, int value);
 int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
@@ -335,6 +336,10 @@ static inline int gpiod_get_direction(const struct gpio_desc *desc)
 	WARN_ON(desc);
 	return -ENOSYS;
 }
+static inline int gpiod_is_single_ended(struct gpio_desc *desc)
+{
+	return 0;
+}
 static inline int gpiod_direction_input(struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-- 
2.43.0

On Sun, Jan 25, 2026 at 8:51 PM Jie Li <lj29312931@gmail.com> wrote:

> The direction of a single-ended (open-drain or open-source) GPIO line
> cannot always be reliably determined by reading hardware registers.
> In true open-drain implementations, the "high" state is achieved by
> entering a high-impedance mode, which many hardware controllers report
> as "input" even if the software intends to use it as an output.
>
> This creates issues for consumer drivers (like I2C) that rely on
> gpiod_get_direction() to decide if a line can be driven.
>
> Introduce gpiod_is_single_ended() to allow consumers to check the
> software configuration (GPIO_FLAG_OPEN_DRAIN/GPIO_FLAG_OPEN_SOURCE) of
> a descriptor. This provides a robust way to identify lines that are
> capable of being driven, regardless of their instantaneous hardware state.
>
> Signed-off-by: Jie Li <jie.i.li@nokia.com>

This makes sense!

> +/**
> + * gpiod_is_single_ended - check if the GPIO is configured as single-ended
> + * @desc: the GPIO descriptor to check
> + *
> + * Returns true if the GPIO is configured as either Open Drain or Open Source.
> + * In these modes, the direction of the line cannot always be reliably
> + * determined by reading hardware registers, as the "off" state (High-Z)
> + * is physically indistinguishable from an input state.
> + */
> +int gpiod_is_single_ended(struct gpio_desc *desc)

Switch to bool
bool gpiod_is_single_ended(struct gpio_desc *desc)

> +{
> +       if (!desc)
> +               return 0;

return false;

> +
> +       if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &desc->flags) ||
> +               test_bit(GPIOD_FLAG_OPEN_SOURCE, &desc->flags))
> +               return 1;

return true;

> +
> +       return 0;

return false;


> +static inline int gpiod_is_single_ended(struct gpio_desc *desc)
> +{
> +       return 0;
> +}

bool

return false;

Yours,
Linus Walleij

Currently, i2c_init_recovery() only assigns the set_sda/set_scl
hooks if gpiod_get_direction() returns 0 (output).

This logic fails on certain SoC controllers where open-drain lines
in a high-impedance state are physically reported as inputs. This
leads to a "deadlock" where the I2C core refuses to assign the
recovery hooks because it incorrectly assumes the pins are
input-only, even though they are fully capable of driving the bus
low for recovery.

Update the recovery initialization to use the new
gpiod_is_single_ended() helper. If a GPIO is configured as
open-drain or open-source in the firmware, it is safe to assume
it can be used for bus recovery, even if the current hardware
direction is reported as input.

Signed-off-by: Jie Li <jie.i.li@nokia.com>
---
 drivers/i2c/i2c-core-base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ae7e9c8b65a6..11bd801418e8 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -446,7 +446,8 @@ static int i2c_init_recovery(struct i2c_adapter *adap)
 		if (bri->sda_gpiod) {
 			bri->get_sda = get_sda_gpio_value;
 			/* FIXME: add proper flag instead of '0' once available */
-			if (gpiod_get_direction(bri->sda_gpiod) == 0)
+			if (gpiod_get_direction(bri->sda_gpiod) == 0 ||
+				gpiod_is_single_ended(bri->sda_gpiod))
 				bri->set_sda = set_sda_gpio_value;
 		}
 	} else if (bri->recover_bus == i2c_generic_scl_recovery) {
-- 
2.43.0

On Sun, Jan 25, 2026 at 8:51 PM Jie Li <lj29312931@gmail.com> wrote:

> Currently, i2c_init_recovery() only assigns the set_sda/set_scl
> hooks if gpiod_get_direction() returns 0 (output).
>
> This logic fails on certain SoC controllers where open-drain lines
> in a high-impedance state are physically reported as inputs. This
> leads to a "deadlock" where the I2C core refuses to assign the
> recovery hooks because it incorrectly assumes the pins are
> input-only, even though they are fully capable of driving the bus
> low for recovery.
>
> Update the recovery initialization to use the new
> gpiod_is_single_ended() helper. If a GPIO is configured as
> open-drain or open-source in the firmware, it is safe to assume
> it can be used for bus recovery, even if the current hardware
> direction is reported as input.
>
> Signed-off-by: Jie Li <jie.i.li@nokia.com>

This looks good!
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Greetings,

Apologies for the delay in responding.

Thank you very much for your review and the specific guidance regarding 
the return types. I really appreciate your patience and time spent 
guiding me through my first contribution to the kernel.

This series (v3) updates the helper function to use the 'bool' type as 
suggested and includes the Reviewed-by tags.

This series addresses a limitation in the I2C bus recovery mechanism where 
certain open-drain GPIOs are incorrectly identified as input-only, 
preventing the recovery logic from functioning.

Following the suggestion from Linus Walleij, this version drops the 
previously proposed "force-set-sda" DT property. Instead, it 
introduces a generic helper in the GPIO subsystem to identify 
single-ended configurations. This allows the I2C core to reliably 
enable recovery for open-drain lines regardless of the 
instantaneous hardware direction reporting.

Changes in v3:
- Patch 1:
  - Changed return type of gpiod_is_single_ended() from int to bool.
  - Updated return values from 0/1 to false/true.
  - Added Reviewed-by: Linus Walleij.
- Patch 2:
  - Added Reviewed-by: Linus Walleij.

Changes in v2:
- Replaced DT-based "force-set-sda" with a gpiolib helper.
- Added gpiod_is_single_ended() to drivers/gpio/gpiolib.c.
- Updated i2c-core-base.c to use the new helper.

Jie Li (2):
  gpiolib: add gpiod_is_single_ended() helper
  i2c: core: support recovery for single-ended GPIOs

 drivers/gpio/gpiolib.c        | 22 ++++++++++++++++++++++
 drivers/i2c/i2c-core-base.c   |  3 ++-
 include/linux/gpio/consumer.h |  5 +++++
 3 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.43.0

The direction of a single-ended (open-drain or open-source) GPIO line
cannot always be reliably determined by reading hardware registers.
In true open-drain implementations, the "high" state is achieved by
entering a high-impedance mode, which many hardware controllers report
as "input" even if the software intends to use it as an output.

This creates issues for consumer drivers (like I2C) that rely on
gpiod_get_direction() to decide if a line can be driven.

Introduce gpiod_is_single_ended() to allow consumers to check the
software configuration (GPIO_FLAG_OPEN_DRAIN/GPIO_FLAG_OPEN_SOURCE) of
a descriptor. This provides a robust way to identify lines that are
capable of being driven, regardless of their instantaneous hardware state.

Signed-off-by: Jie Li <jie.i.li@nokia.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
---
 drivers/gpio/gpiolib.c        | 22 ++++++++++++++++++++++
 include/linux/gpio/consumer.h |  5 +++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1578cf3a8c74..08e6960053f8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -486,6 +486,28 @@ int gpiod_get_direction(struct gpio_desc *desc)
 }
 EXPORT_SYMBOL_GPL(gpiod_get_direction);
 
+/**
+ * gpiod_is_single_ended - check if the GPIO is configured as single-ended
+ * @desc: the GPIO descriptor to check
+ *
+ * Returns true if the GPIO is configured as either Open Drain or Open Source.
+ * In these modes, the direction of the line cannot always be reliably
+ * determined by reading hardware registers, as the "off" state (High-Z)
+ * is physically indistinguishable from an input state.
+ */
+bool gpiod_is_single_ended(struct gpio_desc *desc)
+{
+	if (!desc)
+		return false;
+
+	if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &desc->flags) ||
+		test_bit(GPIOD_FLAG_OPEN_SOURCE, &desc->flags))
+		return true;
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(gpiod_is_single_ended);
+
 /*
  * Add a new chip to the global chips list, keeping the list of chips sorted
  * by range(means [base, base + ngpio - 1]) order.
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index cafeb7a40ad1..12ef6e07ee1a 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -109,6 +109,7 @@ void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc);
 void devm_gpiod_put_array(struct device *dev, struct gpio_descs *descs);
 
 int gpiod_get_direction(struct gpio_desc *desc);
+bool gpiod_is_single_ended(struct gpio_desc *desc);
 int gpiod_direction_input(struct gpio_desc *desc);
 int gpiod_direction_output(struct gpio_desc *desc, int value);
 int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
@@ -335,6 +336,10 @@ static inline int gpiod_get_direction(const struct gpio_desc *desc)
 	WARN_ON(desc);
 	return -ENOSYS;
 }
+static inline bool gpiod_is_single_ended(struct gpio_desc *desc)
+{
+	return false;
+}
 static inline int gpiod_direction_input(struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-- 
2.43.0

Currently, i2c_init_recovery() only assigns the set_sda/set_scl
hooks if gpiod_get_direction() returns 0 (output).

This logic fails on certain SoC controllers where open-drain lines
in a high-impedance state are physically reported as inputs. This
leads to a "deadlock" where the I2C core refuses to assign the
recovery hooks because it incorrectly assumes the pins are
input-only, even though they are fully capable of driving the bus
low for recovery.

Update the recovery initialization to use the new
gpiod_is_single_ended() helper. If a GPIO is configured as
open-drain or open-source in the firmware, it is safe to assume
it can be used for bus recovery, even if the current hardware
direction is reported as input.

Signed-off-by: Jie Li <jie.i.li@nokia.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
---
 drivers/i2c/i2c-core-base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ae7e9c8b65a6..11bd801418e8 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -446,7 +446,8 @@ static int i2c_init_recovery(struct i2c_adapter *adap)
 		if (bri->sda_gpiod) {
 			bri->get_sda = get_sda_gpio_value;
 			/* FIXME: add proper flag instead of '0' once available */
-			if (gpiod_get_direction(bri->sda_gpiod) == 0)
+			if (gpiod_get_direction(bri->sda_gpiod) == 0 ||
+				gpiod_is_single_ended(bri->sda_gpiod))
 				bri->set_sda = set_sda_gpio_value;
 		}
 	} else if (bri->recover_bus == i2c_generic_scl_recovery) {
-- 
2.43.0