From: Shrikant Raskar <raskar.shree97@gmail.com>
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.
Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
---
drivers/iio/proximity/rfd77402.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/iio/proximity/rfd77402.c b/drivers/iio/proximity/rfd77402.c
index 69cc1505b964..716c9330b14e 100644
--- a/drivers/iio/proximity/rfd77402.c
+++ b/drivers/iio/proximity/rfd77402.c
@@ -12,6 +12,7 @@
#include <linux/delay.h>
#include <linux/i2c.h>
+#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/iio/iio.h>
@@ -110,10 +111,23 @@ static int rfd77402_set_state(struct i2c_client *client, u8 state, u16 check)
return 0;
}
-static int rfd77402_measure(struct i2c_client *client)
+static int rfd77402_wait_for_result(struct rfd77402_data *data)
{
+ struct i2c_client *client = data->client;
+ int ret;
+
+ return read_poll_timeout(i2c_smbus_read_byte_data, ret,
+ ret & RFD77402_ICSR_RESULT,
+ 10 * USEC_PER_MSEC,
+ 10 * 10 * USEC_PER_MSEC,
+ false,
+ client, RFD77402_ICSR);
+}
+
+static int rfd77402_measure(struct rfd77402_data *data)
+{
+ struct i2c_client *client = data->client;
int ret;
- int tries = 10;
ret = rfd77402_set_state(client, RFD77402_CMD_MCPU_ON,
RFD77402_STATUS_MCPU_ON);
@@ -126,19 +140,9 @@ static int rfd77402_measure(struct i2c_client *client)
if (ret < 0)
goto err;
- while (tries-- > 0) {
- ret = i2c_smbus_read_byte_data(client, RFD77402_ICSR);
- if (ret < 0)
- goto err;
- if (ret & RFD77402_ICSR_RESULT)
- break;
- msleep(20);
- }
-
- if (tries < 0) {
- ret = -ETIMEDOUT;
+ ret = rfd77402_wait_for_result(data);
+ if (ret < 0)
goto err;
- }
ret = i2c_smbus_read_word_data(client, RFD77402_RESULT_R);
if (ret < 0)
@@ -168,7 +172,7 @@ static int rfd77402_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
mutex_lock(&data->lock);
- ret = rfd77402_measure(data->client);
+ ret = rfd77402_measure(data);
mutex_unlock(&data->lock);
if (ret < 0)
return ret;
--
2.43.0On 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
> > > + 10 * USEC_PER_MSEC, > > + 10 * 10 * USEC_PER_MSEC, > > This makes sleeps shorter by 2. Why? I have considered the timeout values from the RFD77402 datasheet. The timeout values mentioned in section 3.1.1 Single Measure are as below: 1. Every Status Check = 10ms 2. Whole Flow = 100 ms Regards, Shrikant
On Sat, Jan 24, 2026 at 10:26:04PM +0530, Shrikant wrote: ... > > > + 10 * USEC_PER_MSEC, > > > + 10 * 10 * USEC_PER_MSEC, > > > > This makes sleeps shorter by 2. Why? > I have considered the timeout values from the RFD77402 datasheet. > The timeout values mentioned in section 3.1.1 Single Measure are as below: > 1. Every Status Check = 10ms > 2. Whole Flow = 100 ms So, you should do this in a separate change explaining this. -- With Best Regards, Andy Shevchenko
On Mon, Jan 26, 2026 at 3:14 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Sat, Jan 24, 2026 at 10:26:04PM +0530, Shrikant wrote:
>
> ...
>
> > > > + 10 * USEC_PER_MSEC,
> > > > + 10 * 10 * USEC_PER_MSEC,
> > >
> > > This makes sleep shorter by 2. Why?
> > I have considered the timeout values from the RFD77402 datasheet.
> > The timeout values mentioned in section 3.1.1 Single Measure are as below:
> > 1. Every Status Check = 10ms
> > 2. Whole Flow = 100 ms
>
> So, you should do this in a separate change explaining this.
In that case, I can add a small preparatory patch before the
“Use kernel helper for result polling” patch to adjust the
polling interval according to the datasheet.
This would mean changing the existing polling loop, for example:
diff --git a/drivers/iio/proximity/rfd77402.c b/drivers/iio/proximity/rfd77402.c
index 69cc1505b964..3e14660a4bb1 100644
--- a/drivers/iio/proximity/rfd77402.c
+++ b/drivers/iio/proximity/rfd77402.c
@@ -132,7 +132,7 @@ static int rfd77402_measure(struct i2c_client *client)
goto err;
if (ret & RFD77402_ICSR_RESULT)
break;
- msleep(20);
+ msleep(10);
}
if (tries < 0) {
However, this change would then be removed in the very next patch
when the polling loop is replaced by read_poll_timeout().I just want
to confirm that this temporary change–then–removal is acceptable?
Regards,
Shrikant
On Mon, Jan 26, 2026 at 08:58:07PM +0530, Shrikant wrote:
> On Mon, Jan 26, 2026 at 3:14 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Sat, Jan 24, 2026 at 10:26:04PM +0530, Shrikant wrote:
...
> > > > > + 10 * USEC_PER_MSEC,
> > > > > + 10 * 10 * USEC_PER_MSEC,
> > > >
> > > > This makes sleep shorter by 2. Why?
> > > I have considered the timeout values from the RFD77402 datasheet.
> > > The timeout values mentioned in section 3.1.1 Single Measure are as below:
> > > 1. Every Status Check = 10ms
> > > 2. Whole Flow = 100 ms
> >
> > So, you should do this in a separate change explaining this.
> In that case, I can add a small preparatory patch before the
> “Use kernel helper for result polling” patch to adjust the
> polling interval according to the datasheet.
>
> This would mean changing the existing polling loop, for example:
> diff --git a/drivers/iio/proximity/rfd77402.c b/drivers/iio/proximity/rfd77402.c
> index 69cc1505b964..3e14660a4bb1 100644
> --- a/drivers/iio/proximity/rfd77402.c
> +++ b/drivers/iio/proximity/rfd77402.c
> @@ -132,7 +132,7 @@ static int rfd77402_measure(struct i2c_client *client)
> goto err;
> if (ret & RFD77402_ICSR_RESULT)
> break;
> - msleep(20);
> + msleep(10);
> }
>
> if (tries < 0) {
>
> However, this change would then be removed in the very next patch
> when the polling loop is replaced by read_poll_timeout().I just want
> to confirm that this temporary change–then–removal is acceptable?
Yes. These two will be quite different semantically.
--
With Best Regards,
Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.