On Mon, Dec 05, 2022 at 01:14:21PM +0100, Andrzej Pietrasiewicz wrote: > Hi All, > > include/linux/usb/composite.h contains: > > /* messaging utils */ > #define DBG(d, fmt, args...) \ > dev_dbg(&(d)->gadget->dev , fmt , ## args) > #define VDBG(d, fmt, args...) \ > dev_vdbg(&(d)->gadget->dev , fmt , ## args) > #define ERROR(d, fmt, args...) \ > dev_err(&(d)->gadget->dev , fmt , ## args) > #define WARNING(d, fmt, args...) \ > dev_warn(&(d)->gadget->dev , fmt , ## args) > #define INFO(d, fmt, args...) \ > dev_info(&(d)->gadget->dev , fmt , ## args) > > Gadget functions do use these, but not consistently: > > => DBG() vs dev_dbg(): > $ git grep DBG\( drivers/usb/gadget/function | grep -v VDBG | grep -v LDBG | wc > 138 871 11619 > > $ git grep dev_dbg\( drivers/usb/gadget/function | grep -v "##args" | wc > 33 151 2831 > > => VDBG() vs dev_vdbg(): > git grep VDBG\( drivers/usb/gadget/function | grep -v "#define" | wc > 49 269 3954 > > $ git grep dev_vdbg\( drivers/usb/gadget/function | wc > 2 4 135 > > => ERROR() vs dev_err(): > $ git grep ERROR\( drivers/usb/gadget/function | grep -v LERROR | wc > 72 508 6560 > > $ git grep dev_err\( drivers/usb/gadget/function | grep -v "##args" | wc > 65 309 5527 > > => WARNING() vs dev_warn(): > $ git grep WARNING\( drivers/usb/gadget/function | wc > 4 28 383 > > $ git grep dev_warn\( drivers/usb/gadget/function | grep -v "##args" | wc > 3 6 169 > > => INFO() vs dev_info(): > $ git grep INFO\( drivers/usb/gadget/function | grep -v LINFO | grep -v > u_ether | wc > 14 64 1167 > > $ git grep dev_info\( drivers/usb/gadget/function | grep -v "##args" | wc > 0 0 0 Drivers that work properly should be quiet, so no INFO() usage should probably be needed anyway. > > Questions: > > 1) Should we make them use the messaging utils consitently? Yes, converting to use the dev_*() calls is good to do. > 2) How consistent should we become, given that some functions in the relevant > files, for example u_audio_start_capture(), sometimes (but not always) have: > > struct usb_gadget *gadget = audio_dev->gadget; > struct device *dev = &gadget->dev; > > and then they use dev_dbg(dev, ....); dev_dbg() is fine, what's worng with that? > If we were to use DBG(audio_dev, ....); instead, then we effectively get > dev_dbg(&audio_dev->gadget->dev, ....); after macro expansion, which means two > pointer dereferences and taking an address of the result (I'm wondering how > smart the compiler can get if such a pattern repeats several times in a > function). The compiler can get very smart, but this isn't really an issue overall as USB drivers are very slow due to slow hardware. > 3) Maybe the amount of various messages is too large in the first place > and should be reduced before any unifications? Possibly, many might be able to be removed, look and see! thanks, greg k-h
