diff mbox series

[v2,2/3] brcmfmac: set net carrier on via test tool for AP mode

Message ID 20200914031634.190721-3-wright.feng@cypress.com
State New
Headers show
Series brcmfmac: SoftAP creation and dcmd buffer size changes | expand

Commit Message

Wright Feng Sept. 14, 2020, 3:16 a.m. UTC 
From: Kurt Lee <kurt.lee@cypress.com>

In manufacturing line, test tool may be used to enable SoftAP. Such
SoftAP can't pass traffic because netif carrier is off by default. To
allow such use case, let brcmfmac parse ioctl cmd, and then set iftype
to ap mode and report netif_carrier_on to upper layer.

Signed-off-by: Kurt Lee <kurt.lee@cypress.com>
Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/vendor.c    | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Arend Van Spriel Sept. 14, 2020, 8:42 a.m. UTC | #1 
On 9/14/2020 10:20 AM, Kalle Valo wrote:
> Wright Feng <wright.feng@cypress.com> writes:
> 
>> From: Kurt Lee <kurt.lee@cypress.com>
>>
>> In manufacturing line, test tool may be used to enable SoftAP. Such
>> SoftAP can't pass traffic because netif carrier is off by default. To
>> allow such use case, let brcmfmac parse ioctl cmd, and then set iftype
>> to ap mode and report netif_carrier_on to upper layer.
> 
> nl80211 does not use ioctl(), so the commit log is misleading.
> 
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
>> @@ -64,6 +64,15 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
>>   		*(char *)(dcmd_buf + len)  = '\0';
>>   	}
>>   
>> +	if (cmdhdr->cmd == BRCMF_C_SET_AP) {
>> +		if (*(int *)(dcmd_buf) == 1) {
>> +			ifp->vif->wdev.iftype = NL80211_IFTYPE_AP;
>> +			brcmf_net_setcarrier(ifp, true);
>> +		} else {
>> +			ifp->vif->wdev.iftype = NL80211_IFTYPE_STATION;
>> +		}
>> +	}
> 
> We now have rules for the vendor API:
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/nl80211#vendor-specific_api
> 
> As the BRCMF_VNDR_CMDS_DCMD just seems to be a simple pipe between the
> user space and the firmware I'm leaning towards that we should just
> remove support for that command from brcmfmac.
> 
> And besides, for factory tests you should be using NL80211_CMD_TESTMODE.
> The vendor API is meant for normal mode usage.

We talked about this API before. This command used to be a TESTMODE 
command, but with the introduction of vendor commands in cfg80211 I 
changed that. My reasons being that 1) it helps me in cfg80211 callback 
development, and 2) (some of) our customers were not accepting the need 
for a separate driver (build) for manufacturing. My intent has always 
been to keep this opaque for the driver so I am opposed to patches like 
this that are interpreting the opaque blob. The firmware could simply 
generate a CARRIER event and the driver can act on that.

Regards,
Arend
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
index d07e7c7355d9..5edf5ac1167a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
@@ -64,6 +64,15 @@  static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
 		*(char *)(dcmd_buf + len)  = '\0';
 	}
 
+	if (cmdhdr->cmd == BRCMF_C_SET_AP) {
+		if (*(int *)(dcmd_buf) == 1) {
+			ifp->vif->wdev.iftype = NL80211_IFTYPE_AP;
+			brcmf_net_setcarrier(ifp, true);
+		} else {
+			ifp->vif->wdev.iftype = NL80211_IFTYPE_STATION;
+		}
+	}
+
 	if (cmdhdr->set)
 		ret = brcmf_fil_cmd_data_set(ifp, cmdhdr->cmd, dcmd_buf,
 					     ret_len);