linux-kernel - Re: [PATCH v1 07/15] iio: adc: ad7768-1: Add reset gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250112123537.214d0ba6@jic23-huawei>
Date: Sun, 12 Jan 2025 12:35:37 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Jonathan Santos <Jonathan.Santos@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Sergiu Cuciurean
 <sergiu.cuciurean@...log.com>, lars@...afoo.de,
 Michael.Hennerich@...log.com, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, marcelo.schmitt1@...il.com
Subject: Re: [PATCH v1 07/15] iio: adc: ad7768-1: Add reset gpio

On Tue, 7 Jan 2025 17:40:53 -0600
David Lechner <dlechner@...libre.com> wrote:

> On 1/7/25 9:25 AM, Jonathan Santos wrote:
> > From: Sergiu Cuciurean <sergiu.cuciurean@...log.com>
> > 
> > Depending on the controller, the default state of a gpio can vary. This
> > change excludes the probability that the dafult state of the ADC reset
> > gpio will be HIGH if it will be passed as reference in the deivcetree.

devicetree

> > 
> > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@...log.com>
> > ---
> >  drivers/iio/adc/ad7768-1.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> > index 881446462ff5..f73b9aec8b0f 100644
> > --- a/drivers/iio/adc/ad7768-1.c
> > +++ b/drivers/iio/adc/ad7768-1.c
> > @@ -161,6 +161,7 @@ struct ad7768_state {
> >  	struct completion completion;
> >  	struct iio_trigger *trig;
> >  	struct gpio_desc *gpio_sync_in;
> > +	struct gpio_desc *gpio_reset;
> >  	const char *labels[ARRAY_SIZE(ad7768_channels)];
> >  	/*
> >  	 * DMA (thus cache coherency maintenance) may require the
> > @@ -441,6 +442,18 @@ static int ad7768_setup(struct ad7768_state *st)
> >  {
> >  	int ret;
> >  
> > +	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> > +						 GPIOD_OUT_LOW);  
> 
> Could be simplified by setting this to GPIOD_OUT_HIGH and drop
> 
> 	gpiod_direction_output(st->gpio_reset, 1);
> 
> > +	if (IS_ERR(st->gpio_reset))
> > +		return PTR_ERR(st->gpio_reset);
> > +
> > +	if (st->gpio_reset) {
> > +		gpiod_direction_output(st->gpio_reset, 1);

It's an output already, gpiod_set_value() should work I think?
Possibly even then sleep allowing variants


> > +		usleep_range(10, 15);
> > +		gpiod_direction_output(st->gpio_reset, 0);
> > +		usleep_range(10, 15);  
> 
> prefer fsleep()
> 
> > +	}
> > +  
> 
> We can move the code below in an else since we don't need to do 2 resets.
> 
> >  	/*
> >  	 * Two writes to the SPI_RESET[1:0] bits are required to initiate
> >  	 * a software reset. The bits must first be set to 11, and then  
>