Hi Amitkumar, >> From: Marcel Holtmann [mailto:marcel@xxxxxxxxxxxx] >> Sent: Thursday, October 15, 2015 4:39 AM >> To: Amitkumar Karwar >> Cc: linux-bluetooth; Cathy Luo; Zhaoyang Liu >> Subject: Re: [PATCH 4/4] Bluetooth: btmrvl: change debug prints to >> btmrvl_dbg >> >> Hi Amitkumar, >> >>> This patch changes all debug print from BT_INFO/BT_DBG/BT_ERR to >>> marvell bluetooth driver specific debug functions. >> >> so BT_INFO and BT_ERR are not debug prints. They are genuine valid >> information that should be printed when something goes wrong or is >> informational. > > We will correct the words. > >> >>> >>> Signed-off-by: Zhaoyang Liu <liuzy@xxxxxxxxxxx> >>> Signed-off-by: Cathy Luo <cluo@xxxxxxxxxxx> >>> Signed-off-by: Amitkumar Karwar <akarwar@xxxxxxxxxxx> >>> --- >>> drivers/bluetooth/btmrvl_debugfs.c | 3 +- >>> drivers/bluetooth/btmrvl_main.c | 145 ++++++++++------- >>> drivers/bluetooth/btmrvl_sdio.c | 311 ++++++++++++++++++++++------- >> -------- >>> 3 files changed, 275 insertions(+), 184 deletions(-) >>> >>> diff --git a/drivers/bluetooth/btmrvl_debugfs.c >>> b/drivers/bluetooth/btmrvl_debugfs.c >>> index af52b03..7bbe3dc 100644 >>> --- a/drivers/bluetooth/btmrvl_debugfs.c >>> +++ b/drivers/bluetooth/btmrvl_debugfs.c >>> @@ -257,7 +257,8 @@ void btmrvl_debugfs_init(struct hci_dev *hdev) >>> priv->debugfs_data = dbg; >>> >>> if (!dbg) { >>> - BT_ERR("Can not allocate memory for btmrvl_debugfs_data."); >>> + btmrvl_dbg(priv->adapter, ERROR, >>> + "Can not allocate memory for >> btmrvl_debugfs_data."); >> >> Errors are not debug messages. The syntax for btmrvl_dbg is not correct. >> We can not have such a code merged upstream. >> >> I am also feeling to see all this value with debug_mask and so on. The >> Linux kernel supports dynamic debug which is way more powerful than >> trying to invent your own mask based logging. And BT_DBG and other debug >> print functions are hooked into it. So why not use it. >> >> If you prefer having some btmrvl_dev_info wrappers, then that seems >> reasonable, but they either map to BT_INFO or pr_dev_info or similar. >> >> I would really prefer if this patch series gets re-thought and done the >> Linux kernel way. > > CONFIG_DYNAMIC_DEBUG compiler flag is not by default enabled on some of the platforms. > This was the reason we decided to implement this mask based logging. We do have such implementation for our WLAN driver. Also, I can see similar thing in other driver in wireless subsystem. and that is a good enough reason to duplicate functionality? My viewpoint is that it is not and that even wireless drivers should be cleaned up. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
