Re: [PATCH v2 RESEND] qtnfmac: announcement of new FullMAC driver for Quantenna chipsets

[off-list ref] writes:

From: Avinash Patil <redacted>

This patch adds support for new FullMAC WiFi driver for Quantenna
QSR10G chipsets.

QSR10G is Quantenna's 8x8, 160M, 11ac offering.
QSR10G supports 2 simultaneous WMACs- one 5G and one 2G. 5G WMAC
supports 160M, 8x8 configuration.
FW supports 8 concurrent virtual interfaces on each WMAC.

Patch introduces 2 new drivers- qtnfmac.ko for interfacing with
kernel/cfg80211 and qtnfmac_pcie.ko for PCIe bus interface.

Signed-off-by: Dmitrii Lebed <redacted>
Signed-off-by: Sergei Maksimenko <redacted>
Signed-off-by: Sergey Matyukevich <redacted>
Signed-off-by: Bindu Therthala <redacted>
Signed-off-by: Huizhao Wang <redacted>
Signed-off-by: Kamlesh Rath <redacted>
Signed-off-by: Avinash Patil <redacted>
Signed-off-by: Igor Mitsyanko <redacted>

More comments:

+/* FW names */
+
+#define QTN_PCI_FW_NAME		"pearl-linux.lzma.img"

The firmware name gives no indication what this file is about (remember
that linux-firmware.git has a lot of files). Please name it properly,
don't just use what is used in by firmware build scripts :) Take into
account also future hw support, all firmware files need to coexist in
the same repository without user invention. In a way the firmware
filename is part of kernel/userspace interface and needs to be stable.

For example, you could use something like "qtnfmac/qsr10g.img" (assuming
qsr10g is the name of chip).

I forgot already, is the firmware image ready for submission to
linux-firmware.git?

+	pr_info("%s: %sregistered mgmt frame type 0x%x\n", __func__,
+		reg ? "" : "un", frame_type);

The driver seems to be quite spammy with info messages:

qtnfmac/cfg80211.c:     pr_info("%s: %sregistered mgmt frame type 0x%x\n", __func__,
qtnfmac/cfg80211.c:     pr_info("QTNF: %s cipher=%x, idx=%u, pairwise=%u\n", __func__,
qtnfmac/cfg80211.c:     pr_info("QTNF: %s idx=%u, pairwise=%u\n", __func__, key_index,
qtnfmac/cfg80211.c:     pr_info("QTNF: %s idx=%u, unicast=%u, multicast=%u\n", __func__,
qtnfmac/cfg80211.c:     pr_info("QTNF: %s idx=%u\n", __func__, key_index);
qtnfmac/cfg80211.c:     pr_info("%s: initiator=%d, alpha=%c%c, macid=%d\n", __func__,
qtnfmac/cfg80211.c:     pr_info("%s: MAX_IF: %zu; MODES: %.4X; RADAR WIDTHS: %.2X\n", __func__,
qtnfmac/cfg80211.c:     pr_info("macid=%d, phymode=%#x\n", mac->macid, mac->macinfo.phymode);
qtnfmac/commands.c:                     pr_info("%s: unexpected TLV type: %.4X\n",
qtnfmac/commands.c:                     pr_info("%s: STA %pM not found\n", __func__, sta_mac);
qtnfmac/commands.c:     pr_info("country-code from EP: %c%c\n", hwinfo->country_code[0],
qtnfmac/commands.c:     pr_info("fw_version = %d, num_mac=%d, mac_bitmap=%#x\n",
qtnfmac/commands.c:                     pr_info("iface limit record count=%zu\n", record_count);
qtnfmac/commands.c:                     pr_info("MAC%d reported channels %d\n",
qtnfmac/init.c: pr_info("%s: macid=%d\n", __func__, macid);
qtnfmac/pcie.c:                 pr_info("enabled PCIE MSI interrupt\n");
qtnfmac/pcie.c: pr_info("%s: BAR[%u] vaddr=0x%p busaddr=0x%p len=%u\n",
qtnfmac/pcie.c: pr_info("%s: set mps to %d (was %d, max %d)\n",
qtnfmac/pcie.c: pr_info("fw download started: fw start addr = 0x%p, size=%d\n",
qtnfmac/pcie.c: pr_info("fw download completed: totally sent %d blocks\n", blk);
qtnfmac/pcie.c: pr_info("RC is ready to boot EP...\n");
qtnfmac/pcie.c: pr_info("starting download firmware %s...\n", bus->fwname);
qtnfmac/pcie.c:         pr_info("successful init of PCI device %x\n", pdev->device);
qtnfmac/pcie.c: pr_info("Register Quantenna FullMAC PCIE driver\n");
qtnfmac/pcie.c: pr_info("Unregister Quantenna FullMAC PCIE driver\n");
qtnfmac/trans.c:                        pr_info("%s: interrupted\n", __func__);
qtnfmac/trans.c:        pr_info("%s: skb dropped\n", __func__);

Usualle the preference is that driver is quiet until something goes
wrong. I hope some of these could be debug messages.

Why?

-- 
Kalle Valo