Message ID | 20200914031634.190721-3-wright.feng@cypress.com |
---|---|
State | New |
Headers | show |
Series | brcmfmac: SoftAP creation and dcmd buffer size changes | expand |
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 --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);