Re: [PATCH v2 2/3] gpio: gpio-regmap: Use devm_add_action()

[Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>]

On Fri, May 21, 2021 at 12:31 PM Vaittinen, Matti
<Matti.Vaittinen@xxxxxxxxxxxxxxxxx> wrote:
>
>
> On Fri, 2021-05-21 at 10:38 +0200, Bjørn Mork wrote:
> > Michael Walle <michael@xxxxxxxx> writes:
> >
> > > Am 2021-05-21 08:28, schrieb Matti Vaittinen:
> > > > Slightly simplify the devm_gpio_regmap_register() by using the
> > > > devm_add_action().
> > >
> > > Hm, nice, but what bothers me a bit is that no other subsystem
> > > does it that way, eg. hwmon/hwmon.c or watchdog/watchdog_core.c.
> > > They also store just one pointer, thus could be simplified in the
> > > same way. What I don't know is if devm_add_action() was intended
> > > to be used this way. So I can't say much for this patch ;)
> >
> > There are some examples.  Like:
> >
> > int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter
> > *adapter)
> > {
> >         int ret;
> >
> >         ret = i2c_add_adapter(adapter);
> >         if (ret)
> >                 return ret;
> >
> >         return devm_add_action_or_reset(dev, devm_i2c_del_adapter,
> > adapter);
> > }
> >
> >
> > You should probably use the devm_add_action_or_reset() wrapper here
> > too,
> > catching the unlikely devm_add_action() alloc failure.
> >
>
> I was thinking of it but as the gpio registration succeeded I was
> thinking that we could go on with it - (which means we can proceed but
> the gpio is never released.)
>
> I am not sure how much difference it makes in the case of small alloc
> failure ;)
>
> But as it seems I am in any case re-spinning this I can change this to
> the devm_add_action_or_reset() and fail the gpio_regmap registration if
> alloc fails.
>
> Best Regards
>         Matti Vaittinen

Hi Matti,

Please use the reset variant. We always want to roll-back the changes
done in a function before the failure and propagate the error code.

Bart