[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <04aee30f-908b-4390-934a-e49990217d15@baylibre.com>
Date: Mon, 8 Dec 2025 10:00:50 -0600
From: David Lechner <dlechner@...libre.com>
To: Kurt Borja <kuurtb@...il.com>, Jonathan Cameron <jic23@...nel.org>
Cc: 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 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.
>
>>
>> Given that is sane, I agree with you that we should probably keep these separate.
>> I doubt anyone will use different sampling frequencies even if possible but you
>> never know.
>
> I'll leave it as is then.
>
>>
>> Jonathan
>>
>>>
>
>
Powered by blists - more mailing lists