[PATCH v3] i2c: i2c-gpio: Enhance driver for buses with shared SCL

[PATCH v3] i2c: i2c-gpio: Enhance driver for buses with shared SCL

[Markus Stockhausen]

Some lower end hardware (especially Realtek based switches) are
designed with multiple I2C buses that share a single clock line.
E.g. the D-Link DGS-1250-28X realizes 4 I2C SFP busses with 5 gpios.

Enhance the i2c-gpio driver so it can handle such hardware designs.

- Detect GPIOs that are used by multiple I2C buses in the dts by
  using a unique identifier for each managed SCL.

- The first probing instance allocates and requests the shared SCL
  GPIO plus an associated rt_mutex. Subsequent instances detect the
  existing entry via the identifier and increment a reference count
  to reuse the descriptor.

- All data transfers are serialized via custom lock_ops that handle
  both the standard adapter bus lock and the shared SCL mutex. This
  ensures mutual exclusion across adapters sharing the clock line.

This patch was successfully tested on Linksys LGS310C that has two
SFP slots with two GPIO based I2C buses that share a sinlge SCL.
Test environment: OpenWrt snapshot ported to kernel 6.19.14
including CONFIG_GPIO_SHARED=y and CONFIG_GPIO_SHARED_PROXY=y.

Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

---

v2 -> v3
  - Fix lockdep_set_class() with DEBUG_LOCK_ALLOC=y
v2: https://lore.kernel.org/linux-i2c/20260518161013.900504-1-markus.stockhausen@gmx.de/

v1 -> v2
  - Convert fault injector to scl->gpio (reported by test robot)
  - Use rt_mutex and i2c_lock_operations instead of pre/post_xfer
    (logic taken from i2c-cht-wc.c and enhanced)
  - i2c_gpio_lookup_scl()
    - Improve list control flow
    - Improve comment
    - Carve out SCL node comparison into i2c_gpio_scl_matches()
  - Drop "valid" attribute and directly check gpiod instead
  - Improve fwnode args check for #gpio-cells=1 case
  - Add sda/scl cleanup during probe failures
  - Replace dev_info() with dev_dbg()
  - Reflect changed locking in commmit message
  - Tested with config option GPIO_SHARED/GPIO_SHARED_PROXY
    as requested by Bartosz
v1: https://lore.kernel.org/linux-i2c/20260514092042.3265986-1-markus.stockhausen@gmx.de/

v0 -> v1
  - Initially this enhancement was submitted as a new driver with
    a new devicetree structure. After some discussion Wolfram
    advised to make only an enhancement to the i2c-gpio driver.
v0: https://lore.kernel.org/linux-i2c/20260511162528.84508-1-markus.stockhausen@gmx.de/
---
 drivers/i2c/busses/i2c-gpio.c | 208 +++++++++++++++++++++++++++++++---
 1 file changed, 194 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index f4355b17bfbf..80e1347ecbac 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -18,9 +18,23 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 
+static LIST_HEAD(i2c_gpio_scl_list);
+static DEFINE_MUTEX(i2c_gpio_scl_list_lock);
+static struct lock_class_key i2c_gpio_scl_lock_key;
+
+struct i2c_gpio_scl_data {
+	struct fwnode_handle *fw_node;
+	u32 fw_pin;
+	u32 fw_flags;
+	struct gpio_desc *gpio;
+	struct rt_mutex lock;
+	refcount_t ref;
+	struct list_head list;
+};
+
 struct i2c_gpio_private_data {
 	struct gpio_desc *sda;
-	struct gpio_desc *scl;
+	struct i2c_gpio_scl_data *scl;
 	struct i2c_adapter adap;
 	struct i2c_algo_bit_data bit_data;
 	struct i2c_gpio_platform_data pdata;
@@ -31,6 +45,11 @@ struct i2c_gpio_private_data {
 #endif
 };
 
+static inline struct i2c_gpio_private_data *adap_to_priv(struct i2c_adapter *adap)
+{
+	return container_of(adap, struct i2c_gpio_private_data, adap);
+}
+
 /*
  * Toggle SDA by changing the output value of the pin. This is only
  * valid for pins configured as open drain (i.e. setting the value
@@ -53,7 +72,7 @@ static void i2c_gpio_setscl_val(void *data, int state)
 {
 	struct i2c_gpio_private_data *priv = data;
 
-	gpiod_set_value_cansleep(priv->scl, state);
+	gpiod_set_value_cansleep(priv->scl->gpio, state);
 }
 
 static int i2c_gpio_getsda(void *data)
@@ -67,9 +86,41 @@ static int i2c_gpio_getscl(void *data)
 {
 	struct i2c_gpio_private_data *priv = data;
 
-	return gpiod_get_value_cansleep(priv->scl);
+	return gpiod_get_value_cansleep(priv->scl->gpio);
+}
+
+static void i2c_gpio_lock_bus(struct i2c_adapter *adap, unsigned int flags)
+{
+	/* Take care about adapter lock. See i2c_adapter_lock_bus() and others. */
+	rt_mutex_lock_nested(&adap->bus_lock, i2c_adapter_depth(adap));
+	rt_mutex_lock(&adap_to_priv(adap)->scl->lock);
+}
+
+static int i2c_gpio_trylock_bus(struct i2c_adapter *adap, unsigned int flags)
+{
+	if (!rt_mutex_trylock(&adap->bus_lock))
+		return 0;
+
+	if (!rt_mutex_trylock(&adap_to_priv(adap)->scl->lock)) {
+		rt_mutex_unlock(&adap->bus_lock);
+		return 0;
+	}
+
+	return 1;
+}
+
+static void i2c_gpio_unlock_bus(struct i2c_adapter *adap, unsigned int flags)
+{
+	rt_mutex_unlock(&adap_to_priv(adap)->scl->lock);
+	rt_mutex_unlock(&adap->bus_lock);
 }
 
+static const struct i2c_lock_operations i2c_gpio_lock_ops = {
+	.lock_bus = i2c_gpio_lock_bus,
+	.trylock_bus = i2c_gpio_trylock_bus,
+	.unlock_bus = i2c_gpio_unlock_bus,
+};
+
 #ifdef CONFIG_I2C_GPIO_FAULT_INJECTOR
 
 #define setsda(bd, val)	((bd)->setsda((bd)->data, val))
@@ -165,14 +216,14 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_incomplete_write_byte, NULL, fops_incomplete_write
 static int i2c_gpio_fi_act_on_scl_irq(struct i2c_gpio_private_data *priv,
 				       irqreturn_t handler(int, void*))
 {
-	int ret, irq = gpiod_to_irq(priv->scl);
+	int ret, irq = gpiod_to_irq(priv->scl->gpio);
 
 	if (irq < 0)
 		return irq;
 
 	i2c_lock_bus(&priv->adap, I2C_LOCK_ROOT_ADAPTER);
 
-	ret = gpiod_direction_input(priv->scl);
+	ret = gpiod_direction_input(priv->scl->gpio);
 	if (ret)
 		goto unlock;
 
@@ -187,7 +238,7 @@ static int i2c_gpio_fi_act_on_scl_irq(struct i2c_gpio_private_data *priv,
 
 	free_irq(irq, priv);
  output:
-	ret = gpiod_direction_output(priv->scl, 1) ?: ret;
+	ret = gpiod_direction_output(priv->scl->gpio, 1) ?: ret;
  unlock:
 	i2c_unlock_bus(&priv->adap, I2C_LOCK_ROOT_ADAPTER);
 
@@ -308,13 +359,17 @@ static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
 	struct gpio_desc *retdesc;
 	int ret;
 
-	retdesc = devm_gpiod_get(dev, con_id, gflags);
+	/*
+	 * Don't use resource-managed functions. SCL may be shared across adapters and has
+	 * its own lifetime management. SDA uses the same path for consistency.
+	 */
+	retdesc = gpiod_get(dev, con_id, gflags);
 	if (!IS_ERR(retdesc)) {
 		dev_dbg(dev, "got GPIO from name %s\n", con_id);
 		return retdesc;
 	}
 
-	retdesc = devm_gpiod_get_index(dev, NULL, index, gflags);
+	retdesc = gpiod_get_index(dev, NULL, index, gflags);
 	if (!IS_ERR(retdesc)) {
 		dev_dbg(dev, "got GPIO from index %u\n", index);
 		return retdesc;
@@ -336,6 +391,118 @@ static struct gpio_desc *i2c_gpio_get_desc(struct device *dev,
 	return retdesc;
 }
 
+static struct i2c_gpio_scl_data *i2c_gpio_create_scl(struct fwnode_handle *fwnode)
+{
+	struct fwnode_reference_args args;
+	struct i2c_gpio_scl_data *scl;
+	int ret;
+
+	ret = fwnode_property_get_reference_args(fwnode, "scl-gpios",
+						 "#gpio-cells", 0, 0, &args);
+	if (ret)
+		/* try the ancient way */
+		ret = fwnode_property_get_reference_args(fwnode, "gpios",
+							 "#gpio-cells", 0, 1, &args);
+	if (ret)
+		return ERR_PTR(ret);
+
+	if (args.nargs < 1) {
+		fwnode_handle_put(args.fwnode);
+		return ERR_PTR(-EINVAL);
+	}
+
+	scl = kzalloc(sizeof(*scl), GFP_KERNEL);
+	if (!scl) {
+		fwnode_handle_put(args.fwnode);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* The unique identification from the SCL GPIO reference in the device tree */
+	scl->fw_node = args.fwnode;
+	scl->fw_pin = args.args[0];
+	scl->fw_flags = (args.nargs >= 2) ? args.args[1] : 0;
+
+	rt_mutex_init(&scl->lock);
+	lockdep_set_class(&scl->lock, &i2c_gpio_scl_lock_key);
+	refcount_set(&scl->ref, 1);
+
+	return scl;
+}
+
+static void i2c_gpio_free_scl(struct i2c_gpio_scl_data *scl)
+{
+	fwnode_handle_put(scl->fw_node);
+	kfree(scl);
+}
+
+static bool i2c_gpio_scl_matches(struct i2c_gpio_scl_data *a, struct i2c_gpio_scl_data *b)
+{
+    return a->fw_node == b->fw_node && a->fw_pin == b->fw_pin && a->fw_flags == b->fw_flags;
+}
+
+/*
+ * Look up an existing or create a new shared SCL structure described by the device's fwnode.
+ * Optimistic setup sequence always creates and tries to add a new entry to the list. This uses
+ * minimum locking and afterwards requests the GPIO without a lock held. Concurrent probes for
+ * the same SCL pin see the entry and do not race into a second gpiod_get(). Until everything
+ * is setup they terminate with -EPROBE_DEFER.
+ */
+static struct i2c_gpio_scl_data *i2c_gpio_lookup_scl(struct device *dev, enum gpiod_flags gflags)
+{
+	struct i2c_gpio_scl_data *scl, *new_scl;
+	struct gpio_desc *gpio;
+
+	new_scl = i2c_gpio_create_scl(dev_fwnode(dev));
+	if (IS_ERR(new_scl))
+		return new_scl;
+
+	scoped_guard(mutex, &i2c_gpio_scl_list_lock) {
+		list_for_each_entry(scl, &i2c_gpio_scl_list, list) {
+			if (!i2c_gpio_scl_matches(scl, new_scl))
+				continue;
+
+			i2c_gpio_free_scl(new_scl);
+			if (!scl->gpio)
+				return ERR_PTR(-EPROBE_DEFER);
+
+			refcount_inc(&scl->ref);
+			dev_dbg(dev, "reusing shared SCL (%pfwP, pin %u)\n",
+				scl->fw_node, scl->fw_pin);
+
+			return scl;
+		}
+		list_add(&new_scl->list, &i2c_gpio_scl_list);
+	}
+
+	gpio = i2c_gpio_get_desc(dev, "scl", 1, gflags);
+	if (IS_ERR(gpio)) {
+		scoped_guard(mutex, &i2c_gpio_scl_list_lock)
+			list_del(&new_scl->list);
+		i2c_gpio_free_scl(new_scl);
+
+		return ERR_CAST(gpio);
+	}
+
+	scoped_guard(mutex, &i2c_gpio_scl_list_lock)
+		new_scl->gpio = gpio;
+
+	dev_dbg(dev, "registered shared SCL (%pfwP, pin %u)\n",
+		new_scl->fw_node, new_scl->fw_pin);
+
+	return new_scl;
+}
+
+static void i2c_gpio_cleanup_scl(struct i2c_gpio_scl_data *scl)
+{
+	if (!refcount_dec_and_mutex_lock(&scl->ref, &i2c_gpio_scl_list_lock))
+		return;
+
+	list_del(&scl->list);
+	mutex_unlock(&i2c_gpio_scl_list_lock);
+	gpiod_put(scl->gpio);
+	i2c_gpio_free_scl(scl);
+}
+
 static int i2c_gpio_probe(struct platform_device *pdev)
 {
 	struct i2c_gpio_private_data *priv;
@@ -386,11 +553,13 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 		gflags = GPIOD_OUT_HIGH;
 	else
 		gflags = GPIOD_OUT_HIGH_OPEN_DRAIN;
-	priv->scl = i2c_gpio_get_desc(dev, "scl", 1, gflags);
-	if (IS_ERR(priv->scl))
-		return PTR_ERR(priv->scl);
+	priv->scl = i2c_gpio_lookup_scl(dev, gflags);
+	if (IS_ERR(priv->scl)) {
+		ret = PTR_ERR(priv->scl);
+		goto err_cleanup_sda;
+	}
 
-	if (gpiod_cansleep(priv->sda) || gpiod_cansleep(priv->scl))
+	if (gpiod_cansleep(priv->sda) || gpiod_cansleep(priv->scl->gpio))
 		dev_warn(dev, "Slow GPIO pins might wreak havoc into I2C/SMBus bus timing");
 	else
 		bit_data->can_do_atomic = true;
@@ -423,6 +592,8 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 	else
 		snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);
 
+	/* Always use shared SCL aware locking */
+	adap->lock_ops = &i2c_gpio_lock_ops;
 	adap->algo_data = bit_data;
 	adap->class = I2C_CLASS_HWMON;
 	adap->dev.parent = dev;
@@ -431,7 +602,7 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 	adap->nr = pdev->id;
 	ret = i2c_bit_add_numbered_bus(adap);
 	if (ret)
-		return ret;
+		goto err_cleanup_scl;
 
 	platform_set_drvdata(pdev, priv);
 
@@ -441,13 +612,20 @@ static int i2c_gpio_probe(struct platform_device *pdev)
 	 * from the descriptor, then provide that instead.
 	 */
 	dev_info(dev, "using lines %u (SDA) and %u (SCL%s)\n",
-		 desc_to_gpio(priv->sda), desc_to_gpio(priv->scl),
+		 desc_to_gpio(priv->sda), desc_to_gpio(priv->scl->gpio),
 		 pdata->scl_is_output_only
 		 ? ", no clock stretching" : "");
 
 	i2c_gpio_fault_injector_init(pdev);
 
 	return 0;
+
+err_cleanup_scl:
+	i2c_gpio_cleanup_scl(priv->scl);
+err_cleanup_sda:
+	gpiod_put(priv->sda);
+
+	return ret;
 }
 
 static void i2c_gpio_remove(struct platform_device *pdev)
@@ -459,6 +637,8 @@ static void i2c_gpio_remove(struct platform_device *pdev)
 	adap = &priv->adap;
 
 	i2c_del_adapter(adap);
+	i2c_gpio_cleanup_scl(priv->scl);
+	gpiod_put(priv->sda);
 }
 
 static const struct of_device_id i2c_gpio_dt_ids[] = {
-- 
2.54.0

Re: [PATCH v3] i2c: i2c-gpio: Enhance driver for buses with shared SCL

[Sander Vanheule]

Hi Markus,

Thanks for looking into this. It was on my to-do list for _long_ time, but I
never got to it :-(

On Wed, 2026-05-20 at 19:31 +0200, Markus Stockhausen wrote:
> Some lower end hardware (especially Realtek based switches) are
> designed with multiple I2C buses that share a single clock line.
> E.g. the D-Link DGS-1250-28X realizes 4 I2C SFP busses with 5 gpios.
> 
> Enhance the i2c-gpio driver so it can handle such hardware designs.
> 
> - Detect GPIOs that are used by multiple I2C buses in the dts by
>   using a unique identifier for each managed SCL.
> 
> - The first probing instance allocates and requests the shared SCL
>   GPIO plus an associated rt_mutex. Subsequent instances detect the
>   existing entry via the identifier and increment a reference count
>   to reuse the descriptor.
> 
> - All data transfers are serialized via custom lock_ops that handle
>   both the standard adapter bus lock and the shared SCL mutex. This
>   ensures mutual exclusion across adapters sharing the clock line.
> 
> This patch was successfully tested on Linksys LGS310C that has two
> SFP slots with two GPIO based I2C buses that share a sinlge SCL.
> Test environment: OpenWrt snapshot ported to kernel 6.19.14
> including CONFIG_GPIO_SHARED=y and CONFIG_GPIO_SHARED_PROXY=y.
> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>

I've updated my Cisco SG220-26 devicetree and it now boots with:


[..] i2c-gpio i2c-temp: Slow GPIO pins might wreak havoc into I2C/SMBus bus
timing
[..] i2c-gpio i2c-temp: using lines 565 (SDA) and 551 (SCL)
[..] i2c-gpio i2c-fan: Slow GPIO pins might wreak havoc into I2C/SMBus bus
timing
[..] i2c-gpio i2c-fan: using lines 567 (SDA) and 551 (SCL)
[..] i2c-gpio i2c-sfp25: Slow GPIO pins might wreak havoc into I2C/SMBus bus
timing
[..] i2c-gpio i2c-sfp25: using lines 549 (SDA) and 551 (SCL)
[..] i2c-gpio i2c-sfp26: Slow GPIO pins might wreak havoc into I2C/SMBus bus
timing
[..] i2c-gpio i2c-sfp26: using lines 550 (SDA) and 551 (SCL)


Four busses sharing a clock line. Nice! SFP detection works and "sensors"
provides meaningful output for the fan and temperature monitors.

Tested-by: Sander Vanheule <sander@svanheule.net>

Best,
Sander

Re: [PATCH v3] i2c: i2c-gpio: Enhance driver for buses with shared SCL

[Bartosz Golaszewski]

On Wed, May 20, 2026 at 7:31 PM Markus Stockhausen
<markus.stockhausen@gmx.de> wrote:
>
> Some lower end hardware (especially Realtek based switches) are
> designed with multiple I2C buses that share a single clock line.
> E.g. the D-Link DGS-1250-28X realizes 4 I2C SFP busses with 5 gpios.
>
> Enhance the i2c-gpio driver so it can handle such hardware designs.
>
> - Detect GPIOs that are used by multiple I2C buses in the dts by
>   using a unique identifier for each managed SCL.
>
> - The first probing instance allocates and requests the shared SCL
>   GPIO plus an associated rt_mutex. Subsequent instances detect the
>   existing entry via the identifier and increment a reference count
>   to reuse the descriptor.
>
> - All data transfers are serialized via custom lock_ops that handle
>   both the standard adapter bus lock and the shared SCL mutex. This
>   ensures mutual exclusion across adapters sharing the clock line.
>
> This patch was successfully tested on Linksys LGS310C that has two
> SFP slots with two GPIO based I2C buses that share a sinlge SCL.
> Test environment: OpenWrt snapshot ported to kernel 6.19.14
> including CONFIG_GPIO_SHARED=y and CONFIG_GPIO_SHARED_PROXY=y.
>

Thanks for testing!

> Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
>
> ---
>
> v2 -> v3
>   - Fix lockdep_set_class() with DEBUG_LOCK_ALLOC=y
> v2: https://lore.kernel.org/linux-i2c/20260518161013.900504-1-markus.stockhausen@gmx.de/
>
> v1 -> v2
>   - Convert fault injector to scl->gpio (reported by test robot)
>   - Use rt_mutex and i2c_lock_operations instead of pre/post_xfer
>     (logic taken from i2c-cht-wc.c and enhanced)
>   - i2c_gpio_lookup_scl()
>     - Improve list control flow
>     - Improve comment
>     - Carve out SCL node comparison into i2c_gpio_scl_matches()
>   - Drop "valid" attribute and directly check gpiod instead
>   - Improve fwnode args check for #gpio-cells=1 case
>   - Add sda/scl cleanup during probe failures
>   - Replace dev_info() with dev_dbg()
>   - Reflect changed locking in commmit message
>   - Tested with config option GPIO_SHARED/GPIO_SHARED_PROXY
>     as requested by Bartosz
> v1: https://lore.kernel.org/linux-i2c/20260514092042.3265986-1-markus.stockhausen@gmx.de/
>
> v0 -> v1
>   - Initially this enhancement was submitted as a new driver with
>     a new devicetree structure. After some discussion Wolfram
>     advised to make only an enhancement to the i2c-gpio driver.
> v0: https://lore.kernel.org/linux-i2c/20260511162528.84508-1-markus.stockhausen@gmx.de/
> ---
>  drivers/i2c/busses/i2c-gpio.c | 208 +++++++++++++++++++++++++++++++---
>  1 file changed, 194 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> index f4355b17bfbf..80e1347ecbac 100644
> --- a/drivers/i2c/busses/i2c-gpio.c
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -18,9 +18,23 @@
>  #include <linux/property.h>
>  #include <linux/slab.h>
>
> +static LIST_HEAD(i2c_gpio_scl_list);
> +static DEFINE_MUTEX(i2c_gpio_scl_list_lock);
> +static struct lock_class_key i2c_gpio_scl_lock_key;
> +
> +struct i2c_gpio_scl_data {
> +       struct fwnode_handle *fw_node;
> +       u32 fw_pin;
> +       u32 fw_flags;
> +       struct gpio_desc *gpio;
> +       struct rt_mutex lock;
> +       refcount_t ref;
> +       struct list_head list;
> +};
> +
>  struct i2c_gpio_private_data {
>         struct gpio_desc *sda;
> -       struct gpio_desc *scl;
> +       struct i2c_gpio_scl_data *scl;
>         struct i2c_adapter adap;
>         struct i2c_algo_bit_data bit_data;
>         struct i2c_gpio_platform_data pdata;
> @@ -31,6 +45,11 @@ struct i2c_gpio_private_data {
>  #endif
>  };
>
> +static inline struct i2c_gpio_private_data *adap_to_priv(struct i2c_adapter *adap)
> +{
> +       return container_of(adap, struct i2c_gpio_private_data, adap);
> +}
> +
>  /*
>   * Toggle SDA by changing the output value of the pin. This is only
>   * valid for pins configured as open drain (i.e. setting the value
> @@ -53,7 +72,7 @@ static void i2c_gpio_setscl_val(void *data, int state)
>  {
>         struct i2c_gpio_private_data *priv = data;
>
> -       gpiod_set_value_cansleep(priv->scl, state);
> +       gpiod_set_value_cansleep(priv->scl->gpio, state);

That one bothers me a bit. We're driving a clock line but may end up
sleeping? That doesn't sound right. We typically do:

setscl();
udelay();

I know it's been like this before and maybe I'm not understanding the
whole picture so feel free to disregard the comment.

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

AW: [PATCH v3] i2c: i2c-gpio: Enhance driver for buses with shared SCL

[Markus Stockhausen]

Hi all,

> Von: Bartosz Golaszewski <brgl@kernel.org> 
> Gesendet: Donnerstag, 21. Mai 2026 10:59
> An: Markus Stockhausen <markus.stockhausen@gmx.de>
> Betreff: Re: [PATCH v3] i2c: i2c-gpio: Enhance driver for buses with shared SCL
> ...
> >  /*
> >   * Toggle SDA by changing the output value of the pin. This is only
> >   * valid for pins configured as open drain (i.e. setting the value @@ 
> > -53,7 +72,7 @@ static void i2c_gpio_setscl_val(void *data, int state)  
> > {
> >         struct i2c_gpio_private_data *priv = data;
> >
> > -       gpiod_set_value_cansleep(priv->scl, state);
> > +       gpiod_set_value_cansleep(priv->scl->gpio, state);
> 
> That one bothers me a bit. We're driving a clock line but may 
> end up sleeping? That doesn't sound right. We typically do:
>
> setscl();
> udelay();
>
> I know it's been like this before and maybe I'm not understanding 
> the whole picture so feel free to disregard the comment.

ATM I can only ignore that because I have no idea what this code 
is about. Maybe soneone in CC can advise how to handle Bartosz's
question.

> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Thanks for that.

Markus

Re: [PATCH v3] i2c: i2c-gpio: Enhance driver for buses with shared SCL

[Linus Walleij]

On Thu, May 21, 2026 at 10:59 AM Bartosz Golaszewski <brgl@kernel.org> wrote:

> >  /*
> >   * Toggle SDA by changing the output value of the pin. This is only
> >   * valid for pins configured as open drain (i.e. setting the value
> > @@ -53,7 +72,7 @@ static void i2c_gpio_setscl_val(void *data, int state)
> >  {
> >         struct i2c_gpio_private_data *priv = data;
> >
> > -       gpiod_set_value_cansleep(priv->scl, state);
> > +       gpiod_set_value_cansleep(priv->scl->gpio, state);
>
> That one bothers me a bit. We're driving a clock line but may end up
> sleeping? That doesn't sound right. We typically do:
>
> setscl();
> udelay();
>
> I know it's been like this before and maybe I'm not understanding the
> whole picture so feel free to disregard the comment.

I think there are people driving I2C on slow buses, i.e. a bit-banged
GPIO I2C on a GPIO expander which is itself on I2C.

I'm not saying such hardware design is a good idea, but I think
I've seen it...

The only way to make that work is to use gpiod_set_value_cansleep().

Yours,
Linus Walleij