Re: [PATCH v2 16/17] mfd: adp5585: add support for a reset pin

["Nuno Sá" <noname.nuno@gmail.com>]

On Mon, 2025-04-21 at 12:46 +0300, Laurent Pinchart wrote:
> Hi Nuno,
> 
> Thank you for the patch.
> 
> On Tue, Apr 15, 2025 at 03:49:32PM +0100, Nuno Sá via B4 Relay wrote:
> > From: Nuno Sá <nuno.sa@analog.com>
> > 
> > Make sure to perform an Hardware reset during probe  if the pin is given
> > in FW.
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  drivers/mfd/adp5585.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/mfd/adp5585.c b/drivers/mfd/adp5585.c
> > index
> > 667cc5bd0745f64eec60837ec3c00057af0cddeb..2af110fb28054c8ad6709a9a42cb21919b
> > e433e7 100644
> > --- a/drivers/mfd/adp5585.c
> > +++ b/drivers/mfd/adp5585.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/device.h>
> >  #include <linux/err.h>
> >  #include <linux/i2c.h>
> > +#include <linux/gpio/consumer.h>
> 
> Alphabetic order please.
> 
> >  #include <linux/mfd/adp5585.h>
> >  #include <linux/mfd/core.h>
> >  #include <linux/minmax.h>
> > @@ -833,6 +834,7 @@ static int adp5585_i2c_probe(struct i2c_client *i2c)
> >  {
> >  	const struct adp5585_info *info;
> >  	struct adp5585_dev *adp5585;
> > +	struct gpio_desc *gpio;
> >  	unsigned int id;
> >  	int ret;
> >  
> > @@ -854,6 +856,16 @@ static int adp5585_i2c_probe(struct i2c_client *i2c)
> >  	if (ret)
> >  		return ret;
> >  
> > +	gpio = devm_gpiod_get_optional(&i2c->dev, "reset", GPIOD_OUT_HIGH);
> > +	if (IS_ERR(gpio))
> > +		return PTR_ERR(gpio);
> > +
> > +	if (gpio) {
> > +		fsleep(30);
> > +		gpiod_set_value_cansleep(gpio, 0);
> > +		fsleep(60);
> 
> Where do those values come from ? The only possibly related timing
> information I found in the datasheet indicate a GPIO debounce delay of
> 70µs.

Yeps this were just some reasonable values that work. The datasheet does not
mention any timing...

I can add a comment if you like stating there's no defined values in the
datasheet and these values are "just" some values that work.

- Nuno Sá

> 
> > +	}
> > +
> >  	adp5585->regmap = devm_regmap_init_i2c(i2c, info->regmap_config);
> >  	if (IS_ERR(adp5585->regmap))
> >  		return dev_err_probe(&i2c->dev, PTR_ERR(adp5585->regmap),