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

David Lechner
2026-04-06T16:14:35Z

On 4/6/26 3:08 AM, Sanjay Chitroda wrote:
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> 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@gmail.com>
> ---
> 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);
> }
>