Re: [PATCH v4 1/6] NFC: add nfc subsystem core

[Joe Perches <joe@perches.com>]

On Tue, 2011-06-28 at 20:31 -0300, Aloisio Almeida wrote:
> Hi Joe,

Oi Aloisio.

> On Tue, Jun 28, 2011 at 5:18 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2011-06-28 at 15:20 -0300, Aloisio Almeida Jr wrote:
> >> From: Lauro Ramos Venancio <lauro.venancio@openbossa.org>
> >> The NFC subsystem core is responsible for providing the device driver
> >> interface. It is also responsible for providing an interface to the control
> >> operations and data exchange.
> > []
> >> diff --git a/net/nfc/nfc.h b/net/nfc/nfc.h
> > []
> >> +#define NFC_INFO(fmt, arg...) printk(KERN_INFO "NFC: " fmt "\n", ## arg)
> >> +#define NFC_ERR(fmt, arg...) printk(KERN_ERR "%s: " fmt "\n", __func__, ## arg)
> >> +#define NFC_DBG(fmt, arg...) pr_debug("%s: " fmt "\n", __func__, ## arg)
> >
> > I think these #defines and their uses would be
> > better lower case.
> >
> > #define nfc_info(etc...)
> > #define nfc_err(etc...)
> > #define nfc_dbg(etc...)
> >
> > And because these don't really take any nfc specific
> > struct as an argument, may be better removed altogether
> > and replaced with the pr_<level> equivalents.
> I was using pr_*() on previous versions. One of the proposed changes
> for v4 was to create these macros to avoid calling pr_*() functions
> with same parameters every time. The implementation chosen is based on
> BT_* macros from bluetooth subsystem.
> > I think emitting __func__ rarely adds useful information.
> I can say that during the implementation phase I considered __func__
> useful, mainly for tracing the execution. That's the reason I left it
> in the code.

Up to you, but regardless, I think the bluetooth macros
aren't good ones to follow.  Most all of the other
<foo>_<level> output logging forms are lowercase.

cheers, Joe