Re: [PATCH RFC 2/5] iio: adc: ad4030: add driver for ad4030-24

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux