Re: [PATCH v2 06/18] m68k: Replace setup_irq() by request_irq()

From: Greg Ungerer
Date: Thu Feb 27 2020 - 01:37:28 EST



On 27/2/20 8:31 am, Finn Thain wrote:
On Wed, 26 Feb 2020, Greg Ungerer wrote:

On 26/2/20 4:39 pm, Finn Thain wrote:

If -EBUSY means the end user has misconfigured something, printing
"request_irq failed" would be helpful. But does that still happen?

I have seen it many times. Its not at all difficult to get interrupt
assignments wrong, duplicated, or otherwise mistaken when creating
device trees. Not so much m68k/coldfire platforms where they are most
commonly hard coded.


I was thinking of end users and production builds. You seem to be
concerned about developers. Catering to developers argues for pr_debug()
here, if anything.

Perhaps. But most of the kernel boot output as it stands today
is more debug (or maybe notice) than useful.


You say you've seen -16 errors "many times". Have you also seen -22? Did
the ability to distinguish these values help you to fix your device tree?

Probably not. But the real difficulty is trying to diagnose other
peoples problems with just console trace output. The more information
there the better.


...

BTW, one of the benefits of "%s: request_irq failed" is that a
compilation unit with multiple request_irq calls permits the compiler
to coalesce all duplicated format strings. Whereas, that's not
possible with "foo: request_irq failed" and "bar: request_irq failed".

Given the wide variety of message text used with failed request_irq()
calls it would be shear luck that this matched anything else. A quick
grep shows that "%s: request_irq() failed\n" has no other exact matches
in the current kernel source.


You are overlooking the patches in this series that produce multiple
identical format strings.

No I didn't :-) None of these will end up compiled in at the same time.
The various ColdFire SoC parts have a single timer hardware module -
and only the required one will be compiled in, not all of them.

Regards
Greg