date:20180228

On Wed, Feb 28, 2018 at 11:18:59PM +0200, cantabile wrote:
> On 28/02/18 20:48, Luis R. Rodriguez wrote:
> > On Wed, Feb 28, 2018 at 08:02:59PM +0200, cantabile wrote:
> > > On 27/02/18 22:42, Luis R. Rodriguez wrote:
> > OK so we know that the optimization is optional, not a requirement.
> > That may be worth extending in documentation on the driver.
> 
> I may have spoken too soon. mt7601u kind of exploded after resuming from
> suspend to RAM. Maybe it has nothing to do with this change, but well. The
> system froze after the KDE lock screen appeared, and a few minutes later the
> Caps Lock LED turned on. I attached journalctl output in case it's
> interesting.

Oh well, the optimization is not optional then :P

> Feb 28 22:46:04 home kernel: PM: suspend entry (deep)
> Feb 28 22:46:04 home plasmashell[576]: QXcbClipboard: SelectionRequest too old
> Feb 28 22:46:04 home ksmserver[552]: CreateNotify: 60817442
> Feb 28 22:46:05 home kernel: PM: Syncing filesystems ... done.
> Feb 28 22:46:05 home kernel: firmware_class: device_cache_fw_images
> Feb 28 22:46:05 home kernel: firmware_class: cache_firmware: 
> iwlwifi-3945-2.ucode
> Feb 28 22:46:05 home kernel: firmware_class: __allocate_fw_priv: 
> fw-iwlwifi-3945-2.ucode fw_priv=eb639dd2
> Feb 28 22:46:05 home kernel: firmware_class: cache_firmware: mt7601u.bin
> Feb 28 22:46:05 home kernel: firmware_class: __allocate_fw_priv: 
> fw-mt7601u.bin fw_priv=4f840abe
> Feb 28 22:46:05 home kernel: (NULL device *): loading 
> /lib/firmware/updates/4.16.0-rc3-g4edf7856bed8/iwlwifi-3945-2.ucode failed 
> with error -2
> Feb 28 22:46:19 home kernel: (NULL device *): loading 
> /lib/firmware/updates/iwlwifi-3945-2.ucode failed with error -2
> Feb 28 22:46:19 home kernel: (NULL device *): loading 
> /lib/firmware/updates/4.16.0-rc3-g4edf7856bed8/mt7601u.bin failed with error 
> -2
> Feb 28 22:46:19 home kernel: (NULL device *): loading 
> /lib/firmware/4.16.0-rc3-g4edf7856bed8/iwlwifi-3945-2.ucode failed with error 
> -2
> Feb 28 22:46:19 home kernel: (NULL device *): loading 
> /lib/firmware/updates/mt7601u.bin failed with error -2
> Feb 28 22:46:19 home kernel: (NULL device *): loading 
> /lib/firmware/4.16.0-rc3-g4edf7856bed8/mt7601u.bin failed with error -2
> Feb 28 22:46:19 home kernel: (NULL device *): direct-loading mt7601u.bin
> Feb 28 22:46:19 home kernel: firmware_class: fw_set_page_data: fw-mt7601u.bin 
> fw_priv=4f840abe data=991ae8e7 size=45412
> Feb 28 22:46:19 home kernel: firmware_class: cache_firmware: mt7601u.bin ret=0
> Feb 28 22:46:19 home kernel: (NULL device *): direct-loading 
> iwlwifi-3945-2.ucode
> Feb 28 22:46:19 home kernel: firmware_class: fw_set_page_data: 
> fw-iwlwifi-3945-2.ucode fw_priv=eb639dd2 data=9d96fe1e 
> size=150100
> Feb 28 22:46:19 home kernel: firmware_class: cache_firmware: 
> iwlwifi-3945-2.ucode ret=0
> Feb 28 22:46:19 home kernel: Freezing user space processes ... (elapsed 0.002 
> seconds) done.
> Feb 28 22:46:19 home kernel: OOM killer disabled.
> Feb 28 22:46:19 home kernel: Freezing remaining freezable tasks ... (elapsed 
> 0.001 seconds) done.
> Feb 28 22:46:19 home kernel: Suspending console(s) (use no_console_suspend to 
> debug)
> Feb 28 22:46:19 home kernel: sd 0:0:0:0: [sda] Synchronizing SCSI cache
> Feb 28 22:46:19 home kernel: sd 0:0:0:0: [sda] Stopping disk
> Feb 28 22:46:19 home kernel: e1000e: EEE TX LPI TIMER: 
> Feb 28 22:46:19 home kernel: ACPI: EC: interrupt blocked
> Feb 28 22:46:19 home kernel: ACPI: Preparing to enter system sleep state S3
> Feb 28 22:46:19 home kernel: ACPI: EC: event blocked
> Feb 28 22:46:19 home kernel: ACPI: EC: EC stopped
> Feb 28 22:46:19 home kernel: PM: Saving platform NVS memory
> Feb 28 22:46:19 home kernel: Disabling non-boot CPUs ...
> Feb 28 22:46:19 home kernel: smpboot: CPU 1 is now offline
> Feb 28 22:46:19 home kernel: ACPI: Low-level resume complete
> Feb 28 22:46:19 home kernel: ACPI: EC: EC started

Ok so suspend..


> Feb 28 22:46:19 home kernel: PM: Restoring platform NVS memory
> Feb 28 22:46:19 home kernel: Enabling non-boot CPUs ...
> Feb 28 22:46:19 home kernel: x86: Booting SMP configuration:
> Feb 28 22:46:19 home kernel: smpboot: Booting Node 0 Processor 1 APIC 0x1
> Feb 28 22:46:19 home kernel:  cache: parent cpu1 should not be sleeping
> Feb 28 22:46:19 home kernel: microcode: sig=0x6fd, pf=0x80, revision=0xa3
> Feb 28 22:46:19 home kernel: microcode: updated to revision 0xa4, date = 
> 2010-10-02
> Feb 28 22:46:19 home kernel: CPU1 is up
> Feb 28 22:46:19 home kernel: ACPI: Waking up from system sleep state S3
> Feb 28 22:46:19 home kernel: ACPI: EC: interrupt unblocked
> Feb 28 22:46:19 home kernel: ACPI: EC: event unblocked
> Feb 28 22:46:19 home kernel: ACPI: button: The lid device is not compliant to 
> SW_LID.
> Feb 28 22:46:19 home kernel: usb usb3: root hub lost power or was reset
> Feb 28 22:46:19 home kernel: usb usb4: root hub lost power or was reset
> Feb 28 22:46:19 home kernel: usb

[PATCH] mt76x2: remove unnecessary len variable in mt76x2_eeprom_load()

Substitute unnecessary len variable in mt76x2_eeprom_load() with
MT7662_EEPROM_SIZE macro since len is used just to store eeprom
default size.

Signed-off-by: Lorenzo Bianconi 
---
 drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.c 
b/drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.c
index 9c9bf3e785ba..5bb50027c1e8 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2_eeprom.c
@@ -222,11 +222,10 @@ static int
 mt76x2_eeprom_load(struct mt76x2_dev *dev)
 {
void *efuse;
-   int len = MT7662_EEPROM_SIZE;
bool found;
int ret;
 
-   ret = mt76_eeprom_init(>mt76, len);
+   ret = mt76_eeprom_init(>mt76, MT7662_EEPROM_SIZE);
if (ret < 0)
return ret;
 
@@ -234,14 +233,15 @@ mt76x2_eeprom_load(struct mt76x2_dev *dev)
if (found)
found = !mt76x2_check_eeprom(dev);
 
-   dev->mt76.otp.data = devm_kzalloc(dev->mt76.dev, len, GFP_KERNEL);
-   dev->mt76.otp.size = len;
+   dev->mt76.otp.data = devm_kzalloc(dev->mt76.dev, MT7662_EEPROM_SIZE,
+ GFP_KERNEL);
+   dev->mt76.otp.size = MT7662_EEPROM_SIZE;
if (!dev->mt76.otp.data)
return -ENOMEM;
 
efuse = dev->mt76.otp.data;
 
-   if (mt76x2_get_efuse_data(dev, efuse, len))
+   if (mt76x2_get_efuse_data(dev, efuse, MT7662_EEPROM_SIZE))
goto out;
 
if (found) {
@@ -249,7 +249,7 @@ mt76x2_eeprom_load(struct mt76x2_dev *dev)
} else {
/* FIXME: check if efuse data is complete */
found = true;
-   memcpy(dev->mt76.eeprom.data, efuse, len);
+   memcpy(dev->mt76.eeprom.data, efuse, MT7662_EEPROM_SIZE);
}
 
 out:
-- 
2.16.2

Re: [RFT 0/7] firmware: enable caching of firmware for reboot optimization


On 28/02/18 20:45, Luis R. Rodriguez wrote:

On Wed, Feb 28, 2018 at 08:03:26PM +0200, cantabile wrote:

On 28/02/18 01:20, Luis R. Rodriguez wrote:

Cantabile, please give these patches a spin and let me know if it fixes
your reported issue. They depend on other pending patches I have in line
waiting to be merged so the easiest I thing I think is for you to test my
20180227-firmware-cache branch [0], based on Linus' tree. To get that
tree, cd into your Linus git tree and do:

git remote add mcgrof 
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
git checkout -b 20180227-firmware-cache mcgrof/20180227-firmware-cache

Please let me know if this resolves your issue and thanks for your report.



I confirmed that request_firmware_cache is called, and the firmware is
loaded from the cache when resuming from hibernation. The bug is fixed.


Great, thanks for testing.

Curious, is it also called if you suspend to RAM instead of just using 
hibernation?



request_firmware_cache is called when resuming from suspend to RAM.

Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation


On 28/02/18 20:48, Luis R. Rodriguez wrote:

On Wed, Feb 28, 2018 at 08:02:59PM +0200, cantabile wrote:

On 27/02/18 22:42, Luis R. Rodriguez wrote:

I'd be curious if someone who can trigger the situation can test what
happens if you use:

diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c 
b/drivers/net/wireless/mediatek/mt7601u/mcu.c
index 65a8004418ea..04cbffd225a1 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
@@ -421,7 +421,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev *dev)
 MT_USB_DMA_CFG_TX_BULK_EN));
if (firmware_running(dev))
-   return 0;
+   pr_info("Firmware already loaded but going to reload...");
ret = request_firmware(, MT7601U_FIRMWARE, dev->dev);
if (ret)


Curious, will it really fail?


This change brings no new messages from mt7601u in dmesg (other than this
pr_info), and the device works fine, as far as I can tell.


OK so we know that the optimization is optional, not a requirement.
That may be worth extending in documentation on the driver.


I may have spoken too soon. mt7601u kind of exploded after resuming from 
suspend to RAM. Maybe it has nothing to do with this change, but well. 
The system froze after the KDE lock screen appeared, and a few minutes 
later the Caps Lock LED turned on. I attached journalctl output in case 
it's interesting.





Note that I see mt7601u_stop() just calls mt7601u_mac_stop(). The big cleanup
happens via mt7601u_cleanup(), but I see mt7601u_disconnect() calls it.

Just curious, does that not get called on shutdown?

diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c 
b/drivers/net/wireless/mediatek/mt7601u/usb.c
index b9e4f6793138..126ef2ba77c2 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.c
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
@@ -311,6 +311,7 @@ static void mt7601u_disconnect(struct usb_interface 
*usb_intf)
   {
struct mt7601u_dev *dev = usb_get_intfdata(usb_intf);
+   pr_info("Calling mt7601u_disconnect()...");
ieee80211_unregister_hw(dev->hw);
mt7601u_cleanup(dev);

If it does, one option is mt7601u_cleanup() can use some love to really shut 
down
the device more... But its not clear to me what else could be done and I'm very
inclined to believe its not sensible.


"Calling mt7601u_disconnect" does not appear in journalctl after a reboot.


Oh, I didn't expect it to come up during startup, I was wondering if it did
trigger while going down on reboot.



I meant that it doesn't appear among the messages from the shutdown part 
of rebooting. (I can only check after rebooting.)
Feb 28 22:46:03 home org_kde_powerdevil[593]: powerdevil: Suspend session 
triggered with QMap(("Explicit", QVariant(bool, true))("Type", QVariant(uint, 
1)))
Feb 28 22:46:03 home org_kde_powerdevil[593]: powerdevil: Suspend session 
triggered with QMap(("Explicit", QVariant(bool, true))("SkipFade", 
QVariant(bool, true))("Type", QVariant(uint, 1)))
Feb 28 22:46:03 home org_kde_powerdevil[593]: powerdevil: Starting Login1 
suspend job
Feb 28 22:46:03 home org_kde_powerdevil[593]: powerdevil: Pausing all media 
players before suspending
Feb 28 22:46:03 home NetworkManager[363]:   [1519850763.3467] manager: 
sleep: sleep requested (sleeping: no  enabled: yes)
Feb 28 22:46:03 home NetworkManager[363]:   [1519850763.3477] device 
(wlp16s0): state change: disconnected -> unmanaged (reason 'sleeping', 
sys-iface-state: 'managed')
Feb 28 22:46:03 home ksmserver[552]: lock called
Feb 28 22:46:03 home NetworkManager[363]:   [1519850763.3537] device 
(wlp16s0): set-hw-addr: reset MAC address to 00:1F:3C:63:D2:8C (unmanage)
Feb 28 22:46:03 home kdeinit5[532]: bluedevil: About to suspend
Feb 28 22:46:03 home ksmserver[552]: Lock window Id:  16777228
Feb 28 22:46:03 home wpa_supplicant[395]: nl80211: deinit ifname=wlp16s0 
disabled_11b_rates=0
Feb 28 22:46:03 home ksmserver[552]: CreateNotify: 16777228
Feb 28 22:46:03 home NetworkManager[363]:   [1519850763.4256] manager: 
NetworkManager state is now ASLEEP
Feb 28 22:46:03 home NetworkManager[363]:   [1519850763.4323] device 
(wlp0s29f7u3): state change: activated -> deactivating (reason 'sleeping', 
sys-iface-state: 'managed')
Feb 28 22:46:03 home dbus-daemon[349]: [system] Activating via systemd: service 
name='org.freedesktop.nm_dispatcher' 
unit='dbus-org.freedesktop.nm-dispatcher.service' requested by ':1.2' (uid=0 
pid=363 comm="/usr/bin/NetworkManager --no-daemon ")
Feb 28 22:46:03 home kdeinit5[532]: plasma-nm: Unhandled active connection 
state change:  3
Feb 28 22:46:03 home NetworkManager[363]:   [1519850763.4588] device 
(wlp0s29f7u3): state change: deactivating -> disconnected (reason 'sleeping', 
sys-iface-state: 'managed')
Feb 28 22:46:03 home systemd[1]: Starting Network Manager Script Dispatcher 
Service...
Feb 28 22:46:03 home kernel: wlp0s29f7u3: deauthenticating from 
94:44:52:3b:82:eb by local

Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation

On Wed, Feb 28, 2018 at 08:18:33PM +0100, Arend van Spriel wrote:
> On 2/28/2018 7:48 PM, Luis R. Rodriguez wrote:
> > On Wed, Feb 28, 2018 at 08:02:59PM +0200, cantabile wrote:
> > > On 27/02/18 22:42, Luis R. Rodriguez wrote:
> > > > I'd be curious if someone who can trigger the situation can test what
> > > > happens if you use:
> > > > 
> > > > diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c 
> > > > b/drivers/net/wireless/mediatek/mt7601u/mcu.c
> > > > index 65a8004418ea..04cbffd225a1 100644
> > > > --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
> > > > +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
> > > > @@ -421,7 +421,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev 
> > > > *dev)
> > > >  MT_USB_DMA_CFG_TX_BULK_EN));
> > > > if (firmware_running(dev))
> > > > -   return 0;
> > > > +   pr_info("Firmware already loaded but going to 
> > > > reload...");
> > > > ret = request_firmware(, MT7601U_FIRMWARE, dev->dev);
> > > > if (ret)
> > > > 
> > > > 
> > > > Curious, will it really fail?
> > > 
> > > This change brings no new messages from mt7601u in dmesg (other than this
> > > pr_info), and the device works fine, as far as I can tell.
> > 
> > OK so we know that the optimization is optional, not a requirement.
> > That may be worth extending in documentation on the driver.
> > 
> > > > Note that I see mt7601u_stop() just calls mt7601u_mac_stop(). The big 
> > > > cleanup
> > > > happens via mt7601u_cleanup(), but I see mt7601u_disconnect() calls it.
> > > > 
> > > > Just curious, does that not get called on shutdown?
> > > > 
> > > > diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c 
> > > > b/drivers/net/wireless/mediatek/mt7601u/usb.c
> > > > index b9e4f6793138..126ef2ba77c2 100644
> > > > --- a/drivers/net/wireless/mediatek/mt7601u/usb.c
> > > > +++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
> > > > @@ -311,6 +311,7 @@ static void mt7601u_disconnect(struct usb_interface 
> > > > *usb_intf)
> > > >{
> > > > struct mt7601u_dev *dev = usb_get_intfdata(usb_intf);
> > > > +   pr_info("Calling mt7601u_disconnect()...");
> > > > ieee80211_unregister_hw(dev->hw);
> > > > mt7601u_cleanup(dev);
> > > > 
> > > > If it does, one option is mt7601u_cleanup() can use some love to really 
> > > > shut down
> > > > the device more... But its not clear to me what else could be done and 
> > > > I'm very
> > > > inclined to believe its not sensible.
> > > > 
> > > "Calling mt7601u_disconnect" does not appear in journalctl after a reboot.
> > 
> > Oh, I didn't expect it to come up during startup, I was wondering if it did
> > trigger while going down on reboot.
> 
> Hi Luis,
> This driver does not implement a .shutdown() callback so that might be one
> reason and the usb subsystem does not call the disconnect for whatever
> reason.

Ah, that's one missing callback then. Hrm, well I suppose its optional and
without it, its just an optimization given there is no issue in keeping the
power on on the device and it retaining the firmware upon boot.

Note that the on interface stop all the DMA and MAC is stopped so I suppose
that is sufficient from an 802.11 perspective.

Thanks for chiming in!

  Luis

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

[PATCH for-4.16 V3 2/2] brcmfmac: fix P2P_DEVICE ethernet address generation

The firmware has a requirement that the P2P_DEVICE address should
be different from the address of the primary interface. When not
specified by user-space, the driver generates the MAC address for
the P2P_DEVICE interface using the MAC address of the primary
interface and setting the locally administered bit. However, the MAC
address of the primary interface may already have that bit set causing
the creation of the P2P_DEVICE interface to fail with -EBUSY. Fix this
by using a random address instead to determine the P2P_DEVICE address.

Cc: sta...@vger.kernel.org # 3.10.y
Reported-by: Hans de Goede 
Reviewed-by: Hante Meuleman 
Reviewed-by: Pieter-Paul Giesberts 
Reviewed-by: Franky Lin 
Signed-off-by: Arend van Spriel 
---
 V3:
  - use eth_random_addr() suggested by Hans.
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 24 ++
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 2ee5413..82064e9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -462,25 +462,23 @@ static int brcmf_p2p_set_firmware(struct brcmf_if *ifp, 
u8 *p2p_mac)
  * @dev_addr: optional device address.
  *
  * P2P needs mac addresses for P2P device and interface. If no device
- * address it specified, these are derived from the primary net device, ie.
- * the permanent ethernet address of the device.
+ * address it specified, these are derived from a random ethernet
+ * address.
  */
 static void brcmf_p2p_generate_bss_mac(struct brcmf_p2p_info *p2p, u8 
*dev_addr)
 {
-   struct brcmf_if *pri_ifp = p2p->bss_idx[P2PAPI_BSSCFG_PRIMARY].vif->ifp;
-   bool local_admin = false;
+   bool random_addr = false;
 
-   if (!dev_addr || is_zero_ether_addr(dev_addr)) {
-   dev_addr = pri_ifp->mac_addr;
-   local_admin = true;
-   }
+   if (!dev_addr || is_zero_ether_addr(dev_addr))
+   random_addr = true;
 
-   /* Generate the P2P Device Address.  This consists of the device's
-* primary MAC address with the locally administered bit set.
+   /* Generate the P2P Device Address obtaining a random ethernet
+* address with the locally administered bit set.
 */
-   memcpy(p2p->dev_addr, dev_addr, ETH_ALEN);
-   if (local_admin)
-   p2p->dev_addr[0] |= 0x02;
+   if (random_addr)
+   eth_random_addr(p2p->dev_addr);
+   else
+   memcpy(p2p->dev_addr, dev_addr, ETH_ALEN);
 
/* Generate the P2P Interface Address.  If the discovery and connection
 * BSSCFGs need to simultaneously co-exist, then this address must be
-- 
1.9.1

[PATCH for-4.16 V3 1/2] brcmfmac: add possibility to obtain firmware error

The feature module needs to evaluate the actual firmware error return
upon a control command. This adds a flag to struct brcmf_if that the
caller can set. This flag is checked to determine the error code that
needs to be returned.

Fixes: b69c1df47281 ("brcmfmac: separate firmware errors from i/o errors")
Reviewed-by: Hante Meuleman 
Reviewed-by: Pieter-Paul Giesberts 
Reviewed-by: Franky Lin 
Signed-off-by: Arend van Spriel 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h|  2 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 10 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c|  3 +++
 3 files changed, 15 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index df8a1ec..232dcbb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -181,6 +181,7 @@ enum brcmf_netif_stop_reason {
  * @netif_stop_lock: spinlock for update netif_stop from multiple sources.
  * @pend_8021x_cnt: tracks outstanding number of 802.1x frames.
  * @pend_8021x_wait: used for signalling change in count.
+ * @fwil_fwerr: flag indicating fwil layer should return firmware error codes.
  */
 struct brcmf_if {
struct brcmf_pub *drvr;
@@ -198,6 +199,7 @@ struct brcmf_if {
wait_queue_head_t pend_8021x_wait;
struct in6_addr ipv6_addr_tbl[NDOL_MAX_ENTRIES];
u8 ipv6addr_idx;
+   bool fwil_fwerr;
 };
 
 int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 47de35a..bede7b7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -104,6 +104,9 @@ static void brcmf_feat_iovar_int_get(struct brcmf_if *ifp,
u32 data;
int err;
 
+   /* we need to know firmware error */
+   ifp->fwil_fwerr = true;
+
err = brcmf_fil_iovar_int_get(ifp, name, );
if (err == 0) {
brcmf_dbg(INFO, "enabling feature: %s\n", brcmf_feat_names[id]);
@@ -112,6 +115,8 @@ static void brcmf_feat_iovar_int_get(struct brcmf_if *ifp,
brcmf_dbg(TRACE, "%s feature check failed: %d\n",
  brcmf_feat_names[id], err);
}
+
+   ifp->fwil_fwerr = false;
 }
 
 static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
@@ -120,6 +125,9 @@ static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
 {
int err;
 
+   /* we need to know firmware error */
+   ifp->fwil_fwerr = true;
+
err = brcmf_fil_iovar_data_set(ifp, name, data, len);
if (err != -BRCMF_FW_UNSUPPORTED) {
brcmf_dbg(INFO, "enabling feature: %s\n", brcmf_feat_names[id]);
@@ -128,6 +136,8 @@ static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
brcmf_dbg(TRACE, "%s feature check failed: %d\n",
  brcmf_feat_names[id], err);
}
+
+   ifp->fwil_fwerr = false;
 }
 
 #define MAX_CAPS_BUFFER_SIZE   512
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
index f2cfdd3..fc57511 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
@@ -131,6 +131,9 @@ static const char *brcmf_fil_get_errstr(u32 err)
  brcmf_fil_get_errstr((u32)(-fwerr)), fwerr);
err = -EBADE;
}
+   if (ifp->fwil_fwerr)
+   return fwerr;
+
return err;
 }
 
-- 
1.9.1

[PATCH for-4.16 V3 0/2] brcmfmac: small fixes

Here two small fixes. The first patch fixes a regression introduced in v4.16
and the second fix is result from this discussion [1].

So these patches are intended 4.16 and apply to the master branch of the
wireless-drivers repository.

[1] https://www.spinics.net/lists/linux-wireless/thrd12.html#170494

Arend van Spriel (2):
  brcmfmac: add possibility to obtain firmware error
  brcmfmac: fix P2P_DEVICE ethernet address generation

 .../wireless/broadcom/brcm80211/brcmfmac/core.h|  2 ++
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 10 +
 .../wireless/broadcom/brcm80211/brcmfmac/fwil.c|  3 +++
 .../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 24 ++
 4 files changed, 26 insertions(+), 13 deletions(-)

-- 
1.9.1

Re: [PATCH] mt7601u: remove a warning in mt7601u_efuse_physical_size_check()

2018-02-28 Thread Jakub Kicinski

On Wed, 28 Feb 2018 15:26:57 +0100, Lorenzo Bianconi wrote:
> Fix the following sparse warning in mt7601u_efuse_physical_size_check:
> - drivers/net/wireless/mediatek/mt7601u/eeprom.c:77:27: warning:
>   Variable length array is used
> 
> Signed-off-by: Lorenzo Bianconi 

Acked-by: Jakub Kicinski 

Thanks Lorenzo!

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time


On 2/28/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 27-02-18 00:04, Arend van Spriel wrote:

On 2/26/2018 12:22 PM, Hans de Goede wrote:

Hi,

On 26-02-18 12:01, Arend van Spriel wrote:

On 2/26/2018 11:29 AM, Hans de Goede wrote:

Hi,

On 26-02-18 11:22, Arend van Spriel wrote:

On 2/25/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede 


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:


Yup. Before we were passing firmware errors up to user-space, which
was confusing and potentially be misinterpreted. However, looking at
the output below it would have been good to log the firmware error as
well. And staring at it some more I suddenly realize I broke the
feature detection module with this change. Actually only the GSCAN
feature detection.


[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( -> 0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version =
wl0:
Jun  4 2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface
p2p-dev-wlp1s0 type 10 failed: err=-52

[root@localhost ~]# strings
/lib/firmware/brcm/brcmfmac4356-pcie.bin |
tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon



Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04
16:51:38 PDT Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?


I had a look at it and it seems to be a difference in firmware api
that we need to support in the driver. Need to do a bit more digging,
but it seems an actual issue. You could silence it for now, but I
prefer to wait for the fix.


Ok, what is the ETA of a fix for this?


Actually went back to an old log you sent and noticed:

[   15.714569] brcmfmac: brcmf_attach Enter
[   15.714756] brcmfmac: brcmf_fweh_register event handler registered
for PSM_WATCHDOG
[   15.714757] brcmfmac: brcmf_proto_attach Enter
[   15.716598] brcmfmac: brcmf_bus_started
[   15.716603] brcmfmac: brcmf_add_if Enter, bsscfgidx=0, ifidx=0
[   15.716604] brcmfmac: brcmf_add_if allocate netdev interface
[   15.716622] brcmfmac: brcmf_add_if   pid:2a, if:wlan%d
(00:00:00:00:00:00) created ===
[   15.716624] brcmfmac: brcmf_bus_change_state 0 -> 1
[   15.717841] brcmfmac: brcmf_fil_iovar_data_get ifidx=0,
name=cur_etheraddr, len=6
[   15.717843] brcmutil: data
[   15.717847] : 44 2c 05 9e c9 02   D,

So mac address of the device is 44:2c:05:9e:c9. However, further down
I see:

[   17.819113] brcmfmac: brcmf_netdev_set_mac_address Enter,
bsscfgidx=0
[   17.819122] brcmfmac: brcmf_fil_iovar_data_set ifidx=0,
name=cur_etheraddr, len=6
[   17.819127] brcmutil: data
[   17.819135] : aa 3e 81 77 bc 40   .>.w.@
[   17.819864] brcmfmac: brcmf_netdev_set_mac_address updated to
aa:3e:81:77:bc:40

So the mac address in a local admin variant.


Right, this is likely NetworkManager randomizing the mac for privacy
reasons.


Now our firmware has a requirement for the p2p-dev interface that it
should be different from the mac address of the primary interface, ie.
wlp1s0 in this log. In brcmfmac we try to do that by setting the local
admin bit, but... as it is already set we end up using the same mac
address hence the -EBUSY.


Ah, that is good to know, so how can we fix this? Can userspace
specify a
different mac-address when it asks for the p2p-dev intf to be
created? Or
should we do something about this in the kernel?


So this is the patch I tested. Maybe you can verify it works for you
as well.


I can confirm that this fixes the errors, but I do not think that this
is a good
solution, using the permanent mac address for the p2p interface has the
same
privacy problem as using it regularly. IMHO it would be best to just
generate
a random mac address if none is specified. This is what the kernel seems
to do
in general when it needs a mac address and none is specified.

Re: fixed bit rate 9Mbps set as 24Mbps in g mode(legacy)

2018-02-28 Thread Ben Greear

On 02/27/2018 08:03 PM, KAVITA MATHUR wrote:

On Tue, 27 Feb 2018 08:41:50 -0800, Ben Greear wrote

On 02/27/2018 12:49 AM, KAVITA MATHUR wrote:

Hi,

I have configured AP in g mode and tested legacy rates.All basic rates and
supported
rates(1,2,5.5,11,6,9,12,18,24,36,48,54) are set except 9Mbps.

When fixed bit rate 9Mbps is set, it gets set as 24 Mbps. This issue has been
observed
in backports-4.4.2-1 as well as latest git version of ath10k driver.

Is anybody aware of this issue? How can I fix this rate 9Mbps?

You should specify which firmware/chipset you are using, since this might
easily be a firmware issue.

Chipset : QCA988x
ath10k version : git 16 oct 2017(v4.14-rc2-1-24-gc6db471)
firmware version : firmware-5.bin_10.2.4.70.66
Kernel version : 3.12

And, how are you determining that 'it gets set to 24Mbps'. Is that the
encoding you see in a wifi sniff, or is the driver being set to 24Mbps
improperly when you use iw (or whatever) to try configuring the rate?

I have used command "iw wlan0 station dump" to see set data rate as well as I
have
sniffer. Measured data throughput is also as per 24Mbps.

I use more recent (and hacked) kernels/drivers, and my own wave-1 firmware. I
did have
a bug in some code I wrote because the 48Mbps rate code is '0x0', and some of
my code was ignoring it thinking it was not properly set. I did see correct
on-air 48Mbps encoding rates though.

Possibly you have similar problems with rate-code 0x0.

Have you tried doing a capture with an ath9k or some other non-ath10k driver
to see if it is really 24Mbps on air?

Recent upstream mac80211 has broken the ability to set a single tx rate as
well, last I checked.

Other rates are set properly except 9Mbps.By default it sets as 6Mbps, when data
transfer starts, then it set to the mentioned fixed data rate.

Setting the tx rate through normal API will not effect management frames, and
at the beginning of
the connection, perhaps no data frames have been sent/received yet, so you would
see 6Mbps reported.

Thanks,
Ben

Thanks & Regards,
Kavita

Thanks,
Ben

Thanks & Regards,
Kavita

--
To unsubscribe from this list: send the line "unsubscribe backports" in

--
Ben Greear
Candela Technologies Inc http://www.candelatech.com

Thanks & Regards,
कविता माथुर Kavita Mathur
वरिष्ठ अनुसंधान अभियंता Senior Research Engineer
सी-डॉट C-DOT
इलैक्ट्रॉनिक्स सिटी फेज़ I Electronics City Phase I
होसूर रोड, बेंगलूरु Hosur Road, Bengaluru – 560100
फोन Ph 080-28529896
Disclaimer:
--
This email and any files transmitted with it

--
Ben Greear
Candela Technologies Inc http://www.candelatech.com

Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation


On 2/28/2018 7:48 PM, Luis R. Rodriguez wrote:

On Wed, Feb 28, 2018 at 08:02:59PM +0200, cantabile wrote:

On 27/02/18 22:42, Luis R. Rodriguez wrote:

I'd be curious if someone who can trigger the situation can test what
happens if you use:

diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c 
b/drivers/net/wireless/mediatek/mt7601u/mcu.c
index 65a8004418ea..04cbffd225a1 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
@@ -421,7 +421,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev *dev)
 MT_USB_DMA_CFG_TX_BULK_EN));
if (firmware_running(dev))
-   return 0;
+   pr_info("Firmware already loaded but going to reload...");
ret = request_firmware(, MT7601U_FIRMWARE, dev->dev);
if (ret)


Curious, will it really fail?


This change brings no new messages from mt7601u in dmesg (other than this
pr_info), and the device works fine, as far as I can tell.


OK so we know that the optimization is optional, not a requirement.
That may be worth extending in documentation on the driver.


Note that I see mt7601u_stop() just calls mt7601u_mac_stop(). The big cleanup
happens via mt7601u_cleanup(), but I see mt7601u_disconnect() calls it.

Just curious, does that not get called on shutdown?

diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c 
b/drivers/net/wireless/mediatek/mt7601u/usb.c
index b9e4f6793138..126ef2ba77c2 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.c
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
@@ -311,6 +311,7 @@ static void mt7601u_disconnect(struct usb_interface 
*usb_intf)
   {
struct mt7601u_dev *dev = usb_get_intfdata(usb_intf);
+   pr_info("Calling mt7601u_disconnect()...");
ieee80211_unregister_hw(dev->hw);
mt7601u_cleanup(dev);

If it does, one option is mt7601u_cleanup() can use some love to really shut 
down
the device more... But its not clear to me what else could be done and I'm very
inclined to believe its not sensible.


"Calling mt7601u_disconnect" does not appear in journalctl after a reboot.


Oh, I didn't expect it to come up during startup, I was wondering if it did
trigger while going down on reboot.


Hi Luis,
This driver does not implement a .shutdown() callback so that might be 
one reason and the usb subsystem does not call the disconnect for 
whatever reason.


Regards,
Arend

[PATCH] ath10k: Attempt to work around napi_synchronize hang.

2018-02-28 Thread greearb

From: Ben Greear 

Calling napi_disable twice in a row (w/out starting it and/or without
having NAPI active leads to deadlock because napi_disable sets
NAPI_STATE_SCHED and NAPI_STATE_NPSVC when it returns, as far as I
can tell.  So, guard this call to napi_disable.  I believe the
failure case is something like this:
 rmmod ath10k_pci ath10k_core
   Firmware crashes before hif_stop is called by the rmmod path
   The crash handling logic calls hif_stop
   Then rmmod gets around to calling hif_stop, but spins endlessly
   in napi_synchronize.

I think one way this could happen is that ath10k_stop checks
for state != ATH10K_STATE_OFF, but STATE_RESTARTING is also
a possibility.  That might be how we can have hif_stop called twice
without a hif_start in between. --Ben

Signed-off-by: Ben Greear 
---
* since RFC:  Added similar code to ahb
  This seems needed back to at least 4.9 kernels.

 drivers/net/wireless/ath/ath10k/ahb.c  |  9 +++--
 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/pci.c  | 25 +++--
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ahb.c 
b/drivers/net/wireless/ath/ath10k/ahb.c
index da770af..f826c59 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -641,6 +641,8 @@ static int ath10k_ahb_hif_start(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot ahb hif start\n");
 
napi_enable(>napi);
+   ar->napi_enabled = true;
+
ath10k_ce_enable_interrupts(ar);
ath10k_pci_enable_legacy_irq(ar);
 
@@ -660,8 +662,11 @@ static void ath10k_ahb_hif_stop(struct ath10k *ar)
 
ath10k_pci_flush(ar);
 
-   napi_synchronize(>napi);
-   napi_disable(>napi);
+   if (ar->napi_enabled) {
+   napi_synchronize(>napi);
+   napi_disable(>napi);
+   ar->napi_enabled = false;
+   }
 }
 
 static int ath10k_ahb_hif_power_up(struct ath10k *ar)
diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 72b4495..c7ba49f 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1205,6 +1205,7 @@ struct ath10k {
/* NAPI */
struct net_device napi_dev;
struct napi_struct napi;
+   bool napi_enabled;
 
struct work_struct stop_scan_work;
 
diff --git a/drivers/net/wireless/ath/ath10k/pci.c 
b/drivers/net/wireless/ath/ath10k/pci.c
index 398e413..9131e15 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1956,6 +1956,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");
 
napi_enable(>napi);
+   ar->napi_enabled = true;
 
ath10k_pci_irq_enable(ar);
ath10k_pci_rx_post(ar);
@@ -2086,8 +2087,28 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
ath10k_pci_irq_disable(ar);
ath10k_pci_irq_sync(ar);
ath10k_pci_flush(ar);
-   napi_synchronize(>napi);
-   napi_disable(>napi);
+
+   /* Calling napi_disable twice in a row (w/out starting it and/or without
+* having NAPI active leads to deadlock because napi_disable sets
+* NAPI_STATE_SCHED and NAPI_STATE_NPSVC when it returns, as far as I
+* can tell.  So, guard this call to napi_disable.  I believe the
+* failure case is something like this:
+* rmmod ath10k_pci ath10k_core
+*   Firmware crashes before hif_stop is called by the rmmod path
+*   The crash handling logic calls hif_stop
+ *   Then rmmod gets around to calling hif_stop, but spins endlessly
+*   in napi_synchronize.
+*
+*  I think one way this could happen is that ath10k_stop checks
+*  for state != ATH10K_STATE_OFF, but STATE_RESTARTING is also
+*  a possibility.  That might be how we can have hif_stop called twice
+*  without a hif_start in between. --Ben
+*/
+   if (ar->napi_enabled) {
+   napi_synchronize(>napi);
+   napi_disable(>napi);
+   ar->napi_enabled = false;
+   }
 
spin_lock_irqsave(_pci->ps_lock, flags);
WARN_ON(ar_pci->ps_wake_refcount > 0);
-- 
2.4.11

Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation

On Wed, Feb 28, 2018 at 08:02:59PM +0200, cantabile wrote:
> On 27/02/18 22:42, Luis R. Rodriguez wrote:
> > I'd be curious if someone who can trigger the situation can test what
> > happens if you use:
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c 
> > b/drivers/net/wireless/mediatek/mt7601u/mcu.c
> > index 65a8004418ea..04cbffd225a1 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
> > @@ -421,7 +421,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev 
> > *dev)
> >  MT_USB_DMA_CFG_TX_BULK_EN));
> > if (firmware_running(dev))
> > -   return 0;
> > +   pr_info("Firmware already loaded but going to reload...");
> > ret = request_firmware(, MT7601U_FIRMWARE, dev->dev);
> > if (ret)
> > 
> > 
> > Curious, will it really fail?
> 
> This change brings no new messages from mt7601u in dmesg (other than this
> pr_info), and the device works fine, as far as I can tell.

OK so we know that the optimization is optional, not a requirement.
That may be worth extending in documentation on the driver.

> > Note that I see mt7601u_stop() just calls mt7601u_mac_stop(). The big 
> > cleanup
> > happens via mt7601u_cleanup(), but I see mt7601u_disconnect() calls it.
> > 
> > Just curious, does that not get called on shutdown?
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c 
> > b/drivers/net/wireless/mediatek/mt7601u/usb.c
> > index b9e4f6793138..126ef2ba77c2 100644
> > --- a/drivers/net/wireless/mediatek/mt7601u/usb.c
> > +++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
> > @@ -311,6 +311,7 @@ static void mt7601u_disconnect(struct usb_interface 
> > *usb_intf)
> >   {
> > struct mt7601u_dev *dev = usb_get_intfdata(usb_intf);
> > +   pr_info("Calling mt7601u_disconnect()...");
> > ieee80211_unregister_hw(dev->hw);
> > mt7601u_cleanup(dev);
> > 
> > If it does, one option is mt7601u_cleanup() can use some love to really 
> > shut down
> > the device more... But its not clear to me what else could be done and I'm 
> > very
> > inclined to believe its not sensible.
> > 
> "Calling mt7601u_disconnect" does not appear in journalctl after a reboot.

Oh, I didn't expect it to come up during startup, I was wondering if it did
trigger while going down on reboot.

  Luis

Re: [RFT 0/7] firmware: enable caching of firmware for reboot optimization

On Wed, Feb 28, 2018 at 08:03:26PM +0200, cantabile wrote:
> On 28/02/18 01:20, Luis R. Rodriguez wrote:
> > Cantabile, please give these patches a spin and let me know if it fixes
> > your reported issue. They depend on other pending patches I have in line
> > waiting to be merged so the easiest I thing I think is for you to test my
> > 20180227-firmware-cache branch [0], based on Linus' tree. To get that
> > tree, cd into your Linus git tree and do:
> > 
> > git remote add mcgrof 
> > https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
> > git checkout -b 20180227-firmware-cache mcgrof/20180227-firmware-cache
> > 
> > Please let me know if this resolves your issue and thanks for your report.
> > 
> 
> I confirmed that request_firmware_cache is called, and the firmware is
> loaded from the cache when resuming from hibernation. The bug is fixed.

Great, thanks for testing.

Curious, is it also called if you suspend to RAM instead of just using 
hibernation?

  Luis
> Thank you.
> 

-- 
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg

Re: [RFC] ath10k: Attempt to work around napi_synchronize hang.

2018-02-28 Thread Ben Greear


On 02/28/2018 09:31 AM, Michał Kazior wrote:

On 28 February 2018 at 02:22,   wrote:
[...]

@@ -2086,8 +2087,28 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
ath10k_pci_irq_disable(ar);
ath10k_pci_irq_sync(ar);
ath10k_pci_flush(ar);
-   napi_synchronize(>napi);
-   napi_disable(>napi);
+
+   /* Calling napi_disable twice in a row (w/out starting it and/or without
+* having NAPI active leads to deadlock because napi_disable sets
+* NAPI_STATE_SCHED and NAPI_STATE_NPSVC when it returns, as far as I
+* can tell.  So, guard this call to napi_disable.  I believe the
+* failure case is something like this:
+* rmmod ath10k_pci ath10k_core
+*   Firmware crashes before hif_stop is called by the rmmod path
+*   The crash handling logic calls hif_stop
+ *   Then rmmod gets around to calling hif_stop, but spins endlessly
+*   in napi_synchronize.
+*
+*  I think one way this could happen is that ath10k_stop checks
+*  for state != ATH10K_STATE_OFF, but STATE_RESTARTING is also
+*  a possibility.  That might be how we can have hif_stop called twice
+*  without a hif_start in between. --Ben
+*/
+   if (ar->napi_enabled) {
+   napi_synchronize(>napi);
+   napi_disable(>napi);
+   ar->napi_enabled = false;
+   }


Looking at the code and your comment the described fail case seems
legit. I would consider tuning ath10k_stop() so that it calls
ath10k_halt() only if ar->state != OFF & ar->state != RESTARTING
though. Or did you try already?


I did not try tuning ath10k_stop().  The code in this area is quite complex,
and in my opinion trying to keep the start/stop calls exactly matched without
individual 'has_started' flags seems ripe for bugs.


While your approach will probably work it won't prevent other non-NAPI
bad things from happening. Even if there's nothing today it might
creep up in the future. And you'd need to update ahb.c too.


I'll update ahb.c to match.

Thanks,
Ben




Michał




--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com

Re: [RFT 0/7] firmware: enable caching of firmware for reboot optimization


On 28/02/18 01:20, Luis R. Rodriguez wrote:

Cantabile, please give these patches a spin and let me know if it fixes
your reported issue. They depend on other pending patches I have in line
waiting to be merged so the easiest I thing I think is for you to test my
20180227-firmware-cache branch [0], based on Linus' tree. To get that
tree, cd into your Linus git tree and do:

git remote add mcgrof 
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git
git checkout -b 20180227-firmware-cache mcgrof/20180227-firmware-cache

Please let me know if this resolves your issue and thanks for your report.



I confirmed that request_firmware_cache is called, and the firmware is 
loaded from the cache when resuming from hibernation. The bug is fixed. 
Thank you.

Re: [PATCH] mt7601u: Fix system freeze after resuming from hibernation


On 27/02/18 22:42, Luis R. Rodriguez wrote:

I'd be curious if someone who can trigger the situation can test what
happens if you use:

diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c 
b/drivers/net/wireless/mediatek/mt7601u/mcu.c
index 65a8004418ea..04cbffd225a1 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
@@ -421,7 +421,7 @@ static int mt7601u_load_firmware(struct mt7601u_dev *dev)
 MT_USB_DMA_CFG_TX_BULK_EN));
  
  	if (firmware_running(dev))

-   return 0;
+   pr_info("Firmware already loaded but going to reload...");
  
  	ret = request_firmware(, MT7601U_FIRMWARE, dev->dev);

if (ret)
  



Curious, will it really fail?


This change brings no new messages from mt7601u in dmesg (other than 
this pr_info), and the device works fine, as far as I can tell.




Note that I see mt7601u_stop() just calls mt7601u_mac_stop(). The big cleanup
happens via mt7601u_cleanup(), but I see mt7601u_disconnect() calls it.

Just curious, does that not get called on shutdown?

diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c 
b/drivers/net/wireless/mediatek/mt7601u/usb.c
index b9e4f6793138..126ef2ba77c2 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.c
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
@@ -311,6 +311,7 @@ static void mt7601u_disconnect(struct usb_interface 
*usb_intf)
  {
struct mt7601u_dev *dev = usb_get_intfdata(usb_intf);
  
+	pr_info("Calling mt7601u_disconnect()...");

ieee80211_unregister_hw(dev->hw);
mt7601u_cleanup(dev);

If it does, one option is mt7601u_cleanup() can use some love to really shut 
down
the device more... But its not clear to me what else could be done and I'm very
inclined to believe its not sensible.


"Calling mt7601u_disconnect" does not appear in journalctl after a reboot.


The idea of an optimization of *not* having to load firmware one more time if
it already has it since power hasn't been shut off on the device seems sensible
to me.

Give the few deltas above a quick test just to fill in curiosity if you like
and to be complete -- I'll post RFCs shortly for you Cantabile to test, is
that your name?


Yes.

Re: [RFC] ath10k: Attempt to work around napi_synchronize hang.

2018-02-28 Thread Michał Kazior

On 28 February 2018 at 02:22,   wrote:
[...]
> @@ -2086,8 +2087,28 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
> ath10k_pci_irq_disable(ar);
> ath10k_pci_irq_sync(ar);
> ath10k_pci_flush(ar);
> -   napi_synchronize(>napi);
> -   napi_disable(>napi);
> +
> +   /* Calling napi_disable twice in a row (w/out starting it and/or 
> without
> +* having NAPI active leads to deadlock because napi_disable sets
> +* NAPI_STATE_SCHED and NAPI_STATE_NPSVC when it returns, as far as I
> +* can tell.  So, guard this call to napi_disable.  I believe the
> +* failure case is something like this:
> +* rmmod ath10k_pci ath10k_core
> +*   Firmware crashes before hif_stop is called by the rmmod path
> +*   The crash handling logic calls hif_stop
> + *   Then rmmod gets around to calling hif_stop, but spins endlessly
> +*   in napi_synchronize.
> +*
> +*  I think one way this could happen is that ath10k_stop checks
> +*  for state != ATH10K_STATE_OFF, but STATE_RESTARTING is also
> +*  a possibility.  That might be how we can have hif_stop called 
> twice
> +*  without a hif_start in between. --Ben
> +*/
> +   if (ar->napi_enabled) {
> +   napi_synchronize(>napi);
> +   napi_disable(>napi);
> +   ar->napi_enabled = false;
> +   }

Looking at the code and your comment the described fail case seems
legit. I would consider tuning ath10k_stop() so that it calls
ath10k_halt() only if ar->state != OFF & ar->state != RESTARTING
though. Or did you try already?

While your approach will probably work it won't prevent other non-NAPI
bad things from happening. Even if there's nothing today it might
creep up in the future. And you'd need to update ahb.c too.


Michał

Re: [PATCH] rtl8187: Fix NULL pointer dereference in priv->conf_mutex

2018-02-28 Thread Hin-Tak Leung



On Thu, 15/2/18, Sudhir Sreedharan  wrote:

...
 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Sudhir Sreedharan 

Acked-by: Hin-Tak Leung

Re: [PATCH] mt7601u: remove a warning in mt7601u_efuse_physical_size_check()

2018-02-28 Thread Johannes Berg

On Wed, 2018-02-28 at 17:01 +0100, Lorenzo Bianconi wrote:
> > On Wed, 2018-02-28 at 15:26 +0100, Lorenzo Bianconi wrote:
> > > 
> > >   const int map_reads = DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16);
> > > - u8 data[map_reads * 16];
> > > + u8 data[round_up(MT_EFUSE_USAGE_MAP_SIZE, 16)];
> > > 
> > 
> > You could turn it upside down and make
> > 
> > const int map_reads = ARRAY_SIZE(data);
> 
> map_reads is actually 2 since MT_EFUSE_USAGE_MAP_SIZE is 29. Using
> ARRAY_SIZE(data) map_reads will be set to 32

Oh yeah, good point, sorry.

johannes

Re: [PATCH] mt7601u: remove a warning in mt7601u_efuse_physical_size_check()

> On Wed, 2018-02-28 at 15:26 +0100, Lorenzo Bianconi wrote:
>>
>>   const int map_reads = DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16);
>> - u8 data[map_reads * 16];
>> + u8 data[round_up(MT_EFUSE_USAGE_MAP_SIZE, 16)];
>>
> You could turn it upside down and make
>
> const int map_reads = ARRAY_SIZE(data);

map_reads is actually 2 since MT_EFUSE_USAGE_MAP_SIZE is 29. Using
ARRAY_SIZE(data) map_reads will be set to 32

Regards,
Lorenzo

>
> johannes

Re: [PATCH] mt7601u: make write with mask access atomic

Lorenzo Bianconi  writes:

>> Lorenzo Bianconi  writes:
>>
 On Thu, 15 Feb 2018 23:59:24 +0100, Lorenzo Bianconi wrote:
> Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order
> to make mt7601u_rmw and mt7601u_rmc atomic since it is possible that
> read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved
> with a different write operation on the same register.
> Moreover move write trace point in __mt7601u_vendor_single_wr
>
> Signed-off-by: Lorenzo Bianconi 

 Could you provide an example of which accesses make it problematic?
 Is this fixing an actual bug?
>>>
>>> it is not an issue I had experimented, I noticed a theoretical race
>>> reviewing the code.
>>
>> The commit log should always answer the question "Why?". If you find a
>> theoretical issue in the code document that clearly in the commit log.
>> That helps to choose correct tree and sending to stable releases etc.
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_log_does_not_answer_why
>
> I have already sent a v2: https://patchwork.kernel.org/patch/10225849/
> Is the commit message ok?

Yeah, I already applied it.

-- 
Kalle Valo

Re: [PATCH] mt7601u: remove a warning in mt7601u_efuse_physical_size_check()

2018-02-28 Thread Johannes Berg

On Wed, 2018-02-28 at 15:26 +0100, Lorenzo Bianconi wrote:
> 
>   const int map_reads = DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16);
> - u8 data[map_reads * 16];
> + u8 data[round_up(MT_EFUSE_USAGE_MAP_SIZE, 16)];
> 
You could turn it upside down and make 

const int map_reads = ARRAY_SIZE(data);

johannes

Re: [PATCH] mt7601u: make write with mask access atomic

> Lorenzo Bianconi  writes:
>
>>> On Thu, 15 Feb 2018 23:59:24 +0100, Lorenzo Bianconi wrote:
 Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order
 to make mt7601u_rmw and mt7601u_rmc atomic since it is possible that
 read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved
 with a different write operation on the same register.
 Moreover move write trace point in __mt7601u_vendor_single_wr

 Signed-off-by: Lorenzo Bianconi 
>>>
>>> Could you provide an example of which accesses make it problematic?
>>> Is this fixing an actual bug?
>>
>> it is not an issue I had experimented, I noticed a theoretical race
>> reviewing the code.
>
> The commit log should always answer the question "Why?". If you find a
> theoretical issue in the code document that clearly in the commit log.
> That helps to choose correct tree and sending to stable releases etc.
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_log_does_not_answer_why
>
> --
> Kalle Valo

Hi Kalle,

I have already sent a v2: https://patchwork.kernel.org/patch/10225849/
Is the commit message ok?

Regards,
Lorenzo

Re: [v2] mt7601u: make write with mask access atomic

Lorenzo Bianconi  wrote:

> Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order
> to make mt7601u_rmw and mt7601u_rmc atomic. This patch does not fix a
> reported issue but makes the usb access more robust to concurrent
> operations on the same register since it is theoretically possible that
> read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved with
> a different write operation on the same register.
> Moreover using __mt7601u_rr and __mt7601u_vendor_single_wr in
> mt7601u_rmw/mt7601u_rmc allows to grab vendor_req_mutex mutex once
> instead of twice
> 
> Signed-off-by: Lorenzo Bianconi 
> Acked-by: Jakub Kicinski 

Patch applied to wireless-drivers-next.git, thanks.

fee05843801c mt7601u: make write with mask access atomic

-- 
https://patchwork.kernel.org/patch/10225849/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: rtl8187: Fix NULL pointer dereference in priv->conf_mutex

Sudhir Sreedharan  wrote:

> This can be reproduced by bind/unbind the driver multiple times
> in AM3517 board.
> 
> Analysis revealed that rtl8187_start() was invoked before probe
> finishes(ie. before the mutex is initialized).
> 
>  INFO: trying to register non-static key.
>  the code is fine but needs lockdep annotation.
>  turning off the locking correctness validator.
>  CPU: 0 PID: 821 Comm: wpa_supplicant Not tainted 4.9.80-dirty #250
>  Hardware name: Generic AM3517 (Flattened Device Tree)
>  [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
>  [] (show_stack) from [] (register_lock_class+0x4f4/0x55c)
>  [] (register_lock_class) from [] 
> (__lock_acquire+0x74/0x1938)
>  [] (__lock_acquire) from [] (lock_acquire+0xfc/0x23c)
>  [] (lock_acquire) from [] (mutex_lock_nested+0x50/0x3b0)
>  [] (mutex_lock_nested) from [] (rtl8187_start+0x2c/0xd54)
>  [] (rtl8187_start) from [] (drv_start+0xa8/0x320)
>  [] (drv_start) from [] (ieee80211_do_open+0x2bc/0x8e4)
>  [] (ieee80211_do_open) from [] (__dev_open+0xb8/0x120)
>  [] (__dev_open) from [] (__dev_change_flags+0x88/0x14c)
>  [] (__dev_change_flags) from [] 
> (dev_change_flags+0x18/0x48)
>  [] (dev_change_flags) from [] (devinet_ioctl+0x738/0x840)
>  [] (devinet_ioctl) from [] (sock_ioctl+0x164/0x2f4)
>  [] (sock_ioctl) from [] (do_vfs_ioctl+0x8c/0x9d0)
>  [] (do_vfs_ioctl) from [] (SyS_ioctl+0x6c/0x7c)
>  [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x1c)
>  Unable to handle kernel NULL pointer dereference at virtual address 
>  pgd = cd1ec000
>  [] *pgd=8d1de831, *pte=, *ppte=
>  Internal error: Oops: 817 [#1] PREEMPT ARM
>  Modules linked in:
>  CPU: 0 PID: 821 Comm: wpa_supplicant Not tainted 4.9.80-dirty #250
>  Hardware name: Generic AM3517 (Flattened Device Tree)
>  task: ce73eec0 task.stack: cd1ea000
>  PC is at mutex_lock_nested+0xe8/0x3b0
>  LR is at mutex_lock_nested+0xd0/0x3b0
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Sudhir Sreedharan 

Patch applied to wireless-drivers-next.git, thanks.

7972326a26b5 rtl8187: Fix NULL pointer dereference in priv->conf_mutex

-- 
https://patchwork.kernel.org/patch/10220507/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [1/2] mt7601u: move mt7601u_set_macaddr in mac related code

Lorenzo Bianconi  wrote:

> Remove static qualifier from mt7601u_set_macaddr routine and move it
> in mac related code in order to be used to properly support vif with
> different mac address respect to the default one
> 
> Signed-off-by: Lorenzo Bianconi 

2 patches applied to wireless-drivers-next.git, thanks.

e96826bde3db mt7601u: move mt7601u_set_macaddr in mac related code
032a552e8dc9 mt7601u: set device mac address in mt7601u_add_interface()

-- 
https://patchwork.kernel.org/patch/10208071/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: mt76x2: remove warnings in mt76x2_mac_write_txwi()

Lorenzo Bianconi  wrote:

> Fix following sparse warnings in mt76x2_mac_write_txwi:
> - drivers/net/wireless/mediatek/mt76/mt76x2_mac.c:201:26: warning:
>   incorrect type in assignment (different base types)
> - drivers/net/wireless/mediatek/mt76/mt76x2_mac.c:201:26: expected
>   restricted __le32 [usertype] iv
> - drivers/net/wireless/mediatek/mt76/mt76x2_mac.c:201:26: got unsigned
>   int [unsigned] [usertype] 
> - drivers/net/wireless/mediatek/mt76/mt76x2_mac.c:202:27: warning:
>   incorrect type in assignment (different base types)
> - drivers/net/wireless/mediatek/mt76/mt76x2_mac.c:202:27: expected
>   restricted __le32 [usertype] eiv
> - drivers/net/wireless/mediatek/mt76/mt76x2_mac.c:202:27: got unsigned
>   int [unsigned] [usertype] 
> 
> Fixes: 23405236460b ("mt76: fix transmission of encrypted management frames")
> Signed-off-by: Lorenzo Bianconi 

Patch applied to wireless-drivers-next.git, thanks.

09e93f28aa8d mt76x2: remove warnings in mt76x2_mac_write_txwi()

-- 
https://patchwork.kernel.org/patch/10194957/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

2018-02-28 Thread Hans de Goede


Hi,

On 27-02-18 00:04, Arend van Spriel wrote:

On 2/26/2018 12:22 PM, Hans de Goede wrote:

Hi,

On 26-02-18 12:01, Arend van Spriel wrote:

On 2/26/2018 11:29 AM, Hans de Goede wrote:

Hi,

On 26-02-18 11:22, Arend van Spriel wrote:

On 2/25/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede 


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:


Yup. Before we were passing firmware errors up to user-space, which
was confusing and potentially be misinterpreted. However, looking at
the output below it would have been good to log the firmware error as
well. And staring at it some more I suddenly realize I broke the
feature detection module with this change. Actually only the GSCAN
feature detection.


[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( -> 0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version =
wl0:
Jun  4 2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface
p2p-dev-wlp1s0 type 10 failed: err=-52

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac4356-pcie.bin |
tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon


Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04
16:51:38 PDT Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?


I had a look at it and it seems to be a difference in firmware api
that we need to support in the driver. Need to do a bit more digging,
but it seems an actual issue. You could silence it for now, but I
prefer to wait for the fix.


Ok, what is the ETA of a fix for this?


Actually went back to an old log you sent and noticed:

[   15.714569] brcmfmac: brcmf_attach Enter
[   15.714756] brcmfmac: brcmf_fweh_register event handler registered
for PSM_WATCHDOG
[   15.714757] brcmfmac: brcmf_proto_attach Enter
[   15.716598] brcmfmac: brcmf_bus_started
[   15.716603] brcmfmac: brcmf_add_if Enter, bsscfgidx=0, ifidx=0
[   15.716604] brcmfmac: brcmf_add_if allocate netdev interface
[   15.716622] brcmfmac: brcmf_add_if   pid:2a, if:wlan%d
(00:00:00:00:00:00) created ===
[   15.716624] brcmfmac: brcmf_bus_change_state 0 -> 1
[   15.717841] brcmfmac: brcmf_fil_iovar_data_get ifidx=0,
name=cur_etheraddr, len=6
[   15.717843] brcmutil: data
[   15.717847] : 44 2c 05 9e c9 02   D,

So mac address of the device is 44:2c:05:9e:c9. However, further down
I see:

[   17.819113] brcmfmac: brcmf_netdev_set_mac_address Enter, bsscfgidx=0
[   17.819122] brcmfmac: brcmf_fil_iovar_data_set ifidx=0,
name=cur_etheraddr, len=6
[   17.819127] brcmutil: data
[   17.819135] : aa 3e 81 77 bc 40   .>.w.@
[   17.819864] brcmfmac: brcmf_netdev_set_mac_address updated to
aa:3e:81:77:bc:40

So the mac address in a local admin variant.


Right, this is likely NetworkManager randomizing the mac for privacy
reasons.


Now our firmware has a requirement for the p2p-dev interface that it
should be different from the mac address of the primary interface, ie.
wlp1s0 in this log. In brcmfmac we try to do that by setting the local
admin bit, but... as it is already set we end up using the same mac
address hence the -EBUSY.


Ah, that is good to know, so how can we fix this? Can userspace specify a
different mac-address when it asks for the p2p-dev intf to be created? Or
should we do something about this in the kernel?


So this is the patch I tested. Maybe you can verify it works for you as well.


I can confirm that this fixes the errors, but I do not think that this is a good
solution, using the permanent mac address for the p2p interface has the same
privacy problem as using it regularly. IMHO it would be best to just generate
a random mac address if none is specified. This is what the kernel seems to do
in general when it needs a mac address and none is specified. You can use the
eth_random_addr(u8 *addr)

Re: [PATCH] mt7601u: make write with mask access atomic

Lorenzo Bianconi  writes:

>> On Thu, 15 Feb 2018 23:59:24 +0100, Lorenzo Bianconi wrote:
>>> Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order
>>> to make mt7601u_rmw and mt7601u_rmc atomic since it is possible that
>>> read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved
>>> with a different write operation on the same register.
>>> Moreover move write trace point in __mt7601u_vendor_single_wr
>>>
>>> Signed-off-by: Lorenzo Bianconi 
>>
>> Could you provide an example of which accesses make it problematic?
>> Is this fixing an actual bug?
>
> it is not an issue I had experimented, I noticed a theoretical race
> reviewing the code.

The commit log should always answer the question "Why?". If you find a
theoretical issue in the code document that clearly in the commit log.
That helps to choose correct tree and sending to stable releases etc.

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_log_does_not_answer_why

-- 
Kalle Valo

Re: ssb: Prevent build of PCI host features in module

Matt Redfearn  wrote:

> Attempting to build ssb.ko with CONFIG_SSB_DRIVER_PCICORE=y results in
> a build error due to use of symbols not exported from vmlinux:
> 
> ERROR: "pcibios_enable_device" [drivers/ssb/ssb.ko] undefined!
> ERROR: "register_pci_controller" [drivers/ssb/ssb.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
> 
> To prevent this, don't allow the host mode feature to be built if
> CONFIG_SSB=m
> 
> Signed-off-by: Matt Redfearn 

Patch applied to wireless-drivers-next.git, thanks.

882164a4a928 ssb: Prevent build of PCI host features in module

-- 
https://patchwork.kernel.org/patch/10161131/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: bcma: add HP Stream Notebook

Denis 'GNUtoo' Carikli  wrote:

> In this laptop we have the following PCI device:
> 02:00.0 Network controller [0280]: Broadcom Limited BCM43142 802.11b/g/n 
> [14e4:4365] (rev 01)
>   Subsystem: Hewlett-Packard Company BCM43142 802.11b/g/n [103c:804a]
>   [...]
>   Region 0: Memory at 9100 (64-bit, non-prefetchable) [size=32K]
>   [...]
> 
> With this patch, we can now see its WiFi chip:
>   bcma: bus0: Found chip with id 43142, rev 0x01 and package 0x08
>   bcma: bus0: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x28, 
> class 0x0)
>   bcma: bus0: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x21, 
> class 0x0)
>   bcma: bus0: Core 2 found: PCIe (manuf 0x4BF, id 0x820, rev 0x16, class 0x0)
>   bcma: bus0: Core 3 found: UNKNOWN (manuf 0x43B, id 0x368, rev 0x00, class 
> 0x0)
>   bcma: bus0: Found rev 15 PMU (capabilities 0x518C5E0F)
>   bcma: bus0: SPROM offset 0x840
>   bcma: bus0: Found SPROM revision 10
>   bcma: bus0: Workarounds unknown or not needed for device 0xA886
>   bcma: bus0: Bus registered
> 
> But it not yet supported by brcmsmac so it won't work for now:
>   brcmsmac bcma0:1: brcms_b_attach wl0: vendor 0x14e4 device 0x4365
>   brcmsmac: unknown device id 4365
> 
> Signed-off-by: Denis 'GNUtoo' Carikli 

Patch applied to wireless-drivers-next.git, thanks.

985324a16efb bcma: add HP Stream Notebook

-- 
https://patchwork.kernel.org/patch/10207423/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: bcma: Prevent build of PCI host features in module

Matt Redfearn  wrote:

> Attempting to build bcma.ko with BCMA_DRIVER_PCI_HOSTMODE=y results in
> a build error due to use of symbols not exported from vmlinux:
> 
> ERROR: "pcibios_enable_device" [drivers/bcma/bcma.ko] undefined!
> ERROR: "register_pci_controller" [drivers/bcma/bcma.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
> 
> To prevent this, don't allow the host mode feature to be built if
> CONFIG_BCMA=m
> 
> Signed-off-by: Matt Redfearn 

This does not apply to wireless-drivers-next anymore. Also please CC:

linux-m...@linux-mips.org
James Hogan 

Recorded preimage for 'drivers/bcma/Kconfig'
error: Failed to merge in the changes.
Applying: bcma: Prevent build of PCI host features in module
Using index info to reconstruct a base tree...
M   drivers/bcma/Kconfig
Falling back to patching base and 3-way merge...
Auto-merging drivers/bcma/Kconfig
CONFLICT (content): Merge conflict in drivers/bcma/Kconfig
Patch failed at 0001 bcma: Prevent build of PCI host features in module
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/10161087/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH] mt7601u: remove a warning in mt7601u_efuse_physical_size_check()

Fix the following sparse warning in mt7601u_efuse_physical_size_check:
- drivers/net/wireless/mediatek/mt7601u/eeprom.c:77:27: warning:
  Variable length array is used

Signed-off-by: Lorenzo Bianconi 
---
 drivers/net/wireless/mediatek/mt7601u/eeprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c 
b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
index da6faea092d6..a462064b5c91 100644
--- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c
@@ -74,7 +74,7 @@ static int
 mt7601u_efuse_physical_size_check(struct mt7601u_dev *dev)
 {
const int map_reads = DIV_ROUND_UP(MT_EFUSE_USAGE_MAP_SIZE, 16);
-   u8 data[map_reads * 16];
+   u8 data[round_up(MT_EFUSE_USAGE_MAP_SIZE, 16)];
int ret, i;
u32 start = 0, end = 0, cnt_free;
 
-- 
2.14.3

Re: [PATCH] ath10k: Add sta rx packet stats per tid

Vasanthakumar Thiagarajan  writes:

> Added per tid sta counters for the following
>
> - Total number MSDUs received from firmware
> - Number of MSDUs received with errors like decryption, crc, mic ,etc.
> - Number of MSDUs dropped in the driver
> - A-MPDU/A-MSDU subframe stats
> - Number of MSDUS passed to mac80211
>
> All stats other than A-MPDU stats are only for received data frames.
> A-MPDU stats might have stats for management frames when monitor
> interface is active where management frames are notified both in wmi
> and HTT interfaces.
>
> These per tid stats can be enabled with tid bitmask through a debugfs
> like below
>
>  echo  > 
> /sys/kernel/debug/ieee80211/phyX/ath10k/sta_tid_stats_mask
>
>  tid 16 (tid_bitmask 0x1) is used for non-qos data/management frames
>
> The stats are read from
> /sys/kernel/debug/ieee80211/phyX/netdev\:wlanX/stations//dump_tid_stats
>
> Sample output:
>
>  To enable rx stats for tid 0, 5 and 6,
>
>  echo 0x0061 > /sys/kernel/debug/ieee80211/phy0/ath10k/sta_tid_stats_mask
>
> cat 
> /sys/kernel/debug/ieee80211/phy0/netdev\:wlan15/stations/8c\:fd\:f0\:0a\:8e\:df/dump_tid_stats
>
>   Driver Rx pkt stats per tid, ([tid] count)
> --
> MSDUs from FW   [00] 2567[05] 3178[06] 1089
> MSDUs unchained [00] 0   [05] 0   [06] 0
> MSDUs locally dropped:chained   [00] 0   [05] 0   [06] 0
> MSDUs locally dropped:filtered  [00] 0   [05] 0   [06] 0
> MSDUs queued for mac80211   [00] 2567[05] 3178[06] 1089
> MSDUs with error:fcs_err[00] 0   [05] 0   [06] 2
> MSDUs with error:tkip_err   [00] 0   [05] 0   [06] 0
> MSDUs with error:crypt_err  [00] 0   [05] 0   [06] 0
> MSDUs with error:peer_idx_inval [00] 0   [05] 0   [06] 0
>
> A-MPDU num subframes upto 10[00] 2567[05] 3178[06] 1087
> A-MPDU num subframes 11-20  [00] 0   [05] 0   [06] 0
> A-MPDU num subframes 21-30  [00] 0   [05] 0   [06] 0
> A-MPDU num subframes 31-40  [00] 0   [05] 0   [06] 0
> A-MPDU num subframes 41-50  [00] 0   [05] 0   [06] 0
> A-MPDU num subframes 51-60  [00] 0   [05] 0   [06] 0
> A-MPDU num subframes >60[00] 0   [05] 0   [06] 0
>
> A-MSDU num subframes 1  [00] 2567[05] 3178[06] 1089
> A-MSDU num subframes 2  [00] 0   [05] 0   [06] 0
> A-MSDU num subframes 3  [00] 0   [05] 0   [06] 0
> A-MSDU num subframes 4  [00] 0   [05] 0   [06] 0
> A-MSDU num subframes >4 [00] 0   [05] 0   [06] 0
>
> Signed-off-by: Vasanthakumar Thiagarajan 

This added new ath10k-check warnings, I fixed those in the pending
branch:

drivers/net/wireless/ath/ath10k/debug.h:203: Alignment should match open 
parenthesis
drivers/net/wireless/ath/ath10k/debug.h:226: Alignment should match open 
parenthesis
drivers/net/wireless/ath/ath10k/debugfs_sta.c:24: Alignment should match open 
parenthesis
drivers/net/wireless/ath/ath10k/debugfs_sta.c:40: Alignment should match open 
parenthesis
drivers/net/wireless/ath/ath10k/debugfs_sta.c:60: Alignment should match open 
parenthesis

Also I fixed the typo "rages" in debug.h. My changes:

diff --git a/drivers/net/wireless/ath/ath10k/debug.h 
b/drivers/net/wireless/ath/ath10k/debug.h
index 306796dcedae..7ebb9b1e7e69 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -200,9 +200,9 @@ void ath10k_sta_update_rx_tid_stats(struct ath10k *ar, u8 
*first_hdr,
unsigned long int drop_cnt_filter,
unsigned long int queued_msdus);
 void ath10k_sta_update_rx_tid_stats_ampdu(struct ath10k *ar,
-   u16 peer_id, u8 tid,
-   struct htt_rx_indication_mpdu_range *mpdu_ranges,
-   int num_mpdu_rages);
+ u16 peer_id, u8 tid,
+ struct htt_rx_indication_mpdu_range 
*ranges,
+ int num_ranges);
 #else
 static inline
 void ath10k_sta_update_rx_duration(struct ath10k *ar,
@@ -223,9 +223,9 @@ void ath10k_sta_update_rx_tid_stats(struct ath10k *ar, u8 
*first_hdr,
 
 static inline
 void ath10k_sta_update_rx_tid_stats_ampdu(struct ath10k *ar,
-   u16 peer_id, u8 tid,
-   struct htt_rx_indication_mpdu_range *mpdu_ranges,
-   int num_mpdu_rages)
+ u16 peer_id, u8 tid,
+ struct htt_rx_indication_mpdu_range 
*ranges,
+

Re: [PATCH] ath10k: dma unmap mgmt tx buffer if wmi cmd send fails

pill...@codeaurora.org writes:

> From: Rakesh Pillai 
>
> WCN3990 sends mgmt frames by reference via WMI.
> The host dma maps the mgmt frame and sends the physical
> address to the firmware in the wmi command. Since the
> dma mapping is done in the gen_mgmt_tx and if the wmi
> command send fails, the corresponding mgmt frame is
> not being dma unmapped.
>
> Fix the missing dma unmapping of mgmt tx frame when
> wmi command sending fails for mgmt tx by reference
> via WMI. The already exisiting mgmt tx using copy by
> value does not need such dma unmapping.
> Add a separate wmi-tlv op for mgmt tx via ref, which
> takes care of unmapping the dma address, in case of
> wmi command sending failure.
>
> Signed-off-by: Rakesh Pillai 

ath10k-check found two warnings, I fixed those in pending branch:

drivers/net/wireless/ath/ath10k/wmi-ops.h:130: Alignment should match open 
parenthesis
drivers/net/wireless/ath/ath10k/wmi-ops.h:380: Alignment should match open 
parenthesis

-- 
Kalle Valo

Re: Max clients on wifi access point with 7265

Steve deRosier  writes:

> On Tue, Feb 27, 2018 at 12:33 AM, Kalle Valo  wrote:
>> Luca Coelho  writes:
>>
 > We do have wiphy::max_ap_assoc_sta, but I see only ath10k, qtnfmac
 > and
 > rsi_91x setting it. I wish all drivers would use that.
 >
 >  * @max_ap_assoc_sta: maximum number of associated stations
 > supported in AP mode
 >  * (including P2P GO) or 0 to indicate no such limit is advertised.
 > The
 >  * driver is allowed to advertise a theoretical limit that it can
 > reach in
 >  * some cases, but may not always reach.

 Cool, I hadn't noticed this before.  I'll add a patch to iwlwifi to
 add it.
>>>
>>> Actually this is not so straightforward, because every time we add a
>>> p2p vif, we lose one more station.  So the max_ap_assoc_sta value must
>>> be dynamic (or we can state the theoretical lowest number to start
>>> with, which would not be very nice).
>>>
>>> I don't think this feature is worth the trouble, so I'll skip it for
>>> now.
>>
>> I think the documentation answers that part pretty well:
>>
>>   "The driver is allowed to advertise a theoretical limit that it can
>>reach in some cases, but may not always reach."
>>
>> So I still thank having the drivers to advertise the theoretical maximum
>> numbers of client is useful, even if it would not be always 100%
>> correct. For example, an average user most likely will not have any clue
>> if the limit is 10, 50 or 100 clients. And besides, very few people use
>> P2P anyway ;)
>>
>> But of course this is just nice-to-have category cand we have far more
>> important things to fix first.
>>
>
> From a practical point-of-view, many chipsets don't advertise this
> information. Luca was able to look the limit up or knew it for his
> chip. For example, on the ath6kl chips I most commonly have worked
> with, the number is not specified by QCA and was only determined by us
> via experimentation. And even on the same chip, it'll change with
> firmware version as the necessary resources get consumed by new
> features or fixes. If the firmware can send that number up to the
> driver (or the driver can reliably know it because it can't change),
> then expose it, but otherwise I'd advise publishing a value of 0. I'd
> rather see the unknown flag rather than relying on a number that may
> or may not be accurate.

Yeah, definitely. The value needs to be based on facts, not guesses.

-- 
Kalle Valo

Re: [PATCH 1/2] brcmfmac: add possibility to obtain firmware error

Arend van Spriel  writes:

> On 2/28/2018 12:42 AM, Arend van Spriel wrote:
>> The feature module needs to evaluate the actual firmware error returned
>> upon a control command. This adds a field to struct brcmf_if in which
>> the error code is stored whenever a firmware error occurs.
>
> Hi Kalle,
>
> Can you drop this one. It was way too late for me so I screwed it up.
> If needed I will resend both patches. Let me know.

I see that you submitted v2 already so I'll drop both patches 1 and 2.

-- 
Kalle Valo

Re: Problem with bridge (mcast-to-ucast + hairpin) and Broadcom's 802.11f in their FullMAC fw


On 2/27/2018 11:14 AM, Rafał Miłecki wrote:

Sending with a fixed linux-wireless ML address. Please kindly send your
replies using linux-wireless@

On 02/27/2018 11:08 AM, Rafał Miłecki wrote:

I've problem when using OpenWrt/LEDE on a home router with Broadcom's
FullMAC WiFi chipset.


First of all OpenWrt/LEDE uses bridge interface for LAN network with:
1) IFLA_BRPORT_MCAST_TO_UCAST
2) Clients isolation in hostapd
3) Hairpin mode enabled

For more details please see Linus's patch description:
https://patchwork.kernel.org/patch/9530669/
and maybe hairpin mode patch:
https://lwn.net/Articles/347344/

Short version: in that setup packets received from a bridged wireless
interface can be handled back to it for transmission.


Now, Broadcom's firmware for their FullMAC chipsets in AP mode
supports an obsoleted 802.11f AKA IAPP standard. It's a roaming
standard that was replaced by 802.11r.

Whenever a new station associates, firmware generates a packet like:
ff ff ff ff  ff ff ec 10  7b 5f ?? ??  00 06 00 01  af 81 01 00
(just masked 2 bytes of my MAC)

For mode details you can see discussion in my brcmfmac patch thread:
https://patchwork.kernel.org/patch/10191451/


The problem is that bridge (in setup as above) handles such a packet
back to the device.


From reading the referenced links I understand the hairpin mode is 
causing the packet to be sent back to the device, and the hairpin mode 
is required for MCAST_TO_UCAST, right?



That makes Broadcom's FullMAC firmware believe that a given station
just connected to another AP in a network (which doesn't even exist).
As a result firmware immediately disassociates that station. It's
simply impossible to connect to the router. Every association is
followed by immediate disassociation.


Can you see any solution for this problem? Is that an option to stop
multicast-to-unicast from touching 802.11f packets? Some other ideas?
Obviously I can't modify Broadcom's firmware and drop that obsoleted
standard.


As far as I can tell you are correct that the 802.11f amendment was 
never adopted into the 802.11 standard. I will ask internally if we 
still have a reason for carrying it in our firmware.


Regards,
Arend

[PATCH for-4.16 V2 2/2] brcmfmac: fix P2P_DEVICE ethernet address generation

The firmware has a requirement that the P2P_DEVICE address should
be different from the address of the primary interface. When not
specified by user-space, the driver generates the MAC address for
the P2P_DEVICE interface using the MAC address of the primary
interface and setting the locally administered bit. However, the MAC
address of the primary interface may already have that bit set causing
the creation of the P2P_DEVICE interface to fail with -EBUSY. Fix this
by using the permanent address instead to determing the P2P_DEVICE
address.

Cc: sta...@vger.kernel.org # 3.10.y
Reported-by: Hans de Goede 
Reviewed-by: Hante Meuleman 
Reviewed-by: Pieter-Paul Giesberts 
Reviewed-by: Franky Lin 
Signed-off-by: Arend van Spriel 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 2ee5413..ddbb386 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -462,8 +462,8 @@ static int brcmf_p2p_set_firmware(struct brcmf_if *ifp, u8 
*p2p_mac)
  * @dev_addr: optional device address.
  *
  * P2P needs mac addresses for P2P device and interface. If no device
- * address it specified, these are derived from the primary net device, ie.
- * the permanent ethernet address of the device.
+ * address it specified, these are derived from the permanent ethernet
+ * address of the device.
  */
 static void brcmf_p2p_generate_bss_mac(struct brcmf_p2p_info *p2p, u8 
*dev_addr)
 {
@@ -471,12 +471,12 @@ static void brcmf_p2p_generate_bss_mac(struct 
brcmf_p2p_info *p2p, u8 *dev_addr)
bool local_admin = false;
 
if (!dev_addr || is_zero_ether_addr(dev_addr)) {
-   dev_addr = pri_ifp->mac_addr;
+   dev_addr = pri_ifp->drvr->mac;
local_admin = true;
}
 
/* Generate the P2P Device Address.  This consists of the device's
-* primary MAC address with the locally administered bit set.
+* permanent ethernet address with the locally administered bit set.
 */
memcpy(p2p->dev_addr, dev_addr, ETH_ALEN);
if (local_admin)
@@ -486,7 +486,7 @@ static void brcmf_p2p_generate_bss_mac(struct 
brcmf_p2p_info *p2p, u8 *dev_addr)
 * BSSCFGs need to simultaneously co-exist, then this address must be
 * different from the P2P Device Address, but also locally administered.
 */
-   memcpy(p2p->int_addr, p2p->dev_addr, ETH_ALEN);
+   memcpy(p2p->int_addr, dev_addr, ETH_ALEN);
p2p->int_addr[0] |= 0x02;
p2p->int_addr[4] ^= 0x80;
 }
-- 
1.9.1

[PATCH for-4.16 V2 1/2] brcmfmac: add possibility to obtain firmware error

The feature module needs to evaluate the actual firmware error return
upon a control command. This adds a flag to struct brcmf_if that the
caller can set. This flag is checked to determine the error code that
needs to be returned.

Fixes: b69c1df47281 ("brcmfmac: separate firmware errors from i/o errors")
Reviewed-by: Hante Meuleman 
Reviewed-by: Pieter-Paul Giesberts 
Reviewed-by: Franky Lin 
Signed-off-by: Arend van Spriel 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h|  2 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 10 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c|  3 +++
 3 files changed, 15 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index df8a1ec..232dcbb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -181,6 +181,7 @@ enum brcmf_netif_stop_reason {
  * @netif_stop_lock: spinlock for update netif_stop from multiple sources.
  * @pend_8021x_cnt: tracks outstanding number of 802.1x frames.
  * @pend_8021x_wait: used for signalling change in count.
+ * @fwil_fwerr: flag indicating fwil layer should return firmware error codes.
  */
 struct brcmf_if {
struct brcmf_pub *drvr;
@@ -198,6 +199,7 @@ struct brcmf_if {
wait_queue_head_t pend_8021x_wait;
struct in6_addr ipv6_addr_tbl[NDOL_MAX_ENTRIES];
u8 ipv6addr_idx;
+   bool fwil_fwerr;
 };
 
 int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 47de35a..bede7b7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -104,6 +104,9 @@ static void brcmf_feat_iovar_int_get(struct brcmf_if *ifp,
u32 data;
int err;
 
+   /* we need to know firmware error */
+   ifp->fwil_fwerr = true;
+
err = brcmf_fil_iovar_int_get(ifp, name, );
if (err == 0) {
brcmf_dbg(INFO, "enabling feature: %s\n", brcmf_feat_names[id]);
@@ -112,6 +115,8 @@ static void brcmf_feat_iovar_int_get(struct brcmf_if *ifp,
brcmf_dbg(TRACE, "%s feature check failed: %d\n",
  brcmf_feat_names[id], err);
}
+
+   ifp->fwil_fwerr = false;
 }
 
 static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
@@ -120,6 +125,9 @@ static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
 {
int err;
 
+   /* we need to know firmware error */
+   ifp->fwil_fwerr = true;
+
err = brcmf_fil_iovar_data_set(ifp, name, data, len);
if (err != -BRCMF_FW_UNSUPPORTED) {
brcmf_dbg(INFO, "enabling feature: %s\n", brcmf_feat_names[id]);
@@ -128,6 +136,8 @@ static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
brcmf_dbg(TRACE, "%s feature check failed: %d\n",
  brcmf_feat_names[id], err);
}
+
+   ifp->fwil_fwerr = false;
 }
 
 #define MAX_CAPS_BUFFER_SIZE   512
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
index f2cfdd3..fc57511 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
@@ -131,6 +131,9 @@ static const char *brcmf_fil_get_errstr(u32 err)
  brcmf_fil_get_errstr((u32)(-fwerr)), fwerr);
err = -EBADE;
}
+   if (ifp->fwil_fwerr)
+   return fwerr;
+
return err;
 }
 
-- 
1.9.1

[PATCH for-4.16 V2 0/2] brcmfmac: small fixes

Here two small fixes. The first patch fixes a regression introduced in v4.16
and the second fix is result from this discussion [1].

So these patches are intended 4.16 and apply to the master branch of the
wireless-drivers repository.

[1] https://www.spinics.net/lists/linux-wireless/thrd12.html#170494

Arend van Spriel (2):
  brcmfmac: add possibility to obtain firmware error
  brcmfmac: fix P2P_DEVICE ethernet address generation

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h|  2 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 10 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c|  3 +++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 10 +-
 4 files changed, 20 insertions(+), 5 deletions(-)

-- 
1.9.1

Re: Support on vendor id and device id

On 2/28/2018 12:14 AM, Harsha Rao wrote:

My suspicion is that your device, is fundamentally a wilc1000 and that
>>>the existing wilc1000 driver will likely largely work for it and all
>>>you really need to do is modify the existing driver to handle the
>>>quirks of your particular implementation of the wilc1000 chip. And,
>>>often WiFi chips will let you change the VID/PID somewhere within
>>>whatever non-volatile storage it has (like where it stores the MAC
>>>address).

>
>
>So it seems the wilc1000 devices from Microchip/Atmel are also using a
>vendor id they did not buy. Could be that the mentioned 3rd party providing
>the SDIO IP actually owns that vendor id, but if you are building your wifi
>chip on that you should better buy you own vendor id from the SD
>Association. Now if Harsha is actually working for Microchip (unclear to me)
>there is basically one party that should go shopping.
>

I would like to clarify that I am not building anything on top of
microchip wifi device.
We have a different HW . Its been just that 3rd party vendor providing
SDIO IP has given
same ID to different customers.

So it is as I said, ie. you are using the 3rd party SDIO IP as is and 
add your own wifi IP to it? So what does the term "SDIO IP" mean here. 
Is it a piece of hardware that you hook up to your wifi hardware or is 
it VHDL/verilog in which the vendor id is defined. If it is VHDL you 
should really get your own vendor id from the SD Association and fix it. 
Otherwise, the 3rd party hardware should have means to change it. If 
not, you better find another party.

Regards,
Arend

Re: [PATCH 1/2] brcmfmac: add possibility to obtain firmware error