[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <db14ea3c-c224-e263-e134-70e9acc5008a@vivo.com>
Date: Mon, 3 Jul 2023 17:10:22 +0800
From: Yangtao Li <frank.li@...o.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
miquel.raynal@...tlin.com, rafael@...nel.org,
daniel.lezcano@...aro.org, amitk@...nel.org, rui.zhang@...el.com,
mmayer@...adcom.com, bcm-kernel-feedback-list@...adcom.com,
florian.fainelli@...adcom.com, shawnguo@...nel.org,
s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
linux-imx@....com, agross@...nel.org, andersson@...nel.org,
konrad.dybcio@...aro.org, thara.gopinath@...il.com,
heiko@...ech.de, mcoquelin.stm32@...il.com,
alexandre.torgue@...s.st.com, thierry.reding@...il.com,
jonathanh@...dia.com, tglx@...utronix.de, matthias.bgg@...il.com,
angelogioacchino.delregno@...labora.com,
srinivas.pandruvada@...ux.intel.com,
DLG-Adam.Ward.opensource@...renesas.com, shangxiaojing@...wei.com,
bchihi@...libre.com, wenst@...omium.org,
u.kleine-koenig@...gutronix.de, hayashi.kunihiko@...ionext.com,
niklas.soderlund+renesas@...natech.se, chi.minghao@....com.cn,
johan+linaro@...nel.org, jernej.skrabec@...il.com
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org, linux-rockchip@...ts.infradead.org,
linux-stm32@...md-mailman.stormreply.com,
linux-tegra@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 01/15] genirq/devres: Add error information printing for
devm_request_threaded_irq()
On 2023/6/27 15:43, Krzysztof Kozlowski wrote:
> On 27/06/2023 09:16, 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.
>> 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@...o.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..94039a915218 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");
> Just like any memory allocation, I don't think we print anything for
> devres failures. Why do you think we should start doing it?
And tglx point out that:
Having proper and consistent information why the device cannot be used
_is_ useful.
>
>> 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(dev, "Failed to request threaded irq\n");
> I don't like that one path - devm() managed - prints error, but regular
> path does not. Code should be here consistent. Also error message is too
> generic. You need to print at least irq number, maybe also devname?
v3 has been added.
Thx,
Yangtao
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists