| Message ID | 305f0bb4a90aba547de6b46d4c9dcf04a2a4db72.1741268122.git.Jonathan.Santos@analog.com |
|---|---|
| State | New |
| Headers | show |
| Series | iio: adc: ad7768-1: Add features, improvements, and fixes | expand |
On 03/06, 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. The description doesn't seem to match the changes nor the patch title. You are essentinally adding support for hardware reset. Change the commit description to reflect that. The default state of GPIOs would not impact device reset because (in theory) they weren't being connected to the reset pin prevously. > > Reviewed-by: David Lechner <dlechner@baylibre.com> > Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com> > Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > --- > v4 Changes: > * None. > > v3 Changes: > * fixed SoB order. > * increased delay after finishing the reset action to 200us, as the > datasheet recommends. > > 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. > --- > 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 04a26e5b7d5c..86f44d28c478 100644 > --- a/drivers/iio/adc/ad7768-1.c > +++ b/drivers/iio/adc/ad7768-1.c > @@ -166,6 +166,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 > @@ -487,19 +488,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(200); > + } 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 >
On Fri, 7 Mar 2025 10:42:09 -0300 Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote: > On 03/06, 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. > > The description doesn't seem to match the changes nor the patch title. You are > essentinally adding support for hardware reset. Change the commit description to > reflect that. > > The default state of GPIOs would not impact device reset because (in theory) > they weren't being connected to the reset pin prevously. Also possible some other entity was doing appropriate reset (sometimes it's just how the system is wired - puts appropriate voltage to come out of reset after some other action - no kernel involvement). Agreed in general though that this is simpler described as just implementing hardware reset and not worry about those details. Jonathan > > > > > Reviewed-by: David Lechner <dlechner@baylibre.com> > > Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com> > > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com> > > Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com> > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > > --- > > v4 Changes: > > * None. > > > > v3 Changes: > > * fixed SoB order. > > * increased delay after finishing the reset action to 200us, as the > > datasheet recommends. > > > > 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. > > --- > > 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 04a26e5b7d5c..86f44d28c478 100644 > > --- a/drivers/iio/adc/ad7768-1.c > > +++ b/drivers/iio/adc/ad7768-1.c > > @@ -166,6 +166,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 > > @@ -487,19 +488,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(200); > > + } 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 04a26e5b7d5c..86f44d28c478 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -166,6 +166,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 @@ -487,19 +488,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(200); + } 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);