[net-next] sky2: Stop printing VPD info to debugfs

Commit Message

Heiner Kallweit Sept. 16, 2021, 9:40 p.m. UTC

Sky2 is parsing the VPD and adds the parsed information to its debugfs
file. This isn't needed in kernel, userspace tools like lspci can be
used to display such information nicely. Therefore remove this from
the driver.

lspci -vv:

Capabilities: [50] Vital Product Data
	Product Name: Marvell Yukon 88E8070 Gigabit Ethernet Controller
	Read-only fields:
		[PN] Part number: Yukon 88E8070
		[EC] Engineering changes: Rev. 1.0
		[MN] Manufacture ID: Marvell
		[SN] Serial number: AbCdEfG970FD4
		[CP] Extended capability: 01 10 cc 03
		[RV] Reserved: checksum good, 9 byte(s) reserved
	Read/write fields:
		[RW] Read-write area: 1 byte(s) free
	End

Relevant part in debugfs file:

0000:01:00.0 Product Data
Marvell Yukon 88E8070 Gigabit Ethernet Controller
 Part Number: Yukon 88E8070
 Engineering Level: Rev. 1.0
 Manufacturer: Marvell
 Serial Number: AbCdEfG970FD4

Signed-off-by: Heiner Kallweit <[e-mail]>
---
 drivers/net/ethernet/marvell/sky2.c | 84 +----------------------------
 1 file changed, 1 insertion(+), 83 deletions(-)

Comments

Stephen Hemminger Sept. 16, 2021, 10:13 p.m. UTC | #1

On Thu, 16 Sep 2021 23:40:37 +0200
Heiner Kallweit <[e-mail]> wrote:

> Sky2 is parsing the VPD and adds the parsed information to its debugfs
> file. This isn't needed in kernel, userspace tools like lspci can be
> used to display such information nicely. Therefore remove this from
> the driver.
> 
> lspci -vv:
> 
> Capabilities: [50] Vital Product Data
> 	Product Name: Marvell Yukon 88E8070 Gigabit Ethernet Controller
> 	Read-only fields:
> 		[PN] Part number: Yukon 88E8070
> 		[EC] Engineering changes: Rev. 1.0
> 		[MN] Manufacture ID: Marvell
> 		[SN] Serial number: AbCdEfG970FD4
> 		[CP] Extended capability: 01 10 cc 03
> 		[RV] Reserved: checksum good, 9 byte(s) reserved
> 	Read/write fields:
> 		[RW] Read-write area: 1 byte(s) free
> 	End
> 
> Relevant part in debugfs file:
> 
> 0000:01:00.0 Product Data
> Marvell Yukon 88E8070 Gigabit Ethernet Controller
>  Part Number: Yukon 88E8070
>  Engineering Level: Rev. 1.0
>  Manufacturer: Marvell
>  Serial Number: AbCdEfG970FD4
> 
> Signed-off-by: Heiner Kallweit <[e-mail]>

Make sense lspci seems to have gotten better at handling this now.

Acked-by: Stephen Hemminger <[e-mail]>

[e-mail] Sept. 18, 2021, 1:40 a.m. UTC | #2

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Thu, 16 Sep 2021 23:40:37 +0200 you wrote:
> Sky2 is parsing the VPD and adds the parsed information to its debugfs
> file. This isn't needed in kernel, userspace tools like lspci can be
> used to display such information nicely. Therefore remove this from
> the driver.
> 
> lspci -vv:
> 
> [...]

Here is the summary with links:
  - [net-next] sky2: Stop printing VPD info to debugfs
    https://git.kernel.org/netdev/net-next/c/0efcc3f20145

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Patch

diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
index e9fc74e54..3cb9c1271 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -4440,86 +4440,6 @@  static const struct ethtool_ops sky2_ethtool_ops = {
 
 static struct dentry *sky2_debug;
 
-
-/*
- * Read and parse the first part of Vital Product Data
- */
-#define VPD_SIZE	128
-#define VPD_MAGIC	0x82
-
-static const struct vpd_tag {
-	char tag[2];
-	char *label;
-} vpd_tags[] = {
-	{ "PN",	"Part Number" },
-	{ "EC", "Engineering Level" },
-	{ "MN", "Manufacturer" },
-	{ "SN", "Serial Number" },
-	{ "YA", "Asset Tag" },
-	{ "VL", "First Error Log Message" },
-	{ "VF", "Second Error Log Message" },
-	{ "VB", "Boot Agent ROM Configuration" },
-	{ "VE", "EFI UNDI Configuration" },
-};
-
-static void sky2_show_vpd(struct seq_file *seq, struct sky2_hw *hw)
-{
-	size_t vpd_size;
-	loff_t offs;
-	u8 len;
-	unsigned char *buf;
-	u16 reg2;
-
-	reg2 = sky2_pci_read16(hw, PCI_DEV_REG2);
-	vpd_size = 1 << ( ((reg2 & PCI_VPD_ROM_SZ) >> 14) + 8);
-
-	seq_printf(seq, "%s Product Data\n", pci_name(hw->pdev));
-	buf = kmalloc(vpd_size, GFP_KERNEL);
-	if (!buf) {
-		seq_puts(seq, "no memory!\n");
-		return;
-	}
-
-	if (pci_read_vpd(hw->pdev, 0, vpd_size, buf) < 0) {
-		seq_puts(seq, "VPD read failed\n");
-		goto out;
-	}
-
-	if (buf[0] != VPD_MAGIC) {
-		seq_printf(seq, "VPD tag mismatch: %#x\n", buf[0]);
-		goto out;
-	}
-	len = buf[1];
-	if (len == 0 || len > vpd_size - 4) {
-		seq_printf(seq, "Invalid id length: %d\n", len);
-		goto out;
-	}
-
-	seq_printf(seq, "%.*s\n", len, buf + 3);
-	offs = len + 3;
-
-	while (offs < vpd_size - 4) {
-		int i;
-
-		if (!memcmp("RW", buf + offs, 2))	/* end marker */
-			break;
-		len = buf[offs + 2];
-		if (offs + len + 3 >= vpd_size)
-			break;
-
-		for (i = 0; i < ARRAY_SIZE(vpd_tags); i++) {
-			if (!memcmp(vpd_tags[i].tag, buf + offs, 2)) {
-				seq_printf(seq, " %s: %.*s\n",
-					   vpd_tags[i].label, len, buf + offs + 3);
-				break;
-			}
-		}
-		offs += len + 3;
-	}
-out:
-	kfree(buf);
-}
-
 static int sky2_debug_show(struct seq_file *seq, void *v)
 {
 	struct net_device *dev = seq->private;
@@ -4529,9 +4449,7 @@  static int sky2_debug_show(struct seq_file *seq, void *v)
 	unsigned idx, last;
 	int sop;
 
-	sky2_show_vpd(seq, hw);
-
-	seq_printf(seq, "\nIRQ src=%x mask=%x control=%x\n",
+	seq_printf(seq, "IRQ src=%x mask=%x control=%x\n",
 		   sky2_read32(hw, B0_ISRC),
 		   sky2_read32(hw, B0_IMSK),
 		   sky2_read32(hw, B0_Y2_SP_ICR));