All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Ruslan Valiyev <linuxoid@gmail.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	David Lechner <dlechner@baylibre.com>,
	Nuno Sa <nuno.sa@analog.com>, Andy Shevchenko <andy@kernel.org>,
	linux-iio@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] staging: iio: adc: ad7816: add timeout to busy-wait loop
Date: Thu, 26 Feb 2026 11:15:22 +0200	[thread overview]
Message-ID: <aaAPKiwCiGtys-P-@smile.fi.intel.com> (raw)
In-Reply-To: <20260226082547.69616-1-linuxoid@gmail.com>

On Thu, Feb 26, 2026 at 08:25:47AM +0000, Ruslan Valiyev wrote:
> The ad7816_spi_read() function polls the busy GPIO pin in a tight
> loop without any timeout. If the hardware fails to deassert the
> busy signal, the kernel hangs indefinitely in an unbounded
> busy-wait.
> 
> Replace the open-coded while/cpu_relax() loop with
> read_poll_timeout_atomic() which polls every 5 us and returns
> -ETIMEDOUT after 1 ms. Per the AD7816 datasheet the maximum
> conversion time is 27 us (temperature channel), so 1 ms provides
> generous margin.
> 
> Use the atomic variant to preserve the existing busy-wait semantics
> of the original loop.
> 
> Also handle the case where gpiod_get_value() returns a negative
> error code, which the original loop would treat as busy and keep
> spinning on indefinitely.

First of all, start a new email thread for a new version of the patch.

Second, give at least 24h between the versions to have a chance being
reviewed by more people.


>  	if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
> +		int val;
> +
> +		ret = read_poll_timeout_atomic(gpiod_get_value, val, val <= 0,

While val <= 0 is technically correct, semantically it's harder to read,
it's better to use val < 0 || val == 0.

> +					5, 1000, false, chip->busy_pin);

This has an indentation issue now.

> +		if (val < 0)
> +			return val;
> +		if (ret)
> +			return ret;

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2026-02-26  9:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-26  5:26 [PATCH] staging: iio: adc: ad7816: add timeout to busy-wait loop Ruslan Valiyev
2026-02-26  8:01 ` Andy Shevchenko
2026-02-26  8:25   ` Ruslan Valiyev
2026-02-26  9:20     ` Dan Carpenter
2026-02-26  8:25   ` [PATCH v2] " Ruslan Valiyev
2026-02-26  9:15     ` Andy Shevchenko [this message]
2026-02-26  9:29   ` Ruslan Valiyev

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=aaAPKiwCiGtys-P-@smile.fi.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linuxoid@gmail.com \
    --cc=nuno.sa@analog.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.