Re: [PATCH 2/2] Bluetooth: Replaces printk with pr_debug in bt_dbg

[Greg KH]

On Tue, Nov 07, 2023 at 09:32:51PM +0530, Yuran Pereira wrote:
> Hello Greg,
> On Tue, Nov 07, 2023 at 07:31:27AM +0100, Greg KH wrote:
> > 
> > You might have just changed the functionality here, are you SURE this is
> > identical to the original code?  How was it tested?
> > 
> > I'm not saying this is a bad idea to do, just be aware of the
> > consequences for this change and document it properly (hint, the
> > changelog does not document the user-visible change that just happened.)
> > 
> > Note, pr_debug() is NOT identical to printk(), look at the source for
> > the full details.
> > 
> 
> Thank you for the heads-up. 
> Yes, you're right.
> 
> I just took another look and it seems that using pr_debug here
> does defeat the purpose of bt_dbg which was created for situations
> where `DYNAMIC_DEBUG` and `DEBUG` are disabled.
> 
> The likely equivalent would have been `pr_devel` but that also
> depends on `DEBUG`.
> 
> Do you think that a new `pr_devel_uncond` like the one below
> (only to be used in exceptional scenarios) would be a good idea?
> 
> ```
> #define pr_devel_uncond(fmt, ...) \
>     printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> ```
> 
> This would neither depend on `DYNAMIC_DEBUG` nor on `DEBUG`.

No, not at all, the bluetooth subsystem should move to actually use the
proper dynamic debug infrastructure and not have their own "special"
subsystem loging macros/functions.  What you are doing here is the
proper way forward, BUT you need to make everyone aware that it is going
to change how things work from what they do today.

In other words, it's not just a "trivial" change, you need to get
approval to change this type of functionality from the Bluetooth
developers/maintainers.

thanks,

greg k-h