On 01/27, Jonathan Santos wrote:
> From: Sergiu Cuciurean <sergiu.cuciurean@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
> 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
>
