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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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