Re: [PATCH v5 4/5] iio: ssp_sensors: use devm APIs for mutex and IRQ resources

[David Lechner]

On 4/6/26 3:08 AM, Sanjay Chitroda wrote:
> From: Sanjay Chitroda <sanjayembeddedse@xxxxxxxxx>
> 
> Convert mutex initialization and IRQ registration to use devm-managed
> helpers, tying their lifetime to the device.
> 
> This removes the need for explicit cleanup in the error and remove
> paths, as the resources are automatically released on probe failure
> or device unbind.
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@xxxxxxxxx>
> ---
>  drivers/iio/common/ssp_sensors/ssp_dev.c | 36 +++++++++++-------------
>  1 file changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
> index da09c9f3ceb6..f4fc869fc770 100644
> --- a/drivers/iio/common/ssp_sensors/ssp_dev.c
> +++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
> @@ -510,7 +510,11 @@ static int ssp_probe(struct spi_device *spi)
>  	data->spi = spi;
>  	spi_set_drvdata(spi, data);
>  
> -	mutex_init(&data->comm_lock);
> +	ret = devm_mutex_init(&spi->dev, &data->comm_lock);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Failed to init comm_lock mutex\n");
> +		goto err_setup_spi;
> +	}
>  
>  	for (i = 0; i < SSP_SENSOR_MAX; ++i) {
>  		data->delay_buf[i] = SSP_DEFAULT_POLLING_DELAY;
> @@ -523,7 +527,11 @@ static int ssp_probe(struct spi_device *spi)
>  
>  	data->time_syncing = true;
>  
> -	mutex_init(&data->pending_lock);
> +	ret = devm_mutex_init(&spi->dev, &data->pending_lock);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Failed to init pending_lock mutex\n");
> +		goto err_setup_spi;
> +	}
>  	INIT_LIST_HEAD(&data->pending_list);
>  
>  	atomic_set(&data->enable_refcount, 0);
> @@ -533,13 +541,13 @@ static int ssp_probe(struct spi_device *spi)
>  
>  	timer_setup(&data->wdt_timer, ssp_wdt_timer_func, 0);
>  
> -	ret = request_threaded_irq(data->spi->irq, NULL,
> -				   ssp_irq_thread_fn,
> -				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -				   "SSP_Int", data);
> +	ret = devm_request_threaded_irq(&spi->dev, data->spi->irq, NULL,
> +					ssp_irq_thread_fn,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					"SSP_Int", data);
>  	if (ret < 0) {
>  		dev_err(&spi->dev, "Irq request fail\n");
> -		goto err_setup_irq;
> +		goto err_setup_spi;
>  	}
>  
>  	/* Let's start with enabled one so irq balance could be ok */
> @@ -553,21 +561,16 @@ static int ssp_probe(struct spi_device *spi)
>  		ret = ssp_initialize_mcu(data);
>  		if (ret < 0) {
>  			dev_err(&spi->dev, "Initialize_mcu failed\n");
> -			goto err_read_reg;
> +			goto err_setup_spi;
>  		}
>  	} else {
>  		dev_err(&spi->dev, "Firmware version not supported\n");
>  		ret = -EPERM;
> -		goto err_read_reg;
> +		goto err_setup_spi;
>  	}
>  
>  	return 0;
>  
> -err_read_reg:
> -	free_irq(data->spi->irq, data);
> -err_setup_irq:
> -	mutex_destroy(&data->pending_lock);
> -	mutex_destroy(&data->comm_lock);
>  err_setup_spi:
>  	mfd_remove_devices(&spi->dev);
>  
> @@ -589,14 +592,9 @@ static void ssp_remove(struct spi_device *spi)
>  
>  	ssp_clean_pending_list(data);
>  
> -	free_irq(data->spi->irq, data);

Do we need to replace this with an irq_disable() so make sure
no more timers/work is scheduled after we delete them below?

If it is safe the way it is, add an explanation to the commit
message.

Alternative would be to use devm_add_action_or_reset() to
handle all of the other cleanup functions in the correct order
still and get rid of the remove function entirely.

> -
>  	timer_delete_sync(&data->wdt_timer);
>  	cancel_work_sync(&data->work_wdt);
>  
> -	mutex_destroy(&data->comm_lock);
> -	mutex_destroy(&data->pending_lock);
> -
>  	mfd_remove_devices(&spi->dev);
>  }
>