Re: [PATCH v3 1/7] iio: light: opt3001: move device registration to end of probe()

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 21 May 2026 11:55:38 +0200
Joshua Crofts via B4 Relay <devnull+joshua.crofts1.gmail.com@kernel.org> wrote:

> From: Joshua Crofts <joshua.crofts1@gmail.com>
> 
> Move IIO device registration to the end of the probe() function to
> prevent a potential race where userspace can interact with the device
> before IRQ is configured properly.

For this one, we really need a specific problem path rather than
generality that userspace interfaces aren't up yet so a spurious interrupt
causes us trouble.  

The intent is that the IIO core should drop anything it gets sent.
Might not be true in some corner case. If so we need to identify what
that is and whether we should add some more defense.

An example is iio_push_event().  That's guarded on iio_dev_opaque->ev_int
which is initialised in iio_device_register_eventset()
There might be problems in there but it will take more analysis.
The Kfifo looks suspicious as it's initialized a few lines too late
(for no obvious reason - ideally we'd look at whether we can
harden that!)

So question becomes is the kfifo used.  That's a driver question
and depends on a few register values. So chase that through to decide
if there is a bug here.

Otherwise it's just a logic improvement, not a fix.



> Additionally, switch
> devm_iio_device_register() to its unmanaged counterpart as current
> driver implementation mixes managed and unmanaged resources, causing
> potential resource leaks.

> 
> Also, add iio_device_unregister() to remove() function to correctly
> handle teardown.
> 
> Fixes: ac663db3678a ("iio: light: opt3001: enable operation w/o IRQ")
> Reported-by: Jonathan Cameron <jic23@kernel.org>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260511-opt3001-cleanup-v1-0-f7879dc3455c%40gmail.com?part=7
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/light/opt3001.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
> index dac3d3818d0a01b4ca53106fa38bb54b8c5373d4..fe53a072b21ddaa26a23c76e82897fdbc1e4d846 100644
> --- a/drivers/iio/light/opt3001.c
> +++ b/drivers/iio/light/opt3001.c
> @@ -874,12 +874,6 @@ static int	(struct i2c_client *client)
>  	iio->modes = INDIO_DIRECT_MODE;
>  	iio->info = &opt3001_info;
>  
> -	ret = devm_iio_device_register(dev, iio);
> -	if (ret) {
> -		dev_err(dev, "failed to register IIO device\n");
> -		return ret;
> -	}
> -
>  	/* Make use of INT pin only if valid IRQ no. is given */
>  	if (irq > 0) {
>  		ret = request_threaded_irq(irq, NULL, opt3001_irq,
> @@ -894,7 +888,7 @@ static int opt3001_probe(struct i2c_client *client)
>  		dev_dbg(opt->dev, "enabling interrupt-less operation\n");
>  	}
>  
> -	return 0;
> +	return iio_device_register(iio);
>  }
>  
>  static void opt3001_remove(struct i2c_client *client)
> @@ -904,6 +898,8 @@ static void opt3001_remove(struct i2c_client *client)
>  	int ret;
>  	u16 reg;
>  
> +	iio_device_unregister(iio);
> +
>  	if (opt->use_irq)
>  		free_irq(client->irq, iio);
>  
>