Re: [PATCH 06/12] bnx2x: Search VPD with pci_vpd_find_ro_info_keyword()
From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2021-08-24 17:19:34
Also in:
netdev
On Sun, Aug 22, 2021 at 03:54:23PM +0200, Heiner Kallweit wrote:
Use pci_vpd_find_ro_info_keyword() to search for keywords in VPD to simplify the code. str_id_reg and str_id_cap hold the same string and are used in the same comparison. This doesn't make sense, use one string str_id instead.
str_id_reg is printed with "%04x" (lower-case hex letters) and str_id_cap with "%04X" (upper-case hex letters), so the previous code would match either 0xabcd or 0xABCD. After this patch, we'd match only the latter. PCI_VENDOR_ID_DELL is 0x1028, so it shouldn't make any difference, which makes me wonder why somebody bothered with both. But it does seem like a potential landmine to change the case sensitivity. Maybe strncasecmp() instead?
quoted hunk ↗ jump to hunk
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- .../net/ethernet/broadcom/bnx2x/bnx2x_main.c | 57 +++++-------------- 1 file changed, 15 insertions(+), 42 deletions(-)diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index 0466adf8d..2c7bfc416 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c@@ -12189,11 +12189,10 @@ static int bnx2x_get_hwinfo(struct bnx2x *bp) static void bnx2x_read_fwinfo(struct bnx2x *bp) { - int i, block_end, rodi; - char str_id_reg[VENDOR_ID_LEN+1]; - char str_id_cap[VENDOR_ID_LEN+1]; - unsigned int vpd_len; - u8 *vpd_data, len; + char str_id[VENDOR_ID_LEN + 1]; + unsigned int vpd_len, kw_len; + u8 *vpd_data; + int rodi; memset(bp->fw_ver, 0, sizeof(bp->fw_ver));@@ -12201,46 +12200,20 @@ static void bnx2x_read_fwinfo(struct bnx2x *bp) if (IS_ERR(vpd_data)) return; - /* VPD RO tag should be first tag after identifier string, hence - * we should be able to find it in first BNX2X_VPD_LEN chars - */ - i = pci_vpd_find_tag(vpd_data, vpd_len, PCI_VPD_LRDT_RO_DATA); - if (i < 0) - goto out_not_found; - - block_end = i + PCI_VPD_LRDT_TAG_SIZE + - pci_vpd_lrdt_size(&vpd_data[i]); - i += PCI_VPD_LRDT_TAG_SIZE; - - rodi = pci_vpd_find_info_keyword(vpd_data, i, block_end, - PCI_VPD_RO_KEYWORD_MFR_ID); - if (rodi < 0) - goto out_not_found; - - len = pci_vpd_info_field_size(&vpd_data[rodi]); - - if (len != VENDOR_ID_LEN) + rodi = pci_vpd_find_ro_info_keyword(vpd_data, vpd_len, + PCI_VPD_RO_KEYWORD_MFR_ID, &kw_len); + if (rodi < 0 || kw_len != VENDOR_ID_LEN) goto out_not_found; - rodi += PCI_VPD_INFO_FLD_HDR_SIZE; - /* vendor specific info */ - snprintf(str_id_reg, VENDOR_ID_LEN + 1, "%04x", PCI_VENDOR_ID_DELL); - snprintf(str_id_cap, VENDOR_ID_LEN + 1, "%04X", PCI_VENDOR_ID_DELL); - if (!strncmp(str_id_reg, &vpd_data[rodi], VENDOR_ID_LEN) || - !strncmp(str_id_cap, &vpd_data[rodi], VENDOR_ID_LEN)) { - - rodi = pci_vpd_find_info_keyword(vpd_data, i, block_end, - PCI_VPD_RO_KEYWORD_VENDOR0); - if (rodi >= 0) { - len = pci_vpd_info_field_size(&vpd_data[rodi]); - - rodi += PCI_VPD_INFO_FLD_HDR_SIZE; - - if (len < 32 && (len + rodi) <= vpd_len) { - memcpy(bp->fw_ver, &vpd_data[rodi], len); - bp->fw_ver[len] = ' '; - } + snprintf(str_id, VENDOR_ID_LEN + 1, "%04X", PCI_VENDOR_ID_DELL); + if (!strncmp(str_id, &vpd_data[rodi], VENDOR_ID_LEN)) { + rodi = pci_vpd_find_ro_info_keyword(vpd_data, vpd_len, + PCI_VPD_RO_KEYWORD_VENDOR0, + &kw_len); + if (rodi >= 0 && kw_len < sizeof(bp->fw_ver)) { + memcpy(bp->fw_ver, &vpd_data[rodi], kw_len); + bp->fw_ver[kw_len] = ' '; } } out_not_found:-- 2.33.0