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
