From: Shrikant Raskar <raskar.shree97@gmail.com>
Add interrupt handling support to enable event-driven data acquisition
instead of continuous polling. This improves responsiveness, reduces
CPU overhead, and supports low-power operation by allowing the system
to remain idle until an interrupt occurs.
Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
---
drivers/iio/proximity/rfd77402.c | 136 ++++++++++++++++++++++++++++++++++-----
1 file changed, 120 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/proximity/rfd77402.c b/drivers/iio/proximity/rfd77402.c
index 496c1412ebf8..8cddf509f9c6 100644
--- a/drivers/iio/proximity/rfd77402.c
+++ b/drivers/iio/proximity/rfd77402.c
@@ -6,13 +6,14 @@
*
* 7-bit I2C slave address 0x4c
*
- * TODO: interrupt
* https://media.digikey.com/pdf/Data%20Sheets/RF%20Digital%20PDFs/RFD77402.pdf
*/
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
#include <linux/iopoll.h>
#include <linux/iio/iio.h>
@@ -20,6 +21,8 @@
#define RFD77402_DRV_NAME "rfd77402"
#define RFD77402_ICSR 0x00 /* Interrupt Control Status Register */
+#define RFD77402_ICSR_CLR_CFG BIT(0)
+#define RFD77402_ICSR_CLR_TYPE BIT(1)
#define RFD77402_ICSR_INT_MODE BIT(2)
#define RFD77402_ICSR_INT_POL BIT(3)
#define RFD77402_ICSR_RESULT BIT(4)
@@ -27,6 +30,12 @@
#define RFD77402_ICSR_H2M_MSG BIT(6)
#define RFD77402_ICSR_RESET BIT(7)
+#define RFD77402_IER 0x02
+#define RFD77402_IER_RESULT BIT(0)
+#define RFD77402_IER_M2H_MSG BIT(1)
+#define RFD77402_IER_H2M_MSG BIT(2)
+#define RFD77402_IER_RESET BIT(3)
+
#define RFD77402_CMD_R 0x04
#define RFD77402_CMD_SINGLE 0x01
#define RFD77402_CMD_STANDBY 0x10
@@ -77,10 +86,18 @@ static const struct {
{RFD77402_HFCFG_3, 0x45d4},
};
+/**
+ * struct rfd77402_data - device-specific data for the RFD77402 sensor
+ * @client: I2C client handle
+ * @lock: mutex to serialize sensor reads
+ * @completion: completion used for interrupt-driven measurements
+ * @irq_en: indicates whether interrupt mode is enabled
+ */
struct rfd77402_data {
struct i2c_client *client;
- /* Serialize reads from the sensor */
struct mutex lock;
+ struct completion completion;
+ bool irq_en;
};
static const struct iio_chan_spec rfd77402_channels[] = {
@@ -91,6 +108,39 @@ static const struct iio_chan_spec rfd77402_channels[] = {
},
};
+static irqreturn_t rfd77402_interrupt_handler(int irq, void *pdata)
+{
+ struct rfd77402_data *data = pdata;
+ int ret;
+
+ if (!data || !data->client)
+ return IRQ_NONE;
+
+ ret = i2c_smbus_read_byte_data(data->client, RFD77402_ICSR);
+ if (ret < 0)
+ return IRQ_NONE;
+
+ /* Check if the interrupt is from our device */
+ if (!(ret & RFD77402_ICSR_RESULT))
+ return IRQ_NONE;
+
+ /* Signal completion of measurement */
+ complete(&data->completion);
+ return IRQ_HANDLED;
+}
+
+static int rfd77402_wait_for_irq(struct rfd77402_data *data)
+{
+ int ret;
+ /* As per datasheet, single measurement flow takes 100ms */
+ ret = wait_for_completion_timeout(&data->completion,
+ msecs_to_jiffies(100));
+ if (ret == 0)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
static int rfd77402_set_state(struct i2c_client *client, u8 state, u16 check)
{
int ret;
@@ -111,8 +161,9 @@ 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_measure(struct rfd77402_data *data)
{
+ struct i2c_client *client = data->client;
int ret;
ret = rfd77402_set_state(client, RFD77402_CMD_MCPU_ON,
@@ -126,13 +177,19 @@ static int rfd77402_measure(struct i2c_client *client)
if (ret < 0)
goto err;
- /* Poll ICSR until RESULT bit is set */
- ret = read_poll_timeout(i2c_smbus_read_byte_data, ret,
- ret & RFD77402_ICSR_RESULT,
- 10000, /* sleep: 10ms */
- 100000, /* timeout: 100ms */
- false,
- client, RFD77402_ICSR);
+ if (data->irq_en) {
+ /* Re-initialize completion and wait for interrupt */
+ reinit_completion(&data->completion);
+ ret = rfd77402_wait_for_irq(data);
+ } else {
+ /* Poll ICSR until RESULT bit is set */
+ ret = read_poll_timeout(i2c_smbus_read_byte_data, ret,
+ ret & RFD77402_ICSR_RESULT,
+ 10000, /* sleep 10ms */
+ 100000, /* timeout 100ms */
+ false,
+ client, RFD77402_ICSR);
+ }
if (ret < 0)
goto err;
@@ -164,7 +221,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;
@@ -184,8 +241,21 @@ static const struct iio_info rfd77402_info = {
.read_raw = rfd77402_read_raw,
};
+static int rfd77402_config_irq(struct i2c_client *client, u8 csr, u8 ier)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR, csr);
+ if (ret)
+ return ret;
+
+ return i2c_smbus_write_byte_data(client, RFD77402_IER, ier);
+}
+
static int rfd77402_init(struct i2c_client *client)
{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct rfd77402_data *data = iio_priv(indio_dev);
int ret, i;
ret = rfd77402_set_state(client, RFD77402_CMD_STANDBY,
@@ -193,10 +263,24 @@ static int rfd77402_init(struct i2c_client *client)
if (ret < 0)
return ret;
- /* configure INT pad as push-pull, active low */
- ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR,
- RFD77402_ICSR_INT_MODE);
- if (ret < 0)
+ if (data->irq_en) {
+ /* Enable interrupt mode:
+ * - Configure ICSR for auto-clear on read and
+ * push-pull output
+ * - Enable "result ready" interrupt in IER
+ */
+ ret = rfd77402_config_irq(client,
+ RFD77402_ICSR_CLR_CFG |
+ RFD77402_ICSR_INT_MODE,
+ RFD77402_IER_RESULT);
+ } else {
+ /* Disable all interrupts:
+ * - Clear ICSR configuration
+ * - Disable all interrupts in IER
+ */
+ ret = rfd77402_config_irq(client, 0, 0);
+ }
+ if (ret)
return ret;
/* I2C configuration */
@@ -271,7 +355,27 @@ static int rfd77402_probe(struct i2c_client *client)
data = iio_priv(indio_dev);
data->client = client;
- mutex_init(&data->lock);
+ ret = devm_mutex_init(&client->dev, &data->lock);
+ if (ret)
+ return ret;
+
+ init_completion(&data->completion);
+ i2c_set_clientdata(client, indio_dev);
+
+ data->irq_en = false;
+ if (client->irq > 0) {
+ ret = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, rfd77402_interrupt_handler,
+ IRQF_ONESHOT,
+ "rfd77402", data);
+ if (ret)
+ return ret;
+
+ data->irq_en = true;
+ dev_dbg(&client->dev, "Using interrupt mode\n");
+ } else {
+ dev_dbg(&client->dev, "Using polling mode\n");
+ }
indio_dev->info = &rfd77402_info;
indio_dev->channels = rfd77402_channels;
--
2.43.0On Thu, 01 Jan 2026 21:47:41 +0530
Shrikant Raskar via B4 Relay <devnull+raskar.shree97.gmail.com@kernel.org> wrote:
> From: Shrikant Raskar <raskar.shree97@gmail.com>
>
> Add interrupt handling support to enable event-driven data acquisition
> instead of continuous polling. This improves responsiveness, reduces
> CPU overhead, and supports low-power operation by allowing the system
> to remain idle until an interrupt occurs.
>
> Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
> ---
Hi Shrikant,
A few additional (some may overlap with Andy's) comments from me.
Thanks,
Jonathan
> +
> +static int rfd77402_wait_for_irq(struct rfd77402_data *data)
> +{
> + int ret;
Blank line here. Pretty much always the case are declarations
in kernel code.
> + /* As per datasheet, single measurement flow takes 100ms */
> + ret = wait_for_completion_timeout(&data->completion,
> + msecs_to_jiffies(100));
> + if (ret == 0)
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> static int rfd77402_init(struct i2c_client *client)
> {
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct rfd77402_data *data = iio_priv(indio_dev);
> int ret, i;
>
> ret = rfd77402_set_state(client, RFD77402_CMD_STANDBY,
> @@ -193,10 +263,24 @@ static int rfd77402_init(struct i2c_client *client)
> if (ret < 0)
> return ret;
>
> - /* configure INT pad as push-pull, active low */
> - ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR,
> - RFD77402_ICSR_INT_MODE);
> - if (ret < 0)
> + if (data->irq_en) {
> + /* Enable interrupt mode:
/*
* Enable ...
is the comment syntax used in IIO.
> + * - Configure ICSR for auto-clear on read and
> + * push-pull output
> + * - Enable "result ready" interrupt in IER
> + */
> + ret = rfd77402_config_irq(client,
> + RFD77402_ICSR_CLR_CFG |
> + RFD77402_ICSR_INT_MODE,
> + RFD77402_IER_RESULT);
> + } else {
> + /* Disable all interrupts:
As above. Match local style for comments (which is not this!)
Oddly that was correct in v3 (indent is now fixed though)
> + * - Clear ICSR configuration
> + * - Disable all interrupts in IER
> + */
> + ret = rfd77402_config_irq(client, 0, 0);
> + }
> + if (ret)
> return ret;
>
> /* I2C configuration */
> @@ -271,7 +355,27 @@ static int rfd77402_probe(struct i2c_client *client)
>
> data = iio_priv(indio_dev);
> data->client = client;
> - mutex_init(&data->lock);
> + ret = devm_mutex_init(&client->dev, &data->lock);
> + if (ret)
> + return ret;
I'm not that fussy about it given it's so minor but this is an unrelated
changes so in ideal world would be a separate patch.
> +
> + init_completion(&data->completion);
> + i2c_set_clientdata(client, indio_dev);
> +
> + data->irq_en = false;
Not strictly necessary as it's kind of the obvious default and we
kzalloc data anyway so it's already false.
> + if (client->irq > 0) {
> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL, rfd77402_interrupt_handler,
> + IRQF_ONESHOT,
> + "rfd77402", data);
> + if (ret)
> + return ret;
> +
> + data->irq_en = true;
> + dev_dbg(&client->dev, "Using interrupt mode\n");
> + } else {
> + dev_dbg(&client->dev, "Using polling mode\n");
> + }
>
> indio_dev->info = &rfd77402_info;
> indio_dev->channels = rfd77402_channels;
>
On Thu, Jan 01, 2026 at 09:47:41PM +0530, Shrikant Raskar via B4 Relay wrote:
> Add interrupt handling support to enable event-driven data acquisition
> instead of continuous polling. This improves responsiveness, reduces
> CPU overhead, and supports low-power operation by allowing the system
> to remain idle until an interrupt occurs.
...
> #include <linux/module.h>
> #include <linux/i2c.h>
> #include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/completion.h>
> #include <linux/iopoll.h>
Same comment as per previous patch. Do not add even more misordering, please.
...
> +/**
> + * struct rfd77402_data - device-specific data for the RFD77402 sensor
> + * @client: I2C client handle
> + * @lock: mutex to serialize sensor reads
> + * @completion: completion used for interrupt-driven measurements
> + * @irq_en: indicates whether interrupt mode is enabled
> + */
> struct rfd77402_data {
> struct i2c_client *client;
> - /* Serialize reads from the sensor */
> struct mutex lock;
> + struct completion completion;
> + bool irq_en;
> };
The kernel-doc conversion can be a separate patch, but I'm not insisting.
...
> +static irqreturn_t rfd77402_interrupt_handler(int irq, void *pdata)
> +{
> + struct rfd77402_data *data = pdata;
> + int ret;
> + if (!data || !data->client)
> + return IRQ_NONE;
How is this possible to be non-dead code?
> + ret = i2c_smbus_read_byte_data(data->client, RFD77402_ICSR);
> + if (ret < 0)
> + return IRQ_NONE;
> +
> + /* Check if the interrupt is from our device */
> + if (!(ret & RFD77402_ICSR_RESULT))
> + return IRQ_NONE;
> +
> + /* Signal completion of measurement */
> + complete(&data->completion);
> + return IRQ_HANDLED;
> +}
...
> +static int rfd77402_wait_for_irq(struct rfd77402_data *data)
> +{
> + int ret;
Missed blank line. Doesn't checkpatch complain?
> + /* As per datasheet, single measurement flow takes 100ms */
Please, be more specific about datasheet, i.e. which Chapter/Section (with its
number and possible name) or Table specifies this.
> + ret = wait_for_completion_timeout(&data->completion,
> + msecs_to_jiffies(100));
> + if (ret == 0)
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
...
> +static int rfd77402_measure(struct rfd77402_data *data)
> {
> + struct i2c_client *client = data->client;
This (conversion to data instead of client) can be split into a separate
precursor change.a but it seems not a big deal. Up to maintainers.
...
> - /* Poll ICSR until RESULT bit is set */
> - ret = read_poll_timeout(i2c_smbus_read_byte_data, ret,
> - ret & RFD77402_ICSR_RESULT,
> - 10000, /* sleep: 10ms */
> - 100000, /* timeout: 100ms */
> - false,
> - client, RFD77402_ICSR);
> + if (data->irq_en) {
> + /* Re-initialize completion and wait for interrupt */
> + reinit_completion(&data->completion);
> + ret = rfd77402_wait_for_irq(data);
> + } else {
> + /* Poll ICSR until RESULT bit is set */
> + ret = read_poll_timeout(i2c_smbus_read_byte_data, ret,
> + ret & RFD77402_ICSR_RESULT,
> + 10000, /* sleep 10ms */
> + 100000, /* timeout 100ms */
> + false,
> + client, RFD77402_ICSR);
> + }
This is ping-pong type of change. You just introduced it a patch ago. Make sure
you don't remove/modify (too much at least) the lines that were just added.
One of the possible technique to achieve this is to use a helper function.
...
> static int rfd77402_init(struct i2c_client *client)
> {
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct rfd77402_data *data = iio_priv(indio_dev);
Can't this now take the data as above modified functon?
...
> - mutex_init(&data->lock);
> + ret = devm_mutex_init(&client->dev, &data->lock);
> + if (ret)
> + return ret;
In my opinion this deserves a separate change.
--
With Best Regards,
Andy Shevchenko
> > #include <linux/module.h>
> > #include <linux/i2c.h>
> > #include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/completion.h>
> > #include <linux/iopoll.h>
>
> Same comment as per previous patch. Do not add even more misordering, please.
Will it be okay if I re-order the includes as below ?
#include <linux/completion.h>
#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/iio/iio.h>
> > +/**
> > + * struct rfd77402_data - device-specific data for the RFD77402 sensor
> > + * @client: I2C client handle
> > + * @lock: mutex to serialize sensor reads
> > + * @completion: completion used for interrupt-driven measurements
> > + * @irq_en: indicates whether interrupt mode is enabled
> > + */
> > struct rfd77402_data {
> > struct i2c_client *client;
> > - /* Serialize reads from the sensor */
> > struct mutex lock;
> > + struct completion completion;
> > + bool irq_en;
> > };
>
> The kernel-doc conversion can be a separate patch, but I'm not insisting.
I can split this into a separate patch within the same series.
Please let me know if you would prefer it to be handled differently.
> > +static int rfd77402_wait_for_irq(struct rfd77402_data *data)
> > +{
> > + int ret;
>
> Missed blank line. Doesn't checkpatch complain?
Checkpatch did not complain but I will add a blank line.
> > - mutex_init(&data->lock);
> > + ret = devm_mutex_init(&client->dev, &data->lock);
> > + if (ret)
> > + return ret;
>
> In my opinion this deserves a separate change.
I will add a separate patch for this within the same series.
Please let me know if you would prefer it to be handled differently
Regards,
Shrikant
On Tue, Jan 06, 2026 at 05:39:29AM +0530, Shrikant wrote:
...
> > > #include <linux/module.h>
> > > #include <linux/i2c.h>
> > > #include <linux/delay.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/completion.h>
> > > #include <linux/iopoll.h>
> >
> > Same comment as per previous patch. Do not add even more misordering, please.
> Will it be okay if I re-order the includes as below ?
> #include <linux/completion.h>
> #include <linux/delay.h>
> #include <linux/i2c.h>
> #include <linux/interrupt.h>
> #include <linux/iopoll.h>
> #include <linux/module.h>
Just try to squeeze the new inclusions in the longest chain of the sorted ones
(yes, some original ones may be left untouched and hence unordered).
> #include <linux/iio/iio.h>
...
> > > +/**
> > > + * struct rfd77402_data - device-specific data for the RFD77402 sensor
> > > + * @client: I2C client handle
> > > + * @lock: mutex to serialize sensor reads
> > > + * @completion: completion used for interrupt-driven measurements
> > > + * @irq_en: indicates whether interrupt mode is enabled
> > > + */
> > > struct rfd77402_data {
> > > struct i2c_client *client;
> > > - /* Serialize reads from the sensor */
> > > struct mutex lock;
> > > + struct completion completion;
> > > + bool irq_en;
> > > };
> >
> > The kernel-doc conversion can be a separate patch, but I'm not insisting.
> I can split this into a separate patch within the same series.
> Please let me know if you would prefer it to be handled differently.
It's up to maintainers.
--
With Best Regards,
Andy Shevchenko
On Tue, 6 Jan 2026 22:47:17 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Tue, Jan 06, 2026 at 05:39:29AM +0530, Shrikant wrote:
>
> ...
>
> > > > #include <linux/module.h>
> > > > #include <linux/i2c.h>
> > > > #include <linux/delay.h>
> > > > +#include <linux/interrupt.h>
> > > > +#include <linux/completion.h>
> > > > #include <linux/iopoll.h>
> > >
> > > Same comment as per previous patch. Do not add even more misordering, please.
> > Will it be okay if I re-order the includes as below ?
> > #include <linux/completion.h>
> > #include <linux/delay.h>
> > #include <linux/i2c.h>
> > #include <linux/interrupt.h>
> > #include <linux/iopoll.h>
> > #include <linux/module.h>
>
> Just try to squeeze the new inclusions in the longest chain of the sorted ones
> (yes, some original ones may be left untouched and hence unordered).
>
Or a separate cleanup precursor patch woudl be fine.
> > #include <linux/iio/iio.h>
>
> ...
>
> > > > +/**
> > > > + * struct rfd77402_data - device-specific data for the RFD77402 sensor
> > > > + * @client: I2C client handle
> > > > + * @lock: mutex to serialize sensor reads
> > > > + * @completion: completion used for interrupt-driven measurements
> > > > + * @irq_en: indicates whether interrupt mode is enabled
> > > > + */
> > > > struct rfd77402_data {
> > > > struct i2c_client *client;
> > > > - /* Serialize reads from the sensor */
> > > > struct mutex lock;
> > > > + struct completion completion;
> > > > + bool irq_en;
> > > > };
> > >
> > > The kernel-doc conversion can be a separate patch, but I'm not insisting.
> > I can split this into a separate patch within the same series.
> > Please let me know if you would prefer it to be handled differently.
>
> It's up to maintainers.
I'd prefer it split.
Thanks,
Jonathan
>
© 2016 - 2026 Red Hat, Inc.