| Message ID | 20230601054034.43692-1-nealsid@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2] wifi: brcmfmac: Detect corner error case earlier with log | expand |
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?
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
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;
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(+) -- 2.40.1