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

[Marcel Holtmann <marcel@holtmann.org>]

Hi Joe,

> > 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.

it is similar to what we do in the Bluetooth subsystem, but in the end
upper-case vs lower-case is a bit more personal taste.

The pr_<level> ones are nice and I wished we had them all back in the
days, but the NFC ones actually could take the controller as argument
and then us the dev_* versions of these commands.

At this stage of the project it is a bit too early to tell I guess.

> I think emitting __func__ rarely adds useful information.

Depends on how you are using your debug statements. I find it really
helpful since then you can keep the text detail to a minimum.

Regards

Marcel