Re: [PATCH v3 3/6] iio: light: vcnl4000: replace mutex_init with devm_mutex_init

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 14 Mar 2026 18:06:32 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> Replace mutex_init used across driver with its device-managed
> counterpart, so all assigned mutexes get destroyed.
> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
Hi Erikas,

One related area that I noticed whilst checking this patch was
safe wrt to ordering.  I think cleaning that up before this
patch would make this one more obviously correct.

Jonathan

> ---
>  drivers/iio/light/vcnl4000.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 939ff2d65105..0ee307fc5ab7 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -356,6 +356,8 @@ static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
>  
>  static int vcnl4200_init(struct vcnl4000_data *data)
>  {
> +	struct i2c_client *client = data->client;
> +	struct device *dev = &client->dev;
>  	int ret, id;
>  	u16 regval;
>  
> @@ -400,8 +402,14 @@ static int vcnl4200_init(struct vcnl4000_data *data)
>  	}
>  	data->al_scale = data->chip_spec->ulux_step;
>  	data->ps_scale = 16;
> -	mutex_init(&data->vcnl4200_al.lock);
> -	mutex_init(&data->vcnl4200_ps.lock);
> +
> +	ret = devm_mutex_init(dev, &data->vcnl4200_al.lock);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = devm_mutex_init(dev, &data->vcnl4200_ps.lock);
> +	if (ret < 0)
> +		return ret;
I think this is ok because the only thing undone in remove is the power state
setting that is the last call in this function but the mixture of non
devm and devm calls in init is less than helpful for readability.
Given both init() callbacks end with
return data->chip_spec->set_power_state(data, true) and the remove
just calls that callback without any wrapping up in different init functions
I'm thinking it would make it all more readable if we didn't consider
turning on the power as part of the _init() but instead called it
directly from probe().

That would perhaps give more readable code and avoid mix of devm cleanup
and other cleanup in those callbacks.


>  
>  	/* Use 16 bits proximity sensor readings */
>  	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
> @@ -1985,6 +1993,7 @@ static int vcnl4000_probe(struct i2c_client *client)
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	struct vcnl4000_data *data;
>  	struct iio_dev *indio_dev;
> +	struct device *dev = &client->dev;
>  	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> @@ -1997,7 +2006,9 @@ static int vcnl4000_probe(struct i2c_client *client)
>  	data->id = id->driver_data;
>  	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
>  
> -	mutex_init(&data->vcnl4000_lock);
> +	ret = devm_mutex_init(dev, &data->vcnl4000_lock);
> +	if (ret < 0)
> +		return ret;
>  
>  	ret = data->chip_spec->init(data);
>  	if (ret < 0)
>