[PATCH 0/3] iio: proximity: Add DT and interrupt support for RFD77402

[PATCH 0/3] iio: proximity: Add DT and interrupt support for RFD77402

[Shrikant Raskar]

This patch series adds:
 - Add RF Digital vendor prefix
 - YAML binding for RFD77402
 - Device Tree support in the driver
 - Interrupt handling support
 
These changes enable DT-based configuration and event-driven
operation for the RFD77402 Time-of-Flight sensor.

These changes are tested on RPi 3B.
1. Interrupt Mode logs:
pi@raspberrypi:~$ dmesg | grep rfd
[   26.907482] rfd77402 1-004c: Using interrupt mode 
pi@raspberrypi:~$ cat /sys/bus/iio/devices/iio:device0/in_distance_raw
51
1. Polling Mode logs:
pi@raspberrypi:~$ dmesg | grep rfd
[   26.760403] rfd77402 1-004c: No interrupt specified, using polling mode
pi@raspberrypi:~$ cat /sys/bus/iio/devices/iio:device0/in_distance_raw
1990

Shrikant Raskar (3):
  dt-bindings: iio: proximity: Add YAML binding for RFD77402 ToF sensor
  iio: proximity: rfd77402: Add Device Tree support
  iio: proximity: rfd77402: Add interrupt handling support

 .../iio/proximity/rfdigital,rfd77402.yaml     |  55 +++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/iio/proximity/rfd77402.c              | 134 +++++++++++++++---
 3 files changed, 171 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml

-- 
2.43.0

[PATCH 1/3] dt-bindings: iio: proximity: Add YAML binding for RFD77402 ToF sensor

[Shrikant Raskar]

The RFD77402 driver has existed without a formal device tree binding
description. With the recent addition of Device Tree support and
interrupt handling in the driver, it is now necessary to document
the DT properties used for configuring the device.

Since the binding introduces the compatible string "rfdigital,rfd77402",
the "rfdigital" vendor prefix is also added to vendor-prefixes.yaml.

Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
---
 .../iio/proximity/rfdigital,rfd77402.yaml     | 55 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 2 files changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml

diff --git a/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml b/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
new file mode 100644
index 000000000000..93deaa4e8b7a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/proximity/rfdigital,rfd77402.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RF Digital RFD77402 ToF sensor
+
+maintainers:
+  - Shrikant Raskar <raskar.shree97@gmail.com>
+
+description: |
+  The RF Digital RFD77402 is a Time-of-Flight (ToF) proximity and distance
+  sensor providing up to 200 mm range measurement over an I2C interface.
+
+properties:
+  compatible:
+    const: rfdigital,rfd77402
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description: |
+      Generated by the device to announce that a new
+      measurement data is ready in result register.
+
+  vdd-supply:
+    description: Regulator that provides power to the sensor
+
+  vddio-supply:
+    description: Regulator providing I/O interface voltage
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        proximity@4c {
+            compatible = "rfdigital,rfd77402";
+            reg = <0x4c>;
+            interrupt-parent = <&gpio>;
+            interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index f1d1882009ba..a2e113e29e37 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1341,6 +1341,8 @@ patternProperties:
     description: Revolution Robotics, Inc. (Revotics)
   "^rex,.*":
     description: iMX6 Rex Project
+  "^rfdigital,.*":
+    description: RF Digital Corporation
   "^richtek,.*":
     description: Richtek Technology Corporation
   "^ricoh,.*":
-- 
2.43.0

[PATCH 2/3] iio: proximity: rfd77402: Add Device Tree support

[Shrikant Raskar]

This patch enables seamless integration of the RFD77402 ToF sensor
on platforms that use Device Tree for hardware description

Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
---
 drivers/iio/proximity/rfd77402.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/proximity/rfd77402.c b/drivers/iio/proximity/rfd77402.c
index aff60a3c1a6f..3262af6f6882 100644
--- a/drivers/iio/proximity/rfd77402.c
+++ b/drivers/iio/proximity/rfd77402.c
@@ -313,10 +313,17 @@ static const struct i2c_device_id rfd77402_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rfd77402_id);
 
+static const struct of_device_id rfd77402_of_match[] = {
+	{ .compatible = "rfdigital,rfd77402" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rfd77402_of_match);
+
 static struct i2c_driver rfd77402_driver = {
 	.driver = {
 		.name   = RFD77402_DRV_NAME,
 		.pm     = pm_sleep_ptr(&rfd77402_pm_ops),
+		.of_match_table = rfd77402_of_match,
 	},
 	.probe = rfd77402_probe,
 	.id_table = rfd77402_id,
-- 
2.43.0

[PATCH 3/3] iio: proximity: rfd77402: Add interrupt handling support

[Shrikant Raskar]

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 | 127 ++++++++++++++++++++++++++-----
 1 file changed, 107 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/proximity/rfd77402.c b/drivers/iio/proximity/rfd77402.c
index 3262af6f6882..0e4763b34267 100644
--- a/drivers/iio/proximity/rfd77402.c
+++ b/drivers/iio/proximity/rfd77402.c
@@ -6,19 +6,22 @@
  *
  * 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/of.h>
 #include <linux/iio/iio.h>
 
 #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)
@@ -26,6 +29,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
@@ -80,6 +89,10 @@ struct rfd77402_data {
 	struct i2c_client *client;
 	/* Serialize reads from the sensor */
 	struct mutex lock;
+	/* Completion for interrupt-driven measurements */
+	struct completion completion;
+	/* Flag to indicate if interrupt is available */
+	bool irq_en;
 };
 
 static const struct iio_chan_spec rfd77402_channels[] = {
@@ -110,8 +123,24 @@ static int rfd77402_set_state(struct i2c_client *client, u8 state, u16 check)
 	return 0;
 }
 
-static int rfd77402_measure(struct i2c_client *client)
+static irqreturn_t rfd77402_interrupt_handler(int irq, void *dev_id)
+{
+	struct rfd77402_data *data = dev_id;
+	int ret;
+
+	/* Check if the interrupt is from our device */
+	ret = i2c_smbus_read_byte_data(data->client, RFD77402_ICSR);
+	if (ret < 0 || !(ret & RFD77402_ICSR_RESULT))
+		return IRQ_NONE;
+
+	/* Signal completion of measurement */
+	complete(&data->completion);
+	return IRQ_HANDLED;
+}
+
+static int rfd77402_measure(struct rfd77402_data *data)
 {
+	struct i2c_client *client = data->client;
 	int ret;
 	int tries = 10;
 
@@ -120,24 +149,39 @@ static int rfd77402_measure(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	/* Initialize completion for interrupt mode */
+	if (data->irq_en)
+		reinit_completion(&data->completion);
+
 	ret = i2c_smbus_write_byte_data(client, RFD77402_CMD_R,
 					RFD77402_CMD_SINGLE |
 					RFD77402_CMD_VALID);
 	if (ret < 0)
 		goto err;
 
-	while (tries-- > 0) {
-		ret = i2c_smbus_read_byte_data(client, RFD77402_ICSR);
-		if (ret < 0)
+	if (data->irq_en) {
+		/* Wait for interrupt-driven completion */
+		ret = wait_for_completion_timeout(&data->completion,
+						  msecs_to_jiffies(200));
+		if (ret == 0) {
+			ret = -ETIMEDOUT;
 			goto err;
-		if (ret & RFD77402_ICSR_RESULT)
-			break;
-		msleep(20);
-	}
-
-	if (tries < 0) {
-		ret = -ETIMEDOUT;
-		goto err;
+		}
+	} else {
+		/* Fallback to polling mode */
+		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;
+			goto err;
+		}
 	}
 
 	ret = i2c_smbus_read_word_data(client, RFD77402_RESULT_R);
@@ -168,7 +212,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;
@@ -190,6 +234,8 @@ static const struct iio_info rfd77402_info = {
 
 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,
@@ -197,11 +243,29 @@ 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)
-		return ret;
+	if (data->irq_en) {
+		/* Configure ICSR: auto-clear on read, push-pull, falling edge */
+		ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR,
+						RFD77402_ICSR_CLR_CFG |
+						RFD77402_ICSR_INT_MODE);
+		if (ret < 0)
+			return ret;
+
+		/* Enable 'new data available' interrupt in IER */
+		ret = i2c_smbus_write_byte_data(client, RFD77402_IER,
+						RFD77402_IER_RESULT);
+		if (ret < 0)
+			return ret;
+	} else {
+		/* Disable interrupts */
+		ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR, 0);
+		if (ret < 0)
+			return ret;
+
+		ret = i2c_smbus_write_byte_data(client, RFD77402_IER, 0);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* I2C configuration */
 	ret = i2c_smbus_write_word_data(client, RFD77402_I2C_INIT_CFG,
@@ -277,6 +341,29 @@ static int rfd77402_probe(struct i2c_client *client)
 	data->client = client;
 	mutex_init(&data->lock);
 
+	init_completion(&data->completion);
+
+	i2c_set_clientdata(client, indio_dev);
+
+	/* Check if interrupt is mentioned in device tree */
+	data->irq_en = false;
+	if (client->irq > 0) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, rfd77402_interrupt_handler,
+						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+						"rfd77402", data);
+		if (ret == 0) {
+			data->irq_en = true;
+			dev_info(&client->dev, "Using interrupt mode\n");
+		} else {
+			dev_warn(&client->dev,
+				 "Failed to request IRQ %d, using polling mode: %d\n",
+				 client->irq, ret);
+		}
+	} else {
+		dev_info(&client->dev, "No interrupt specified, using polling mode\n");
+	}
+
 	indio_dev->info = &rfd77402_info;
 	indio_dev->channels = rfd77402_channels;
 	indio_dev->num_channels = ARRAY_SIZE(rfd77402_channels);
-- 
2.43.0

Re: [PATCH 2/3] iio: proximity: rfd77402: Add Device Tree support

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 08:44:39AM +0530, Shrikant Raskar wrote:
> This patch enables seamless integration of the RFD77402 ToF sensor
> on platforms that use Device Tree for hardware description

Missing period at the end. Otherwise LGTM.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: proximity: rfd77402: Add interrupt handling support

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 08:44:40AM +0530, Shrikant Raskar 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>
> -

Stray removal of blank line.

> +#include <linux/interrupt.h>
> +#include <linux/completion.h>

> +#include <linux/of.h>

Please, avoid using of.h in a new code.

>  #include <linux/iio/iio.h>

...

> +static irqreturn_t rfd77402_interrupt_handler(int irq, void *dev_id)
> +{
> +	struct rfd77402_data *data = dev_id;
> +	int ret;

> +	/* Check if the interrupt is from our device */

This comment only for the second part and I would split the condition to make
it clearer.

> +	ret = i2c_smbus_read_byte_data(data->client, RFD77402_ICSR);
> +	if (ret < 0 || !(ret & RFD77402_ICSR_RESULT))
> +		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;
> +}

...

> -	while (tries-- > 0) {
> -		ret = i2c_smbus_read_byte_data(client, RFD77402_ICSR);
> -		if (ret < 0)
> +	if (data->irq_en) {
> +		/* Wait for interrupt-driven completion */
> +		ret = wait_for_completion_timeout(&data->completion,
> +						  msecs_to_jiffies(200));
> +		if (ret == 0) {
> +			ret = -ETIMEDOUT;
>  			goto err;
> -		if (ret & RFD77402_ICSR_RESULT)
> -			break;
> -		msleep(20);
> -	}
> -
> -	if (tries < 0) {
> -		ret = -ETIMEDOUT;
> -		goto err;
> +		}
> +	} else {
> +		/* Fallback to polling mode */
> +		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;
> +			goto err;
> +		}
>  	}

Instead, move the current code into a helper (in a separate patch) and alter it
here with new conditional. So in the result it will be something like

	if (...)
		ret = call_new_helper_for_irq();
	else
		ret = call_old_helper_for_polling();

...

> +	if (data->irq_en) {
> +		/* Configure ICSR: auto-clear on read, push-pull, falling edge */
> +		ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR,
> +						RFD77402_ICSR_CLR_CFG |
> +						RFD77402_ICSR_INT_MODE);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Enable 'new data available' interrupt in IER */
> +		ret = i2c_smbus_write_byte_data(client, RFD77402_IER,
> +						RFD77402_IER_RESULT);
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		/* Disable interrupts */
> +		ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR, 0);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = i2c_smbus_write_byte_data(client, RFD77402_IER, 0);
> +		if (ret < 0)
> +			return ret;
> +	}

This can be also factored out to a helper(s). Something like this, perhaps

	if (irq_en)
		ret = call_a_helper(client, $CSR, $ER);
	else
		ret = call_a_helper(client, 0, 0);

...

> +	/* Check if interrupt is mentioned in device tree */
> +	data->irq_en = false;
> +	if (client->irq > 0) {
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +						NULL, rfd77402_interrupt_handler,
> +						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +						"rfd77402", data);
> +		if (ret == 0) {
> +			data->irq_en = true;
> +			dev_info(&client->dev, "Using interrupt mode\n");
> +		} else {
> +			dev_warn(&client->dev,
> +				 "Failed to request IRQ %d, using polling mode: %d\n",
> +				 client->irq, ret);

If we asked for interrupt and didn't get it due to "linux" errors, we should
not fallback. No need to work around bugs in the DT, the DT description must
be fixed instead.

> +		}
> +	} else {
> +		dev_info(&client->dev, "No interrupt specified, using polling mode\n");
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] dt-bindings: iio: proximity: Add YAML binding for RFD77402 ToF sensor

[Krzysztof Kozlowski]

On Wed, Nov 26, 2025 at 08:44:38AM +0530, Shrikant Raskar wrote:
> The RFD77402 driver has existed without a formal device tree binding
> description. With the recent addition of Device Tree support and
> interrupt handling in the driver, it is now necessary to document
> the DT properties used for configuring the device.

This is all irrelevant here. It does not matter for the bindings if the
driver existed or not.

Please rather document here the hardware.

A nit, subject: drop second/last, redundant "YAML binding for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
For sure don't use "YAML binding" - there is no such thing.

> 
> Since the binding introduces the compatible string "rfdigital,rfd77402",
> the "rfdigital" vendor prefix is also added to vendor-prefixes.yaml.

Also redundant, we can see the diff.

> 
> Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
> ---
>  .../iio/proximity/rfdigital,rfd77402.yaml     | 55 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
>  2 files changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml b/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
> new file mode 100644
> index 000000000000..93deaa4e8b7a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/proximity/rfdigital,rfd77402.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RF Digital RFD77402 ToF sensor
> +
> +maintainers:
> +  - Shrikant Raskar <raskar.shree97@gmail.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The RF Digital RFD77402 is a Time-of-Flight (ToF) proximity and distance
> +  sensor providing up to 200 mm range measurement over an I2C interface.
> +
> +properties:
> +  compatible:
> +    const: rfdigital,rfd77402
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    description: |

Same, also a bit odd wrapping of the text below

> +      Generated by the device to announce that a new
> +      measurement data is ready in result register.
> +
> +  vdd-supply:
> +    description: Regulator that provides power to the sensor
> +
> +  vddio-supply:
> +    description: Regulator providing I/O interface voltage
> +
> +required:
> +  - compatible
> +  - reg

supplies should be required, devices rarely work without power. If you
think hardware works without power, this is something unusual thus you
should explain it in the commit msg.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        proximity@4c {
> +            compatible = "rfdigital,rfd77402";
> +            reg = <0x4c>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <4 IRQ_TYPE_EDGE_FALLING>;

Supplies

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: iio: proximity: Add YAML binding for RFD77402 ToF sensor

[Shrikant]

On Wed, Nov 26, 2025 at 3:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Nov 26, 2025 at 08:44:38AM +0530, Shrikant Raskar wrote:
> > The RFD77402 driver has existed without a formal device tree binding
> > description. With the recent addition of Device Tree support and
> > interrupt handling in the driver, it is now necessary to document
> > the DT properties used for configuring the device.
>
> This is all irrelevant here. It does not matter for the bindings if the
> driver existed or not.
>
> Please rather document here the hardware.
>
> A nit, subject: drop second/last, redundant "YAML binding for". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> For sure don't use "YAML binding" - there is no such thing.
>
> >
> > Since the binding introduces the compatible string "rfdigital,rfd77402",
> > the "rfdigital" vendor prefix is also added to vendor-prefixes.yaml.
>
> Also redundant, we can see the diff.
>
> >
> > Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
> > ---
> >  .../iio/proximity/rfdigital,rfd77402.yaml     | 55 +++++++++++++++++++
> >  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
> >  2 files changed, 57 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml b/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
> > new file mode 100644
> > index 000000000000..93deaa4e8b7a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/proximity/rfdigital,rfd77402.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/proximity/rfdigital,rfd77402.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RF Digital RFD77402 ToF sensor
> > +
> > +maintainers:
> > +  - Shrikant Raskar <raskar.shree97@gmail.com>
> > +
> > +description: |
>
> Do not need '|' unless you need to preserve formatting.
>
> > +  The RF Digital RFD77402 is a Time-of-Flight (ToF) proximity and distance
> > +  sensor providing up to 200 mm range measurement over an I2C interface.
> > +
> > +properties:
> > +  compatible:
> > +    const: rfdigital,rfd77402
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description: |
>
> Same, also a bit odd wrapping of the text below
>
> > +      Generated by the device to announce that a new
> > +      measurement data is ready in result register.
> > +
> > +  vdd-supply:
> > +    description: Regulator that provides power to the sensor
> > +
> > +  vddio-supply:
> > +    description: Regulator providing I/O interface voltage
> > +
> > +required:
> > +  - compatible
> > +  - reg
>
> supplies should be required, devices rarely work without power. If you
> think hardware works without power, this is something unusual thus you
> should explain it in the commit msg.
>
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        proximity@4c {
> > +            compatible = "rfdigital,rfd77402";
> > +            reg = <0x4c>;
> > +            interrupt-parent = <&gpio>;
> > +            interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
>
> Supplies
Thank you for reviewing the patch. I will update the dt-binding as
per the feedback and will share the v2 of the patch.

Regards,
Shrikant

Re: [PATCH 3/3] iio: proximity: rfd77402: Add interrupt handling support

[Shrikant]

On Wed, Nov 26, 2025 at 1:05 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, Nov 26, 2025 at 08:44:40AM +0530, Shrikant Raskar 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>
> > -
>
> Stray removal of blank line.
>
> > +#include <linux/interrupt.h>
> > +#include <linux/completion.h>
>
> > +#include <linux/of.h>
>
> Please, avoid using of.h in a new code.
>
> >  #include <linux/iio/iio.h>
>
> ...
>
> > +static irqreturn_t rfd77402_interrupt_handler(int irq, void *dev_id)
> > +{
> > +     struct rfd77402_data *data = dev_id;
> > +     int ret;
>
> > +     /* Check if the interrupt is from our device */
>
> This comment only for the second part and I would split the condition to make
> it clearer.
>
> > +     ret = i2c_smbus_read_byte_data(data->client, RFD77402_ICSR);
> > +     if (ret < 0 || !(ret & RFD77402_ICSR_RESULT))
> > +             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;
> > +}
>
> ...
>
> > -     while (tries-- > 0) {
> > -             ret = i2c_smbus_read_byte_data(client, RFD77402_ICSR);
> > -             if (ret < 0)
> > +     if (data->irq_en) {
> > +             /* Wait for interrupt-driven completion */
> > +             ret = wait_for_completion_timeout(&data->completion,
> > +                                               msecs_to_jiffies(200));
> > +             if (ret == 0) {
> > +                     ret = -ETIMEDOUT;
> >                       goto err;
> > -             if (ret & RFD77402_ICSR_RESULT)
> > -                     break;
> > -             msleep(20);
> > -     }
> > -
> > -     if (tries < 0) {
> > -             ret = -ETIMEDOUT;
> > -             goto err;
> > +             }
> > +     } else {
> > +             /* Fallback to polling mode */
> > +             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;
> > +                     goto err;
> > +             }
> >       }
>
> Instead, move the current code into a helper (in a separate patch) and alter it
> here with new conditional. So in the result it will be something like
>
>         if (...)
>                 ret = call_new_helper_for_irq();
>         else
>                 ret = call_old_helper_for_polling();
>
> ...
>
> > +     if (data->irq_en) {
> > +             /* Configure ICSR: auto-clear on read, push-pull, falling edge */
> > +             ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR,
> > +                                             RFD77402_ICSR_CLR_CFG |
> > +                                             RFD77402_ICSR_INT_MODE);
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             /* Enable 'new data available' interrupt in IER */
> > +             ret = i2c_smbus_write_byte_data(client, RFD77402_IER,
> > +                                             RFD77402_IER_RESULT);
> > +             if (ret < 0)
> > +                     return ret;
> > +     } else {
> > +             /* Disable interrupts */
> > +             ret = i2c_smbus_write_byte_data(client, RFD77402_ICSR, 0);
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             ret = i2c_smbus_write_byte_data(client, RFD77402_IER, 0);
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
>
> This can be also factored out to a helper(s). Something like this, perhaps
>
>         if (irq_en)
>                 ret = call_a_helper(client, $CSR, $ER);
>         else
>                 ret = call_a_helper(client, 0, 0);
>
> ...
>
> > +     /* Check if interrupt is mentioned in device tree */
> > +     data->irq_en = false;
> > +     if (client->irq > 0) {
> > +             ret = devm_request_threaded_irq(&client->dev, client->irq,
> > +                                             NULL, rfd77402_interrupt_handler,
> > +                                             IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > +                                             "rfd77402", data);
> > +             if (ret == 0) {
> > +                     data->irq_en = true;
> > +                     dev_info(&client->dev, "Using interrupt mode\n");
> > +             } else {
> > +                     dev_warn(&client->dev,
> > +                              "Failed to request IRQ %d, using polling mode: %d\n",
> > +                              client->irq, ret);
>
> If we asked for interrupt and didn't get it due to "linux" errors, we should
> not fallback. No need to work around bugs in the DT, the DT description must
> be fixed instead.
>
> > +             }
> > +     } else {
> > +             dev_info(&client->dev, "No interrupt specified, using polling mode\n");
> > +     }
>
Thank you for the detailed feedback. I will update the code as per
feedback and will share the v2 of the patch.

Regards,
Shrikant

Re: [PATCH 3/3] iio: proximity: rfd77402: Add interrupt handling support

[Jonathan Cameron]

On Fri, 28 Nov 2025 21:36:43 +0530
Shrikant <raskar.shree97@gmail.com> wrote:

> On Wed, Nov 26, 2025 at 1:05 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Wed, Nov 26, 2025 at 08:44:40AM +0530, Shrikant Raskar 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.  
> >
> > ...
...

> Thank you for the detailed feedback. I will update the code as per
> feedback and will share the v2 of the patch.

Hi Shrikant

Small process points in the interests of efficiency.
1. Crop to only the stuff that is relevant...
2. Skip replying at all if you accept all feedback. It's noise.
   The place for that and thanks is in the change log below the --- in the
   next version of the patch (thus no extra emails for people to scroll through!)

Thanks

Jonathan


> 
> Regards,
> Shrikant