Thread (62 messages) 62 messages, 7 authors, 2021-07-16

Re: [PATCH 04/24] rtw89: add debug files

From: Oleksij Rempel <o.rempel@pengutronix.de>
Date: 2021-07-05 09:23:12
Also in: netdev

On Mon, Jul 05, 2021 at 11:08:07AM +0300, Kalle Valo wrote:
Oleksij Rempel [off-list ref] writes:
quoted
On Fri, Jul 02, 2021 at 10:08:53AM -0700, Brian Norris wrote:
quoted
On Fri, Jul 2, 2021 at 12:23 AM Oleksij Rempel [off-list ref] wrote:
quoted
On Fri, Jun 18, 2021 at 02:46:05PM +0800, Ping-Ke Shih wrote:
quoted
+#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.
quoted
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 |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help