Re: [PATCH] mac8390: change an error return code and some cleanup, take 4

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 31, 2010 at 11:21,  <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
On Mon, 31 May 2010, David Miller wrote:
This is getting tiring Finn.

I agree. My patch addresses all of the criticism of the earlier
submissions.

To make it plain: there are 25 files or so that use ei_debug. Three of
those that now have the KERN_DEBUG printk's suppresed by the DEBUG macro
only do so as an apparently unintended side effect of a commit that claims
to "implement dynmic debug infrastructure". (Go figure.)

http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commitdiff;h=dd0fab5b940c0b65f26ac5b01485bac1f690ace6

Your suggestion to use pr_debug is invoking compile time infrastructure
(the DEBUG macro), so it is not in the spirit of this commit, and it is
not relevant to any criticism from you or Joe of the earlier submissions.

Please apply the patch.

`pr_debug()' indeed now may generate code if DEBUG is not defined,
i.e. if CONFIG_DYNAMIC_DEBUG is enabled.
This is intented for debug infrastructure the user may want to enable later.

If you want the old behavior, you can use `pr_devel()' instead, which
only generates code if DEBUG is defined.
This is intended for debug infrastructure for developers only.

However, you used `printk(KERN_DEBUG pr_fmt()...)`, which always generates code.
I'm still not 100% sure that was intentional?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux