mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Stepan Ionichev <sozdayvek@gmail.com>
Cc: lars@metafoo.de, Michael.Hennerich@analog.com,
	dlechner@baylibre.com, nuno.sa@analog.com, andy@kernel.org,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: resolver: ad2s1210: notify trigger and clear state on fault read error
Date: Sat, 16 May 2026 12:28:38 +0100	[thread overview]
Message-ID: <20260516122838.163a77d3@jic23-huawei> (raw)
In-Reply-To: <20260515133138.32319-1-sozdayvek@gmail.com>

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.

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

> ---
>  drivers/iio/resolver/ad2s1210.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
> index 774728c80..1be19fe8a 100644
> --- a/drivers/iio/resolver/ad2s1210.c
> +++ b/drivers/iio/resolver/ad2s1210.c
> @@ -1334,7 +1334,7 @@ static irqreturn_t ad2s1210_trigger_handler(int irq, void *p)
>  
>  		ret = regmap_read(st->regmap, AD2S1210_REG_FAULT, &reg_val);
>  		if (ret < 0)
> -			return ret;
> +			goto error_ret;
>  
>  		st->sample.fault = reg_val;
>  	}


  reply	other threads:[~2026-05-16 11:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 13:31 Stepan Ionichev
2026-05-16 11:28 ` Jonathan Cameron [this message]
2026-05-16 14:49   ` David Lechner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260516122838.163a77d3@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=sozdayvek@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®