2023-06-01 05:47:37

by Neal Sidhwaney

[permalink] [raw]
Subject: [PATCH v2] wifi: brcmfmac: Detect corner error case earlier with log

In some corner cases, an I/O read can fail and return -1, and this
patch detects this slightly earlier than is done today and logs an
appropriate message.

Signed-off-by: Neal Sidhwaney <[email protected]>
---

v2: Add const to variable holding error code & fix patch submission

drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 9f9bf08a70bb..39f3d913c1bc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -972,6 +972,7 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
u32 regdata;
u32 socitype;
int ret;
+ const u32 READ_FAILED = 0xFFFFFFFF;

/* Get CC core rev
* Chipid is assume to be at offset 0 from SI_ENUM_BASE
@@ -980,6 +981,11 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
*/
regdata = ci->ops->read32(ci->ctx,
CORE_CC_REG(ci->pub.enum_base, chipid));
+ if (regdata == READ_FAILED) {
+ brcmf_err("MMIO read failed: 0x%08x\n", regdata);
+ return -ENODEV;
+ }
+
ci->pub.chip = regdata & CID_ID_MASK;
ci->pub.chiprev = (regdata & CID_REV_MASK) >> CID_REV_SHIFT;
socitype = (regdata & CID_TYPE_MASK) >> CID_TYPE_SHIFT;
--
2.40.1


2023-06-02 05:43:09

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Detect corner error case earlier with log

Neal Sidhwaney <[email protected]> writes:

> In some corner cases, an I/O read can fail and return -1, and this
> patch detects this slightly earlier than is done today and logs an
> appropriate message.
>
> Signed-off-by: Neal Sidhwaney <[email protected]>

The formatting seems to be correct now, at least patchwork looks ok:

https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

But the commit log should always answer to the question "why?". Is there
a specific reason why you want to do it earlier?

> @@ -980,6 +981,11 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
> */
> regdata = ci->ops->read32(ci->ctx,
> CORE_CC_REG(ci->pub.enum_base, chipid));
> + if (regdata == READ_FAILED) {
> + brcmf_err("MMIO read failed: 0x%08x\n", regdata);
> + return -ENODEV;
> + }

Indentation here does not look correct, did you run checkpatch?

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

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

2023-06-03 06:15:26

by Neal Sidhwaney

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Detect corner error case earlier with log

On Fri, Jun 2, 2023 at 1:32 AM Kalle Valo <[email protected]> wrote:
> But the commit log should always answer to the question "why?". Is there
> a specific reason why you want to do it earlier?

Added context & motivation to the commit log.

>
> Indentation here does not look correct, did you run checkpatch?

Sorry, my mistake again. I ran checkpatch for this version of the
patch, but missed it in the docs the first time because it's in the
"large patches" paragraph, which is very much not the case with this
patch ;)

Thank you,

Neal