[PATCH 0/5] wifi: wilc1000: minor fixes

[PATCH 0/5] wifi: wilc1000: minor fixes

[Alexis Lothoré]

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

[PATCH 1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

[Alexis Lothoré]

From: Ajay Singh <ajay.kathat@microchip.com>

Changed the default value preamble to WILC_FW_PREAMBLE_AUTO in
wilc_init_fw_config().

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 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 91d71e0f7ef2..8923eb64c964 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -284,7 +284,7 @@ static int wilc_init_fw_config(struct net_device *dev, struct wilc_vif *vif)
 	if (!wilc_wlan_cfg_set(vif, 0, WID_11G_OPERATING_MODE, &b, 1, 0, 0))
 		goto fail;
 
-	b = WILC_FW_PREAMBLE_SHORT;
+	b = WILC_FW_PREAMBLE_AUTO;
 	if (!wilc_wlan_cfg_set(vif, 0, WID_PREAMBLE, &b, 1, 0, 0))
 		goto fail;
 

-- 
2.42.1

[PATCH 2/5] wifi: wilc1000: fix driver_handler when committing initial configuration

[Alexis Lothoré]

From: Ajay Singh <ajay.kathat@microchip.com>

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 <ajay.kathat@microchip.com>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 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

[PATCH 3/5] wilc: wifi: do not realloc workqueue everytime an interface is added

[Alexis Lothoré]

From: Ajay Singh <ajay.kathat@microchip.com>

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 <ajay.kathat@microchip.com>
Co-developed-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
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

[PATCH 4/5] wifi: wilc1000: fix incorrect power down sequence

[Alexis Lothoré]

From: Ajay Singh <ajay.kathat@microchip.com>

Use the correct register configuration when the WILC chip is down so the
successive interface up operation is successful. The modified registers
values during chip down helps to avoid the "FW not responding" debug
message which sometimes occurs because of temporary bus communication
failure during the next start. Also, make sure on first communication with
the chip that it is indeed woken up.

Reported-by: Michael Walle <mwalle@kernel.org>
Closes: https://lore.kernel.org/linux-wireless/20221026085415.6jgwrhq4sunqaypm@0002.3ffe.de/
Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 drivers/net/wireless/microchip/wilc1000/wlan.c | 33 +++++++++++++++-----------
 drivers/net/wireless/microchip/wilc1000/wlan.h |  6 +++++
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
index 9eb115c79c90..6b2f2269ddf8 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -1198,27 +1198,32 @@ int wilc_wlan_stop(struct wilc *wilc, struct wilc_vif *vif)
 
 	acquire_bus(wilc, WILC_BUS_ACQUIRE_AND_WAKEUP);
 
-	ret = wilc->hif_func->hif_read_reg(wilc, WILC_GP_REG_0, &reg);
-	if (ret) {
-		netdev_err(vif->ndev, "Error while reading reg\n");
+	ret = wilc->hif_func->hif_read_reg(wilc, GLOBAL_MODE_CONTROL, &reg);
+	if (ret)
 		goto release;
-	}
 
-	ret = wilc->hif_func->hif_write_reg(wilc, WILC_GP_REG_0,
-					(reg | WILC_ABORT_REQ_BIT));
-	if (ret) {
-		netdev_err(vif->ndev, "Error while writing reg\n");
+	reg &= ~WILC_GLOBAL_MODE_ENABLE_WIFI;
+	ret = wilc->hif_func->hif_write_reg(wilc, GLOBAL_MODE_CONTROL, reg);
+	if (ret)
+		goto release;
+
+	ret = wilc->hif_func->hif_read_reg(wilc, PWR_SEQ_MISC_CTRL, &reg);
+	if (ret)
+		goto release;
+
+	reg &= ~WILC_PWR_SEQ_ENABLE_WIFI_SLEEP;
+	ret = wilc->hif_func->hif_write_reg(wilc, PWR_SEQ_MISC_CTRL, reg);
+	if (ret)
 		goto release;
-	}
 
-	ret = wilc->hif_func->hif_read_reg(wilc, WILC_FW_HOST_COMM, &reg);
+	ret = wilc->hif_func->hif_read_reg(wilc, WILC_GP_REG_0, &reg);
 	if (ret) {
 		netdev_err(vif->ndev, "Error while reading reg\n");
 		goto release;
 	}
-	reg = BIT(0);
 
-	ret = wilc->hif_func->hif_write_reg(wilc, WILC_FW_HOST_COMM, reg);
+	ret = wilc->hif_func->hif_write_reg(wilc, WILC_GP_REG_0,
+					(reg | WILC_ABORT_REQ_BIT));
 	if (ret) {
 		netdev_err(vif->ndev, "Error while writing reg\n");
 		goto release;
@@ -1410,7 +1415,7 @@ static int init_chip(struct net_device *dev)
 	struct wilc_vif *vif = netdev_priv(dev);
 	struct wilc *wilc = vif->wilc;
 
-	acquire_bus(wilc, WILC_BUS_ACQUIRE_ONLY);
+	acquire_bus(wilc, WILC_BUS_ACQUIRE_AND_WAKEUP);
 
 	chipid = wilc_get_chipid(wilc, true);
 
@@ -1440,7 +1445,7 @@ static int init_chip(struct net_device *dev)
 	}
 
 release:
-	release_bus(wilc, WILC_BUS_RELEASE_ONLY);
+	release_bus(wilc, WILC_BUS_RELEASE_ALLOW_SLEEP);
 
 	return ret;
 }
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.h b/drivers/net/wireless/microchip/wilc1000/wlan.h
index a72cd5cac81d..f02775f7e41f 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.h
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.h
@@ -156,6 +156,12 @@
 #define WILC_GP_REG_0			0x149c
 #define WILC_GP_REG_1			0x14a0
 
+#define GLOBAL_MODE_CONTROL		0x1614
+#define PWR_SEQ_MISC_CTRL		0x3008
+
+#define WILC_GLOBAL_MODE_ENABLE_WIFI	BIT(0)
+#define WILC_PWR_SEQ_ENABLE_WIFI_SLEEP	BIT(28)
+
 #define WILC_HAVE_SDIO_IRQ_GPIO		BIT(0)
 #define WILC_HAVE_USE_PMU		BIT(1)
 #define WILC_HAVE_SLEEP_CLK_SRC_RTC	BIT(2)

-- 
2.42.1

[PATCH 5/5] wifi: wilc1000: fix multi-vif management when deleting a vif

[Alexis Lothoré]

From: Ajay Singh <ajay.kathat@microchip.com>

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 <ajay.kathat@microchip.com>
Co-developed-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
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

Re: [PATCH 1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

[Kalle Valo]

Alexis Lothoré <alexis.lothore@bootlin.com> wrote:

> From: Ajay Singh <ajay.kathat@microchip.com>
> 
> Changed the default value preamble to WILC_FW_PREAMBLE_AUTO in
> wilc_init_fw_config().
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>

The commit message should always answer to the question "Why?". I can add that
if you tell me what to add.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240115-wilc_1000_fixes-v1-1-54d29463a738@bootlin.com/

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

Re: [PATCH 2/5] wifi: wilc1000: fix driver_handler when committing initial configuration

[Kalle Valo]

Alexis Lothoré <alexis.lothore@bootlin.com> wrote:

> From: Ajay Singh <ajay.kathat@microchip.com>
> 
> 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 <ajay.kathat@microchip.com>
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>

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/20240115-wilc_1000_fixes-v1-2-54d29463a738@bootlin.com/

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

Re: [PATCH 1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

[Alexis Lothoré]

Hi Kalle,

On 1/18/24 10:31, Kalle Valo wrote:
> Alexis Lothoré <alexis.lothore@bootlin.com> wrote:
> 
>> From: Ajay Singh <ajay.kathat@microchip.com>
>>
>> Changed the default value preamble to WILC_FW_PREAMBLE_AUTO in
>> wilc_init_fw_config().
>>
>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
>> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> The commit message should always answer to the question "Why?". I can add that
> if you tell me what to add.

Yeah, sorry for the lack of description, I may have forgotten to update this
one. I suggest to update it with the following message:

"WILC driver currently applies some default configuration whenever the firmware
is initialized, and sets the default preamble size to short. However, despite
this passed option, firmware is also able to successfully connect to access
points only using long preamble, so this setting does not really enforce short
preambles and is misleading regarding applied configuration.

Update default configuration and make it match the firmware behavior by passing
the existing WILC_FW_PREAMBLE_AUTO value (2 instead of 0). The updated setting
does not really alter firmware behavior since it is still capable to connect to
both short preamble and long preamble access points, but at list the setting now
expresses for real the corresponding firmware behavior"

To give a bit of context around this one: I do not have access to the firmware
internals, I just took the patch from Ajay and I merely did some tests around it
with multiple APs (basically, making a WILC STA connect and ping the AP), and
ensured with wireshark to get at least one AP be really "locked" with long
preamble, with WILC managing to connect to it.

Thanks,

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

[Ajay.Kathat]

On 1/18/24 08:08, Alexis Lothoré wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Kalle,
> 
> On 1/18/24 10:31, Kalle Valo wrote:
>> Alexis Lothoré <alexis.lothore@bootlin.com> wrote:
>>
>>> From: Ajay Singh <ajay.kathat@microchip.com>
>>>
>>> Changed the default value preamble to WILC_FW_PREAMBLE_AUTO in
>>> wilc_init_fw_config().
>>>
>>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
>>> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
>>
>> The commit message should always answer to the question "Why?". I can add that
>> if you tell me what to add.
> 
> Yeah, sorry for the lack of description, I may have forgotten to update this
> one. I suggest to update it with the following message:
> 
> "WILC driver currently applies some default configuration whenever the firmware
> is initialized, and sets the default preamble size to short. However, despite
> this passed option, firmware is also able to successfully connect to access
> points only using long preamble, so this setting does not really enforce short
> preambles and is misleading regarding applied configuration.
> 
> Update default configuration and make it match the firmware behavior by passing
> the existing WILC_FW_PREAMBLE_AUTO value (2 instead of 0). The updated setting
> does not really alter firmware behavior since it is still capable to connect to
> both short preamble and long preamble access points, but at list the setting now
> expresses for real the corresponding firmware behavior"
> 
> To give a bit of context around this one: I do not have access to the firmware
> internals, I just took the patch from Ajay and I merely did some tests around it
> with multiple APs (basically, making a WILC STA connect and ping the AP), and
> ensured with wireshark to get at least one AP be really "locked" with long
> preamble, with WILC managing to connect to it.
> 

Here are some more details about this change. It have been implemented
to address the transmission(Tx) blackout issue observed in the 802.11b
mode. The modification has no impact on the other modes, which will
continue to work as they did in the previous implementation. This change
will allow the 802.11b transmission(2, 5.5, 11Mbps) to use long preamble.


Regards,
Ajay

Re: [PATCH 1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

[Alexis Lothoré]

On 1/18/24 17:52, Ajay.Kathat@microchip.com wrote:
> On 1/18/24 08:08, Alexis Lothoré wrote:

[...]

>> "WILC driver currently applies some default configuration whenever the firmware
>> is initialized, and sets the default preamble size to short. However, despite
>> this passed option, firmware is also able to successfully connect to access
>> points only using long preamble, so this setting does not really enforce short
>> preambles and is misleading regarding applied configuration.
>>
>> Update default configuration and make it match the firmware behavior by passing
>> the existing WILC_FW_PREAMBLE_AUTO value (2 instead of 0). The updated setting
>> does not really alter firmware behavior since it is still capable to connect to
>> both short preamble and long preamble access points, but at list the setting now
>> expresses for real the corresponding firmware behavior"
>>
>> To give a bit of context around this one: I do not have access to the firmware
>> internals, I just took the patch from Ajay and I merely did some tests around it
>> with multiple APs (basically, making a WILC STA connect and ping the AP), and
>> ensured with wireshark to get at least one AP be really "locked" with long
>> preamble, with WILC managing to connect to it.
>>
> 
> Here are some more details about this change. It have been implemented
> to address the transmission(Tx) blackout issue observed in the 802.11b
> mode. The modification has no impact on the other modes, which will
> continue to work as they did in the previous implementation. This change
> will allow the 802.11b transmission(2, 5.5, 11Mbps) to use long preamble.

Ah, so it fixes a specific bug (and makes parts of my suggested description
wrong). Has there been any report about this "TX blackout issue" on the mailing
lists ? If so, we could enrich the message with some details from the report and
add some missing Reported-By/Fixes/Closes tags.

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

[Ajay.Kathat]

Hi Alexis,

On 1/19/24 00:43, Alexis Lothoré wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 1/18/24 17:52, Ajay.Kathat@microchip.com wrote:
>> On 1/18/24 08:08, Alexis Lothoré wrote:
> 
> [...]
> 
>>> "WILC driver currently applies some default configuration whenever the firmware
>>> is initialized, and sets the default preamble size to short. However, despite
>>> this passed option, firmware is also able to successfully connect to access
>>> points only using long preamble, so this setting does not really enforce short
>>> preambles and is misleading regarding applied configuration.
>>>
>>> Update default configuration and make it match the firmware behavior by passing
>>> the existing WILC_FW_PREAMBLE_AUTO value (2 instead of 0). The updated setting
>>> does not really alter firmware behavior since it is still capable to connect to
>>> both short preamble and long preamble access points, but at list the setting now
>>> expresses for real the corresponding firmware behavior"
>>>
>>> To give a bit of context around this one: I do not have access to the firmware
>>> internals, I just took the patch from Ajay and I merely did some tests around it
>>> with multiple APs (basically, making a WILC STA connect and ping the AP), and
>>> ensured with wireshark to get at least one AP be really "locked" with long
>>> preamble, with WILC managing to connect to it.
>>>
>>
>> Here are some more details about this change. It have been implemented
>> to address the transmission(Tx) blackout issue observed in the 802.11b
>> mode. The modification has no impact on the other modes, which will
>> continue to work as they did in the previous implementation. This change
>> will allow the 802.11b transmission(2, 5.5, 11Mbps) to use long preamble.
> 
> Ah, so it fixes a specific bug (and makes parts of my suggested description
> wrong). Has there been any report about this "TX blackout issue" on the mailing
> lists ? If so, we could enrich the message with some details from the report and
> add some missing Reported-By/Fixes/Closes tags.
> 

For this issue, there are no details on the mailing lists. It was
discovered by internal QA team.

Regards,
Ajay

Re: [1/5] wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

[Kalle Valo]

Alexis Lothoré <alexis.lothore@bootlin.com> wrote:

> From: Ajay Singh <ajay.kathat@microchip.com>
> 
> WILC driver currently applies some default configuration whenever the firmware
> is initialized, and sets the default preamble size to short. However, despite
> this passed option, firmware is also able to successfully connect to access
> points only using long preamble, so this setting does not really enforce short
> preambles and is misleading regarding applied configuration.
> 
> Update default configuration and make it match the firmware behavior by passing
> the existing WILC_FW_PREAMBLE_AUTO value (2 instead of 0). The updated setting
> does not really alter firmware behavior since it is still capable to connect to
> both short preamble and long preamble access points, but at list the setting now
> expresses for real the corresponding firmware behavior.
> 
> More info: it has been implemented to address the transmission (Tx) blackout
> issue observed in the 802.11b mode. The modification has no impact on the other
> modes, which will continue to work as they did in the previous implementation.
> This change will allow the 802.11b transmission (2, 5.5, 11Mbps) to use long
> preamble.
> 
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>

Patch applied to wireless-next.git, thanks.

a8e5fefa9123 wifi: wilc1000: set preamble size to auto as default in wilc_init_fw_config()

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240115-wilc_1000_fixes-v1-1-54d29463a738@bootlin.com/

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