Re: [PATCH] iio: resolver: ad2s1210: notify trigger and clear state on fault read error

[David Lechner <dlechner@baylibre.com>]

On 5/16/26 6:28 AM, Jonathan Cameron wrote:
> On Fri, 15 May 2026 18:31:38 +0500
> Stepan Ionichev <sozdayvek@gmail.com> wrote:
> 
>> ad2s1210_trigger_handler() walks several scan-mask branches and uses
>> "goto error_ret" to land on the iio_trigger_notify_done() teardown at
>> the bottom of the function for every I/O error -- except the
>> MOD_CONFIG fault-register read, which uses a bare "return ret":
>>
>> 	if (st->fixed_mode == MOD_CONFIG) {
>> 		unsigned int reg_val;
>>
>> 		ret = regmap_read(st->regmap, AD2S1210_REG_FAULT, &reg_val);
>> 		if (ret < 0)
>> 			return ret;
>> 		...
>> 	}
>>
>> Two problems on that path:
>>
>>   - the handler returns a negative errno where the prototype expects
>>     an irqreturn_t (IRQ_HANDLED / IRQ_NONE), so the caller in the
>>     IIO core sees a value outside the enum;
>>   - iio_trigger_notify_done() is skipped, leaving the trigger
>>     busy-flag set. A single transient SPI/regmap error on the fault
>>     read then wedges the trigger so subsequent samples are dropped
>>     until the consumer is detached.
>>
>> Convert the error path to "goto error_ret" so the failure path goes
>> through the same notify_done() teardown as every other error in the
>> handler.
>>
>> Fixes: f9b9ff95be8c ("iio: resolver: ad2s1210: add support for adi,fixed-mode")
>> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> 
> Not related to this as the fix is good, but I was obviously half asleep
> when guard() got added to this function which is full of gotos :(
> 
> If you have time could you look at factoring out helper that basically lifts
> everything before trigger_notify_done() which should not be done with
> the guard held - at least in this case it's not a deadlock source
> as the device doesn't have any triggers!
> 
> That helper can use guard() and do direct returns to simplify the code flow.

I can do that since my name is all over the driver.

> 
> Anyhow, this fix is good so applied and marked for stable.

Reviewed-by: David Lechner <dlechner@baylibre.com>