Skip to contentSkip to conversation

brcmfmac: return -EPERM when getting error in vendor command handler

PATCH In review Wright Feng 2 participants 3 messages Sep 4, 2017 – Sep 29, 2017 Broadcom BRCM80211 IEEE802.11 wireless drivers

lore
Share & tools
lore

Wright Feng

author
#0 lore raw

PATCH 1/1 brcmfmac: return -EPERM when getting error in vendor command handler

Firmware returns proprietary error code when getting error in fil_cmd_data_set or fil_cmd_data_get. The vendor tools or utilities which uses libnl may stuck in some commands when wl is down. For example, issue "scan" command after issuing "down" command, the "scan" command will be the blocking call and stuck as no response from libnl. It is caused by that firmware returns BCME_NOTUP(-4) when wl is down, but the -4 is -EINTR in Linux kernel, so libnl catches the error and not passes to upper layer.
Because of that, the driver should return Linux error code instead of the proprietary error code, and the tools or utilities need to get the real firmware error code by command "bcmerror" or "bcmerrorstr" after receiving the error.

Signed-off-by Wright Feng <[email protected]>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c +5 -1
index 8eff275..2b88ba1 100644
@@ -80,8 +80,12 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
 	else
 		ret = brcmf_fil_cmd_data_get(ifp, cmdhdr->cmd, dcmd_buf,
 					     ret_len);
-	if (ret != 0)
+
+	if (ret != 0) {
+		brcmf_dbg(INFO, "error(%d), return -EPERM\n", ret);
+		ret = -EPERM;
 		goto exit;
+	}
 
 	wr_pointer = dcmd_buf;
 	while (ret_len > 0) {
Signature · 1 line
1.9.1
#1 lore raw
Quoted 23 lines · Wright Feng

Firmware returns proprietary error code when getting error in fil_cmd_data_set or fil_cmd_data_get. The vendor tools or utilities which uses libnl may stuck in some commands when wl is down. For example, issue "scan" command after issuing "down" command, the "scan" command will be the blocking call and stuck as no response from libnl. It is caused by that firmware returns BCME_NOTUP(-4) when wl is down, but the -4 is -EINTR in Linux kernel, so libnl catches the error and not passes to upper layer.
Because of that, the driver should return Linux error code instead of the proprietary error code, and the tools or utilities need to get the real firmware error code by command "bcmerror" or "bcmerrorstr" after receiving the error.

Signed-off-by Wright Feng <[email protected]>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c +0 -0
index 8eff275..2b88ba1 100644

drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy, · line 88

Quoted 10 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c +5 -1
@@ -80,8 +80,12 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
        else
                ret = brcmf_fil_cmd_data_get(ifp, cmdhdr->cmd, dcmd_buf,
                                             ret_len);
-       if (ret != 0)
+
+       if (ret != 0) {
+               brcmf_dbg(INFO, "error(%d), return -EPERM\n", ret);
+               ret = -EPERM;
                goto exit;
+       }

It would be better to handle the conversion in brcmf_fil_cmd_data so everyone can benefit. Also please try to assign an appropriate error code when possible to provide more information to the upper layer. For this case "ENETDOWN" seems to be a better fit for BCME_NOTUP.

Thanks,
Franky

Quoted 5 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c +0 -1
        wr_pointer = dcmd_buf;
        while (ret_len > 0) {
--

1.9.1

author
#2 lore raw

Hi Franky,

Quoted 29 lines · Franky Lin
Quoted 28 lines · Wright Feng

Firmware returns proprietary error code when getting error in fil_cmd_data_set or fil_cmd_data_get. The vendor tools or utilities which uses libnl may stuck in some commands when wl is down. For example, issue "scan" command after issuing "down" command, the "scan" command will be the blocking call and stuck as no response from libnl. It is caused by that firmware returns BCME_NOTUP(-4) when wl is down, but the -4 is -EINTR in Linux kernel, so libnl catches the error and not passes to upper layer.
Because of that, the driver should return Linux error code instead of the proprietary error code, and the tools or utilities need to get the real firmware error code by command "bcmerror" or "bcmerrorstr" after receiving the error.

Signed-off-by Wright Feng <[email protected]>
---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c +1 -1
index 8eff275..2b88ba1 100644
@@ -80,8 +80,12 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
         else
                 ret = brcmf_fil_cmd_data_get(ifp, cmdhdr->cmd, dcmd_buf,
                                              ret_len);
-       if (ret != 0)
+
Quoted 8 lines

drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy, · line 88

Quoted 5 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c +4 -0
+       if (ret != 0) {
+               brcmf_dbg(INFO, "error(%d), return -EPERM\n", ret);
+               ret = -EPERM;
                 goto exit;
+       }

It would be better to handle the conversion in brcmf_fil_cmd_data so everyone can benefit. Also please try to assign an appropriate error

Thanks for the suggestion and sorry for replying the mail lately.(We got some mail server issue a couple days ago.)

I will move the conversion to brcmf_proto_bcdc_query_dcmd/brcmf_proto_bcdc_set_dcmd for bcdc and
brcmf_msgbuf_query_dcmd for msgbuf.

Quoted 2 lines

code when possible to provide more information to the upper layer. For this case "ENETDOWN" seems to be a better fit for BCME_NOTUP.

One more thing I need your suggestion.
Some of firmware error codes do not have proper error code mapping to Linux, like "BCME_NOTUP", "BCME_ERROR", "BCME_NOTAP", "BCME_NOTSTA", "BCME_ASSOCIATED"... Is that okay using -EIO for those errors which do have proper mapping error code? Or do you know the error code for the generic error?

Regards,
Wright

Quoted 9 lines

Thanks,
Franky

Quoted 5 lines
drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c +0 -1
         wr_pointer = dcmd_buf;
         while (ret_len > 0) {
--

1.9.1