Am Donnerstag, 20. Februar 2025, 10:56:52 MEZ schrieb Quentin Schulz: > 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> this is missing #include <linux/gpio/consumer.h> because otherwise you end up with ../drivers/gpio/gpio-pcf857x.c: In function ‘pcf857x_probe’: ../drivers/gpio/gpio-pcf857x.c:300:21: error: implicit declaration of function ‘devm_gpiod_get_optional’; did you mean ‘devm_regulator_get_optional’? [-Wimplicit-function-declaration] 300 | rstn_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH); | ^~~~~~~~~~~~~~~~~~~~~~~ | devm_regulator_get_optional ../drivers/gpio/gpio-pcf857x.c:300:68: error: ‘GPIOD_OUT_HIGH’ undeclared (first use in this function) 300 | rstn_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH); | ^~~~~~~~~~~~~~ ../drivers/gpio/gpio-pcf857x.c:300:68: note: each undeclared identifier is reported only once for each function it appears in ../drivers/gpio/gpio-pcf857x.c:309:17: error: implicit declaration of function ‘gpiod_set_value’ [-Wimplicit-function-declaration] 309 | gpiod_set_value(rstn_gpio, 0); | ^~~~~~~~~~~~~~~ > @@ -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); > + usleep_range(100, 200); /* trst > 100uS */ > + > + /* > + * Reset "will initialize to their default states of all I/Os to > + * inputs with weak current source to VDD", which is the same as > + * writing 1 for all I/Os which is 0 in n_latch. > + */ > + n_latch = 0; > + } else { > + device_property_read_u32(&client->dev, "lines-initial-states", > + &n_latch); device_property_read_u32 will not fill n_latch if the property is missing. Before n_latch was always set to 0 at the declaration point above. I guess that should be kept, because we want 0, except if device_property_read_u32 provides a different value. Heiko