[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260111124908.1adec88b@jic23-huawei>
Date: Sun, 11 Jan 2026 12:49:08 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Shrikant Raskar via B4 Relay
<devnull+raskar.shree97.gmail.com@...nel.org>
Cc: raskar.shree97@...il.com, David Lechner <dlechner@...libre.com>, Nuno
Sá <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, skhan@...uxfoundation.org,
david.hunter.linux@...il.com, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/4] iio: proximity: rfd77402: Add interrupt handling
support
On Thu, 01 Jan 2026 21:47:41 +0530
Shrikant Raskar via B4 Relay <devnull+raskar.shree97.gmail.com@...nel.org> wrote:
> From: Shrikant Raskar <raskar.shree97@...il.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@...il.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;
>
Powered by blists - more mailing lists