Hi Marcel, Thanks for your review. > 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. Regards, Amitkumar -- 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
