-----Original Message-----
From: Brian Norris [mailto:briannorris@chromium.org]
Sent: Saturday, July 03, 2021 2:38 AM
To: Oleksij Rempel
Cc: Pkshih; Kalle Valo; linux-wireless; <redacted>; Sascha Hauer
Subject: Re: [PATCH 04/24] rtw89: add debug files

On Fri, Jul 2, 2021 at 10:57 AM Oleksij Rempel [off-list ref] wrote:

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

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.

OK! So I see you've touched a lot of ath9k, >3 years ago. You might
notice that it is one such example -- it supports exactly the same
kind of debugfs file, with a set of ath_dbg() log types. Why doesn't
it use this netif debug logging?

quoted

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!" ?

Well mainly, I don't really like people dreaming up arbitrary rules
and enforcing them only on new submissions. If such a change was
Recommended, it seems like a better first step would be to prove that
existing drivers (where there are numerous examples) can be converted
nicely, instead of pushing the work to new contributors arbitrarily.
Otherwise, the bar for new contributions gets exceedingly high -- this
one has already sat for more than 6 months with depressingly little
useful feedback.

I also know very little about this netif log level feature, but if it
really depends on ethtool (seems like it does?) -- I don't even bother
installing ethtool on most of my systems. It's much easier to poke at
debugfs, sysfs, etc., than to construct the relevant ethtool ioctl()s
or netlink messages. It also seems that these debug knobs can't be set
before the driver finishes coming up, so one would still need a module
parameter to mirror some of the same features. Additionally, a WiFi
driver doesn't even have a netdev to speak of until after a lot of the
interesting stuff comes up (much of the mac80211 framework focuses on
a |struct ieee80211_hw| and a |struct wiphy|), so I'm not sure your
suggestion really fits these sorts of drivers (and the things they
might like to support debug-logging for) at all.

Anyway, if Ping-Ke wants to paint this bikeshed for you, I won't stop him.

I encounter the problems you mentioned mostly:
1. no netdev to be the parameter 'dev' of 'netif_dbg(priv, type, dev, format, args...)'
2. predefined 'type' isn't enough for wifi application. There're many wifi- or vendor-
   specific components, such as RF calibration, BT coexistence, DIG, CFO and so on.

So, I don't plan to change them for now.

--
Ping-Ke