Hi Heiner,
> -----Original Message-----
> From: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> Sent: Sunday, August 22, 2021 7:23 PM
> To: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; Ariel Elior <aelior@xxxxxxxxxxx>;
> Sudarsana Reddy Kalluru <skalluru@xxxxxxxxxxx>; GR-everest-linux-l2 <GR-
> everest-linux-l2@xxxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; David
> Härdeman <david@xxxxxxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx
> Subject: [PATCH 05/12] bnx2x: Read VPD with pci_vpd_alloc()
>
> External Email
>
> ----------------------------------------------------------------------
> Use pci_vpd_alloc() to dynamically allocate a properly sized buffer and
> read the full VPD data into it.
>
> This simplifies the code, and we no longer have to make assumptions about
> VPD size.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 1 -
> .../net/ethernet/broadcom/bnx2x/bnx2x_main.c | 44 +++++--------------
> 2 files changed, 10 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> index d04994840..e789430f4 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> @@ -2407,7 +2407,6 @@ void bnx2x_igu_clear_sb_gen(struct bnx2x *bp, u8
> func, u8 idu_sb_id,
> #define ETH_MAX_RX_CLIENTS_E2 ETH_MAX_RX_CLIENTS_E1H
> #endif
>
> -#define BNX2X_VPD_LEN 128
> #define VENDOR_ID_LEN 4
>
> #define VF_ACQUIRE_THRESH 3
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 6d9813491..0466adf8d 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -12189,50 +12189,29 @@ static int bnx2x_get_hwinfo(struct bnx2x *bp)
>
> static void bnx2x_read_fwinfo(struct bnx2x *bp)
> {
> - int cnt, i, block_end, rodi;
> - char vpd_start[BNX2X_VPD_LEN+1];
> + int i, block_end, rodi;
> char str_id_reg[VENDOR_ID_LEN+1];
> char str_id_cap[VENDOR_ID_LEN+1];
> - char *vpd_data;
> - char *vpd_extended_data = NULL;
> - u8 len;
> + unsigned int vpd_len;
> + u8 *vpd_data, len;
>
> - cnt = pci_read_vpd(bp->pdev, 0, BNX2X_VPD_LEN, vpd_start);
> memset(bp->fw_ver, 0, sizeof(bp->fw_ver));
>
> - if (cnt < BNX2X_VPD_LEN)
> - goto out_not_found;
> + vpd_data = pci_vpd_alloc(bp->pdev, &vpd_len);
Definition of pci_vpd_alloc() is below as per repo "git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git" and branch wip/heiner-vpd-api
void *pci_vpd_alloc(struct pci_dev *dev, unsigned int *size)
{
unsigned int len = dev->vpd.len;
void *buf;
--
--
if (size)
*size = len;
}
Here is len is already part of pci_dev.
So why cannot same be set in caller function i.e. vpd_len = pb->pdev->vpd.len
> + 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_start, BNX2X_VPD_LEN,
> PCI_VPD_LRDT_RO_DATA);
> + 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_start[i]);
> -
> + pci_vpd_lrdt_size(&vpd_data[i]);
> i += PCI_VPD_LRDT_TAG_SIZE;
>
> - if (block_end > BNX2X_VPD_LEN) {
> - vpd_extended_data = kmalloc(block_end, GFP_KERNEL);
> - if (vpd_extended_data == NULL)
> - goto out_not_found;
> -
> - /* read rest of vpd image into vpd_extended_data */
> - memcpy(vpd_extended_data, vpd_start, BNX2X_VPD_LEN);
> - cnt = pci_read_vpd(bp->pdev, BNX2X_VPD_LEN,
> - block_end - BNX2X_VPD_LEN,
> - vpd_extended_data + BNX2X_VPD_LEN);
> - if (cnt < (block_end - BNX2X_VPD_LEN))
> - goto out_not_found;
> - vpd_data = vpd_extended_data;
> - } else
> - vpd_data = vpd_start;
> -
> - /* now vpd_data holds full vpd content in both cases */
> -
> rodi = pci_vpd_find_info_keyword(vpd_data, i, block_end,
> PCI_VPD_RO_KEYWORD_MFR_ID);
> if (rodi < 0)
> @@ -12258,17 +12237,14 @@ static void bnx2x_read_fwinfo(struct bnx2x *bp)
>
> rodi += PCI_VPD_INFO_FLD_HDR_SIZE;
>
> - if (len < 32 && (len + rodi) <= BNX2X_VPD_LEN) {
> + if (len < 32 && (len + rodi) <= vpd_len) {
> memcpy(bp->fw_ver, &vpd_data[rodi], len);
> bp->fw_ver[len] = ' ';
> }
> }
> - kfree(vpd_extended_data);
> - return;
> }
> out_not_found:
> - kfree(vpd_extended_data);
> - return;
> + kfree(vpd_data);
As vpd_data allocation done in PCI layer.
It will be logical to also free vpd_data in PCI layer.
--pk
