On Mon, Jul 05, 2021 at 11:08:07AM +0300, Kalle Valo wrote: > Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> writes: > > > On Fri, Jul 02, 2021 at 10:08:53AM -0700, Brian Norris wrote: > >> On Fri, Jul 2, 2021 at 12:23 AM Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote: > >> > On Fri, Jun 18, 2021 at 02:46:05PM +0800, Ping-Ke Shih wrote: > >> > > +#ifdef CONFIG_RTW89_DEBUGMSG > >> > > +unsigned int rtw89_debug_mask; > >> > > +EXPORT_SYMBOL(rtw89_debug_mask); > >> > > +module_param_named(debug_mask, rtw89_debug_mask, uint, 0644); > >> > > +MODULE_PARM_DESC(debug_mask, "Debugging mask"); > >> > > +#endif > >> > > >> > > >> > For dynamic debugging we usually use ethtool msglvl. > >> > Please, convert all dev_err/warn/inf.... to netif_ counterparts > >> > >> Have you ever looked at a WiFi driver? > > > > Yes. You can parse the kernel log for my commits. > > > >> I haven't seen a single one that uses netif_*() for logging. > >> On the other hand, almost every > >> single one has a similar module parameter or debugfs knob for enabling > >> different types of debug messages. > >> > >> As it stands, the NETIF_* categories don't really align at all with > >> the kinds of message categories most WiFi drivers support. Do you > >> propose adding a bunch of new options to the netif debug feature? > > > > Why not? It make no sense or it is just "it is tradition, we never do > > it!" ? > > > > Even dynamic printk provide even more granularity. So module parameter looks > > like stone age against all existing possibilities. > > I'm all for improving wireless driver debugging features, but let's > please keep that as a separate thread from reviewing new drivers. I > think there are 4-5 new drivers in the queue at the moment so to keep > all this manageable let's have the review process as simple as possible, > please. Ok, there is enough work to do. > Using a module parameter for setting the debug mask is a standard > feature in wireless drivers so it shouldn't block rtw89. If we want a > generic debug framework for wireless drivers, an rfc patch for an > existing upstream wireless driver is a good way to get that discussion > forward. Ok, sounds good. Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
