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

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

[Neal Sidhwaney]

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 <nealsid@gmail.com>
---

 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

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

[Kalle Valo]

Neal Sidhwaney <nealsid@gmail.com> 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 <nealsid@gmail.com>

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

https://patchwork.kernel.org/project/linux-wireless/patch/20230601054034.43692-1-nealsid@gmail.com/

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

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

[Neal Sidhwaney]

On Fri, Jun 2, 2023 at 1:32 AM Kalle Valo <kvalo@kernel.org> 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