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

[Jonathan Cameron <jic23@kernel.org>]

On 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;
>