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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251214144839.2eec58f9@jic23-huawei>
Date: Sun, 14 Dec 2025 14:48:39 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Kurt Borja <kuurtb@...il.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Tobias Sperling
 <tobias.sperling@...ting.com>, David Lechner <dlechner@...libre.com>, Nuno
 Sá <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Jonathan Cameron
 <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v8 2/2] iio: adc: Add ti-ads1018 driver

On Thu, 11 Dec 2025 23:25:44 -0500
Kurt Borja <kuurtb@...il.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@...nel.org>
> Signed-off-by: Kurt Borja <kuurtb@...il.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.

> +}