Le 27/06/2024 à 13:59, Esteban Blanc a écrit :
This adds a new driver for the Analog Devices INC. AD4030-24 ADC. The driver implements basic support for the AD4030-24 1 channel differential ADC with hardware gain and offset control. Signed-off-by: Esteban Blanc <eblanc-rdvid1DuHRBWk0Htik3J/w@xxxxxxxxxxxxxxxx> ---
Hi, a few nitpick below, should it help. ...
+static int ad4030_read_avail(struct iio_dev *indio_dev, + struct iio_chan_spec const *channel, + const int **vals, int *type, + int *length, long mask) +{ + struct ad4030_state *st = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_CALIBBIAS: + *vals = ad4030_get_offset_avail(st); + *type = IIO_VAL_INT; + return IIO_AVAIL_RANGE; + + case IIO_CHAN_INFO_CALIBSCALE: + *vals = (void *)ad4030_gain_avail; + *type = IIO_VAL_INT_PLUS_MICRO; + return IIO_AVAIL_RANGE;
Nitpick: in other switch below, there is a blank line before default:
+ default: + return -EINVAL; + } +}
...
+static int ad4030_regulators_get(struct ad4030_state *st) +{ + struct device *dev = &st->spi->dev; + static const char * const ids[] = {"vdd-5v", "vdd-1v8"}; + int ret; + + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(ids), ids); + if (ret) + return dev_err_probe(dev, ret, "Failed to enable regulators\n"); + + st->vio_uv = devm_regulator_get_enable_read_voltage(dev, "vio"); + if (st->vio_uv < 0) + return dev_err_probe(dev, st->vio_uv, + "Failed to enable and read vio voltage");
Nitpick: missing \n
+ + st->vref_uv = devm_regulator_get_enable_read_voltage(dev, "ref"); + if (st->vref_uv < 0) { + if (st->vref_uv != -ENODEV) + return dev_err_probe(dev, st->vref_uv, + "Failed to read vref voltage");
Nitpick: missing \n
+ + /* if not using optional REF, the internal REFIN must be used */ + st->vref_uv = devm_regulator_get_enable_read_voltage(dev, "refin"); + if (st->vref_uv < 0) + return dev_err_probe(dev, st->vref_uv, + "Failed to read vrefin voltage");
Nitpick: missing \n
+ } + + if (st->vref_uv < AD4030_VREF_MIN_UV || st->vref_uv > AD4030_VREF_MAX_UV) + return dev_err_probe(dev, -EINVAL, + "vref(%d) must be in the range [%lu %lu]\n", + st->vref_uv, AD4030_VREF_MIN_UV, + AD4030_VREF_MAX_UV); + + return 0; +}
...
+static int ad4030_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + struct iio_dev *indio_dev; + struct ad4030_state *st; + int ret; + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
Nitpick: dev could be used. It is the same &spi->dev
+ if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->spi = spi; + + /* Make sure the SPI clock is within range to read register */ + st->conversion_speed_hz = spi->max_speed_hz; + if (spi->max_speed_hz > AD4030_SPI_MAX_REG_XFER_SPEED) + spi->max_speed_hz = AD4030_SPI_MAX_REG_XFER_SPEED; + + st->regmap = devm_regmap_init(&spi->dev, &ad4030_regmap_bus, st,
Nitpick: dev could be used. It is the same &spi->dev
+ &ad4030_regmap_config); + if (IS_ERR(st->regmap)) + dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
Nitpick: dev could be used. It is the same &spi->dev
+ "Failed to initialize regmap\n"); + + st->chip = spi_get_device_match_data(spi); + if (!st->chip) + return -EINVAL; + + ret = ad4030_regulators_get(st); + if (ret) + return ret; + + ret = ad4030_reset(st); + if (ret) + return ret; + + ret = ad4030_detect_chip_info(st); + if (ret) + return ret; + + ret = ad4030_config(st); + if (ret) + return ret; + + st->cnv_gpio = devm_gpiod_get(dev, "cnv", GPIOD_OUT_LOW); + if (IS_ERR(st->cnv_gpio)) + return dev_err_probe(dev, PTR_ERR(st->cnv_gpio), + "Failed to get cnv gpio");
Nitpick: missing \n
+ + indio_dev->name = st->chip->name; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &ad4030_iio_info; + indio_dev->channels = st->chip->channels; + indio_dev->num_channels = 2 * st->chip->num_channels + 1;
Nitpick : 2 spaces after =
+ indio_dev->available_scan_masks = st->chip->available_masks; + indio_dev->masklength = st->chip->available_masks_len; + + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, + iio_pollfunc_store_time, + ad4030_trigger_handler, + &ad4030_buffer_setup_ops); + if (ret) + return dev_err_probe(dev, ret, "Failed to setup triggered buffer\n"); + + return devm_iio_device_register(dev, indio_dev); +}
... CJ