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@xxxxxxxxxx> wrote:
>
> On Wed,  3 Jun 2026 09:51:33 -0500
> Maxwell Doose <m32285159@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >  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