| Message ID | 20220303233801.242870-4-jonathan.lemon@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,1/2] ptp: ocp: add nvmem interface for accessing eeprom | expand |
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.
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
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?
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
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)
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
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.
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 = ®, - }, - { - .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); }
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(-)