2019-02-21 10:33:34

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH wireless-drivers-next 1/2] brcmfmac: fix size of the struct msgbuf_ring_status

From: Rafał Miłecki <[email protected]>

This updates host struct to match the in-firmawre definition. It's a
cosmetic change as it only applies to the reserved struct space.

Fixes: c988b78244df ("brcmfmac: print firmware reported ring status errors")
Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
index aef2d4092872..d711dc8ed606 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -139,7 +139,7 @@ struct msgbuf_ring_status {
struct msgbuf_common_hdr msg;
struct msgbuf_completion_hdr compl_hdr;
__le16 write_idx;
- __le32 rsvd0[5];
+ __le16 rsvd0[5];
};

struct msgbuf_rx_event {
--
2.20.1



2019-02-21 10:33:39

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH wireless-drivers-next 2/2] brcmfmac: print firmware reported general status errors

From: Rafał Miłecki <[email protected]>

Firmware may report general errors using a special message type. Add
basic support for it by simply decoding & printing an error number.

A sample situation in which firmware reports a buf error:
CONSOLE: 027084.733 no host response IOCTL buffer available..so fail the request
will now produce a "Firmware reported general error: 9" on the host.

Signed-off-by: Rafał Miłecki <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/msgbuf.c | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
index d711dc8ed606..d3780eae7f19 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -134,6 +134,14 @@ struct msgbuf_completion_hdr {
__le16 flow_ring_id;
};

+/* Data struct for the MSGBUF_TYPE_GEN_STATUS */
+struct msgbuf_gen_status {
+ struct msgbuf_common_hdr msg;
+ struct msgbuf_completion_hdr compl_hdr;
+ __le16 write_idx;
+ __le32 rsvd0[3];
+};
+
/* Data struct for the MSGBUF_TYPE_RING_STATUS */
struct msgbuf_ring_status {
struct msgbuf_common_hdr msg;
@@ -1194,6 +1202,18 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf)
brcmf_netif_rx(ifp, skb);
}

+static void brcmf_msgbuf_process_gen_status(struct brcmf_msgbuf *msgbuf,
+ void *buf)
+{
+ struct msgbuf_gen_status *gen_status = buf;
+ struct brcmf_pub *drvr = msgbuf->drvr;
+ int err;
+
+ err = le16_to_cpu(gen_status->compl_hdr.status);
+ if (err)
+ bphy_err(drvr, "Firmware reported general error: %d\n", err);
+}
+
static void brcmf_msgbuf_process_ring_status(struct brcmf_msgbuf *msgbuf,
void *buf)
{
@@ -1273,6 +1293,10 @@ static void brcmf_msgbuf_process_msgtype(struct brcmf_msgbuf *msgbuf, void *buf)

msg = (struct msgbuf_common_hdr *)buf;
switch (msg->msgtype) {
+ case MSGBUF_TYPE_GEN_STATUS:
+ brcmf_dbg(MSGBUF, "MSGBUF_TYPE_GEN_STATUS\n");
+ brcmf_msgbuf_process_gen_status(msgbuf, buf);
+ break;
case MSGBUF_TYPE_RING_STATUS:
brcmf_dbg(MSGBUF, "MSGBUF_TYPE_RING_STATUS\n");
brcmf_msgbuf_process_ring_status(msgbuf, buf);
--
2.20.1


2019-02-21 10:45:28

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH wireless-drivers-next 2/2] brcmfmac: print firmware reported general status errors



On 2/21/2019 11:33 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> Firmware may report general errors using a special message type. Add
> basic support for it by simply decoding & printing an error number.
>
> A sample situation in which firmware reports a buf error:
> CONSOLE: 027084.733 no host response IOCTL buffer available..so fail the request
> will now produce a "Firmware reported general error: 9" on the host.

Could have meaningful message instead of a number. I can do that in
follow up patch if you do not have the information.

Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> .../broadcom/brcm80211/brcmfmac/msgbuf.c | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> index d711dc8ed606..d3780eae7f19 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> @@ -134,6 +134,14 @@ struct msgbuf_completion_hdr {
> __le16 flow_ring_id;
> };
>
> +/* Data struct for the MSGBUF_TYPE_GEN_STATUS */
> +struct msgbuf_gen_status {
> + struct msgbuf_common_hdr msg;
> + struct msgbuf_completion_hdr compl_hdr;
> + __le16 write_idx;
> + __le32 rsvd0[3];
> +};
> +
> /* Data struct for the MSGBUF_TYPE_RING_STATUS */
> struct msgbuf_ring_status {
> struct msgbuf_common_hdr msg;
> @@ -1194,6 +1202,18 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf)
> brcmf_netif_rx(ifp, skb);
> }
>
> +static void brcmf_msgbuf_process_gen_status(struct brcmf_msgbuf *msgbuf,
> + void *buf)
> +{
> + struct msgbuf_gen_status *gen_status = buf;
> + struct brcmf_pub *drvr = msgbuf->drvr;
> + int err;
> +
> + err = le16_to_cpu(gen_status->compl_hdr.status);
> + if (err)
> + bphy_err(drvr, "Firmware reported general error: %d\n", err);
> +}
> +
> static void brcmf_msgbuf_process_ring_status(struct brcmf_msgbuf *msgbuf,
> void *buf)
> {
> @@ -1273,6 +1293,10 @@ static void brcmf_msgbuf_process_msgtype(struct brcmf_msgbuf *msgbuf, void *buf)
>
> msg = (struct msgbuf_common_hdr *)buf;
> switch (msg->msgtype) {
> + case MSGBUF_TYPE_GEN_STATUS:
> + brcmf_dbg(MSGBUF, "MSGBUF_TYPE_GEN_STATUS\n");
> + brcmf_msgbuf_process_gen_status(msgbuf, buf);
> + break;
> case MSGBUF_TYPE_RING_STATUS:
> brcmf_dbg(MSGBUF, "MSGBUF_TYPE_RING_STATUS\n");
> brcmf_msgbuf_process_ring_status(msgbuf, buf);
>

2019-02-21 16:02:09

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH wireless-drivers-next 2/2] brcmfmac: print firmware reported general status errors

On 2019-02-21 11:45, Arend Van Spriel wrote:
> On 2/21/2019 11:33 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <[email protected]>
>>
>> Firmware may report general errors using a special message type. Add
>> basic support for it by simply decoding & printing an error number.
>>
>> A sample situation in which firmware reports a buf error:
>> CONSOLE: 027084.733 no host response IOCTL buffer available..so fail
>> the request
>> will now produce a "Firmware reported general error: 9" on the host.
>
> Could have meaningful message instead of a number. I can do that in
> follow up patch if you do not have the information.

That would be nice, thanks!

2019-02-28 08:28:22

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH wireless-drivers-next 1/2] brcmfmac: fix size of the struct msgbuf_ring_status

Rafał Miłecki wrote:

> From: Rafał Miłecki <[email protected]>
>
> This updates host struct to match the in-firmawre definition. It's a
> cosmetic change as it only applies to the reserved struct space.
>
> Fixes: c988b78244df ("brcmfmac: print firmware reported ring status errors")
> Signed-off-by: Rafał Miłecki <[email protected]>

2 patches applied to wireless-drivers-next.git, thanks.

0c7051610c57 brcmfmac: fix size of the struct msgbuf_ring_status
c91377495192 brcmfmac: print firmware reported general status errors

--
https://patchwork.kernel.org/patch/10823623/

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