Re: [PATCH v8 2/2] iio: adc: Add ti-ads1018 driver

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 11 Dec 2025 23:25:44 -0500
Kurt Borja <kuurtb@gmail.com> wrote:

> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
> analog-to-digital converters.
> 
> This chips' MOSI pin is shared with a data-ready interrupt. Defining
> this interrupt in devicetree is optional, therefore we only create an
> IIO trigger if one is found.
> 
> Handling this interrupt requires some considerations. When enabling the
> trigger the CS line is tied low (active), thus we need to hold
> spi_bus_lock() too, to avoid state corruption. This is done inside the
> set_trigger_state() callback, to let users use other triggers without
> wasting a bus lock.
> 
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>

Hi Kurt, 

A couple of minor formatting things. All trivial so I tweaked whilst
applying. Applied to the testing branch of iio.git. I'll rebase that
on rc1 once available then push out as togreg for linux-next to pick
it up.

Thanks,

Jonathan



> diff --git a/drivers/iio/adc/ti-ads1018.c b/drivers/iio/adc/ti-ads1018.c
> new file mode 100644
> index 000000000000..e3087bb47699
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads1018.c

> +
> +static int
> +ads1018_write_raw_get_fmt(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan, long mask)
I'm not immediately seeing why this particular line wrap mapes sense.
static int ads1018_write_raw_get_fmt(struct iio_dev *indio_dev,
			  	     struct iio_chan_spec const *chan,
				     long mask)

Is more common way to do this particular combination.

There are a few other places where the wrap choice wasn't quite what
I'd have gone with, so I tweaked those as well.


> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +}
> +
> +static const struct iio_info ads1018_iio_info = {
> +	.read_raw = ads1018_read_raw,
> +	.read_avail = ads1018_read_avail,
> +	.write_raw = ads1018_write_raw,
> +	.write_raw_get_fmt = ads1018_write_raw_get_fmt,
> +};


> +static int ads1018_spi_probe(struct spi_device *spi)
> +{
> +	const struct ads1018_chip_info *info = spi_get_device_match_data(spi);
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev;
> +	struct ads1018 *ads1018;
> +	int ret;

...

> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> +					      iio_pollfunc_store_time,
> +					      ads1018_trigger_handler,
> +					      &ads1018_buffer_ops);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);

You have dev, so makes sense to use it here too.

> +}