Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Fri, 21 Jul 2023 12:45:11 +0200 | Subject | Re: [PATCH v5 02/22] thermal/drivers/sun8i: convert to use devm_request*_irq_probe() |
| |
On Fri, Jul 21, 2023 at 11:47 AM Yangtao Li <frank.li@vivo.com> wrote: > > There are more than 700 calls to devm_request_threaded_irq method and > more than 1000 calls to devm_request_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 2000 lines > of code can be saved by removing the msg in the driver. > > And tglx point out that: > > If we actually look at the call sites of > devm_request_threaded_irq() then the vast majority of them print more or > less lousy error messages. A quick grep/sed/awk/sort/uniq revealed > > 519 messages total (there are probably more) > > 352 unique messages > > 323 unique messages after lower casing > > Those 323 are mostly just variants of the same patterns with > slight modifications in formatting and information provided. > > 186 of these messages do not deliver any useful information, > e.g. "no irq", " > > The most useful one of all is: "could request wakeup irq: %d" > > So there is certainly an argument to be made that this particular > function should print a well formatted and informative error message. > > It's not a general allocator like kmalloc(). It's specialized and in the > vast majority of cases failing to request the interrupt causes the > device probe to fail. So having proper and consistent information why > the device cannot be used _is_ useful. > > So convert to use devm_request*_irq_probe() API, which 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. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Krzysztof Kozlowski <krzk@kernel.org> > Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> > Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > Signed-off-by: Yangtao Li <frank.li@vivo.com> > Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
It is not clear to me what the purpose of sending these patches is.
Because the devm_request_threaded_irq_probe() definition is not there in the current -rc kernels AFAICS, it looks like they are sent in order to collect tags from people. If so, there should be a cover letter making that clear.
As it stands, it is also unclear how you want them to be merged.
Moreover, sending the series without patch [01/22] to linux-pm has not been helpful.
Thanks!
> --- > drivers/thermal/sun8i_thermal.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c > index 195f3c5d0b38..a952804ff993 100644 > --- a/drivers/thermal/sun8i_thermal.c > +++ b/drivers/thermal/sun8i_thermal.c > @@ -512,9 +512,9 @@ static int sun8i_ths_probe(struct platform_device *pdev) > * registered yet, we deffer the registration of the interrupt to > * the end. > */ > - ret = devm_request_threaded_irq(dev, irq, NULL, > - sun8i_irq_thread, > - IRQF_ONESHOT, "ths", tmdev); > + ret = devm_request_threaded_irq_probe(dev, irq, NULL, > + sun8i_irq_thread, > + IRQF_ONESHOT, "ths", tmdev, NULL); > if (ret) > return ret; > > -- > 2.39.0 >
| |