On Sat, 14 Mar 2026 18:06:32 +0200 Erikas Bitovtas <[email protected]> wrote:
> Replace mutex_init used across driver with its device-managed > counterpart, so all assigned mutexes get destroyed. > > Signed-off-by: Erikas Bitovtas <[email protected]> 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) >
