| Message ID | 20250407-gpiochip-set-rv-iio-v1-1-8431b003a145@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: convert GPIO chips to using new value setters | expand |
On Mon, 07 Apr 2025 09:18:09 +0200 Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > The locks used here are initialized but never released which causes > resource leaks with mutex debugging enabled. Add missing calls to > mutex_destroy() or use devres if applicable. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Hi Bartosz, > --- > drivers/iio/dac/ad5592r-base.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c > index 50d19304bacb..fe4c35689d4d 100644 > --- a/drivers/iio/dac/ad5592r-base.c > +++ b/drivers/iio/dac/ad5592r-base.c > @@ -155,6 +155,8 @@ static void ad5592r_gpio_cleanup(struct ad5592r_state *st) > { > if (st->gpio_map) > gpiochip_remove(&st->gpiochip); > + > + mutex_destroy(&st->gpio_lock); > } > > static int ad5592r_reset(struct ad5592r_state *st) > @@ -622,7 +624,9 @@ int ad5592r_probe(struct device *dev, const char *name, > iio_dev->info = &ad5592r_info; > iio_dev->modes = INDIO_DIRECT_MODE; > > - mutex_init(&st->lock); > + ret = devm_mutex_init(dev, &st->lock); > + if (ret) > + goto error_disable_reg; Please don't mix devm and gotos. That tends to make for complex logic to review for limited gain. Devm then gotos is fine though (i.e. a transition from all devm to gotos only mid way through probe). Easiest solution would be to move this mutex init before the regulator is enabled (so up about 10 lines). Then we finish the devm stuff before starting the non devm part. A more complex cleanup would be to drop the support for dynamic vref (as that is almost certainly copied and pasted from elsewhere rather than a real thing) and then use devm to manage the vref. That wouldn't be related to this series though so I'd just move that mutex init up. > > ad5592r_init_scales(st, ad5592r_get_vref(st)); > >
diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c index 50d19304bacb..fe4c35689d4d 100644 --- a/drivers/iio/dac/ad5592r-base.c +++ b/drivers/iio/dac/ad5592r-base.c @@ -155,6 +155,8 @@ static void ad5592r_gpio_cleanup(struct ad5592r_state *st) { if (st->gpio_map) gpiochip_remove(&st->gpiochip); + + mutex_destroy(&st->gpio_lock); } static int ad5592r_reset(struct ad5592r_state *st) @@ -622,7 +624,9 @@ int ad5592r_probe(struct device *dev, const char *name, iio_dev->info = &ad5592r_info; iio_dev->modes = INDIO_DIRECT_MODE; - mutex_init(&st->lock); + ret = devm_mutex_init(dev, &st->lock); + if (ret) + goto error_disable_reg; ad5592r_init_scales(st, ad5592r_get_vref(st));