Hello,
this series aims to bring some minor fixes on WILC1000 driver. Those fixes
already live in Microchip internal tree and had been initiated by Ajay
- some initial firmware configuration adjustments, no visible impact on
user
- a workqueue leak when dealing with multiple VIF
- a multi-vif fix (adding and removing a vif currently breaks the first
one)
- a power down sequence fix to prevent "Fw not responding" errors on next
boot
Those have been tested on SAMA5D2 with WILC1000 SD. All those fixes are
independent from each other.
---
Ajay Singh (5):
wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()
wifi: wilc1000: fix driver_handler when committing initial configuration
wilc: wifi: do not realloc workqueue everytime an interface is added
wifi: wilc1000: fix incorrect power down sequence
wifi: wilc1000: fix multi-vif management when deleting a vif
drivers/net/wireless/microchip/wilc1000/cfg80211.c | 12 ++++++--
drivers/net/wireless/microchip/wilc1000/netdev.c | 14 ++-------
drivers/net/wireless/microchip/wilc1000/wlan.c | 33 +++++++++++++---------
drivers/net/wireless/microchip/wilc1000/wlan.h | 6 ++++
4 files changed, 38 insertions(+), 27 deletions(-)
---
base-commit: 03b2a1757348d2e8b62d4e8cbcbcd3ff59413d01
change-id: 20240115-wilc_1000_fixes-d41573764468
Best regards,
--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
From: Ajay Singh <[email protected]>
During firmware initial configuration in wilc_init_fw_config, the special
driver_handler 0 should be used instead of targeting a specific virtual
interface (either 1 or 2)
The issue does not seem to have real consequence (both virtual interfaces
seems to answer correctly to a Add Block Ack request with the Immediate
policy), but lets make everything homogeneous
Signed-off-by: Ajay Singh <[email protected]>
Signed-off-by: Alexis Lothoré <[email protected]>
---
drivers/net/wireless/microchip/wilc1000/netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index 8923eb64c964..f3b9709f8730 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -416,7 +416,7 @@ static int wilc_init_fw_config(struct net_device *dev, struct wilc_vif *vif)
b = 1;
if (!wilc_wlan_cfg_set(vif, 0, WID_11N_IMMEDIATE_BA_ENABLED, &b, 1,
- 1, 1))
+ 1, 0))
goto fail;
return 0;
--
2.42.1
From: Ajay Singh <[email protected]>
Commit 09ed8bfc5215 ("wilc1000: Rename workqueue from "WILC_wq" to
"NETDEV-wq"") moved workqueue creation in wilc_netdev_ifc_init in order to
set the interface name in the workqueue name. However, while the driver
needs only one workqueue, the wilc_netdev_ifc_init is called each time we
add an interface over a phy, which in turns overwrite the workqueue with a
new one. This can be observed with the following commands:
for i in $(seq 0 10)
do
iw phy phy0 interface add wlan1 type managed
iw dev wlan1 del
done
ps -eo pid,comm|grep wlan
39 kworker/R-wlan0
98 kworker/R-wlan1
102 kworker/R-wlan1
105 kworker/R-wlan1
108 kworker/R-wlan1
111 kworker/R-wlan1
114 kworker/R-wlan1
117 kworker/R-wlan1
120 kworker/R-wlan1
123 kworker/R-wlan1
126 kworker/R-wlan1
129 kworker/R-wlan1
Fix this leakage by putting back hif_workqueue allocation in
wilc_cfg80211_init. Regarding the workqueue name, it is indeed relevant to
set it lowercase, however it is not attached to a specific netdev, so
enforcing netdev name in the name is not so relevant. Still, enrich the
name with the wiphy name to make it clear which phy is using the workqueue.
Fixes: 09ed8bfc5215 ("wilc1000: Rename workqueue from "WILC_wq" to "NETDEV-wq"")
Signed-off-by: Ajay Singh <[email protected]>
Co-developed-by: Alexis Lothoré <[email protected]>
Signed-off-by: Alexis Lothoré <[email protected]>
---
This patch has initially been done by Ajay, and I slightly reworked it
09ed8bfc5215 ("wilc1000: Rename workqueue from "WILC_wq" to "NETDEV-wq"")
also mentions that this workqueue allocation move has also been done to
make wq alloc/dealloc symmetric: the revert set back the assymetry, and that
remains something to be fixed. Deallocation should likely be moved from
netdev.c to cfg80211, but that would be a dedicated rework topic
---
drivers/net/wireless/microchip/wilc1000/cfg80211.c | 11 ++++++++++-
drivers/net/wireless/microchip/wilc1000/netdev.c | 10 +---------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index ad2509d8c99a..2d0474e6404e 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -1804,15 +1804,24 @@ int wilc_cfg80211_init(struct wilc **wilc, struct device *dev, int io_type,
INIT_LIST_HEAD(&wl->rxq_head.list);
INIT_LIST_HEAD(&wl->vif_list);
+ wl->hif_workqueue = alloc_ordered_workqueue("%s", WQ_MEM_RECLAIM,
+ wiphy_name(wl->wiphy));
+ if (!wl->hif_workqueue) {
+ ret = -ENOMEM;
+ goto free_cfg;
+ }
vif = wilc_netdev_ifc_init(wl, "wlan%d", WILC_STATION_MODE,
NL80211_IFTYPE_STATION, false);
if (IS_ERR(vif)) {
ret = PTR_ERR(vif);
- goto free_cfg;
+ goto free_hq;
}
return 0;
+free_hq:
+ destroy_workqueue(wl->hif_workqueue);
+
free_cfg:
wilc_wlan_cfg_deinit(wl);
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index f3b9709f8730..da52f4a9c1fe 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -989,13 +989,6 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
goto error;
}
- wl->hif_workqueue = alloc_ordered_workqueue("%s-wq", WQ_MEM_RECLAIM,
- ndev->name);
- if (!wl->hif_workqueue) {
- ret = -ENOMEM;
- goto unregister_netdev;
- }
-
ndev->needs_free_netdev = true;
vif->iftype = vif_type;
vif->idx = wilc_get_available_idx(wl);
@@ -1008,12 +1001,11 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name,
return vif;
-unregister_netdev:
+error:
if (rtnl_locked)
cfg80211_unregister_netdevice(ndev);
else
unregister_netdev(ndev);
- error:
free_netdev(ndev);
return ERR_PTR(ret);
}
--
2.42.1
From: Ajay Singh <[email protected]>
Adding then removing a second vif currently makes the first vif not working
anymore. This is visible for example when we have a first interface
connected to some access point:
- create a wpa_supplicant.conf with some AP credentials
- wpa_supplicant -Dnl80211 -c /etc/wpa_supplicant.conf -i wlan0
- dhclient wlan0
- iw phy phy0 interface add wlan1 type managed
- iw dev wlan1 del
wlan0 does not manage properly traffic anymore (eg: ping not working)
This is due to vif mode being incorrectly reconfigured with some default
values in del_virtual_intf, affecting by default first vif.
Prevent first vif from being affected on second vif removal by removing vif
mode change command in del_virtual_intf
Fixes: 9bc061e88054 ("staging: wilc1000: added support to dynamically add/remove interfaces")
Signed-off-by: Ajay Singh <[email protected]>
Co-developed-by: Alexis Lothoré <[email protected]>
Signed-off-by: Alexis Lothoré <[email protected]>
---
This patch has been initiated by Ajay, and I reworked it a bit/split it.
I could have just fixed the parameters passed to wilc_set_operation_mode,
but I observed that even calling wilc_set_operation_mode seems faulty in
del_virtual_intf: for example the thread in charge of sending commands to
the chip is possibly not running (because interface is not up). Also, the
vif mode is only configured in change_virtual_intf and not in
add_virtual_intf, which tends to validate the removal from
del_virtual_intf.
---
drivers/net/wireless/microchip/wilc1000/cfg80211.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index 2d0474e6404e..f03fd15c0c97 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -1609,7 +1609,6 @@ static int del_virtual_intf(struct wiphy *wiphy, struct wireless_dev *wdev)
cfg80211_unregister_netdevice(vif->ndev);
vif->monitor_flag = 0;
- wilc_set_operation_mode(vif, 0, 0, 0);
mutex_lock(&wl->vif_mutex);
list_del_rcu(&vif->list);
wl->vif_num--;
--
2.42.1
Alexis Lothoré <[email protected]> wrote:
> From: Ajay Singh <[email protected]>
>
> During firmware initial configuration in wilc_init_fw_config, the special
> driver_handler 0 should be used instead of targeting a specific virtual
> interface (either 1 or 2)
> The issue does not seem to have real consequence (both virtual interfaces
> seems to answer correctly to a Add Block Ack request with the Immediate
> policy), but lets make everything homogeneous
>
> Signed-off-by: Ajay Singh <[email protected]>
> Signed-off-by: Alexis Lothoré <[email protected]>
4 patches applied to wireless-next.git, thanks.
52284952cbf3 wifi: wilc1000: fix driver_handler when committing initial configuration
328efda22af8 wifi: wilc1000: do not realloc workqueue everytime an interface is added
a4f1a05b832e wifi: wilc1000: fix incorrect power down sequence
12cfc9c8d3fa wifi: wilc1000: fix multi-vif management when deleting a vif
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches