[PATCH 1/5] wilc1000: increase firmware version array size

[PATCH 1/5] wilc1000: increase firmware version array size

[Ajay.Kathat]

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

Increase firmware version array size to hold complete version information.
The firmware commit id(Build:) information is also part of the firmware
version string.

Firmware version format:
WILC_WIFI_FW_REL_XX_XX Build: XXXXX

e.g.
WILC_WIFI_FW_REL_15_6 Build: 12804

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/net/wireless/microchip/wilc1000/netdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index 643bddaae32a..3c292e3464c2 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -14,6 +14,7 @@
 #include "wlan_cfg.h"
 
 #define WILC_MULTICAST_TABLE_SIZE	8
+#define WILC_MAX_FW_VERSION_STR_SIZE	50
 
 /* latest API version supported */
 #define WILC1000_API_VER		1
@@ -522,7 +523,7 @@ static int wilc_wlan_initialize(struct net_device *dev, struct wilc_vif *vif)
 
 		if (wilc_wlan_cfg_get(vif, 1, WID_FIRMWARE_VERSION, 1, 0)) {
 			int size;
-			char firmware_ver[20];
+			char firmware_ver[WILC_MAX_FW_VERSION_STR_SIZE];
 
 			size = wilc_wlan_cfg_get_val(wl, WID_FIRMWARE_VERSION,
 						     firmware_ver,
-- 
2.25.1

[PATCH 2/5] wilc1000: use fixed function base register value to access SDIO_FBR_ENABLE_CSA

[Ajay.Kathat]

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

The function number was not correct(reset to 0) when host resumes from
suspend state. Use hardcoded value in function base information
register(FBR base address) to re-initialize correctly on host resume.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/net/wireless/microchip/wilc1000/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
index ec595dbd8959..7962c11cfe84 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -598,7 +598,7 @@ static int wilc_sdio_init(struct wilc *wilc, bool resume)
 	cmd.read_write = 1;
 	cmd.function = 0;
 	cmd.raw = 1;
-	cmd.address = SDIO_FBR_BASE(func->num);
+	cmd.address = SDIO_FBR_BASE(1);
 	cmd.data = SDIO_FBR_ENABLE_CSA;
 	ret = wilc_sdio_cmd52(wilc, &cmd);
 	if (ret) {
-- 
2.25.1

[PATCH 4/5] wilc1000: use 'u64' datatype for cookie variable

[Ajay.Kathat]

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

Use 'u64' instead of 'u32' for the cookie variable as expected by cfg80211
callback function argument.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/net/wireless/microchip/wilc1000/hif.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/hif.h b/drivers/net/wireless/microchip/wilc1000/hif.h
index cccd54ed0518..77616fc77575 100644
--- a/drivers/net/wireless/microchip/wilc1000/hif.h
+++ b/drivers/net/wireless/microchip/wilc1000/hif.h
@@ -123,7 +123,7 @@ struct wilc_remain_ch {
 	u32 duration;
 	void (*expired)(void *priv, u64 cookie);
 	void *arg;
-	u32 cookie;
+	u64 cookie;
 };
 
 struct wilc;
-- 
2.25.1

[PATCH 3/5] wilc1000: fix crash observed in AP mode with cfg80211_register_netdevice()

[Ajay.Kathat]

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

Monitor(mon.) interface is used for handling the AP mode and 'ieee80211_ptr'
reference is not getting set for it. Like earlier implementation,
use register_netdevice() instead of cfg80211_register_netdevice() which
expects valid 'ieee80211_ptr' reference to avoid the possible crash.

Fixes: 2fe8ef106238 ("cfg80211: change netdev registration/unregistration semantics")
Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/net/wireless/microchip/wilc1000/mon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/mon.c b/drivers/net/wireless/microchip/wilc1000/mon.c
index 6bd63934c2d8..b5a1b65c087c 100644
--- a/drivers/net/wireless/microchip/wilc1000/mon.c
+++ b/drivers/net/wireless/microchip/wilc1000/mon.c
@@ -233,7 +233,7 @@ struct net_device *wilc_wfi_init_mon_interface(struct wilc *wl,
 	wl->monitor_dev->netdev_ops = &wilc_wfi_netdev_ops;
 	wl->monitor_dev->needs_free_netdev = true;
 
-	if (cfg80211_register_netdevice(wl->monitor_dev)) {
+	if (register_netdevice(wl->monitor_dev)) {
 		netdev_err(real_dev, "register_netdevice failed\n");
 		free_netdev(wl->monitor_dev);
 		return NULL;
@@ -251,7 +251,7 @@ void wilc_wfi_deinit_mon_interface(struct wilc *wl, bool rtnl_locked)
 		return;
 
 	if (rtnl_locked)
-		cfg80211_unregister_netdevice(wl->monitor_dev);
+		unregister_netdevice(wl->monitor_dev);
 	else
 		unregister_netdev(wl->monitor_dev);
 	wl->monitor_dev = NULL;
-- 
2.25.1

[PATCH 5/5] wilc1000: add valid vmm_entry check before fetching from TX queue

[Ajay.Kathat]

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

'vmm_table' array contains the size of data buffer length including host
header length. In 'vmm_table' array, the Zero value means the end of
vmm_entries that needs to transfer to firmware which is calculated based on
VMM free size in firmware.

Use 'vmm_table' valid entry check before fetching the entry from TX queue to
only copy valid number of entries to avoid possible NULL pointer exception
observed sometimes during large file transfers.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/net/wireless/microchip/wilc1000/wlan.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
index fb5633a05fd5..48441f0389ca 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -875,14 +875,15 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
 		char *bssid;
 		u8 mgmt_ptk = 0;
 
+		if (vmm_table[i] == 0 || vmm_entries_ac[i] >= NQUEUES)
+			break;
+
 		tqe = wilc_wlan_txq_remove_from_head(wilc, vmm_entries_ac[i]);
-		ac_pkt_num_to_chip[vmm_entries_ac[i]]++;
 		if (!tqe)
 			break;
 
+		ac_pkt_num_to_chip[vmm_entries_ac[i]]++;
 		vif = tqe->vif;
-		if (vmm_table[i] == 0)
-			break;
 
 		le32_to_cpus(&vmm_table[i]);
 		vmm_sz = FIELD_GET(WILC_VMM_BUFFER_SIZE, vmm_table[i]);
-- 
2.25.1

Re: [PATCH 3/5] wilc1000: fix crash observed in AP mode with cfg80211_register_netdevice()

[Kalle Valo]

<Ajay.Kathat@microchip.com> writes:

> From: Ajay Singh <ajay.kathat@microchip.com>
>
> Monitor(mon.) interface is used for handling the AP mode and 'ieee80211_ptr'
> reference is not getting set for it. Like earlier implementation,
> use register_netdevice() instead of cfg80211_register_netdevice() which
> expects valid 'ieee80211_ptr' reference to avoid the possible crash.
>
> Fixes: 2fe8ef106238 ("cfg80211: change netdev registration/unregistration semantics")
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>

Is there a reason why wilc1000 does not use ieee8011_ptr? And what
earlier implementation are you referring to?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 3/5] wilc1000: fix crash observed in AP mode with cfg80211_register_netdevice()

[Ajay.Kathat]

On 09/05/22 17:20, Kalle Valo wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> <Ajay.Kathat@microchip.com> writes:
>
>> From: Ajay Singh <ajay.kathat@microchip.com>
>>
>> Monitor(mon.) interface is used for handling the AP mode and 'ieee80211_ptr'
>> reference is not getting set for it. Like earlier implementation,
>> use register_netdevice() instead of cfg80211_register_netdevice() which
>> expects valid 'ieee80211_ptr' reference to avoid the possible crash.
>>
>> Fixes: 2fe8ef106238 ("cfg80211: change netdev registration/unregistration semantics")
>> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> Is there a reason why wilc1000 does not use ieee8011_ptr? And what
> earlier implementation are you referring to?


As I understand, in WILC "ieee80211_ptr" is used for the station/AP 
interface but not to configure the monitor interface which gets 
additionally added in AP mode. In AP mode, for an interface( wlan0 ), an 
additional monitor interface( mon.wlan0 ) gets created. A netdevice is 
register for monitor interface(mon.wlan0) to transmit/receive frames 
from/to hostapd. That interface doesn't explicitly set up 
'ieee80211_ptr' because the original interface(wlan0) uses it, so using 
"cfg80211_register/unregister" API's for mon.wlan0 interfaces fails.

For AP monitor interface, the earlier implementation was using 
register_netdevice()/unregister_netdevice() API which doesn't depends on 
'ieee80211_ptr' for netdevice registration so retained the previous API.


Regards,
Ajay

Re: [PATCH 3/5] wilc1000: fix crash observed in AP mode with cfg80211_register_netdevice()

[Johannes Berg]

On Mon, 2022-05-09 at 13:49 +0000, Ajay.Kathat@microchip.com wrote:
> 
> As I understand, in WILC "ieee80211_ptr" is used for the station/AP 
> interface but not to configure the monitor interface which gets 
> additionally added in AP mode. In AP mode, for an interface( wlan0 ), an 
> additional monitor interface( mon.wlan0 ) gets created. A netdevice is 
> register for monitor interface(mon.wlan0) to transmit/receive frames 
> from/to hostapd. That interface doesn't explicitly set up 
> 'ieee80211_ptr' because the original interface(wlan0) uses it, so using 
> "cfg80211_register/unregister" API's for mon.wlan0 interfaces fails.

Btw, this probably should just be removed in favour of using the nl80211
based APIs for hostapd ...

But I think as a quick bugfix goes this is fine, but it'd be better to
remove this and either allow hostapd to create the interface with the
proper ieee80211_ptr, or to just use the nl80211-based TX/RX.

johannes

Re: [PATCH 3/5] wilc1000: fix crash observed in AP mode with cfg80211_register_netdevice()

[Ajay.Kathat]

On 10/05/22 01:14, Johannes Berg wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Mon, 2022-05-09 at 13:49 +0000, Ajay.Kathat@microchip.com wrote:
>> As I understand, in WILC "ieee80211_ptr" is used for the station/AP
>> interface but not to configure the monitor interface which gets
>> additionally added in AP mode. In AP mode, for an interface( wlan0 ), an
>> additional monitor interface( mon.wlan0 ) gets created. A netdevice is
>> register for monitor interface(mon.wlan0) to transmit/receive frames
>> from/to hostapd. That interface doesn't explicitly set up
>> 'ieee80211_ptr' because the original interface(wlan0) uses it, so using
>> "cfg80211_register/unregister" API's for mon.wlan0 interfaces fails.
> Btw, this probably should just be removed in favour of using the nl80211
> based APIs for hostapd ...
>
> But I think as a quick bugfix goes this is fine, but it'd be better to
> remove this and either allow hostapd to create the interface with the
> proper ieee80211_ptr, or to just use the nl80211-based TX/RX.


Thanks Johannes. Yes, this patch would help to make the AP mode work.
I will check on using nl80211 based APIs for hostapd but might take some 
time to change.

Regards,
Ajay

Re: [PATCH 3/5] wilc1000: fix crash observed in AP mode with cfg80211_register_netdevice()

[Kalle Valo]

<Ajay.Kathat@microchip.com> writes:

> On 10/05/22 01:14, Johannes Berg wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On Mon, 2022-05-09 at 13:49 +0000, Ajay.Kathat@microchip.com wrote:
>>> As I understand, in WILC "ieee80211_ptr" is used for the station/AP
>>> interface but not to configure the monitor interface which gets
>>> additionally added in AP mode. In AP mode, for an interface( wlan0 ), an
>>> additional monitor interface( mon.wlan0 ) gets created. A netdevice is
>>> register for monitor interface(mon.wlan0) to transmit/receive frames
>>> from/to hostapd. That interface doesn't explicitly set up
>>> 'ieee80211_ptr' because the original interface(wlan0) uses it, so using
>>> "cfg80211_register/unregister" API's for mon.wlan0 interfaces fails.
>> Btw, this probably should just be removed in favour of using the nl80211
>> based APIs for hostapd ...
>>
>> But I think as a quick bugfix goes this is fine, but it'd be better to
>> remove this and either allow hostapd to create the interface with the
>> proper ieee80211_ptr, or to just use the nl80211-based TX/RX.
>
>
> Thanks Johannes. Yes, this patch would help to make the AP mode work.
> I will check on using nl80211 based APIs for hostapd but might take some 
> time to change.

Ok, I'll take this as a quick fix but please switch to using proper
interfaces.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 1/5] wilc1000: increase firmware version array size

[Kalle Valo]

<Ajay.Kathat@microchip.com> wrote:

> From: Ajay Singh <ajay.kathat@microchip.com>
> 
> Increase firmware version array size to hold complete version information.
> The firmware commit id(Build:) information is also part of the firmware
> version string.
> 
> Firmware version format:
> WILC_WIFI_FW_REL_XX_XX Build: XXXXX
> 
> e.g.
> WILC_WIFI_FW_REL_15_6 Build: 12804
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>

5 patches applied to wireless-next.git, thanks.

4ee8a915730f wilc1000: increase firmware version array size
72ebd6751f9e wilc1000: use fixed function base register value to access SDIO_FBR_ENABLE_CSA
868f0e28290c wilc1000: fix crash observed in AP mode with cfg80211_register_netdevice()
819b161b9487 wilc1000: use 'u64' datatype for cookie variable
62296b3e19dd wilc1000: add valid vmm_entry check before fetching from TX queue

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20220504161924.2146601-1-ajay.kathat@microchip.com/

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