Re: [QUESTION] i2c: designware: Why use GPIOD_OUT_HIGH instead of OPEN_DRAIN for recovery?

Re: [QUESTION] i2c: designware: Why use GPIOD_OUT_HIGH instead of OPEN_DRAIN for recovery?

[Andy Shevchenko]

On Mon, Jan 12, 2026 at 10:10:02PM +0800, Alex Ivy wrote:

> I am currently looking at the bus recovery implementation in
> drivers/i2c/busses/i2c-designware-master.c. I noticed that
> i2c_dw_init_recovery_info() seems to manually reimplement much of the logic
> already present in the I2C core's i2c_gpio_init_generic_recovery().
> 
> However, there are two key differences that caught my attention:
> 
> GPIO Flags: The I2C core uses GPIOD_OUT_HIGH_OPEN_DRAIN, while
> i2c-designware uses GPIOD_OUT_HIGH. Is this specifically intended to
> support SoC GPIO controllers that do not implement the OPEN_DRAIN flag,
> which would otherwise cause devm_gpiod_get() to fail in the core's generic
> helper?
> 
> Initialization Flow: The core's helper performs a pinctrl state toggle
> (GPIO -> Default) during initialization, whereas i2c-designware only looks
> up the states and defers the actual switching to prepare_recovery.

This might affect the workflow. Note, in the past one of such conversion was
presumably done without real testing and brought a regression (I'm talking
about PXA version).

> My question is: Would it be possible (or welcomed) to refactor this to use
> the core's generic helper, or is the current manual initialization required
> to maintain compatibility with specific DesignWare-integrated SoCs (like
> certain Intel or ARM platforms) that have restrictive GPIO/Pinctrl
> requirements?
> 
> I would appreciate your insights on the historical background of these
> choices.

Do you have an access to real HW?

-- 
With Best Regards,
Andy Shevchenko

Re: [QUESTION] i2c: designware: Why use GPIOD_OUT_HIGH instead of OPEN_DRAIN for recovery?

[Alex Ivy]

Hi Andy,

As for the  GPIOD FLAGS, I solved my problem  by using GPIO_INPUT to
simulate OPEN_DRAIN for i2c recovery, by just updating the Device Tree
to include GPIO_OPEN_DRAIN for the recovery gpios.

Board: ARM64 linux 6.1.158 , with snps,designware-i2c and snps,dw-apb-gpio.

STEP1:
I performed a test on my ARM board using the logic found in commit
d2d0ad2aec4a ("i2c: imx: use open drain for recovery
GPIO").https://lkml.org/lkml/2018/7/13/682
diff --git a/drivers/i2c/busses/i2c-designware-master.c
b/drivers/i2c/busses/i2c-designware-master.c
index ddf9673ec742..2cd257bc06f5 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -914,7 +914,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
                return -1;
        }

-       gpio = devm_gpiod_get_optional(dev->dev, "scl", GPIOD_OUT_HIGH);
+       gpio = devm_gpiod_get_optional(dev->dev, "scl",
GPIOD_OUT_HIGH_OPEN_DRAIN);
        if (IS_ERR_OR_NULL(gpio))
                return PTR_ERR_OR_ZERO(gpio);

Then "[    0.653908] gpio-496 (scl): enforced open drain please flag
it properly in DT/ACPI DSDT/board file"

STEP2:
I updated the Device Tree to include GPIO_OPEN_DRAIN for the recovery gpios.

Result: The gpiolib warning (enforced open drain...) has disappeared,
and the bus recovery still functions correctly.

STEP3:
Then I discard the modification in i2c-designware-master.c and use the
Device Tree change only, and the GPIO_OPEN_DRAIN flags remain in the
gpio's desc.

This solved my problem.

However, my question still remains why the imx changes get merged. Is
it needed to apply the same patch for the designware i2c?(which my
need many dts change)

Best regards, Alex Ivy


Andy Shevchenko <andriy.shevchenko@linux.intel.com> 于2026年1月13日周二 00:57写道：
>
> On Mon, Jan 12, 2026 at 10:10:02PM +0800, Alex Ivy wrote:
>
> > I am currently looking at the bus recovery implementation in
> > drivers/i2c/busses/i2c-designware-master.c. I noticed that
> > i2c_dw_init_recovery_info() seems to manually reimplement much of the logic
> > already present in the I2C core's i2c_gpio_init_generic_recovery().
> >
> > However, there are two key differences that caught my attention:
> >
> > GPIO Flags: The I2C core uses GPIOD_OUT_HIGH_OPEN_DRAIN, while
> > i2c-designware uses GPIOD_OUT_HIGH. Is this specifically intended to
> > support SoC GPIO controllers that do not implement the OPEN_DRAIN flag,
> > which would otherwise cause devm_gpiod_get() to fail in the core's generic
> > helper?
> >
> > Initialization Flow: The core's helper performs a pinctrl state toggle
> > (GPIO -> Default) during initialization, whereas i2c-designware only looks
> > up the states and defers the actual switching to prepare_recovery.
>
> This might affect the workflow. Note, in the past one of such conversion was
> presumably done without real testing and brought a regression (I'm talking
> about PXA version).
>
> > My question is: Would it be possible (or welcomed) to refactor this to use
> > the core's generic helper, or is the current manual initialization required
> > to maintain compatibility with specific DesignWare-integrated SoCs (like
> > certain Intel or ARM platforms) that have restrictive GPIO/Pinctrl
> > requirements?
> >
> > I would appreciate your insights on the historical background of these
> > choices.
>
> Do you have an access to real HW?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [QUESTION] i2c: designware: Why use GPIOD_OUT_HIGH instead of OPEN_DRAIN for recovery?

[Alex Ivy]

Hi Andy,

I was not familiar with the LKML reply format and learned to send the
reply again.

As for the  GPIOD FLAGS, I solved my problem  by using GPIO_INPUT to
simulate OPEN_DRAIN for i2c recovery, by just updating the Device Tree
to include GPIO_OPEN_DRAIN for the recovery gpios.

Andy Shevchenko <andriy.shevchenko@linux.intel.com> Wrote：
>
> On Mon, Jan 12, 2026 at 10:10:02PM +0800, Alex Ivy wrote:
>
...
> >
> > Initialization Flow: The core's helper performs a pinctrl state toggle
> > (GPIO -> Default) during initialization, whereas i2c-designware only looks
> > up the states and defers the actual switching to prepare_recovery.
>
> This might affect the workflow. Note, in the past one of such conversion was
> presumably done without real testing and brought a regression (I'm talking
> about PXA version).
...

Got it, so let's talk about the GPIOD flags this time.

...
>
> > My question is: Would it be possible (or welcomed) to refactor this to use
> > the core's generic helper, or is the current manual initialization required
> > to maintain compatibility with specific DesignWare-integrated SoCs (like
> > certain Intel or ARM platforms) that have restrictive GPIO/Pinctrl
> > requirements?
> >
> > I would appreciate your insights on the historical background of these
> > choices.
>
> Do you have an access to real HW?
...

Board: ARM64 linux 6.1.158 , with snps,designware-i2c and snps,dw-apb-gpio.


STEP1:
I performed a test on my ARM board using the logic found in commit
d2d0ad2aec4a ("i2c: imx: use open drain for recovery
GPIO").https://lkml.org/lkml/2018/7/13/682
diff --git a/drivers/i2c/busses/i2c-designware-master.c
b/drivers/i2c/busses/i2c-designware-master.c
index ddf9673ec742..2cd257bc06f5 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -914,7 +914,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
                return -1;
        }

-       gpio = devm_gpiod_get_optional(dev->dev, "scl", GPIOD_OUT_HIGH);
+       gpio = devm_gpiod_get_optional(dev->dev, "scl",
GPIOD_OUT_HIGH_OPEN_DRAIN);
        if (IS_ERR_OR_NULL(gpio))
                return PTR_ERR_OR_ZERO(gpio);

Then "[    0.653908] gpio-496 (scl): enforced open drain please flag
it properly in DT/ACPI DSDT/board file"

STEP2:
I updated the Device Tree to include GPIO_OPEN_DRAIN for the recovery gpios.

Result: The gpiolib warning (enforced open drain...) has disappeared,
and the bus recovery still functions correctly.

STEP3:
Then I discard the modification in i2c-designware-master.c and use the
Device Tree change only, and the GPIO_OPEN_DRAIN flags remain in the
gpiod desc.

This solved my problem.

However, my question still remains why the imx changes get merged. Is
it needed to apply the same patch for the designware i2c?(which my
need many dts changes)

--
Best regards,
Alex Ivy