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

[David Lechner <dlechner@baylibre.com>]

On 12/15/25 10:54 AM, Kurt Borja wrote:
> On Mon Dec 15, 2025 at 10:55 AM -05, David Lechner wrote:
>> On 12/11/25 10:25 PM, Kurt Borja wrote:
>>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
>>> analog-to-digital converters.
>>>

...

>>> +static irqreturn_t ads1018_trigger_handler(int irq, void *p)
>>> +{
>>> +	struct iio_poll_func *pf = p;
>>> +	struct iio_dev *indio_dev = pf->indio_dev;
>>> +	struct ads1018 *ads1018 = iio_priv(indio_dev);
>>> +	struct {
>>> +		__be16 conv;
>>> +		aligned_s64 ts;
>>> +	} scan = {};
>>> +	int ret;
>>> +
>>
>>
>>> +	if (iio_device_claim_buffer_mode(indio_dev))
>>> +		goto out_notify_done;
>>
>> This should not be needed. It should not be possible to
>> exit buffer mode without triggers being stopped first.
>> (No other driver is doing this.)
> 
> Previously I had my own lock here because ads1018_spi_read_exclusive()
> needs locking.

What exactly are we protecting against here? I.e. give side-by-side
lists of possible concurrent function calls where there could be a
problem.

Any call to iio_device_claim_direct() will already fail without
calling iio_device_claim_buffer_mode() here. And since this is
an interrupt handler, we don't have to worry about reentrancy (it
can't be called again until the previous call returns). And nowhere
else in the driver is calling iio_device_claim_buffer_mode(). So
calling it here doesn't actually add any protection AFAICT.

> I dropped it per your suggestion in v1 I believe, but
> given the discussion in the cleanup.h series I was thinking in bringing
> it back.
> 
> But yes, the scope can be adjusted too.
>