On Sat, 16 Jan 2021 14:45:25 +0100 Heiner Kallweit wrote:
> The comment is quite weird, there is no such thing as a vendor-specific
> VPD id. 0x82 is the value of PCI_VPD_LRDT_ID_STRING. So what we are
> doing here is simply checking whether the byte at VPD address VPD_BASE
> is a valid string LRDT, same as what is done a few lines later in
> the code.
> LRDT = Large Resource Data Tag, see PCI 2.2 spec, VPD chapter
>
> Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com>
Did you find this by code inspection?
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> index 2c80371f9..48f20a6a0 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> @@ -2689,7 +2689,6 @@ void t4_get_regs(struct adapter *adap, void *buf,
> size_t buf_size)
> #define VPD_BASE 0x400
> #define VPD_BASE_OLD 0
> #define VPD_LEN 1024
> -#define CHELSIO_VPD_UNIQUE_ID 0x82
>
> /**
> * t4_eeprom_ptov - translate a physical EEPROM address to virtual
> @@ -2743,9 +2742,9 @@ int t4_seeprom_wp(struct adapter *adapter, bool enable)
> */
> int t4_get_raw_vpd_params(struct adapter *adapter, struct vpd_params *p)
> {
> - int i, ret = 0, addr;
> + int i, ret = 0, addr = VPD_BASE;
IMHO it's more readable if the addr is set to BASE or BASE_OLD in one
place rather than having a default value at variable init which may be
overwritten.
> int ec, sn, pn, na;
> - u8 *vpd, csum;
> + u8 *vpd, csum, base_val = 0;
> unsigned int vpdr_len, kw_offset, id_len;
>
> vpd = vmalloc(VPD_LEN);
> @@ -2755,17 +2754,12 @@ int t4_get_raw_vpd_params(struct adapter *adapter,
> struct vpd_params *p)
> /* Card information normally starts at VPD_BASE but early cards had
> * it at 0.
> */
> - ret = pci_read_vpd(adapter->pdev, VPD_BASE, sizeof(u32), vpd);
> + ret = pci_read_vpd(adapter->pdev, VPD_BASE, 1, &base_val);
Are we sure this works? I've seen silicon out there which has problems
with small PCI accesses (granted those were not VPD accesses).
> if (ret < 0)
> goto out;
>
> - /* The VPD shall have a unique identifier specified by the PCI SIG.
> - * For chelsio adapters, the identifier is 0x82. The first byte of a VPD
> - * shall be CHELSIO_VPD_UNIQUE_ID (0x82). The VPD programming software
> - * is expected to automatically put this entry at the
> - * beginning of the VPD.
> - */
> - addr = *vpd == CHELSIO_VPD_UNIQUE_ID ? VPD_BASE : VPD_BASE_OLD;
> + if (base_val != PCI_VPD_LRDT_ID_STRING)
> + addr = VPD_BASE_OLD;
>
> ret = pci_read_vpd(adapter->pdev, addr, VPD_LEN, vpd);
> if (ret < 0)
