[PATCH net-next] ptp: ocp: Add serial port information to the debug summary

[PATCH net-next] ptp: ocp: Add serial port information to the debug summary

[Jonathan Lemon]

On the debug summary page, show the /dev/ttyS<port> mapping.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index cfe744b80407..5e3e06acaf87 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -2109,6 +2109,14 @@ ptp_ocp_summary_show(struct seq_file *s, void *data)
 	sma_out = ioread32(&bp->sma->gpio2);
 
 	seq_printf(s, "%7s: /dev/ptp%d\n", "PTP", ptp_clock_index(bp->ptp));
+	if (bp->gnss_port != -1)
+		seq_printf(s, "%7s: /dev/ttyS%d\n", "GNSS1", bp->gnss_port);
+	if (bp->gnss2_port != -1)
+		seq_printf(s, "%7s: /dev/ttyS%d\n", "GNSS2", bp->gnss2_port);
+	if (bp->mac_port != -1)
+		seq_printf(s, "%7s: /dev/ttyS%d\n", "MAC", bp->mac_port);
+	if (bp->nmea_port != -1)
+		seq_printf(s, "%7s: /dev/ttyS%d\n", "NMEA", bp->nmea_port);
 
 	sma1_show(dev, NULL, buf);
 	seq_printf(s, "   sma1: %s", buf);
-- 
2.31.1

[PATCH net-next] ptp: ocp: correct label for error path

[Jonathan Lemon]

When devlink_register() was removed from the error path, the
corresponding label was not updated.   Rename the label for
readability puposes, no functional change.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 608d1a0eb141..cfe744b80407 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -2600,7 +2600,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	err = pci_enable_device(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "pci_enable_device\n");
-		goto out_unregister;
+		goto out_free;
 	}
 
 	bp = devlink_priv(devlink);
@@ -2646,7 +2646,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	pci_set_drvdata(pdev, NULL);
 out_disable:
 	pci_disable_device(pdev);
-out_unregister:
+out_free:
 	devlink_free(devlink);
 	return err;
 }
-- 
2.31.1

[PATCH net-next 0/2] ptp: ocp: update devlink information

[Jonathan Lemon]

Both of these patches update the information displayed via devlink.

Jonathan Lemon (2):
  ptp: ocp: add nvmem interface for accessing eeprom
  ptp: ocp: Update devlink firmware display path.

 drivers/ptp/ptp_ocp.c | 242 ++++++++++++++++++++++++++----------------
 1 file changed, 149 insertions(+), 93 deletions(-)

-- 
2.31.1

[PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jonathan Lemon]

Add the at24 drivers for the eeprom, and use those accessors
via the nvmem API instead of direct i2c accesses.  This makes
things cleaner.

Add an eeprom map table which specifies where the manufacturer
pre-defined information is located.  Retrieve this and export
this via the devlink interface.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 201 ++++++++++++++++++++++++++++--------------
 1 file changed, 133 insertions(+), 68 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 5e3e06acaf87..330a85cda1bd 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -11,12 +11,14 @@
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/i2c-xiic.h>
 #include <linux/ptp_clock_kernel.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/xilinx_spi.h>
 #include <net/devlink.h>
 #include <linux/i2c.h>
 #include <linux/mtd/mtd.h>
+#include <linux/nvmem-consumer.h>
 
 #ifndef PCI_VENDOR_ID_FACEBOOK
 #define PCI_VENDOR_ID_FACEBOOK 0x1d9b
@@ -204,6 +206,10 @@ struct ptp_ocp_ext_src {
 	int			irq_vec;
 };
 
+#define OCP_BOARD_MFR_LEN		8
+#define OCP_BOARD_ID_LEN		13
+#define OCP_SERIAL_LEN			6
+
 struct ptp_ocp {
 	struct pci_dev		*pdev;
 	struct device		dev;
@@ -230,6 +236,7 @@ struct ptp_ocp {
 	struct platform_device	*spi_flash;
 	struct clk_hw		*i2c_clk;
 	struct timer_list	watchdog;
+	const struct ptp_ocp_eeprom_map *eeprom_map;
 	struct dentry		*debug_root;
 	time64_t		gnss_lost;
 	int			id;
@@ -238,8 +245,10 @@ struct ptp_ocp {
 	int			gnss2_port;
 	int			mac_port;	/* miniature atomic clock */
 	int			nmea_port;
-	u8			serial[6];
-	bool			has_serial;
+	u8			board_mfr[OCP_BOARD_MFR_LEN];
+	u8			board_id[OCP_BOARD_ID_LEN];
+	u8			serial[OCP_SERIAL_LEN];
+	bool			has_eeprom_data;
 	u32			pps_req_map;
 	int			flash_start;
 	u32			utc_tai_offset;
@@ -268,6 +277,29 @@ static int ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r);
 static irqreturn_t ptp_ocp_ts_irq(int irq, void *priv);
 static int ptp_ocp_ts_enable(void *priv, u32 req, bool enable);
 
+struct ptp_ocp_eeprom_map {
+	u16	off;
+	u16	len;
+	u32	bp_offset;
+	const void * const tag;
+};
+
+#define EEPROM_ENTRY(addr, member)				\
+	.off = addr,						\
+	.len = sizeof_field(struct ptp_ocp, member),		\
+	.bp_offset = offsetof(struct ptp_ocp, member)
+
+#define BP_MAP_ENTRY_ADDR(bp, map) ({				\
+	(void *)((uintptr_t)(bp) + (map)->bp_offset);		\
+})
+
+static struct ptp_ocp_eeprom_map fb_eeprom_map[] = {
+	{ EEPROM_ENTRY(0x43, board_id) },
+	{ EEPROM_ENTRY(0x79, board_mfr) },
+	{ EEPROM_ENTRY(0x00, serial), .tag = "mac" },
+	{ }
+};
+
 #define bp_assign_entry(bp, res, val) ({				\
 	uintptr_t addr = (uintptr_t)(bp) + (res)->bp_offset;		\
 	*(typeof(val) *)addr = val;					\
@@ -396,6 +428,15 @@ static struct ocp_resource ocp_fb_resource[] = {
 		.extra = &(struct ptp_ocp_i2c_info) {
 			.name = "xiic-i2c",
 			.fixed_rate = 50000000,
+			.data_size = sizeof(struct xiic_i2c_platform_data),
+			.data = &(struct xiic_i2c_platform_data) {
+				.num_devices = 2,
+				.devices = (struct i2c_board_info[]) {
+					{ I2C_BOARD_INFO("24c02", 0x50) },
+					{ I2C_BOARD_INFO("24mac402", 0x58),
+					  .platform_data = "mac" },
+				},
+			},
 		},
 	},
 	{
@@ -919,78 +960,88 @@ ptp_ocp_tod_gnss_name(int idx)
 	return gnss_name[idx];
 }
 
-static int
-ptp_ocp_firstchild(struct device *dev, void *data)
-{
-	return 1;
-}
+struct ptp_ocp_nvmem_match_info {
+	struct ptp_ocp *bp;
+	const void * const tag;
+};
 
 static int
-ptp_ocp_read_i2c(struct i2c_adapter *adap, u8 addr, u8 reg, u8 sz, u8 *data)
+ptp_ocp_nvmem_match(struct device *dev, const void *data)
 {
-	struct i2c_msg msgs[2] = {
-		{
-			.addr = addr,
-			.len = 1,
-			.buf = &reg,
-		},
-		{
-			.addr = addr,
-			.flags = I2C_M_RD,
-			.len = 2,
-			.buf = data,
-		},
-	};
-	int err;
-	u8 len;
+	const struct ptp_ocp_nvmem_match_info *info = data;
 
-	/* xiic-i2c for some stupid reason only does 2 byte reads. */
-	while (sz) {
-		len = min_t(u8, sz, 2);
-		msgs[1].len = len;
-		err = i2c_transfer(adap, msgs, 2);
-		if (err != msgs[1].len)
-			return err;
-		msgs[1].buf += len;
-		reg += len;
-		sz -= len;
-	}
+	dev = dev->parent;
+	if (!i2c_verify_client(dev) || info->tag != dev->platform_data)
+		return 0;
+
+	while ((dev = dev->parent))
+		if (dev->driver && !strcmp(dev->driver->name, KBUILD_MODNAME))
+			return info->bp == dev_get_drvdata(dev);
 	return 0;
 }
 
-static void
-ptp_ocp_get_serial_number(struct ptp_ocp *bp)
+static inline struct nvmem_device *
+ptp_ocp_nvmem_device_get(struct ptp_ocp *bp, const void * const tag)
 {
-	struct i2c_adapter *adap;
-	struct device *dev;
-	int err;
+	struct ptp_ocp_nvmem_match_info info = { .bp = bp, .tag = tag };
+
+	return nvmem_device_find(&info, ptp_ocp_nvmem_match);
+}
+
+static inline void
+ptp_ocp_nvmem_device_put(struct nvmem_device **nvmemp)
+{
+	if (*nvmemp != NULL) {
+		nvmem_device_put(*nvmemp);
+		*nvmemp = NULL;
+	}
+}
+
+static void
+ptp_ocp_read_eeprom(struct ptp_ocp *bp)
+{
+	const struct ptp_ocp_eeprom_map *map;
+	struct nvmem_device *nvmem;
+	const void *tag;
+	int ret;
 
 	if (!bp->i2c_ctrl)
 		return;
 
-	dev = device_find_child(&bp->i2c_ctrl->dev, NULL, ptp_ocp_firstchild);
-	if (!dev) {
-		dev_err(&bp->pdev->dev, "Can't find I2C adapter\n");
-		return;
+	tag = NULL;
+	nvmem = NULL;
+
+	for (map = bp->eeprom_map; map->len; map++) {
+		if (map->tag != tag) {
+			tag = map->tag;
+			ptp_ocp_nvmem_device_put(&nvmem);
+		}
+		if (!nvmem) {
+			nvmem = ptp_ocp_nvmem_device_get(bp, tag);
+			if (!nvmem)
+				goto out;
+		}
+		ret = nvmem_device_read(nvmem, map->off, map->len,
+					BP_MAP_ENTRY_ADDR(bp, map));
+		if (ret != map->len)
+			goto read_fail;
 	}
 
-	adap = i2c_verify_adapter(dev);
-	if (!adap) {
-		dev_err(&bp->pdev->dev, "device '%s' isn't an I2C adapter\n",
-			dev_name(dev));
-		goto out;
-	}
-
-	err = ptp_ocp_read_i2c(adap, 0x58, 0x9A, 6, bp->serial);
-	if (err) {
-		dev_err(&bp->pdev->dev, "could not read eeprom: %d\n", err);
-		goto out;
-	}
-
-	bp->has_serial = true;
+	bp->has_eeprom_data = true;
 
 out:
-	put_device(dev);
+	ptp_ocp_nvmem_device_put(&nvmem);
+	return;
+
+read_fail:
+	dev_err(&bp->pdev->dev, "could not read eeprom: %d\n", ret);
+	goto out;
+}
+
+static int
+ptp_ocp_firstchild(struct device *dev, void *data)
+{
+	return 1;
 }
 
 static struct device *
@@ -1109,16 +1160,29 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 			return err;
 	}
 
-	if (!bp->has_serial)
-		ptp_ocp_get_serial_number(bp);
-
-	if (bp->has_serial) {
-		sprintf(buf, "%pM", bp->serial);
-		err = devlink_info_serial_number_put(req, buf);
-		if (err)
-			return err;
+	if (!bp->has_eeprom_data) {
+		ptp_ocp_read_eeprom(bp);
+		if (!bp->has_eeprom_data)
+			return 0;
 	}
 
+	sprintf(buf, "%pM", bp->serial);
+	err = devlink_info_serial_number_put(req, buf);
+	if (err)
+		return err;
+
+	err = devlink_info_version_fixed_put(req,
+			DEVLINK_INFO_VERSION_GENERIC_BOARD_MANUFACTURE,
+			bp->board_mfr);
+	if (err)
+		return err;
+
+	err = devlink_info_version_fixed_put(req,
+			DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
+			bp->board_id);
+	if (err)
+		return err;
+
 	return 0;
 }
 
@@ -1412,6 +1476,7 @@ static int
 ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
 {
 	bp->flash_start = 1024 * 4096;
+	bp->eeprom_map = fb_eeprom_map;
 
 	ptp_ocp_tod_init(bp);
 	ptp_ocp_nmea_out_init(bp);
@@ -1810,8 +1875,8 @@ serialnum_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct ptp_ocp *bp = dev_get_drvdata(dev);
 
-	if (!bp->has_serial)
-		ptp_ocp_get_serial_number(bp);
+	if (!bp->has_eeprom_data)
+		ptp_ocp_read_eeprom(bp);
 
 	return sysfs_emit(buf, "%pM\n", bp->serial);
 }
-- 
2.31.1

[PATCH net-next 2/2] ptp: ocp: Update devlink firmware display path.

[Jonathan Lemon]

Cache the firmware version when the card is initialized,
and use this field to populate the devlink firmware information.

The cached firmware version will be used for feature gating in
upcoming patches.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 330a85cda1bd..55f7e8a7e9b4 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -245,6 +245,7 @@ struct ptp_ocp {
 	int			gnss2_port;
 	int			mac_port;	/* miniature atomic clock */
 	int			nmea_port;
+	u32			fw_version;
 	u8			board_mfr[OCP_BOARD_MFR_LEN];
 	u8			board_id[OCP_BOARD_ID_LEN];
 	u8			serial[OCP_SERIAL_LEN];
@@ -1142,23 +1143,15 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	if (err)
 		return err;
 
-	if (bp->image) {
-		u32 ver = ioread32(&bp->image->version);
-
-		if (ver & 0xffff) {
-			sprintf(buf, "%d", ver);
-			err = devlink_info_version_running_put(req,
-							       "fw",
-							       buf);
-		} else {
-			sprintf(buf, "%d", ver >> 16);
-			err = devlink_info_version_running_put(req,
-							       "loader",
-							       buf);
-		}
-		if (err)
-			return err;
+	if (bp->fw_version & 0xffff) {
+		sprintf(buf, "%d", bp->fw_version);
+		err = devlink_info_version_running_put(req, "fw", buf);
+	} else {
+		sprintf(buf, "%d", bp->fw_version >> 16);
+		err = devlink_info_version_running_put(req, "loader", buf);
 	}
+	if (err)
+		return err;
 
 	if (!bp->has_eeprom_data) {
 		ptp_ocp_read_eeprom(bp);
@@ -1477,6 +1470,7 @@ ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
 {
 	bp->flash_start = 1024 * 4096;
 	bp->eeprom_map = fb_eeprom_map;
+	bp->fw_version = ioread32(&bp->image->version);
 
 	ptp_ocp_tod_init(bp);
 	ptp_ocp_nmea_out_init(bp);
@@ -2585,17 +2579,14 @@ ptp_ocp_info(struct ptp_ocp *bp)
 
 	ptp_ocp_phc_info(bp);
 
-	if (bp->image) {
-		u32 ver = ioread32(&bp->image->version);
+	dev_info(dev, "version %x\n", bp->fw_version);
+	if (bp->fw_version & 0xffff)
+		dev_info(dev, "regular image, version %d\n",
+			 bp->fw_version & 0xffff);
+	else
+		dev_info(dev, "golden image, version %d\n",
+			 bp->fw_version >> 16);
 
-		dev_info(dev, "version %x\n", ver);
-		if (ver & 0xffff)
-			dev_info(dev, "regular image, version %d\n",
-				 ver & 0xffff);
-		else
-			dev_info(dev, "golden image, version %d\n",
-				 ver >> 16);
-	}
 	ptp_ocp_serial_info(dev, "GNSS", bp->gnss_port, 115200);
 	ptp_ocp_serial_info(dev, "GNSS2", bp->gnss2_port, 115200);
 	ptp_ocp_serial_info(dev, "MAC", bp->mac_port, 57600);
-- 
2.31.1

Re: [PATCH net-next 0/2] ptp: ocp: update devlink information

[Jakub Kicinski]

On Thu,  3 Mar 2022 15:37:59 -0800 Jonathan Lemon wrote:
> Both of these patches update the information displayed via devlink.

You'll need to repost these patches with the threading not being
broken...

Re: [PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jakub Kicinski]

On Thu,  3 Mar 2022 15:38:00 -0800 Jonathan Lemon wrote:
> manufacturer

The generic string is for manufacture, i.e. fab; that's different 
from manufacture*r* i.e. vendor. It's when you multi-source a single
board design at multiple factories.

Re: [PATCH net-next 0/2] ptp: ocp: update devlink information

[Jonathan Lemon]

On 3 Mar 2022, at 20:56, Jakub Kicinski wrote:

> On Thu,  3 Mar 2022 15:37:59 -0800 Jonathan Lemon wrote:
>> Both of these patches update the information displayed via devlink.
>
> You'll need to repost these patches with the threading not being
> broken...

Hmm.  Interesting tool behavior.  Will resend.
—
Jonathan

Re: [PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jonathan Lemon]

On 3 Mar 2022, at 21:01, Jakub Kicinski wrote:

> On Thu,  3 Mar 2022 15:38:00 -0800 Jonathan Lemon wrote:
>> manufacturer
>
> The generic string is for manufacture, i.e. fab; that's different
> from manufacture*r* i.e. vendor. It's when you multi-source a single
> board design at multiple factories.

The documentation seems unclear:

board.manufacture
-----------------
An identifier of the company or the facility which produced the part.


There isn’t a board.vendor (or manufacturer) in devlink.h.

The board design is open source, there’s several variants of
the design being produced, so I’m looking for a simple way to
identify the design (other than the opaque board id)
—
Jonathan

Re: [PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jakub Kicinski]

On Thu, 03 Mar 2022 21:39:48 -0800 Jonathan Lemon wrote:
> On 3 Mar 2022, at 21:01, Jakub Kicinski wrote:
> > On Thu,  3 Mar 2022 15:38:00 -0800 Jonathan Lemon wrote:  
> >> manufacturer  
> >
> > The generic string is for manufacture, i.e. fab; that's different
> > from manufacture*r* i.e. vendor. It's when you multi-source a single
> > board design at multiple factories.  
> 
> The documentation seems unclear:
> 
> board.manufacture
> -----------------
> An identifier of the company or the facility which produced the part.

Yeah, so this is for standard NICs. Say you have a NIC made by
Chelsio (just picking a random company that's unlikely to have its 
own fabs), the vendor is Chelsio but they will contract out building
the boards to whatever contractors. The contractor just puts the board
together and runs manufacturing tests, tho, no real IP work.

> There isn’t a board.vendor (or manufacturer) in devlink.h.
> 
> The board design is open source, there’s several variants of
> the design being produced, so I’m looking for a simple way to
> identify the design (other than the opaque board id)

And all of them use Facebook PCI_ID, hm. But AFAIU the cards are not
identical, right? Are they using the same exact board design or
something derived from the reference board design that matches 
the OCP spec?

And AFAIU the company delivering the card writes / assembles the
firmware, you can't take FW load from company A and flash it onto
company B's card, no?

Re: [PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jonathan Lemon]

On 4 Mar 2022, at 8:18, Jakub Kicinski wrote:

> On Thu, 03 Mar 2022 21:39:48 -0800 Jonathan Lemon wrote:
>> On 3 Mar 2022, at 21:01, Jakub Kicinski wrote:
>>> On Thu,  3 Mar 2022 15:38:00 -0800 Jonathan Lemon wrote:
>>>> manufacturer
>>>
>>> The generic string is for manufacture, i.e. fab; that's different
>>> from manufacture*r* i.e. vendor. It's when you multi-source a single
>>> board design at multiple factories.
>>
>> The documentation seems unclear:
>>
>> board.manufacture
>> -----------------
>> An identifier of the company or the facility which produced the part.
>
> Yeah, so this is for standard NICs. Say you have a NIC made by
> Chelsio (just picking a random company that's unlikely to have its
> own fabs), the vendor is Chelsio but they will contract out building
> the boards to whatever contractors. The contractor just puts the board
> together and runs manufacturing tests, tho, no real IP work.
>
>> There isn’t a board.vendor (or manufacturer) in devlink.h.
>>
>> The board design is open source, there’s several variants of
>> the design being produced, so I’m looking for a simple way to
>> identify the design (other than the opaque board id)
>
> And all of them use Facebook PCI_ID, hm. But AFAIU the cards are not
> identical, right? Are they using the same exact board design or
> something derived from the reference board design that matches
> the OCP spec?

A reference design, apparently with optional features.


> And AFAIU the company delivering the card writes / assembles the
> firmware, you can't take FW load from company A and flash it onto
> company B's card, no?

Nope.  There are currently 3 designs, and 3 firmware variants.
I’m looking for a way to tell them apart, especially since the
firmware file must match the card.  Suggestions?

[root@timecard net-next]# devlink dev info
pci/0000:02:00.0:
  driver ptp_ocp
  serial_number fc:c2:3d:2e:d7:c0
  versions:
      fixed:
        board.manufacture GOTHAM
        board.id RSH04940
      running:
        fw 21
pci/0000:65:00.0:
  driver ptp_ocp
  serial_number 4e:75:6d:00:00:00
  versions:
      fixed:
        board.manufacture O2S
        board.id R3006G000100
      running:
        fw 9
pci/0000:b3:00.0:
  driver ptp_ocp
  serial_number 3d:00:00:0e:37:73
  versions:
      fixed:
        board.manufacture CLS
        board.id R4006G000101
      running:
        fw 32773

—
Jonathan

Re: [PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jakub Kicinski]

On Fri, 04 Mar 2022 08:50:02 -0800 Jonathan Lemon wrote:
> > And AFAIU the company delivering the card writes / assembles the
> > firmware, you can't take FW load from company A and flash it onto
> > company B's card, no?  
> 
> Nope.  There are currently 3 designs, and 3 firmware variants.
> I’m looking for a way to tell them apart, especially since the
> firmware file must match the card.  Suggestions?
> 
> [root@timecard net-next]# devlink dev info
> pci/0000:02:00.0:
>   driver ptp_ocp
>   serial_number fc:c2:3d:2e:d7:c0
>   versions:
>       fixed:
>         board.manufacture GOTHAM
>         board.id RSH04940
>       running:
>         fw 21
> pci/0000:65:00.0:
>   driver ptp_ocp
>   serial_number 4e:75:6d:00:00:00
>   versions:
>       fixed:
>         board.manufacture O2S
>         board.id R3006G000100
>       running:
>         fw 9
> pci/0000:b3:00.0:
>   driver ptp_ocp
>   serial_number 3d:00:00:0e:37:73
>   versions:
>       fixed:
>         board.manufacture CLS
>         board.id R4006G000101
>       running:
>         fw 32773

Thanks for the output!

In my limited experience the right fit here would be PCI Subsystem
Vendor ID. This will also allow lspci to pretty print the vendor
name like:

30:00.0 Dunno controller: OCP Time Card whatever (Vendor X)

Re: [PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jonathan Lemon]

On 4 Mar 2022, at 19:11, Jakub Kicinski wrote:

> On Fri, 04 Mar 2022 08:50:02 -0800 Jonathan Lemon wrote:
>>> And AFAIU the company delivering the card writes / assembles the
>>> firmware, you can't take FW load from company A and flash it onto
>>> company B's card, no?
>>
>> Nope.  There are currently 3 designs, and 3 firmware variants.
>> I’m looking for a way to tell them apart, especially since the
>> firmware file must match the card.  Suggestions?
>>
>> [root@timecard net-next]# devlink dev info
>> pci/0000:02:00.0:
>>   driver ptp_ocp
>>   serial_number fc:c2:3d:2e:d7:c0
>>   versions:
>>       fixed:
>>         board.manufacture GOTHAM
>>         board.id RSH04940
>>       running:
>>         fw 21
>> pci/0000:65:00.0:
>>   driver ptp_ocp
>>   serial_number 4e:75:6d:00:00:00
>>   versions:
>>       fixed:
>>         board.manufacture O2S
>>         board.id R3006G000100
>>       running:
>>         fw 9
>> pci/0000:b3:00.0:
>>   driver ptp_ocp
>>   serial_number 3d:00:00:0e:37:73
>>   versions:
>>       fixed:
>>         board.manufacture CLS
>>         board.id R4006G000101
>>       running:
>>         fw 32773
>
> Thanks for the output!
>
> In my limited experience the right fit here would be PCI Subsystem
> Vendor ID. This will also allow lspci to pretty print the vendor
> name like:
>
> 30:00.0 Dunno controller: OCP Time Card whatever (Vendor X)

Unfortunately, that’s not going to work for a while, until the
relevant numbers get through the PCI approval body.

I believe that board.manufacture is correct.  In this particular
example, the 3 boards are fabbed in 3 different locations, but
there are 2 “vendors”.

So what this does is identify the contractor who assembled the
particular board.  Isn’t that what this is intended for?
—
Jonathan

Re: [PATCH net-next 1/2] ptp: ocp: add nvmem interface for accessing eeprom

[Jakub Kicinski]

On Sun, 06 Mar 2022 14:53:42 -0800 Jonathan Lemon wrote:
> > In my limited experience the right fit here would be PCI Subsystem
> > Vendor ID. This will also allow lspci to pretty print the vendor
> > name like:
> >
> > 30:00.0 Dunno controller: OCP Time Card whatever (Vendor X)  
> 
> Unfortunately, that’s not going to work for a while, until the
> relevant numbers get through the PCI approval body.

There's no approval for sub ids. Vendor whose ID is used can 
do whatever they want there.

> I believe that board.manufacture is correct.  In this particular
> example, the 3 boards are fabbed in 3 different locations, but
> there are 2 “vendors”.
> 
> So what this does is identify the contractor who assembled the
> particular board.  Isn’t that what this is intended for?

Not really, as I explained this field is to differentiate _identical_
board designs delivered by different fabs. I did not see that case in
your example output. The point of devlink info is to expose the
information not covered in standard PCI device fields, so the industry
standard approach takes precedence.