Hello all,
There is another thing that I forgot to mention inline with my last email.
> Add support for AD7770, AD7771, AD7779 ADCs. The device is capable of
> sending out data both on DOUT lines interface,as on the SDO line.
> The driver currently implements only the SDO data streaming mode. SPI
> communication is used alternatively for accessing registers and
> streaming data. Register access are protected by crc8.
>
> Signed-off-by: Ramona Alexandra Nechita <ramona.nechita@xxxxxxxxxx>
Hi Ramona,
....
>> +static int ad7779_probe(struct spi_device *spi) {
>> + struct iio_dev *indio_dev;
>> + struct ad7779_state *st;
>> + struct gpio_desc *reset_gpio, *start_gpio;
>> + int ret;
>> +
>> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> +
>> + st = iio_priv(indio_dev);
>> +
>> + st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
>> + if (IS_ERR(st->mclk))
>> + return PTR_ERR(st->mclk);
>> +
>> + if (!spi->irq)
>> + return dev_err_probe(&spi->dev, ret,
>> + "DRDY irq not present\n");
>> +
>> + reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
>> + if (IS_ERR(reset_gpio))
>> + return PTR_ERR(reset_gpio);
>> +
>> + start_gpio = devm_gpiod_get(&spi->dev, "start", GPIOD_OUT_HIGH);
>> + if (IS_ERR(start_gpio))
>> + return PTR_ERR(start_gpio);
>> +
>> + crc8_populate_msb(ad7779_crc8_table, AD7779_CRC8_POLY);
>> + st->spi = spi;
>> +
>> + st->chip_info = spi_get_device_match_data(spi);
>> + if (!st->chip_info)
>> + return -ENODEV;
>> +
>> + ret = ad7779_reset(indio_dev, reset_gpio);
>> + if (ret)
>> + return ret;
>> +
>> + ad7779_powerup(st, start_gpio);
>> + if (ret)
>> + return ret;
>What powers the device down again if we hit an error?
>
>Probably need a devm_add_action_or_reset() or if it self powers down may a comment on that.
In the powerup function there are only some register writes and the start gpio is only a synchronization pulse (perhaps the name powerup is not very appropriate),
would an action or reset be necessary in this case? Since the regulators are not used in the driver, should there be a function disabling them anyway?
Best Regards,
Ramona
