public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] i2c: add support for forced SDA recovery
@ 2026-01-14 14:13 Jie Li
  2026-01-14 14:13 ` [PATCH v1 1/2] i2c: core: add "force-set-sda" flag for open-drain SDA without "FLAG_IS_OUT" bit Jie Li
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Jie Li @ 2026-01-14 14:13 UTC (permalink / raw)
  To: wsa
  Cc: linux-i2c, robh, krzk+dt, conor+dt, devicetree, linus.walleij,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v1 1/2] i2c: core: add "force-set-sda" flag for open-drain SDA without "FLAG_IS_OUT" bit
  2026-01-14 14:13 [PATCH v1 0/2] i2c: add support for forced SDA recovery Jie Li
@ 2026-01-14 14:13 ` Jie Li
  2026-01-14 14:13 ` [PATCH v1 2/2] dt-bindings: i2c: add force-set-sda property Jie Li
  2026-01-15  9:27 ` [PATCH v1 0/2] i2c: add support for forced SDA recovery Linus Walleij
  2 siblings, 0 replies; 17+ messages in thread
From: Jie Li @ 2026-01-14 14:13 UTC (permalink / raw)
  To: wsa
  Cc: linux-i2c, robh, krzk+dt, conor+dt, devicetree, linus.walleij,
	linux-kernel, Jie Li

On certain specialized SoC platforms, the I2C SDA pin is physically
open-drain but lacks the "FLAG_IS_OUT" bit in the GPIO subsystem.
In such cases, the set_sda function isn't assigned, causing bus
recovery to fail.

This patch introduces a new optional pinctrl flag "force-set-sda".
When this flag is present in the device tree, the I2C recovery
mechanism will explicitly attempt to toggle the SDA line through
the pinctrl state, ensuring the bus can be freed even when the
default recovery logic is insufficient for this specific hardware
implementation.

This change is necessary to improve the robustness of I2C
communication on hardware where the SDA line can remain stuck
low and standard recovery fails.

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

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ae7e9c8b65a6..ffbab3e4528d 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -42,6 +42,9 @@
 #include <linux/rwsem.h>
 #include <linux/slab.h>
 #include <linux/string_choices.h>
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/fwnode.h>
 
 #include "i2c-core.h"
 
@@ -422,9 +425,25 @@ static int i2c_gpio_init_recovery(struct i2c_adapter *adap)
 	return i2c_gpio_init_generic_recovery(adap);
 }
 
+/* Check if SDA can be driven for recovery when
+ * GPIO direction reporting is unavailable.
+ * Usage: add new flag "force-set-sda" in dts pinctrl.
+ */
+static bool force_set_sda(struct device *dev)
+{
+	if (!dev || !dev->of_node)
+		return false;
+
+	if (of_property_read_bool(dev->of_node, "force-set-sda"))
+		return true;
+	else
+		return false;
+}
+
 static int i2c_init_recovery(struct i2c_adapter *adap)
 {
 	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+	struct device *dev = &adap->dev;
 	bool is_error_level = true;
 	char *err_str;
 
@@ -446,7 +465,7 @@ 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 || force_set_sda(dev))
 				bri->set_sda = set_sda_gpio_value;
 		}
 	} else if (bri->recover_bus == i2c_generic_scl_recovery) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 2/2] dt-bindings: i2c: add force-set-sda property
  2026-01-14 14:13 [PATCH v1 0/2] i2c: add support for forced SDA recovery Jie Li
  2026-01-14 14:13 ` [PATCH v1 1/2] i2c: core: add "force-set-sda" flag for open-drain SDA without "FLAG_IS_OUT" bit Jie Li
@ 2026-01-14 14:13 ` Jie Li
  2026-01-16 14:14   ` Krzysztof Kozlowski
  2026-01-15  9:27 ` [PATCH v1 0/2] i2c: add support for forced SDA recovery Linus Walleij
  2 siblings, 1 reply; 17+ messages in thread
From: Jie Li @ 2026-01-14 14:13 UTC (permalink / raw)
  To: wsa
  Cc: linux-i2c, robh, krzk+dt, conor+dt, devicetree, linus.walleij,
	linux-kernel, Jie Li

Document the new "force-set-sda" optional property.
This property is used for hardware where the SDA line is open-drain
but the standard driver-level check (like gpiod_get_direction) might
not correctly reflect the ability to drive the line for bus recovery.

Signed-off-by: Jie Li <jie.i.li@nokia.com>
---
 Documentation/devicetree/bindings/i2c/i2c-gpio.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
index afd4925c2a7d..82713fcf87e4 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.yaml
@@ -37,6 +37,13 @@ properties:
     description: sda as output only
     type: boolean
 
+  force-set-sda:
+    type: boolean
+    description:
+      Force the use of the SDA output toggle during I2C bus recovery.
+      This is needed for some hardware where the SDA pin is open-drain
+      but the GPIO subsystem cannot automatically detect its output capability.
+
   i2c-gpio,scl-output-only:
     description: scl as output only
     type: boolean
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 0/2] i2c: add support for forced SDA recovery
  2026-01-14 14:13 [PATCH v1 0/2] i2c: add support for forced SDA recovery Jie Li
  2026-01-14 14:13 ` [PATCH v1 1/2] i2c: core: add "force-set-sda" flag for open-drain SDA without "FLAG_IS_OUT" bit Jie Li
  2026-01-14 14:13 ` [PATCH v1 2/2] dt-bindings: i2c: add force-set-sda property Jie Li
@ 2026-01-15  9:27 ` Linus Walleij
  2026-01-15 13:12   ` 李杰
  2 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2026-01-15  9:27 UTC (permalink / raw)
  To: Jie Li
  Cc: wsa, linux-i2c, robh, krzk+dt, conor+dt, devicetree,
	linus.walleij, linux-kernel, Bartosz Golaszewski,
	Linux pin control

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 0/2] i2c: add support for forced SDA recovery
  2026-01-15  9:27 ` [PATCH v1 0/2] i2c: add support for forced SDA recovery Linus Walleij
@ 2026-01-15 13:12   ` 李杰
  2026-01-16 13:59     ` Linus Walleij
  0 siblings, 1 reply; 17+ messages in thread
From: 李杰 @ 2026-01-15 13:12 UTC (permalink / raw)
  To: Linus Walleij
  Cc: wsa, linux-i2c, robh, krzk+dt, conor+dt, devicetree,
	linus.walleij, linux-kernel, Bartosz Golaszewski,
	Linux pin control

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 0/2] i2c: add support for forced SDA recovery
  2026-01-15 13:12   ` 李杰
@ 2026-01-16 13:59     ` Linus Walleij
  2026-01-25 19:51       ` [PATCH v2 0/2] i2c: improve bus recovery for single-ended GPIOs Jie Li
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2026-01-16 13:59 UTC (permalink / raw)
  To: 李杰
  Cc: wsa, linux-i2c, robh, krzk+dt, conor+dt, devicetree,
	linus.walleij, linux-kernel, Bartosz Golaszewski,
	Linux pin control

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/2] dt-bindings: i2c: add force-set-sda property
  2026-01-14 14:13 ` [PATCH v1 2/2] dt-bindings: i2c: add force-set-sda property Jie Li
@ 2026-01-16 14:14   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-16 14:14 UTC (permalink / raw)
  To: Jie Li, wsa
  Cc: linux-i2c, robh, krzk+dt, conor+dt, devicetree, linus.walleij,
	linux-kernel, Jie Li

On 14/01/2026 15:13, Jie Li wrote:
> Document the new "force-set-sda" optional property.
> This property is used for hardware where the SDA line is open-drain
> but the standard driver-level check (like gpiod_get_direction) might
> not correctly reflect the ability to drive the line for bus recovery.

I think Linus explained well why this does not fit DT, so also formally
responding here - use proper GPIO flags, when SDA is open-drain.

> 
> Signed-off-by: Jie Li <jie.i.li@nokia.com>

This does not match From address. You need to always check your patches
with checkpatch.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 0/2] i2c: improve bus recovery for single-ended GPIOs
  2026-01-16 13:59     ` Linus Walleij
@ 2026-01-25 19:51       ` Jie Li
  2026-01-25 19:51         ` [PATCH v2 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
  2026-01-25 19:51         ` [PATCH v2 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
  0 siblings, 2 replies; 17+ messages in thread
From: Jie Li @ 2026-01-25 19:51 UTC (permalink / raw)
  To: wsa, linus.walleij; +Cc: linux-i2c, linux-gpio, linux-kernel, Jie Li

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/2] gpiolib: add gpiod_is_single_ended() helper
  2026-01-25 19:51       ` [PATCH v2 0/2] i2c: improve bus recovery for single-ended GPIOs Jie Li
@ 2026-01-25 19:51         ` Jie Li
  2026-01-27  9:46           ` Linus Walleij
  2026-01-25 19:51         ` [PATCH v2 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
  1 sibling, 1 reply; 17+ messages in thread
From: Jie Li @ 2026-01-25 19:51 UTC (permalink / raw)
  To: wsa, linus.walleij; +Cc: linux-i2c, linux-gpio, linux-kernel, Jie Li

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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/2] i2c: core: support recovery for single-ended GPIOs
  2026-01-25 19:51       ` [PATCH v2 0/2] i2c: improve bus recovery for single-ended GPIOs Jie Li
  2026-01-25 19:51         ` [PATCH v2 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
@ 2026-01-25 19:51         ` Jie Li
  2026-01-27  9:47           ` Linus Walleij
  1 sibling, 1 reply; 17+ messages in thread
From: Jie Li @ 2026-01-25 19:51 UTC (permalink / raw)
  To: wsa, linus.walleij; +Cc: linux-i2c, linux-gpio, linux-kernel, Jie Li

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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 1/2] gpiolib: add gpiod_is_single_ended() helper
  2026-01-25 19:51         ` [PATCH v2 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
@ 2026-01-27  9:46           ` Linus Walleij
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2026-01-27  9:46 UTC (permalink / raw)
  To: Jie Li; +Cc: wsa, linus.walleij, linux-i2c, linux-gpio, linux-kernel, Jie Li

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/2] i2c: core: support recovery for single-ended GPIOs
  2026-01-25 19:51         ` [PATCH v2 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
@ 2026-01-27  9:47           ` Linus Walleij
  2026-02-01 11:18             ` [PATCH v3 0/2] i2c: improve bus " Jie Li
  0 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2026-01-27  9:47 UTC (permalink / raw)
  To: Jie Li; +Cc: wsa, linus.walleij, linux-i2c, linux-gpio, linux-kernel, Jie Li

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 0/2] i2c: improve bus recovery for single-ended GPIOs
  2026-01-27  9:47           ` Linus Walleij
@ 2026-02-01 11:18             ` Jie Li
  2026-02-01 11:18               ` [PATCH v3 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
                                 ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Jie Li @ 2026-02-01 11:18 UTC (permalink / raw)
  To: wsa, linusw; +Cc: linux-i2c, linux-gpio, linux-kernel, Jie Li

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 1/2] gpiolib: add gpiod_is_single_ended() helper
  2026-02-01 11:18             ` [PATCH v3 0/2] i2c: improve bus " Jie Li
@ 2026-02-01 11:18               ` Jie Li
  2026-02-01 11:18               ` [PATCH v3 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
  2026-04-22 18:47               ` [PATCH v3 0/2] i2c: improve bus " 李杰
  2 siblings, 0 replies; 17+ messages in thread
From: Jie Li @ 2026-02-01 11:18 UTC (permalink / raw)
  To: wsa, linusw; +Cc: linux-i2c, linux-gpio, linux-kernel, Jie Li

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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 2/2] i2c: core: support recovery for single-ended GPIOs
  2026-02-01 11:18             ` [PATCH v3 0/2] i2c: improve bus " Jie Li
  2026-02-01 11:18               ` [PATCH v3 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
@ 2026-02-01 11:18               ` Jie Li
  2026-05-04 12:14                 ` Wolfram Sang
  2026-04-22 18:47               ` [PATCH v3 0/2] i2c: improve bus " 李杰
  2 siblings, 1 reply; 17+ messages in thread
From: Jie Li @ 2026-02-01 11:18 UTC (permalink / raw)
  To: wsa, linusw; +Cc: linux-i2c, linux-gpio, linux-kernel, Jie Li

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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 0/2] i2c: improve bus recovery for single-ended GPIOs
  2026-02-01 11:18             ` [PATCH v3 0/2] i2c: improve bus " Jie Li
  2026-02-01 11:18               ` [PATCH v3 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
  2026-02-01 11:18               ` [PATCH v3 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
@ 2026-04-22 18:47               ` 李杰
  2 siblings, 0 replies; 17+ messages in thread
From: 李杰 @ 2026-04-22 18:47 UTC (permalink / raw)
  To: wsa, linusw; +Cc: linux-i2c, linux-gpio, linux-kernel, Jie Li

Hi Wolfram,

I'm just gently pinging this patch series to see if you have any
further comments or if it's ready to be picked up for the I2C tree.

This v3 series already includes the Reviewed-by tags from Linus
Walleij for both patches.

Thanks for your time!

Best regards,
Jie Li

On Sun, Feb 1, 2026 at 12:18 PM Jie Li <lj29312931@gmail.com> wrote:
>
> 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
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] i2c: core: support recovery for single-ended GPIOs
  2026-02-01 11:18               ` [PATCH v3 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
@ 2026-05-04 12:14                 ` Wolfram Sang
  0 siblings, 0 replies; 17+ messages in thread
From: Wolfram Sang @ 2026-05-04 12:14 UTC (permalink / raw)
  To: Jie Li; +Cc: wsa, linusw, linux-i2c, linux-gpio, linux-kernel, Jie Li

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

Hi all,

On Sun, Feb 01, 2026 at 12:18:12PM +0100, Jie Li 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>
> Reviewed-by: Linus Walleij <linusw@kernel.org>

I think this should go via the GPIO tree? If so:

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

This probably needs a respin, though, because I will push out a patch
today which will use the new GPIO_LINE_DIRECTION_OUT macro instead of 0.

Thanks and happy hacking,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-05-04 12:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-14 14:13 [PATCH v1 0/2] i2c: add support for forced SDA recovery Jie Li
2026-01-14 14:13 ` [PATCH v1 1/2] i2c: core: add "force-set-sda" flag for open-drain SDA without "FLAG_IS_OUT" bit Jie Li
2026-01-14 14:13 ` [PATCH v1 2/2] dt-bindings: i2c: add force-set-sda property Jie Li
2026-01-16 14:14   ` Krzysztof Kozlowski
2026-01-15  9:27 ` [PATCH v1 0/2] i2c: add support for forced SDA recovery Linus Walleij
2026-01-15 13:12   ` 李杰
2026-01-16 13:59     ` Linus Walleij
2026-01-25 19:51       ` [PATCH v2 0/2] i2c: improve bus recovery for single-ended GPIOs Jie Li
2026-01-25 19:51         ` [PATCH v2 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
2026-01-27  9:46           ` Linus Walleij
2026-01-25 19:51         ` [PATCH v2 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
2026-01-27  9:47           ` Linus Walleij
2026-02-01 11:18             ` [PATCH v3 0/2] i2c: improve bus " Jie Li
2026-02-01 11:18               ` [PATCH v3 1/2] gpiolib: add gpiod_is_single_ended() helper Jie Li
2026-02-01 11:18               ` [PATCH v3 2/2] i2c: core: support recovery for single-ended GPIOs Jie Li
2026-05-04 12:14                 ` Wolfram Sang
2026-04-22 18:47               ` [PATCH v3 0/2] i2c: improve bus " 李杰

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox