Hi Marcel, > From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth- > owner@xxxxxxxxxxxxxxx] On Behalf Of Marcel Holtmann > Sent: Thursday, October 15, 2015 8:21 PM > 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, > > >> 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. > This has been implemented this just to get logs for the issues reported by end user where DYNAMIC DEBUG is not enabled. If isn't a standard way in Linux kernel, please drop these patches. I suppose you can still consider 1/4. Regards, Amit -- 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
