All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: pca953x: Add GPIO settle delay
@ 2026-07-10 23:04 Judith Mendez
  2026-07-13  7:07 ` Francesco Dolcini
  2026-07-13 12:00 ` Kathpalia, Tanmay
  0 siblings, 2 replies; 5+ messages in thread
From: Judith Mendez @ 2026-07-10 23:04 UTC (permalink / raw)
  To: Judith Mendez, Tom Rini, Simon Glass; +Cc: Francois Berder, u-boot

Some board designs have slow RC circuits on GPIO outputs controlled by
PCA953X I2C expanders. On some board designs, pin voltage transitions
can take significant time after I2C write command completes.

Add an optional "gpio-settle-delay-ms" device tree property for the
pca953x driver to add a delay after I2C write if specified, allowing to
compensate for slow RC circuits.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/gpio/pca953x_gpio.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
index 965a5fcf30b..54b957069c9 100644
--- a/drivers/gpio/pca953x_gpio.c
+++ b/drivers/gpio/pca953x_gpio.c
@@ -28,6 +28,7 @@
 #include <dm/device_compat.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <linux/bitops.h>
+#include <linux/delay.h>
 
 #define PCA953X_INPUT           0
 #define PCA953X_OUTPUT          1
@@ -89,6 +90,7 @@ static const struct pca95xx_reg pca957x_regs = {
  * @reg_output: array to hold the value of output registers
  * @reg_direction: array to hold the value of direction registers
  * @regs: struct to hold the registers addresses
+ * @settle_delay_ms: GPIO pin settle delay in milliseconds
  */
 struct pca953x_info {
 	struct udevice *dev;
@@ -100,6 +102,7 @@ struct pca953x_info {
 	u8 reg_output[MAX_BANK];
 	u8 reg_direction[MAX_BANK];
 	const struct pca95xx_reg *regs;
+	u32 settle_delay_ms;
 };
 
 static int pca953x_write_single(struct udevice *dev, int reg, u8 val,
@@ -116,6 +119,9 @@ static int pca953x_write_single(struct udevice *dev, int reg, u8 val,
 		return ret;
 	}
 
+	if (info->settle_delay_ms)
+		mdelay(info->settle_delay_ms);
+
 	return 0;
 }
 
@@ -333,6 +339,8 @@ static int pca953x_probe(struct udevice *dev)
 
 	info->bank_count = DIV_ROUND_UP(info->gpio_count, BANK_SZ);
 
+	info->settle_delay_ms = dev_read_u32_default(dev, "gpio-settle-delay-ms", 0);
+
 	ret = pca953x_read_regs(dev, info->regs->output, info->reg_output);
 	if (ret) {
 		dev_err(dev, "Error reading output register\n");
-- 
2.54.0


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

* Re: [PATCH] gpio: pca953x: Add GPIO settle delay
  2026-07-10 23:04 [PATCH] gpio: pca953x: Add GPIO settle delay Judith Mendez
@ 2026-07-13  7:07 ` Francesco Dolcini
  2026-07-13 20:48   ` Mendez, Judith
  2026-07-13 12:00 ` Kathpalia, Tanmay
  1 sibling, 1 reply; 5+ messages in thread
From: Francesco Dolcini @ 2026-07-13  7:07 UTC (permalink / raw)
  To: Judith Mendez; +Cc: Tom Rini, Simon Glass, Francois Berder, u-boot

Hello Judith,

On Fri, Jul 10, 2026 at 06:04:09PM -0500, Judith Mendez wrote:
> Some board designs have slow RC circuits on GPIO outputs controlled by
> PCA953X I2C expanders. On some board designs, pin voltage transitions
> can take significant time after I2C write command completes.
> 
> Add an optional "gpio-settle-delay-ms" device tree property for the
> pca953x driver to add a delay after I2C write if specified, allowing to
> compensate for slow RC circuits.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>  drivers/gpio/pca953x_gpio.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
> index 965a5fcf30b..54b957069c9 100644
> --- a/drivers/gpio/pca953x_gpio.c
> +++ b/drivers/gpio/pca953x_gpio.c
> @@ -333,6 +339,8 @@ static int pca953x_probe(struct udevice *dev)
>  
>  	info->bank_count = DIV_ROUND_UP(info->gpio_count, BANK_SZ);
>  
> +	info->settle_delay_ms = dev_read_u32_default(dev, "gpio-settle-delay-ms", 0);

Is this property supported also in Linux and defined in the Linux
binding? I do not think we should have U-Boot specific DT properties.

Francesco


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

* Re: [PATCH] gpio: pca953x: Add GPIO settle delay
  2026-07-10 23:04 [PATCH] gpio: pca953x: Add GPIO settle delay Judith Mendez
  2026-07-13  7:07 ` Francesco Dolcini
@ 2026-07-13 12:00 ` Kathpalia, Tanmay
  2026-07-13 20:49   ` Mendez, Judith
  1 sibling, 1 reply; 5+ messages in thread
From: Kathpalia, Tanmay @ 2026-07-13 12:00 UTC (permalink / raw)
  To: Judith Mendez, Tom Rini, Simon Glass; +Cc: Francois Berder, u-boot

Hi Judith,

On 7/11/2026 4:34 AM, Judith Mendez wrote:
> Some board designs have slow RC circuits on GPIO outputs controlled by
> PCA953X I2C expanders. On some board designs, pin voltage transitions
> can take significant time after I2C write command completes.
>
> Add an optional "gpio-settle-delay-ms" device tree property for the
> pca953x driver to add a delay after I2C write if specified, allowing to
> compensate for slow RC circuits.
>
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>   drivers/gpio/pca953x_gpio.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
> index 965a5fcf30b..54b957069c9 100644
> --- a/drivers/gpio/pca953x_gpio.c
> +++ b/drivers/gpio/pca953x_gpio.c
> @@ -28,6 +28,7 @@
>   #include <dm/device_compat.h>
>   #include <dt-bindings/gpio/gpio.h>
>   #include <linux/bitops.h>
> +#include <linux/delay.h>
>   
>   #define PCA953X_INPUT           0
>   #define PCA953X_OUTPUT          1
> @@ -89,6 +90,7 @@ static const struct pca95xx_reg pca957x_regs = {
>    * @reg_output: array to hold the value of output registers
>    * @reg_direction: array to hold the value of direction registers
>    * @regs: struct to hold the registers addresses
> + * @settle_delay_ms: GPIO pin settle delay in milliseconds
>    */
>   struct pca953x_info {
>   	struct udevice *dev;
> @@ -100,6 +102,7 @@ struct pca953x_info {
>   	u8 reg_output[MAX_BANK];
>   	u8 reg_direction[MAX_BANK];
>   	const struct pca95xx_reg *regs;
> +	u32 settle_delay_ms;
>   };
>   
>   static int pca953x_write_single(struct udevice *dev, int reg, u8 val,
> @@ -116,6 +119,9 @@ static int pca953x_write_single(struct udevice *dev, int reg, u8 val,
>   		return ret;
>   	}
>   
> +	if (info->settle_delay_ms)
> +		mdelay(info->settle_delay_ms);
> +
>   	return 0;
>   }

This delay is placed inside pca953x_write_single(), which is called by both
pca953x_set_value() and pca953x_set_direction(). This results in a 
double delay
for every direction_output call, which I think is unintended.

Additionally, a settle delay is only meaningful after changing an output
level — it represents the time for the pin voltage to physically settle 
(e.g.
through an RC circuit). Firing it on direction register writes does not make
sense. The delay should be scoped only to pca953x_set_value().

>   
> @@ -333,6 +339,8 @@ static int pca953x_probe(struct udevice *dev)
>   
>   	info->bank_count = DIV_ROUND_UP(info->gpio_count, BANK_SZ);
>   
> +	info->settle_delay_ms = dev_read_u32_default(dev, "gpio-settle-delay-ms", 0);
> +

As Francesco mentioned, adding u-boot specifc properties causes the 
U-Boot DT
to diverge from the Linux DT, which violates U-Boot's policy of 
maintaining DT
compatibility with Linux.

>   	ret = pca953x_read_regs(dev, info->regs->output, info->reg_output);
>   	if (ret) {
>   		dev_err(dev, "Error reading output register\n");

Given that the delay is due to a slow RC circuit on the output, I would 
suggest
looking at the gpio-delay (gpio-delay.c, binding at gpio-delay.yaml). It is
designed precisely for this scenario:
"This binding describes an electrical setup where setting a GPIO output is
delayed by some external setup, e.g. RC circuit."

Regards,
Tanmay

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

* Re: [PATCH] gpio: pca953x: Add GPIO settle delay
  2026-07-13  7:07 ` Francesco Dolcini
@ 2026-07-13 20:48   ` Mendez, Judith
  0 siblings, 0 replies; 5+ messages in thread
From: Mendez, Judith @ 2026-07-13 20:48 UTC (permalink / raw)
  To: Francesco Dolcini; +Cc: Tom Rini, Simon Glass, Francois Berder, u-boot

Hi Francesco,

On 7/13/2026 2:07 AM, Francesco Dolcini wrote:
> Hello Judith,
> 
> On Fri, Jul 10, 2026 at 06:04:09PM -0500, Judith Mendez wrote:
>> Some board designs have slow RC circuits on GPIO outputs controlled by
>> PCA953X I2C expanders. On some board designs, pin voltage transitions
>> can take significant time after I2C write command completes.
>>
>> Add an optional "gpio-settle-delay-ms" device tree property for the
>> pca953x driver to add a delay after I2C write if specified, allowing to
>> compensate for slow RC circuits.
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   drivers/gpio/pca953x_gpio.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
>> index 965a5fcf30b..54b957069c9 100644
>> --- a/drivers/gpio/pca953x_gpio.c
>> +++ b/drivers/gpio/pca953x_gpio.c
>> @@ -333,6 +339,8 @@ static int pca953x_probe(struct udevice *dev)
>>   
>>   	info->bank_count = DIV_ROUND_UP(info->gpio_count, BANK_SZ);
>>   
>> +	info->settle_delay_ms = dev_read_u32_default(dev, "gpio-settle-delay-ms", 0);
> 
> Is this property supported also in Linux and defined in the Linux
> binding? I do not think we should have U-Boot specific DT properties.
yea you are right. I guess we can use gpio-delay instead.

~ Judith

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

* Re: [PATCH] gpio: pca953x: Add GPIO settle delay
  2026-07-13 12:00 ` Kathpalia, Tanmay
@ 2026-07-13 20:49   ` Mendez, Judith
  0 siblings, 0 replies; 5+ messages in thread
From: Mendez, Judith @ 2026-07-13 20:49 UTC (permalink / raw)
  To: Kathpalia, Tanmay, Tom Rini, Simon Glass; +Cc: Francois Berder, u-boot

Hi Tanmay,

On 7/13/2026 7:00 AM, Kathpalia, Tanmay wrote:
> Hi Judith,
> 
> On 7/11/2026 4:34 AM, Judith Mendez wrote:
>> Some board designs have slow RC circuits on GPIO outputs controlled by
>> PCA953X I2C expanders. On some board designs, pin voltage transitions
>> can take significant time after I2C write command completes.
>>
>> Add an optional "gpio-settle-delay-ms" device tree property for the
>> pca953x driver to add a delay after I2C write if specified, allowing to
>> compensate for slow RC circuits.
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   drivers/gpio/pca953x_gpio.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
>> index 965a5fcf30b..54b957069c9 100644
>> --- a/drivers/gpio/pca953x_gpio.c
>> +++ b/drivers/gpio/pca953x_gpio.c
>> @@ -28,6 +28,7 @@
>>   #include <dm/device_compat.h>
>>   #include <dt-bindings/gpio/gpio.h>
>>   #include <linux/bitops.h>
>> +#include <linux/delay.h>
>>   #define PCA953X_INPUT           0
>>   #define PCA953X_OUTPUT          1
>> @@ -89,6 +90,7 @@ static const struct pca95xx_reg pca957x_regs = {
>>    * @reg_output: array to hold the value of output registers
>>    * @reg_direction: array to hold the value of direction registers
>>    * @regs: struct to hold the registers addresses
>> + * @settle_delay_ms: GPIO pin settle delay in milliseconds
>>    */
>>   struct pca953x_info {
>>       struct udevice *dev;
>> @@ -100,6 +102,7 @@ struct pca953x_info {
>>       u8 reg_output[MAX_BANK];
>>       u8 reg_direction[MAX_BANK];
>>       const struct pca95xx_reg *regs;
>> +    u32 settle_delay_ms;
>>   };
>>   static int pca953x_write_single(struct udevice *dev, int reg, u8 val,
>> @@ -116,6 +119,9 @@ static int pca953x_write_single(struct udevice 
>> *dev, int reg, u8 val,
>>           return ret;
>>       }
>> +    if (info->settle_delay_ms)
>> +        mdelay(info->settle_delay_ms);
>> +
>>       return 0;
>>   }
> 
> This delay is placed inside pca953x_write_single(), which is called by both
> pca953x_set_value() and pca953x_set_direction(). This results in a 
> double delay
> for every direction_output call, which I think is unintended.
> 
> Additionally, a settle delay is only meaningful after changing an output
> level — it represents the time for the pin voltage to physically settle 
> (e.g.
> through an RC circuit). Firing it on direction register writes does not 
> make
> sense. The delay should be scoped only to pca953x_set_value().

Yea did not intended.

> 
>> @@ -333,6 +339,8 @@ static int pca953x_probe(struct udevice *dev)
>>       info->bank_count = DIV_ROUND_UP(info->gpio_count, BANK_SZ);
>> +    info->settle_delay_ms = dev_read_u32_default(dev, "gpio-settle- 
>> delay-ms", 0);
>> +
> 
> As Francesco mentioned, adding u-boot specifc properties causes the U- 
> Boot DT
> to diverge from the Linux DT, which violates U-Boot's policy of 
> maintaining DT
> compatibility with Linux.
> 
>>       ret = pca953x_read_regs(dev, info->regs->output, info->reg_output);
>>       if (ret) {
>>           dev_err(dev, "Error reading output register\n");
> 
> Given that the delay is due to a slow RC circuit on the output, I would 
> suggest
> looking at the gpio-delay (gpio-delay.c, binding at gpio-delay.yaml). It is
> designed precisely for this scenario:
> "This binding describes an electrical setup where setting a GPIO output is
> delayed by some external setup, e.g. RC circuit."
Thanks, Ill check that out.

~ Judith

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

end of thread, other threads:[~2026-07-13 20:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 23:04 [PATCH] gpio: pca953x: Add GPIO settle delay Judith Mendez
2026-07-13  7:07 ` Francesco Dolcini
2026-07-13 20:48   ` Mendez, Judith
2026-07-13 12:00 ` Kathpalia, Tanmay
2026-07-13 20:49   ` Mendez, Judith

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.