Re: [PATCH 1/3] gpio: Add GPIO delay driver

Michal Simek
2026-01-30T08:30:20Z

On 1/30/26 08:21, Peng Fan wrote:
> Hi Michal,
>
> On Thu, Jan 22, 2026 at 01:50:33PM +0100, Michal Simek wrote:
>> Add a GPIO controller driver that provides configurable delays when
>> setting GPIO output values. This is useful for hardware that requires
>> specific timing delays during power sequencing or GPIO state changes.
>>
>> The driver wraps underlying GPIO controllers and adds programmable
>> ramp-up and ramp-down delays specified in microseconds through the
>> device tree. Each GPIO can have independent delay timings.
>>
>> Device tree binding matches Linux.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> drivers/gpio/Kconfig | 8 +++
>> drivers/gpio/Makefile | 1 +
>> drivers/gpio/gpio-delay.c | 133 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 142 insertions(+)
>> create mode 100644 drivers/gpio/gpio-delay.c
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 60c5c54688e6..f69919abc05b 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -1,3 +1,11 @@
>> +config GPIO_DELAY
>> + bool "GPIO delay driver"
>> + depends on DM_GPIO
>> + help
>> + Enable the GPIO delay driver.
>> + This driver allows wrapping another GPIO controller and inserting
>> + ramp-up/ramp-down delays on output changes, as described in the
>> + Linux gpio-delay binding.
>
> Add an entry for SPL?

Why? I don't have a need for it and someone should test it if this should be
used in SPL. That can be done on the top of this when tested.

>> #
>> # GPIO infrastructure and drivers
>> #
>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
>> index 910478c0c7a9..fec258f59f52 100644
>> --- a/drivers/gpio/Makefile
>> +++ b/drivers/gpio/Makefile
>> @@ -21,6 +21,7 @@ obj-$(CONFIG_ATMEL_PIO4) += atmel_pio4.o
>> obj-$(CONFIG_BCM6345_GPIO) += bcm6345_gpio.o
>> obj-$(CONFIG_CORTINA_GPIO) += cortina_gpio.o
>> obj-$(CONFIG_FXL6408_GPIO) += gpio-fxl6408.o
>> +obj-$(CONFIG_GPIO_DELAY) += gpio-delay.o
>> obj-$(CONFIG_INTEL_GPIO) += intel_gpio.o
>> obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o
>> obj-$(CONFIG_INTEL_BROADWELL_GPIO) += intel_broadwell_gpio.o
>> diff --git a/drivers/gpio/gpio-delay.c b/drivers/gpio/gpio-delay.c
>> new file mode 100644
>> index 000000000000..0c0d05ccb493
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-delay.c
>> @@ -0,0 +1,133 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2025 - 2026, Advanced Micro Devices, Inc.
>
> Drop 2025?

Why? Driver was developed in 2025 but upstreaming happens in 2026.

>
>> + *
>> + * Michal Simek <michal.simek@amd.com>
>> + */
>> +
>> +#include <dm.h>
>> +#include <dm/device_compat.h>
>> +#include <dm/devres.h>
>> +#include <asm/gpio.h>
>> +#include <linux/delay.h>
>> +
>> +struct gpio_delay_desc {
>> + struct gpio_desc real_gpio;
>> + u32 ramp_up_us;
>> + u32 ramp_down_us;
>> +};
>> +
>> +struct gpio_delay_priv {
>> + struct gpio_delay_desc *descs;
>> +};
>> +
>> +static int gpio_delay_direction_input(struct udevice *dev, unsigned int offset)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> +static int gpio_delay_get_value(struct udevice *dev, unsigned int offset)
>> +{
>> + return -ENOSYS;
>> +}
>> +
>> +static int gpio_delay_set_value(struct udevice *dev, unsigned int offset,
>> + int value)
>> +{
>> + struct gpio_delay_priv *priv = dev_get_priv(dev);
>> + struct gpio_delay_desc *desc = &priv->descs[offset];
>> + int ret = dm_gpio_set_value(&desc->real_gpio, value);
>> + u32 wait;
>
>
> Check return value.
> if (ret) {
> dev_err(dev, "Failed to set gpio %d\n", offset);
> }

will add.

>
>> +
>> + dev_dbg(dev, "gpio %d set to %d\n", offset, value);
>> +
>> + if (value)
>> + wait = desc->ramp_up_us;
>> + else
>> + wait = desc->ramp_down_us;
>> +
>> + udelay(wait);
>> +
>> + dev_dbg(dev, "waited for %d us\n", wait);
>> +
>> + return ret;
>> +}
>> +
>> +static int gpio_delay_direction_output(struct udevice *dev, unsigned int offset,
>> + int value)
>> +{
>> + return gpio_delay_set_value(dev, offset, value);
>> +}
>> +
>> +static int gpio_delay_xlate(struct udevice *dev, struct gpio_desc *desc,
>> + struct ofnode_phandle_args *args)
>> +{
>> + struct gpio_delay_priv *priv = dev_get_priv(dev);
>> +
>> + if (args->args_count < 3)
>> + return -EINVAL;
>> +
>> + if (args->args[0] >= 32)
>> + return -EINVAL;
>> +
>> + struct gpio_delay_desc *d = &priv->descs[args->args[0]];
>> +
>> + d->ramp_up_us = args->args[1];
>> + d->ramp_down_us = args->args[2];
>> +
>> + dev_dbg(dev, "pin: %d, ramp_up_us: %d, ramp_down_us: %d\n",
>> + args->args[0], d->ramp_up_us, d->ramp_down_us);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dm_gpio_ops gpio_delay_ops = {
>> + .direction_output = gpio_delay_direction_output,
>> + .direction_input = gpio_delay_direction_input,
>> + .get_value = gpio_delay_get_value,
>> + .set_value = gpio_delay_set_value,
>> + .xlate = gpio_delay_xlate,
>> +};
>> +
>> +static int gpio_delay_probe(struct udevice *dev)
>> +{
>> + struct gpio_delay_priv *priv = dev_get_priv(dev);
>> + struct gpio_delay_desc *d;
>> + ofnode node = dev_ofnode(dev);
>> + int i = 0, ret, ngpio;
>> +
>> + ngpio = gpio_get_list_count(dev, "gpios");
>> + if (ngpio < 0)
>> + return ngpio;
>> +
>> + dev_dbg(dev, "gpios: %d\n", ngpio);
>> +
>> + priv->descs = devm_kmalloc_array(dev, ngpio, sizeof(*d), GFP_KERNEL);
>> + if (!priv->descs)
>> + return -ENOMEM;
>> +
>> + /* Request all GPIOs described in the controller node */
>> + for (i = 0; i < ngpio; i++) {
>> + d = &priv->descs[i];
>> + ret = gpio_request_by_name_nodev(node, "gpios", i,
>> + &d->real_gpio, GPIOD_IS_OUT);
>
> This will configure the GPIO as output and ACTIVE high/low flag will also
> be used per my understanding.
>
> Should the dir and value be deferred until set_value is invoked?

It is clear that it should be output because you just setting it up later.
But I don't have any issue not to pass any flag here.
Please let me know if you want me to change it to 0.

Thanks,
Michal