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

[fthain]

On Mon, 31 May 2010, Geert Uytterhoeven wrote:

> 
> `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?

Geert, in the beginning, I decided that it should be KERN_DEBUG, not 
KERN_INFO, and made that change in the first patch submission. I used 
pr_debug().

Then Joe pointed out that this required DEBUG defined, which I'd forgotten 
(I didn't know about CONFIG_DYNAMIC_DEBUG). So, to retain the old 
behaviour, while following the example of other usages of ei_debug in 
lib8390 and drivers, I changed it to printk(KERN_DEBUG ...).

Then Joe pointed out that I should take advantage of pr_fmt(), so the 
third submission made that change.

(Then David said I should propagate the return code from request_irq, so I 
made the present patch submission.)

Apparently David now wants me to submit this again --

if (ei_debug)
	pr_debug(...)

David, if that code is acceptable, please let me know.

If that code is mandatory, why didn't you say so upon the second patch 
submission?

Alternatively, if the following is preferred (as implied by your last 
email):

if (ei_debug)
	pr_info(...)

Then let me know, and I'll quit bothering you.

Or just go ahead and change my patch if you wish to save us both some 
time.

Finn

> 
> Gr{oetje,eeting}s,
> 
> 						Geert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/