[PATCH] Remove error prints for devm_add_action_or_reset()

[Uwe Kleine-König]

Hello Andy,

On Tue, Jul 01, 2025 at 08:57:02PM +0300, Andy Shevchenko wrote:
> On Tue, Jul 1, 2025 at 8:44 PM Uwe Kleine-König <ukleinek at kernel.org> wrote:
> > On Tue, Jul 01, 2025 at 05:03:33PM +0200, Waqar Hameed wrote:
> 
> ...
> 
> > With that
> >
> >         ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
> >                                        meson->channels[i].clk);
> >         if (ret)
> >                 return dev_err_probe(dev, ret,
> >                                      "Failed to add clk_put action\n");
> >
> > from drivers/pwm/pwm-meson.c is optimized to
> >
> >         ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
> >                                        meson->channels[i].clk);
> >         if (ret)
> >                 return ret;
> >
> > .
> >
> > I would prefer this approach, because a) there is no need to drop all
> > dev_err_probe()s after devm_add_action_or_reset() and b) the
> > dev_err_probe()s could stay for consistency in the error paths of a
> > driver.
> 
> Why do we need a dev_err_probe() after devm_add_action*()? I would
> expect that the original call (if needed) can spit out a message.

I'm not a big fan of API functions that emit an error message. In
general the caller knows better what went wrong (here:
devm_add_action_or_reset() doesn't know this to be about the clk_put
action), so the error message can be more expressive.

Also in general an API function doesn't know if a failure is fatal or if
the consumer handles the failure just well and if the call is part of a
driver's .probe() so it's unclear if dev_err_probe() can/should be used.
(I admit that the last two probably don't apply to
devm_add_action_or_reset() but that's not a good enough reason to
make this function special. Every special case is a maintanance burden.)

My two ¢,
Uwe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-phy/attachments/20250702/a501dc3e/attachment.sig>