[PATCH] iio: chemical: scd30: Use devm_mutex_init() over non-devm mutex_init()

[PATCH] iio: chemical: scd30: Use devm_mutex_init() over non-devm mutex_init()

[Maxwell Doose]

The current code uses mutex_init() instead of devm_mutex_init(), which
is incorrect as the rest of the file uses the devm automatic resource
management API. Fix this so that the mutex is set up in the same way as
the rest of the device data structure.

Fixes: 64b3d8b1b0f5c ("iio: chemical: scd30: add core driver")
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/chemical/scd30_core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
index db5cc295aeab..f00979c0c196 100644
--- a/drivers/iio/chemical/scd30_core.c
+++ b/drivers/iio/chemical/scd30_core.c
@@ -714,7 +714,10 @@ int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
 	state->pressure_comp = SCD30_PRESSURE_COMP_DEFAULT;
 	state->meas_interval = SCD30_MEAS_INTERVAL_DEFAULT;
 	state->command = command;
-	mutex_init(&state->lock);
+	ret = devm_mutex_init(dev, &state->lock);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to init mutex\n");
+
 	init_completion(&state->meas_ready);
 
 	dev_set_drvdata(dev, indio_dev);
-- 
2.54.0

Re: [PATCH] iio: chemical: scd30: Use devm_mutex_init() over non-devm mutex_init()

[Jonathan Cameron]

On Wed,  3 Jun 2026 09:51:33 -0500
Maxwell Doose <m32285159@gmail.com> wrote:

> The current code uses mutex_init() instead of devm_mutex_init(), which
> is incorrect as the rest of the file uses the devm automatic resource
> management API. Fix this so that the mutex is set up in the same way as
> the rest of the device data structure.
> 
> Fixes: 64b3d8b1b0f5c ("iio: chemical: scd30: add core driver")
Hi Maxwell,

No to this being a fix.  This enhances one corner case of debug, slightly.
So to me it isn't worth a fixes tag.

> Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> ---
>  drivers/iio/chemical/scd30_core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
> index db5cc295aeab..f00979c0c196 100644
> --- a/drivers/iio/chemical/scd30_core.c
> +++ b/drivers/iio/chemical/scd30_core.c
> @@ -714,7 +714,10 @@ int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
>  	state->pressure_comp = SCD30_PRESSURE_COMP_DEFAULT;
>  	state->meas_interval = SCD30_MEAS_INTERVAL_DEFAULT;
>  	state->command = command;
> -	mutex_init(&state->lock);
> +	ret = devm_mutex_init(dev, &state->lock);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to init mutex\n");
The only thing it can return is -ENOMEM I think. Which doesn't print anything
so 
	if (ret)
		return ret;

is enough.

> +
>  	init_completion(&state->meas_ready);
>  
>  	dev_set_drvdata(dev, indio_dev);

Re: [PATCH] iio: chemical: scd30: Use devm_mutex_init() over non-devm mutex_init()

[Maxwell Doose]

On Wed, Jun 3, 2026 at 11:44 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed,  3 Jun 2026 09:51:33 -0500
> Maxwell Doose <m32285159@gmail.com> wrote:
>
> > The current code uses mutex_init() instead of devm_mutex_init(), which
> > is incorrect as the rest of the file uses the devm automatic resource
> > management API. Fix this so that the mutex is set up in the same way as
> > the rest of the device data structure.
> >
> > Fixes: 64b3d8b1b0f5c ("iio: chemical: scd30: add core driver")
> Hi Maxwell,
>
> No to this being a fix.  This enhances one corner case of debug, slightly.
> So to me it isn't worth a fixes tag.
>

Can drop. I guess I was just erring on the side of "we're fixing the
original maintainer's/contributor's mistake when they did devm" but I
guess that's a fallacy since the only way any issues could arise is if
the device gets deregistered and reregistered.

> > Signed-off-by: Maxwell Doose <m32285159@gmail.com>
> > ---
> >  drivers/iio/chemical/scd30_core.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
> > index db5cc295aeab..f00979c0c196 100644
> > --- a/drivers/iio/chemical/scd30_core.c
> > +++ b/drivers/iio/chemical/scd30_core.c
> > @@ -714,7 +714,10 @@ int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
> >       state->pressure_comp = SCD30_PRESSURE_COMP_DEFAULT;
> >       state->meas_interval = SCD30_MEAS_INTERVAL_DEFAULT;
> >       state->command = command;
> > -     mutex_init(&state->lock);
> > +     ret = devm_mutex_init(dev, &state->lock);
> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "Failed to init mutex\n");
> The only thing it can return is -ENOMEM I think. Which doesn't print anything
> so
>         if (ret)
>                 return ret;
>
> is enough.
>

If I'm not wrong (and after a quick google search), I think it also
may return -EINVAL but then that means the device struct is invalid.
Still may be worth doing dev_err_probe(dev, ret, "Failed to init mutex
with error %d", ret); or something along those lines. If you want I
can get this out today or I can also wait for more reviewer feedback,
but your recommended changes seem pretty simple. TBH though I'd still
argue for the case of either dev_err_probe() or dev_warn() since not
having an initialized mutex is certainly an error.

-- 
best regards,
max

Re: [PATCH] iio: chemical: scd30: Use devm_mutex_init() over non-devm mutex_init()

[Andy Shevchenko]

On Wed, Jun 03, 2026 at 12:27:18PM -0500, Maxwell Doose wrote:
> On Wed, Jun 3, 2026 at 11:44 AM Jonathan Cameron <jic23@kernel.org> wrote:
> > On Wed,  3 Jun 2026 09:51:33 -0500
> > Maxwell Doose <m32285159@gmail.com> wrote:

...

> > > +     ret = devm_mutex_init(dev, &state->lock);
> > > +     if (ret)
> > > +             return dev_err_probe(dev, ret, "Failed to init mutex\n");
> > The only thing it can return is -ENOMEM I think. Which doesn't print anything
> > so
> >         if (ret)
> >                 return ret;
> >
> > is enough.
> 
> If I'm not wrong (and after a quick google search), I think it also
> may return -EINVAL but then that means the device struct is invalid.

What?!

> Still may be worth doing dev_err_probe(dev, ret, "Failed to init mutex
> with error %d", ret); or something along those lines. If you want I
> can get this out today or I can also wait for more reviewer feedback,
> but your recommended changes seem pretty simple. TBH though I'd still
> argue for the case of either dev_err_probe() or dev_warn() since not
> having an initialized mutex is certainly an error.

I am on the side of dropping the message altogether. Invalid dev here is
nonsense. Just read the code.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: chemical: scd30: Use devm_mutex_init() over non-devm mutex_init()

[Maxwell Doose]

On Thu, Jun 4, 2026 at 2:43 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, Jun 03, 2026 at 12:27:18PM -0500, Maxwell Doose wrote:
> > On Wed, Jun 3, 2026 at 11:44 AM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Wed,  3 Jun 2026 09:51:33 -0500
> > > Maxwell Doose <m32285159@gmail.com> wrote:
>
> ...
>
> > > > +     ret = devm_mutex_init(dev, &state->lock);
> > > > +     if (ret)
> > > > +             return dev_err_probe(dev, ret, "Failed to init mutex\n");
> > > The only thing it can return is -ENOMEM I think. Which doesn't print anything
> > > so
> > >         if (ret)
> > >                 return ret;
> > >
> > > is enough.
> >
> > If I'm not wrong (and after a quick google search), I think it also
> > may return -EINVAL but then that means the device struct is invalid.
>
> What?!
>

Then I was either misled or I'm just stupid :(

> > Still may be worth doing dev_err_probe(dev, ret, "Failed to init mutex
> > with error %d", ret); or something along those lines. If you want I
> > can get this out today or I can also wait for more reviewer feedback,
> > but your recommended changes seem pretty simple. TBH though I'd still
> > argue for the case of either dev_err_probe() or dev_warn() since not
> > having an initialized mutex is certainly an error.
>
> I am on the side of dropping the message altogether. Invalid dev here is
> nonsense. Just read the code.
>

All of these changes have been made in v2.

-- 
best regards,
max

Re: [PATCH] iio: chemical: scd30: Use devm_mutex_init() over non-devm mutex_init()

[Andy Shevchenko]

On Thu, Jun 04, 2026 at 08:20:25AM -0500, Maxwell Doose wrote:
> On Thu, Jun 4, 2026 at 2:43 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Wed, Jun 03, 2026 at 12:27:18PM -0500, Maxwell Doose wrote:
> > > On Wed, Jun 3, 2026 at 11:44 AM Jonathan Cameron <jic23@kernel.org> wrote:
> > > > On Wed,  3 Jun 2026 09:51:33 -0500
> > > > Maxwell Doose <m32285159@gmail.com> wrote:

...

> > > > > +     ret = devm_mutex_init(dev, &state->lock);
> > > > > +     if (ret)
> > > > > +             return dev_err_probe(dev, ret, "Failed to init mutex\n");
> > > > The only thing it can return is -ENOMEM I think. Which doesn't print anything
> > > > so
> > > >         if (ret)
> > > >                 return ret;
> > > >
> > > > is enough.
> > >
> > > If I'm not wrong (and after a quick google search), I think it also
> > > may return -EINVAL but then that means the device struct is invalid.
> >
> > What?!
> >
> 
> Then I was either misled or I'm just stupid :(
> 
> > > Still may be worth doing dev_err_probe(dev, ret, "Failed to init mutex
> > > with error %d", ret); or something along those lines. If you want I
> > > can get this out today or I can also wait for more reviewer feedback,
> > > but your recommended changes seem pretty simple. TBH though I'd still
> > > argue for the case of either dev_err_probe() or dev_warn() since not
> > > having an initialized mutex is certainly an error.
> >
> > I am on the side of dropping the message altogether. Invalid dev here is
> > nonsense. Just read the code.
> 
> All of these changes have been made in v2.

I am not sure I follow. I was specifically talking about implementation of
dev_mutex_init().

-- 
With Best Regards,
Andy Shevchenko