Re: [PATCH v2 15/23] counter: 104-quad-8: Convert to new counter registration

From: Uwe Kleine-König
Date: Tue Dec 28 2021 - 06:06:30 EST


On Mon, Dec 27, 2021 at 01:19:25PM +0100, Lars-Peter Clausen wrote:
> On 12/27/21 10:45 AM, Uwe Kleine-König wrote:
> > [...]
> > - return devm_counter_register(dev, &priv->counter);
> > + err = devm_counter_add(dev, counter);
> > + if (err < 0)
> > + return dev_err_probe(dev, err, "Failed to add counter\n");
> I wonder if we should put that dev_err_probe into the devm_counter_add since
> every driver wants to have it anyway.

Personally I'm not a big fan of API functions that emit an error
message. Usually the consumer knows best if a certain failing function
call is worth an error message. Look at platform_get_irq() that some
time ago got a dev_err call. The result was to introduce a
silent variant (platform_get_irq_optional()) because emitting an error
message isn't the right thing in all cases.

Also the API function probably won't call
device_set_deferred_probe_reason() and so the consumer has to care for
that anyhow (or not which makes -EPROBE_DEFER problems harder to debug).

Furthermore the consumer can emit in some cases a better error message
than the core. (Well, this doesn't apply here, but in the example of
platform_get_irq() the driver knows the irq's purpose and can call it
"TX irq" in the error message which the irq core cannot.)

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature