Messages in this thread | | | Date | Tue, 27 Jun 2023 13:00:25 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v2 01/15] genirq/devres: Add error information printing for devm_request_threaded_irq() |
| |
Hello,
On Tue, Jun 27, 2023 at 06:12:01PM +0800, Yangtao Li wrote: > Ensure that all error handling branches print error information. In this > way, when this function fails, the upper-layer functions can directly > return an error code without missing debugging information. Otherwise, > the error message will be printed redundantly or missing. > > There are more than 700 calls to the devm_request_threaded_irq method. > Most drivers only request one interrupt resource, and these error > messages are basically the same. If error messages are printed > everywhere, more than 1000 lines of code can be saved by removing the > msg in the driver. > > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > kernel/irq/devres.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c > index f6e5515ee077..fcb946ffb7ec 100644 > --- a/kernel/irq/devres.c > +++ b/kernel/irq/devres.c > @@ -58,8 +58,10 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq, > > dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres), > GFP_KERNEL); > - if (!dr) > + if (!dr) { > + dev_err(dev, "Failed to allocate device resource data\n"); > return -ENOMEM; > + } > > if (!devname) > devname = dev_name(dev); > @@ -67,6 +69,7 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq, > rc = request_threaded_irq(irq, handler, thread_fn, irqflags, devname, > dev_id); > if (rc) { > + dev_err_probe(dev, rc, "Failed to request threaded irq%d: %d\n", irq, rc);
This changes semantics because dev_err_probe() is only supposed to be called in probe functions. Not sure about devm_request_threaded_irq, but its friend request_irq is called in the setup_irq (or open IIRC) callback of serial drivers.
While I assume changing to dev_err_probe is a result of my concern that no error should be printed when rc=-EPROBEDEFER, my other concern that adding an error message to a generic allocation function is a bad idea still stands.
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |