> > >> >> +uint debug; > > >> >> +module_param(debug, uint, 0); > > > >>> +MODULE_PARM_DESC(debug, "Default debug msglevel"); > > >> > > >> >Why are you adding this as a module parameter? > > >> > > >> I believe this is mostly to follow same line as qede which also defines > > > > 'debug' module parameter for allowing easy user control of debug > > > > prints [& specifically for probe prints, which can't be controlled > > > > otherwise]. > > > > > Can you give us an example where dynamic debug and tracing infrastructures > > > are not enough? > > > > > AFAIK, most of these debug module parameters are legacy copy/paste > > > code which is useless in real life scenarios. > > > > Define 'enough'; Using dynamic debug you can provide all the necessary > > information and at an even better granularity that's achieved by suggested > > infrastructure, but is harder for an end-user to use. Same goes for > > tracing. > > > > The 'debug' option provides an easy grouping for prints related to a > > specific > > area in the driver. > > It is hard to agree with you that user which knows how-to load modules > with parameters won't success to enable debug prints.
I think you're giving too much credit to the end-user. :-D > In addition, global increase in debug level for whole driver will create > printk storm in dmesg and give nothing to debuggability. So basically, what you're claiming is that ethtool 'msglvl' setting for devices is completely obselete. While this *might* be true, we use it extensively in our qede and qed drivers; The debug module parameter merely provides a manner of setting the debug value prior to initial probe for all interfaces. qedr follows the same practice.