diff mbox series

[net-next] tg3: improve PCI VPD access

Message ID cb9e9113-0861-3904-87e0-d4c4ab3c8860@gmail.com (mailing list archive)
State Accepted
Delegated to: Netdev Maintainers
Headers show
Series [net-next] tg3: improve PCI VPD access | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 12 this patch: 12
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 55 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 12 this patch: 12
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Heiner Kallweit Jan. 22, 2021, 12:08 p.m. UTC
When working on the PCI VPD code I also tested with a Broadcom BCM95719
card. tg3 uses internal NVRAM access with this card, so I forced it to
PCI VPD mode for testing. PCI VPD access fails
(i + PCI_VPD_LRDT_TAG_SIZE + j > len) because only TG3_NVM_VPD_LEN (256)
bytes are read, but PCI VPD has 400 bytes on this card.

So add a constant TG3_NVM_PCI_VPD_MAX_LEN that defines the maximum
PCI VPD size. The actual VPD size is returned by pci_read_vpd().
In addition it's not worth looping over pci_read_vpd(). If we miss the
125ms timeout per VPD dword read then definitely something is wrong,
and if the tg3 module loading is killed then there's also not much
benefit in retrying the VPD read.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 30 +++++++++++------------------
 drivers/net/ethernet/broadcom/tg3.h |  1 +
 2 files changed, 12 insertions(+), 19 deletions(-)

Comments

Michael Chan Jan. 25, 2021, 10:09 p.m. UTC | #1
On Fri, Jan 22, 2021 at 4:08 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> When working on the PCI VPD code I also tested with a Broadcom BCM95719
> card. tg3 uses internal NVRAM access with this card, so I forced it to
> PCI VPD mode for testing. PCI VPD access fails
> (i + PCI_VPD_LRDT_TAG_SIZE + j > len) because only TG3_NVM_VPD_LEN (256)
> bytes are read, but PCI VPD has 400 bytes on this card.
>
> So add a constant TG3_NVM_PCI_VPD_MAX_LEN that defines the maximum
> PCI VPD size. The actual VPD size is returned by pci_read_vpd().
> In addition it's not worth looping over pci_read_vpd(). If we miss the
> 125ms timeout per VPD dword read then definitely something is wrong,
> and if the tg3 module loading is killed then there's also not much
> benefit in retrying the VPD read.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Michael Chan <michael.chan@broadcom.com>

Thanks.
Jakub Kicinski Jan. 25, 2021, 11:24 p.m. UTC | #2
On Mon, 25 Jan 2021 14:09:16 -0800 Michael Chan wrote:
> On Fri, Jan 22, 2021 at 4:08 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >
> > When working on the PCI VPD code I also tested with a Broadcom BCM95719
> > card. tg3 uses internal NVRAM access with this card, so I forced it to
> > PCI VPD mode for testing. PCI VPD access fails
> > (i + PCI_VPD_LRDT_TAG_SIZE + j > len) because only TG3_NVM_VPD_LEN (256)
> > bytes are read, but PCI VPD has 400 bytes on this card.
> >
> > So add a constant TG3_NVM_PCI_VPD_MAX_LEN that defines the maximum
> > PCI VPD size. The actual VPD size is returned by pci_read_vpd().
> > In addition it's not worth looping over pci_read_vpd(). If we miss the
> > 125ms timeout per VPD dword read then definitely something is wrong,
> > and if the tg3 module loading is killed then there's also not much
> > benefit in retrying the VPD read.
> >
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>  
> 
> Reviewed-by: Michael Chan <michael.chan@broadcom.com>

Applied, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 7def6d815..4ee9da498 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12826,11 +12826,13 @@  static __be32 *tg3_vpd_readblock(struct tg3 *tp, u32 *vpdlen)
 
 			offset = tg3_nvram_logical_addr(tp, offset);
 		}
-	}
 
-	if (!offset || !len) {
-		offset = TG3_NVM_VPD_OFF;
-		len = TG3_NVM_VPD_LEN;
+		if (!offset || !len) {
+			offset = TG3_NVM_VPD_OFF;
+			len = TG3_NVM_VPD_LEN;
+		}
+	} else {
+		len = TG3_NVM_PCI_VPD_MAX_LEN;
 	}
 
 	buf = kmalloc(len, GFP_KERNEL);
@@ -12846,26 +12848,16 @@  static __be32 *tg3_vpd_readblock(struct tg3 *tp, u32 *vpdlen)
 			if (tg3_nvram_read_be32(tp, offset + i, &buf[i/4]))
 				goto error;
 		}
+		*vpdlen = len;
 	} else {
-		u8 *ptr;
 		ssize_t cnt;
-		unsigned int pos = 0;
-
-		ptr = (u8 *)&buf[0];
-		for (i = 0; pos < len && i < 3; i++, pos += cnt, ptr += cnt) {
-			cnt = pci_read_vpd(tp->pdev, pos,
-					   len - pos, ptr);
-			if (cnt == -ETIMEDOUT || cnt == -EINTR)
-				cnt = 0;
-			else if (cnt < 0)
-				goto error;
-		}
-		if (pos != len)
+
+		cnt = pci_read_vpd(tp->pdev, 0, len, (u8 *)buf);
+		if (cnt < 0)
 			goto error;
+		*vpdlen = cnt;
 	}
 
-	*vpdlen = len;
-
 	return buf;
 
 error:
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 1000c8940..46ec4fdfd 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2101,6 +2101,7 @@ 
 /* Hardware Legacy NVRAM layout */
 #define TG3_NVM_VPD_OFF			0x100
 #define TG3_NVM_VPD_LEN			256
+#define TG3_NVM_PCI_VPD_MAX_LEN		512
 
 /* Hardware Selfboot NVRAM layout */
 #define TG3_NVM_HWSB_CFG1		0x00000004