<igor.mitsyanko.os@xxxxxxxxxxxxx> writes: > From: Avinash Patil <avinashp@xxxxxxxxxxxxx> > > 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 <dlebed@xxxxxxxxxxxxx> > Signed-off-by: Sergei Maksimenko <smaksimenko@xxxxxxxxxxxxx> > Signed-off-by: Sergey Matyukevich <smatyukevich@xxxxxxxxxxxxx> > Signed-off-by: Bindu Therthala <btherthala@xxxxxxxxxxxxx> > Signed-off-by: Huizhao Wang <hwang@xxxxxxxxxxxxx> > Signed-off-by: Kamlesh Rath <krath@xxxxxxxxxxxxx> > Signed-off-by: Avinash Patil <avinashp@xxxxxxxxxxxxx> > Signed-off-by: Igor Mitsyanko <igor.mitsyanko.os@xxxxxxxxxxxxx> 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. > --- /dev/null > +++ b/drivers/net/wireless/quantenna/qtnfmac/pcie.c > @@ -0,0 +1,1374 @@ > +/** > + * Copyright (c) 2015-2016 Quantenna Communications, Inc. > + * All rights reserved. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + **/ > + > +#undef DEBUG Why? -- Kalle Valo