Re: [PATCH 2/2] gpio: pcf857x: add support for reset-gpios on (most) PCA967x

[Bartosz Golaszewski]

On Thu, Feb 20, 2025 at 10:57 AM Quentin Schulz <foss+kernel@xxxxxxxxx> wrote:
>
> From: Quentin Schulz <quentin.schulz@xxxxxxxxx>
>
> The PCA9670, PCA9671, PCA9672 and PCA9673 all have a RESETN input pin
> that is used to reset the I2C GPIO expander.
>
> One needs to hold this pin low for at least 4us and the reset should be
> finished after about 100us according to the datasheet[1]. Once the reset
> is done, the "registers and I2C-bus state machine will be held in their
> default state until the RESET input is once again HIGH.".
>
> Because the logic is reset, the latch values eventually provided in the
> Device Tree via lines-initial-states property are inapplicable so they
> are simply ignored if a reset GPIO is provided.
>
> [1] https://www.nxp.com/docs/en/data-sheet/PCA9670.pdf 8.5 and fig 22.
> Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxx>
> ---
>  drivers/gpio/gpio-pcf857x.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
> index 7c57eaeb0afeba8953d998d8eec60a65b40efb6d..94077208e24ae99a1e8762e783f0eabc580fa520 100644
> --- a/drivers/gpio/gpio-pcf857x.c
> +++ b/drivers/gpio/gpio-pcf857x.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2007 David Brownell
>   */
>
> +#include <linux/delay.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> @@ -272,12 +273,11 @@ static const struct irq_chip pcf857x_irq_chip = {
>
>  static int pcf857x_probe(struct i2c_client *client)
>  {
> +       struct gpio_desc *rstn_gpio;
>         struct pcf857x *gpio;
> -       unsigned int n_latch = 0;
> +       unsigned int n_latch;
>         int status;
>
> -       device_property_read_u32(&client->dev, "lines-initial-states", &n_latch);
> -
>         /* Allocate, initialize, and register this gpio_chip. */
>         gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
>         if (!gpio)
> @@ -297,6 +297,29 @@ static int pcf857x_probe(struct i2c_client *client)
>         gpio->chip.direction_output     = pcf857x_output;
>         gpio->chip.ngpio                = (uintptr_t)i2c_get_match_data(client);
>
> +       rstn_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(rstn_gpio)) {
> +               return dev_err_probe(&client->dev, PTR_ERR(rstn_gpio),
> +                                    "failed to get reset GPIO\n");
> +       }
> +
> +       if (rstn_gpio) {
> +               /* Reset already held with devm_gpiod_get_optional with GPIOD_OUT_HIGH */
> +               usleep_range(4, 8); /* tw(rst) > 4us */
> +               gpiod_set_value(rstn_gpio, 0);

Should probably be gpiod_set_value_cansleep().

Bartosz