| Message ID | 722340b0efff3ed22a763ce6581c96ca403316d8.1737985435.git.Jonathan.Santos@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add features, improvements, and fixes | expand |
On 1/27/25 9:12 AM, Jonathan Santos wrote: > From: Sergiu Cuciurean <sergiu.cuciurean@analog.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 devicetree. > > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com> Since you (Jonathan Santos) are the one submitting these patches, you should add your Signed-off-by: here on the last line since you are the last one touching the patch. And if you feel you made significant changes on top of Sergiu's original patch, could could even add a Co-developed-by: line before that. More info: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin > --- > v2 Changes: > * Replaced usleep_range() for fsleep() and gpiod_direction_output() for > gpiod_set_value_cansleep(). > * Reset via SPI register is performed if the Reset GPIO is not defined. > --- Reviewed-by: David Lechner <dlechner@baylibre.com>
On 01/27, Jonathan Santos wrote: > From: Sergiu Cuciurean <sergiu.cuciurean@analog.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 devicetree. > > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com> > --- > v2 Changes: > * Replaced usleep_range() for fsleep() and gpiod_direction_output() for > gpiod_set_value_cansleep(). > * Reset via SPI register is performed if the Reset GPIO is not defined. > --- LGTM. Aside from including your the SoB, one minor thing about reset timings. Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > drivers/iio/adc/ad7768-1.c | 36 ++++++++++++++++++++++++------------ > 1 file changed, 24 insertions(+), 12 deletions(-) > > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > index fb8d6fae5f8a..17a49bf74637 100644 > --- a/drivers/iio/adc/ad7768-1.c > +++ b/drivers/iio/adc/ad7768-1.c > @@ -163,6 +163,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 > @@ -453,19 +454,30 @@ static int ad7768_setup(struct ad7768_state *st) > { > int ret; > > - /* > - * 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 > - * to 10. When the sequence is detected, the reset occurs. > - * See the datasheet, page 70. > - */ > - ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3); > - if (ret) > - return ret; > + st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset", > + GPIOD_OUT_HIGH); > + if (IS_ERR(st->gpio_reset)) > + return PTR_ERR(st->gpio_reset); > > - ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2); > - if (ret) > - return ret; > + if (st->gpio_reset) { > + fsleep(10); > + gpiod_set_value_cansleep(st->gpio_reset, 0); > + fsleep(10); Is 10 us enough time here? AD7768-1 datasheet page 58 sais "The time taken from RESET to an SPI write must be at least 200 μs." > + } else { > + /* > + * 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 > + * to 10. When the sequence is detected, the reset occurs. > + * See the datasheet, page 70. > + */ > + ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3); > + if (ret) > + return ret; > + > + ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2); > + if (ret) > + return ret; > + } > > st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in", > GPIOD_OUT_LOW); > -- > 2.34.1 >
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index fb8d6fae5f8a..17a49bf74637 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -163,6 +163,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 @@ -453,19 +454,30 @@ static int ad7768_setup(struct ad7768_state *st) { int ret; - /* - * 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 - * to 10. When the sequence is detected, the reset occurs. - * See the datasheet, page 70. - */ - ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3); - if (ret) - return ret; + st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset", + GPIOD_OUT_HIGH); + if (IS_ERR(st->gpio_reset)) + return PTR_ERR(st->gpio_reset); - ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2); - if (ret) - return ret; + if (st->gpio_reset) { + fsleep(10); + gpiod_set_value_cansleep(st->gpio_reset, 0); + fsleep(10); + } else { + /* + * 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 + * to 10. When the sequence is detected, the reset occurs. + * See the datasheet, page 70. + */ + ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x3); + if (ret) + return ret; + + ret = regmap_write(st->regmap, AD7768_REG_SYNC_RESET, 0x2); + if (ret) + return ret; + } st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in", GPIOD_OUT_LOW);