linux-kernel - Re: [PATCH v6 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] [day] [month] [year] [list]
Message-ID: <20251213161638.4b5df606@jic23-huawei>
Date: Sat, 13 Dec 2025 16:16:38 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: "Kurt Borja" <kuurtb@...il.com>
Cc: "David Lechner" <dlechner@...libre.com>, "Rob Herring"
 <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor
 Dooley" <conor+dt@...nel.org>, "Tobias Sperling"
 <tobias.sperling@...ting.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 v6 2/2] iio: adc: Add ti-ads1018 driver

On Tue, 09 Dec 2025 23:08:33 -0500
"Kurt Borja" <kuurtb@...il.com> wrote:

> On Mon Dec 8, 2025 at 11:00 AM -05, David Lechner wrote:
> > On 12/7/25 10:06 PM, Kurt Borja wrote:  
> >> On Sun Dec 7, 2025 at 2:56 PM -05, Jonathan Cameron wrote:  
> >>> On Sun, 7 Dec 2025 11:12:51 -0600
> >>> David Lechner <dlechner@...libre.com> wrote:
> >>>  
> >>>> On 12/7/25 10:02 AM, Kurt Borja wrote:  
> >>>>> On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:    
> >>>>>> On Thu, 04 Dec 2025 13:01:28 -0500
> >>>>>> Kurt Borja <kuurtb@...il.com> wrote:
> >>>>>>    
> >>>>>>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
> >>>>>>> analog-to-digital converters.
> >>>>>>>
> >>>>>>> These 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.
> >>>>>>>
> >>>>>>> Signed-off-by: Kurt Borja <kuurtb@...il.com>    
> >>>>>
> >>>>> ...
> >>>>>     
> >>>>>>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
> >>>>>>> +	.type = IIO_VOLTAGE,							\
> >>>>>>> +	.channel = _chan,							\
> >>>>>>> +	.scan_index = _index,							\
> >>>>>>> +	.scan_type = {								\
> >>>>>>> +		.sign = 's',							\
> >>>>>>> +		.realbits = _realbits,						\
> >>>>>>> +		.storagebits = 16,						\
> >>>>>>> +		.shift = 16 - _realbits,					\
> >>>>>>> +		.endianness = IIO_BE,						\
> >>>>>>> +	},									\
> >>>>>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
> >>>>>>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
> >>>>>>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\    
> >>>>>>
> >>>>>> What motivates per channel sampling frequency?
> >>>>>>
> >>>>>> Given you have to write it each time you configure I guess it doesn't matter much
> >>>>>> either way.    
> >>>>>
> >>>>> I guess making it shared by all is simpler too, so I'll go with that.
> >>>>>     
> >>>> Just keep in mind that if there is ever some use case we don't know
> >>>> about that would require a different rate per channel, we can't change
> >>>> it without breaking usespace. Once the decision is made, we are
> >>>> locked in. Keeping it per-channel seems more future-proof to me.  
> >>>
> >>> Only way I can think of that might cause that to matter would be
> >>> if the complex dance to avoid the onehot buffer restriction is added.
> >>> Given you gave this response I went looking and that might make
> >>> sense as an enhancement as the SPI protocol would allow a crafted message
> >>> sequence to do this efficiently.  Extension of figure 15 where first message
> >>> sets config and after that they read out channel and set config for next one.  
> >> 
> >> This is possible, yes. But would the timestamp even make sense in this
> >> case? Even in the fastest sampling rate, we would have to wait at least
> >> 1 ms for each channel and the timestamp would become stale.
> >> 
> >> That was my reasoning for using the onehot restriction.
> >> 
> >> Is that acceptable? Or maybe we would need to disallow the timestamp
> >> channel if more than one channel is selected?  
> >
> > Yes. We have pretty much the same situation with timestamps on every
> > other ADC. The timestamp is usually when one full set of samples is
> > triggered. Not when the actual individual conversions are performed.  
> 
> This is good to know for future patches or drivers. Thanks!
In theory we have the ABI to describe the relative timing, but meh, it's
rarely useful to do so. Hence we don't use it except in very odd corner cases.
See docs for in_voltageY_convdelay in Documentation/ABI/testing/sysfs-bus-iio

I've vaguely thought about suggesting we use that more widely but no one has
asked for it.  Use cases that I came across are things like 
complex filters across multiple channels (think of orientation tracking
if they were accelerometer axis).

So generally probably not a good idea to describe it at all. :)

Jonathan


> 
>