Re: [PATCH v4 05/21] thermal/drivers/tegra: convert to use devm_request*_irq_probe()

[Krzysztof Kozlowski <krzk@kernel.org>]

On 10/07/2023 15:16, Thierry Reding wrote:
> On Mon, Jul 10, 2023 at 05:59:09PM +0800, Yangtao Li 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.
> 
> Do we really need to keep repeating this same commit message for each
> and everyone of these commits? It's already in the cover letter and
> presumably on the patch that introduces the new helper, so surely we can
> come up with a denser version for individual subsystem patches.

Yeah, this is way too long to put in every commit doing the same but for
different drivers.

Best regards,
Krzysztof