linux-kernel - Re: [PATCH v6 2/5] iio: proximity: rfd77402: Use kernel helper for result polling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXCWz6O2CdU_Sp8r@smile.fi.intel.com>
Date: Wed, 21 Jan 2026 11:05:19 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: raskar.shree97@...il.com
Cc: Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Nuno Sá <nuno.sa@...log.com>,
	Andy Shevchenko <andy@...nel.org>, skhan@...uxfoundation.org,
	david.hunter.linux@...il.com, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 2/5] iio: proximity: rfd77402: Use kernel helper for
 result polling

On Wed, Jan 21, 2026 at 02:05:42AM +0530, Shrikant Raskar via B4 Relay wrote:

> Replace the manually written polling loop with read_poll_timeout(),
> the kernel's standard helper for waiting on hardware status.
> Move the polling logic into a dedicated helper function, as it will
> be reused by future updates.
> 
> This makes the code easier to read and avoids repeating the same
> polling code in the driver.

It has some repetitions, I would rephrase as:

  Replace the manually written polling loop with read_poll_timeout(),
  the kernel's standard helper for waiting on hardware status. This
  makes the code easier to read.

  Move the polling logic into a dedicated helper function, as it will
  be reused by future updates.

(also mind the blank lines and paragraphs).

...

> +static int rfd77402_wait_for_result(struct rfd77402_data *data)
>  {
> +	struct i2c_client *client = data->client;

> +	int ret;

I would named it "data" to distinguish from the usual returned code of the calls,
so like in more verbose case

	int data;
	int ret;

	ret = read_poll_timeout(..., data, data & ..., ...);
	if (data < 0)
		return data;
	if (ret)
		return ret;

> +	return read_poll_timeout(i2c_smbus_read_byte_data, ret,
> +				 ret & RFD77402_ICSR_RESULT,

'data' (ex-"ret") may be negative and this will be triggered.
I think you want 'data < 0 || (data & RFD77402_ICSR_RESULT)

> +				 10 * USEC_PER_MSEC,
> +				 10 * 10 * USEC_PER_MSEC,

This makes sleeps shorter by 2. Why?

> +				 false,
> +				 client, RFD77402_ICSR);
> +}

-- 
With Best Regards,
Andy Shevchenko