[PATCH ethtool-next 0/4] Extend module EEPROM API

[PATCH ethtool-next 0/4] Extend module EEPROM API

[Moshe Shemesh]

Ethtool supports module EEPROM dumps via the `ethtool -m <dev>` command.
But in current state its functionality is limited - offset and length
parameters, which are used to specify a linear desired region of EEPROM
data to dump, is not enough, considering emergence of complex module
EEPROM layouts such as CMIS 4.0.

Moreover, CMIS 4.0 extends the amount of pages that may be accessible by
introducing another parameter for page addressing - banks.
Besides, currently module EEPROM is represented as a chunk of
concatenated pages, where lower 128 bytes of all pages, except page 00h,
are omitted. Offset and length are used to address parts of this fake
linear memory. But in practice drivers, which implement
get_module_info() and get_module_eeprom() ethtool ops still calculate
page number and set I2C address on their own.

This series adds support in `ethtool -m` of dumping an arbitrary page
specified by page number, bank number and I2C address. Implement netlink
handler for `ethtool -m` in order to make such requests to the kernel
and extend CLI by adding corresponding parameters.
New command line format:
 ethtool -m <dev> [hex on|off] [raw on|off] [offset N] [length N] [page N] [bank N] [i2c N]

Netlink infrastructure works on per-page basis and allows dumps of a
single page at once. But in case user requests human-readable output,
which currently may require more than one page, userspace can make such
additional calls to kernel on demand and place pages in a linked list.
It allows to get pages from cache on demand and pass them to refactored
SFF decoders.

Vladyslav Tarasiuk (4):
  ethtool: Add netlink handler for getmodule (-m)
  ethtool: Refactor human-readable module EEPROM output for new API
  ethtool: Rename QSFP-DD identifiers to use CMIS 4.0
  ethtool: Update manpages to reflect changes to getmodule (-m) command

 Makefile.am             |   3 +-
 qsfp-dd.c => cmis4.c    | 220 +++++++++++---------
 cmis4.h                 | 128 ++++++++++++
 ethtool.8.in            |  14 ++
 ethtool.c               |   4 +
 internal.h              |  12 ++
 list.h                  |  34 ++++
 netlink/desc-ethtool.c  |  13 ++
 netlink/extapi.h        |   2 +
 netlink/module-eeprom.c | 438 ++++++++++++++++++++++++++++++++++++++++
 qsfp-dd.h               | 125 ------------
 qsfp.c                  | 129 +++++++-----
 qsfp.h                  |  52 ++---
 sff-common.c            |   3 +
 sff-common.h            |   3 +-
 15 files changed, 876 insertions(+), 304 deletions(-)
 rename qsfp-dd.c => cmis4.c (55%)
 create mode 100644 cmis4.h
 create mode 100644 list.h
 create mode 100644 netlink/module-eeprom.c
 delete mode 100644 qsfp-dd.h

-- 
2.26.2

[PATCH ethtool-next 1/4] ethtool: Add netlink handler for getmodule (-m)

[Moshe Shemesh]

From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

Implement "ethtool -m <dev>" subcommand using netlink and extend the
interface for new module EEPROM standards. Currently, ethtool supports
module EEPROM dumps of continuous memory regions, which are specified
using a pair of parameters - offset and length. But due to emergence of
new standards such as CMIS 4.0, which further extends possible
addressed memory, this approach shows its limitations.

Extend command line interface in order to support dumps of arbitrary
pages including CMIS 4.0-specific banked pages:
 ethtool -m <dev> [page N] [bank N] [i2c N]

Command example:
 # ethtool -m eth2 page 1 offset 0x80 length 0x20

 Offset          Values
 ------          ------
 0x0080:         11 00 23 80 00 00 00 00 00 00 00 08 ff 00 00 00
 0x0090:         00 00 01 a0 4d 65 6c 6c 61 6e 6f 78 20 20 20 20

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
---
 Makefile.am             |   1 +
 ethtool.c               |   4 +
 internal.h              |  10 +
 list.h                  |  34 ++++
 netlink/desc-ethtool.c  |  13 ++
 netlink/extapi.h        |   2 +
 netlink/module-eeprom.c | 425 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 489 insertions(+)
 create mode 100644 list.h
 create mode 100644 netlink/module-eeprom.c

diff --git a/Makefile.am b/Makefile.am
index e3e311d..9734bde 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -37,6 +37,7 @@ ethtool_SOURCES += \
 		  netlink/channels.c netlink/coalesce.c netlink/pause.c \
 		  netlink/eee.c netlink/tsinfo.c \
 		  netlink/desc-ethtool.c netlink/desc-genlctrl.c \
+		  netlink/module-eeprom.c \
 		  netlink/desc-rtnl.c netlink/cable_test.c netlink/tunnels.c \
 		  uapi/linux/ethtool_netlink.h \
 		  uapi/linux/netlink.h uapi/linux/genetlink.h \
diff --git a/ethtool.c b/ethtool.c
index a8339c8..f7e8d28 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5881,11 +5881,15 @@ static const struct option args[] = {
 	{
 		.opts	= "-m|--dump-module-eeprom|--module-info",
 		.func	= do_getmodule,
+		.nlfunc = nl_getmodule,
 		.help	= "Query/Decode Module EEPROM information and optical diagnostics if available",
 		.xhelp	= "		[ raw on|off ]\n"
 			  "		[ hex on|off ]\n"
 			  "		[ offset N ]\n"
 			  "		[ length N ]\n"
+			  "		[ page N ]\n"
+			  "		[ bank N ]\n"
+			  "		[ i2c N ]\n"
 	},
 	{
 		.opts	= "--show-eee",
diff --git a/internal.h b/internal.h
index 27da8ea..2affebe 100644
--- a/internal.h
+++ b/internal.h
@@ -216,6 +216,16 @@ static inline int ethtool_link_mode_set_bit(unsigned int nr, u32 *mask)
 	return 0;
 }
 
+/* Struct for managing module EEPROM pages */
+struct ethtool_module_eeprom {
+	u32	offset;
+	u32	length;
+	u8	page;
+	u8	bank;
+	u8	i2c_address;
+	u8	*data;
+};
+
 /* Context for sub-commands */
 struct cmd_context {
 	const char *devname;	/* net device name */
diff --git a/list.h b/list.h
new file mode 100644
index 0000000..aa97fdd
--- /dev/null
+++ b/list.h
@@ -0,0 +1,34 @@
+#ifndef ETHTOOL_LIST_H__
+#define ETHTOOL_LIST_H__
+
+#include <unistd.h>
+
+/* Generic list utilities */
+
+struct list_head {
+	struct list_head *next, *prev;
+};
+
+#define LIST_HEAD_INIT(name) { &(name), &(name) }
+
+static inline void list_add(struct list_head *new, struct list_head *head)
+{
+	head->next->prev = new;
+	new->next = head->next;
+	new->prev = head;
+	head->next = new;
+}
+
+static inline void list_del(struct list_head *entry)
+{
+	entry->next->prev = entry->prev;
+	entry->prev->next = entry->next;
+	entry->next = NULL;
+	entry->prev = NULL;
+}
+
+#define list_for_each_safe(pos, n, head) \
+	for (pos = (head)->next, n = pos->next; pos != (head); \
+		pos = n, n = pos->next)
+
+#endif
diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c
index fe5d7ba..3aacf05 100644
--- a/netlink/desc-ethtool.c
+++ b/netlink/desc-ethtool.c
@@ -318,6 +318,17 @@ const struct pretty_nla_desc __tunnel_info_desc[] = {
 	NLATTR_DESC_NESTED(ETHTOOL_A_TUNNEL_INFO_UDP_PORTS, tunnel_udp),
 };
 
+const struct pretty_nla_desc __module_eeprom_desc[] = {
+	NLATTR_DESC_INVALID(ETHTOOL_A_MODULE_EEPROM_UNSPEC),
+	NLATTR_DESC_NESTED(ETHTOOL_A_MODULE_EEPROM_HEADER, header),
+	NLATTR_DESC_U32(ETHTOOL_A_MODULE_EEPROM_OFFSET),
+	NLATTR_DESC_U32(ETHTOOL_A_MODULE_EEPROM_LENGTH),
+	NLATTR_DESC_U8(ETHTOOL_A_MODULE_EEPROM_PAGE),
+	NLATTR_DESC_U8(ETHTOOL_A_MODULE_EEPROM_BANK),
+	NLATTR_DESC_U8(ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS),
+	NLATTR_DESC_BINARY(ETHTOOL_A_MODULE_EEPROM_DATA)
+};
+
 const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC_INVALID(ETHTOOL_MSG_USER_NONE),
 	NLMSG_DESC(ETHTOOL_MSG_STRSET_GET, strset),
@@ -348,6 +359,7 @@ const struct pretty_nlmsg_desc ethnl_umsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_ACT, cable_test),
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_TDR_ACT, cable_test_tdr),
 	NLMSG_DESC(ETHTOOL_MSG_TUNNEL_INFO_GET, tunnel_info),
+	NLMSG_DESC(ETHTOOL_MSG_MODULE_EEPROM_GET, module_eeprom),
 };
 
 const unsigned int ethnl_umsg_n_desc = ARRAY_SIZE(ethnl_umsg_desc);
@@ -383,6 +395,7 @@ const struct pretty_nlmsg_desc ethnl_kmsg_desc[] = {
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_NTF, cable_test_ntf),
 	NLMSG_DESC(ETHTOOL_MSG_CABLE_TEST_TDR_NTF, cable_test_tdr_ntf),
 	NLMSG_DESC(ETHTOOL_MSG_TUNNEL_INFO_GET_REPLY, tunnel_info),
+	NLMSG_DESC(ETHTOOL_MSG_MODULE_EEPROM_GET_REPLY, module_eeprom),
 };
 
 const unsigned int ethnl_kmsg_n_desc = ARRAY_SIZE(ethnl_kmsg_desc);
diff --git a/netlink/extapi.h b/netlink/extapi.h
index 761cafb..72308e2 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -39,6 +39,7 @@ int nl_cable_test(struct cmd_context *ctx);
 int nl_cable_test_tdr(struct cmd_context *ctx);
 int nl_gtunnels(struct cmd_context *ctx);
 int nl_monitor(struct cmd_context *ctx);
+int nl_getmodule(struct cmd_context *ctx);
 
 void nl_monitor_usage(void);
 
@@ -87,6 +88,7 @@ static inline void nl_monitor_usage(void)
 #define nl_cable_test		NULL
 #define nl_cable_test_tdr	NULL
 #define nl_gtunnels		NULL
+#define nl_getmodule		NULL
 
 #endif /* ETHTOOL_ENABLE_NETLINK */
 
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
new file mode 100644
index 0000000..7298087
--- /dev/null
+++ b/netlink/module-eeprom.c
@@ -0,0 +1,425 @@
+/*
+ * module-eeprom.c - netlink implementation of module eeprom get command
+ *
+ * ethtool -m <dev>
+ */
+
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+#include <stddef.h>
+
+#include "../sff-common.h"
+#include "../qsfp.h"
+#include "../qsfp-dd.h"
+#include "../internal.h"
+#include "../common.h"
+#include "../list.h"
+#include "netlink.h"
+#include "parser.h"
+
+#define ETH_I2C_ADDRESS_LOW	0x50
+#define ETH_I2C_ADDRESS_HIGH	0x51
+#define ETH_I2C_MAX_ADDRESS	0x7F
+
+static struct cmd_params
+{
+	u8 dump_hex;
+	u8 dump_raw;
+	u32 offset;
+	u32 length;
+	u32 page;
+	u32 bank;
+	u32 i2c_address;
+} getmodule_cmd_params;
+
+static const struct param_parser getmodule_params[] = {
+	{
+		.arg		= "hex",
+		.handler	= nl_parse_u8bool,
+		.dest_offset	= offsetof(struct cmd_params, dump_hex),
+		.min_argc	= 1,
+	},
+	{
+		.arg		= "raw",
+		.handler	= nl_parse_u8bool,
+		.dest_offset	= offsetof(struct cmd_params, dump_raw),
+		.min_argc	= 1,
+	},
+	{
+		.arg		= "offset",
+		.handler	= nl_parse_direct_u32,
+		.dest_offset	= offsetof(struct cmd_params, offset),
+		.min_argc	= 1,
+	},
+	{
+		.arg		= "length",
+		.handler	= nl_parse_direct_u32,
+		.dest_offset	= offsetof(struct cmd_params, length),
+		.min_argc	= 1,
+	},
+	{
+		.arg		= "page",
+		.handler	= nl_parse_direct_u32,
+		.dest_offset	= offsetof(struct cmd_params, page),
+		.min_argc	= 1,
+	},
+	{
+		.arg		= "bank",
+		.handler	= nl_parse_direct_u32,
+		.dest_offset	= offsetof(struct cmd_params, bank),
+		.min_argc	= 1,
+	},
+	{
+		.arg		= "i2c",
+		.handler	= nl_parse_direct_u32,
+		.dest_offset	= offsetof(struct cmd_params, i2c_address),
+		.min_argc	= 1,
+	},
+	{}
+};
+
+struct page_entry {
+	struct list_head link;
+	struct ethtool_module_eeprom *page;
+};
+
+static struct list_head page_list = LIST_HEAD_INIT(page_list);
+
+static int cache_add(struct ethtool_module_eeprom *page)
+{
+	struct page_entry *list_element;
+
+	if (!page)
+		return -1;
+	list_element = malloc(sizeof(*list_element));
+	if (!list_element)
+		return -ENOMEM;
+	list_element->page = page;
+
+	list_add(&list_element->link, &page_list);
+	return 0;
+}
+
+static void page_free(struct ethtool_module_eeprom *page)
+{
+	free(page->data);
+	free(page);
+}
+
+static void cache_del(struct ethtool_module_eeprom *page)
+{
+	struct ethtool_module_eeprom *entry;
+	struct list_head *head, *next;
+
+	list_for_each_safe(head, next, &page_list) {
+		entry = ((struct page_entry *)head)->page;
+		if (entry == page) {
+			list_del(head);
+			free(head);
+			page_free(entry);
+			break;
+		}
+	}
+}
+
+static void cache_free(void)
+{
+	struct ethtool_module_eeprom *entry;
+	struct list_head *head, *next;
+
+	list_for_each_safe(head, next, &page_list) {
+		entry = ((struct page_entry *)head)->page;
+		list_del(head);
+		free(head);
+		page_free(entry);
+	}
+}
+
+static struct ethtool_module_eeprom *page_join(struct ethtool_module_eeprom *page_a,
+					       struct ethtool_module_eeprom *page_b)
+{
+	struct ethtool_module_eeprom *joined_page;
+	u32 total_length;
+
+	if (!page_a || !page_b ||
+	    page_a->page != page_b->page ||
+	    page_a->bank != page_b->bank ||
+	    page_a->i2c_address != page_b->i2c_address)
+		return NULL;
+
+	total_length = page_a->length + page_b->length;
+	joined_page = calloc(1, sizeof(*joined_page));
+	joined_page->data = calloc(1, total_length);
+	joined_page->page = page_a->page;
+	joined_page->bank = page_a->bank;
+	joined_page->length = total_length;
+	joined_page->i2c_address = page_a->i2c_address;
+
+	if (page_a->offset < page_b->offset) {
+		memcpy(joined_page->data, page_a->data, page_a->length);
+		memcpy(joined_page->data + page_a->length, page_b->data, page_b->length);
+		joined_page->offset = page_a->offset;
+	} else {
+		memcpy(joined_page->data, page_b->data, page_b->length);
+		memcpy(joined_page->data + page_b->length, page_a->data, page_a->length);
+		joined_page->offset = page_b->offset;
+	}
+
+	return joined_page;
+}
+
+static struct ethtool_module_eeprom *cache_get(u32 page, u32 bank, u8 i2c_address)
+{
+	struct ethtool_module_eeprom *entry;
+	struct list_head *head, *next;
+
+	list_for_each_safe(head, next, &page_list) {
+		entry = ((struct page_entry *)head)->page;
+		if (entry->page == page && entry->bank == bank &&
+		    entry->i2c_address == i2c_address)
+			return entry;
+	}
+
+	return NULL;
+}
+
+static int getmodule_page_fetch_reply_cb(const struct nlmsghdr *nlhdr,
+					 void *data)
+{
+	const struct nlattr *tb[ETHTOOL_A_MODULE_EEPROM_DATA + 1] = {};
+	DECLARE_ATTR_TB_INFO(tb);
+	struct ethtool_module_eeprom *lower_page;
+	struct ethtool_module_eeprom *response;
+	struct ethtool_module_eeprom *request;
+	struct ethtool_module_eeprom *joined;
+	u8 *eeprom_data;
+	int ret;
+
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return ret;
+
+	if (!tb[ETHTOOL_A_MODULE_EEPROM_DATA]) {
+		fprintf(stderr, "Malformed netlink message (getmodule)\n");
+		return MNL_CB_ERROR;
+	}
+
+	response = calloc(1, sizeof(*response));
+	if (!response)
+		return -ENOMEM;
+
+	request = (struct ethtool_module_eeprom *)data;
+	response->offset = request->offset;
+	response->page = request->page;
+	response->bank = request->bank;
+	response->i2c_address = request->i2c_address;
+	response->length = mnl_attr_get_payload_len(tb[ETHTOOL_A_MODULE_EEPROM_DATA]);
+	eeprom_data = mnl_attr_get_payload(tb[ETHTOOL_A_MODULE_EEPROM_DATA]);
+
+	response->data = malloc(response->length);
+	if (!response->data) {
+		free(response);
+		return -ENOMEM;
+	}
+	memcpy(response->data, eeprom_data, response->length);
+
+	if (!request->page) {
+		lower_page = cache_get(request->page, request->bank, response->i2c_address);
+		if (lower_page) {
+			joined = page_join(lower_page, response);
+			page_free(response);
+			cache_del(lower_page);
+			return cache_add(joined);
+		}
+	}
+
+	return cache_add(response);
+}
+
+static int page_fetch(struct nl_context *nlctx, const struct ethtool_module_eeprom *request)
+{
+	struct nl_socket *nlsock = nlctx->ethnl_socket;
+	struct nl_msg_buff *msg = &nlsock->msgbuff;
+	struct ethtool_module_eeprom *page;
+	int ret;
+
+	if (!request || request->i2c_address > ETH_I2C_MAX_ADDRESS)
+		return -EINVAL;
+
+	/* Satisfy request right away, if region is already in cache */
+	page = cache_get(request->page, request->bank, request->i2c_address);
+	if (page && page->offset <= request->offset &&
+	    page->offset + page->length >= request->offset + request->length) {
+		return 0;
+	}
+
+	ret = nlsock_prep_get_request(nlsock, ETHTOOL_MSG_MODULE_EEPROM_GET,
+				      ETHTOOL_A_MODULE_EEPROM_HEADER, 0);
+	if (ret < 0)
+		return ret;
+
+	if (ethnla_put_u32(msg, ETHTOOL_A_MODULE_EEPROM_LENGTH, request->length) ||
+	    ethnla_put_u32(msg, ETHTOOL_A_MODULE_EEPROM_OFFSET, request->offset) ||
+	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_PAGE, request->page) ||
+	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_BANK, request->bank) ||
+	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS, request->i2c_address))
+		return -EMSGSIZE;
+
+	ret = nlsock_sendmsg(nlsock, NULL);
+	if (ret < 0)
+		return ret;
+	ret = nlsock_process_reply(nlsock, getmodule_page_fetch_reply_cb, (void *)request);
+	if (ret < 0)
+		return ret;
+
+	return nlsock_process_reply(nlsock, nomsg_reply_cb, NULL);
+}
+
+static bool page_available(struct ethtool_module_eeprom *which)
+{
+	struct ethtool_module_eeprom *page_zero = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
+	u8 id = page_zero->data[SFF8636_ID_OFFSET];
+	u8 flat_mem = page_zero->data[2] & 0x80;
+
+	switch (id) {
+	case SFF8024_ID_SOLDERED_MODULE:
+	case SFF8024_ID_SFP:
+		return (!which->bank && which->page <= 1);
+	case SFF8024_ID_QSFP:
+	case SFF8024_ID_QSFP28:
+	case SFF8024_ID_QSFP_PLUS:
+		return (!which->bank && which->page <= 3);
+	case SFF8024_ID_QSFP_DD:
+		return !(flat_mem && which->page);
+	default:
+		return true;
+	}
+}
+
+static int decoder_prefetch(struct nl_context *nlctx)
+{
+	struct ethtool_module_eeprom *page_zero_lower = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
+	struct ethtool_module_eeprom request = {0};
+	u8 module_id = page_zero_lower->data[0];
+	int err = 0;
+
+	/* Fetch rest of page 00 */
+	request.i2c_address = ETH_I2C_ADDRESS_LOW;
+	request.offset = 128;
+	request.length = 128;
+	err = page_fetch(nlctx, &request);
+	if (err)
+		return err;
+
+	switch (module_id) {
+	case SFF8024_ID_QSFP:
+	case SFF8024_ID_QSFP28:
+	case SFF8024_ID_QSFP_PLUS:
+		memset(&request, 0, sizeof(request));
+		request.i2c_address = ETH_I2C_ADDRESS_LOW;
+		request.offset = 128;
+		request.length = 128;
+		request.page = 3;
+		break;
+	case SFF8024_ID_QSFP_DD:
+		memset(&request, 0, sizeof(request));
+		request.i2c_address = ETH_I2C_ADDRESS_LOW;
+		request.offset = 128;
+		request.length = 128;
+		request.page = 1;
+		break;
+	}
+
+	return page_fetch(nlctx, &request);
+}
+
+static void decoder_print(void)
+{
+	struct ethtool_module_eeprom *page_zero = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
+	u8 module_id = page_zero->data[SFF8636_ID_OFFSET];
+
+	switch (module_id) {
+	case SFF8024_ID_SFP:
+		sff8079_show_all(page_zero->data);
+		break;
+	default:
+		dump_hex(stdout, page_zero->data, page_zero->length, page_zero->offset);
+		break;
+	}
+}
+
+int nl_getmodule(struct cmd_context *ctx)
+{
+	struct ethtool_module_eeprom request = {0};
+	struct ethtool_module_eeprom *reply_page;
+	struct nl_context *nlctx = ctx->nlctx;
+	u32 dump_length;
+	u8 *eeprom_data;
+	int ret;
+
+	if (netlink_cmd_check(ctx, ETHTOOL_MSG_MODULE_EEPROM_GET, false))
+		return -EOPNOTSUPP;
+
+	nlctx->cmd = "-m";
+	nlctx->argp = ctx->argp;
+	nlctx->argc = ctx->argc;
+	nlctx->devname = ctx->devname;
+	ret = nl_parser(nlctx, getmodule_params, &getmodule_cmd_params, PARSER_GROUP_NONE, NULL);
+	if (ret < 0)
+		return 1;
+
+	if (getmodule_cmd_params.dump_hex && getmodule_cmd_params.dump_raw) {
+		fprintf(stderr, "Hex and raw dump cannot be specified together\n");
+		return 1;
+	}
+
+	request.i2c_address = ETH_I2C_ADDRESS_LOW;
+	request.length = 128;
+	ret = page_fetch(nlctx, &request);
+	if (ret)
+		goto cleanup;
+
+#ifdef ETHTOOL_ENABLE_PRETTY_DUMP
+	if (getmodule_cmd_params.page || getmodule_cmd_params.bank ||
+	    getmodule_cmd_params.offset || getmodule_cmd_params.length)
+#endif
+		getmodule_cmd_params.dump_hex = true;
+
+	request.offset = getmodule_cmd_params.offset;
+	request.length = getmodule_cmd_params.length ?: 128;
+	request.page = getmodule_cmd_params.page;
+	request.bank = getmodule_cmd_params.bank;
+	request.i2c_address = getmodule_cmd_params.i2c_address ?: ETH_I2C_ADDRESS_LOW;
+
+	if (getmodule_cmd_params.dump_hex || getmodule_cmd_params.dump_raw) {
+		if (!page_available(&request))
+			goto err_invalid;
+
+		ret = page_fetch(nlctx, &request);
+		if (ret < 0)
+			return ret;
+		reply_page = cache_get(request.page, request.bank, request.i2c_address);
+		if (!reply_page)
+			goto err_invalid;
+
+		eeprom_data = reply_page->data + (request.offset - reply_page->offset);
+		dump_length = reply_page->length < request.length ? reply_page->length
+								  : request.length;
+		if (getmodule_cmd_params.dump_raw)
+			fwrite(eeprom_data, 1, request.length, stdout);
+		else
+			dump_hex(stdout, eeprom_data, dump_length, request.offset);
+	} else {
+		ret = decoder_prefetch(nlctx);
+		if (ret)
+			goto cleanup;
+		decoder_print();
+	}
+
+err_invalid:
+	ret = -EINVAL;
+cleanup:
+	cache_free();
+	return ret;
+}
-- 
2.26.2

[PATCH ethtool-next 2/4] ethtool: Refactor human-readable module EEPROM output

[Moshe Shemesh]

From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

Reuse existing SFF8636 and QSFP-DD infrastructures to implement
EEPROM decoders, which work with paged memory. Add support for
human-readable output for QSFP, QSFP28, QSFP Plus, QSFP-DD and DSFP
transreceivers from netlink 'ethtool -m' handler.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
---
 internal.h              |   2 +
 netlink/module-eeprom.c |  13 ++++
 qsfp-dd.c               |  44 +++++++++++---
 qsfp-dd.h               |  29 +++++----
 qsfp.c                  | 127 +++++++++++++++++++++++-----------------
 qsfp.h                  |  52 ++++++++--------
 sff-common.c            |   3 +
 sff-common.h            |   3 +-
 8 files changed, 171 insertions(+), 102 deletions(-)

diff --git a/internal.h b/internal.h
index 2affebe..33e619b 100644
--- a/internal.h
+++ b/internal.h
@@ -391,6 +391,8 @@ void sff8472_show_all(const __u8 *id);
 
 /* QSFP Optics diagnostics */
 void sff8636_show_all(const __u8 *id, __u32 eeprom_len);
+void sff8636_show_all_paged(const struct ethtool_module_eeprom *page_zero,
+			    const struct ethtool_module_eeprom *page_three);
 
 /* FUJITSU Extended Socket network device */
 int fjes_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index 7298087..768a261 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -291,6 +291,7 @@ static bool page_available(struct ethtool_module_eeprom *which)
 	case SFF8024_ID_QSFP_PLUS:
 		return (!which->bank && which->page <= 3);
 	case SFF8024_ID_QSFP_DD:
+	case SFF8024_ID_DSFP:
 		return !(flat_mem && which->page);
 	default:
 		return true;
@@ -323,6 +324,7 @@ static int decoder_prefetch(struct nl_context *nlctx)
 		request.page = 3;
 		break;
 	case SFF8024_ID_QSFP_DD:
+	case SFF8024_ID_DSFP:
 		memset(&request, 0, sizeof(request));
 		request.i2c_address = ETH_I2C_ADDRESS_LOW;
 		request.offset = 128;
@@ -336,13 +338,24 @@ static int decoder_prefetch(struct nl_context *nlctx)
 
 static void decoder_print(void)
 {
+	struct ethtool_module_eeprom *page_three = cache_get(3, 0, ETH_I2C_ADDRESS_LOW);
 	struct ethtool_module_eeprom *page_zero = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
+	struct ethtool_module_eeprom *page_one = cache_get(1, 0, ETH_I2C_ADDRESS_LOW);
 	u8 module_id = page_zero->data[SFF8636_ID_OFFSET];
 
 	switch (module_id) {
 	case SFF8024_ID_SFP:
 		sff8079_show_all(page_zero->data);
 		break;
+	case SFF8024_ID_QSFP:
+	case SFF8024_ID_QSFP28:
+	case SFF8024_ID_QSFP_PLUS:
+		sff8636_show_all_paged(page_zero, page_three);
+		break;
+	case SFF8024_ID_QSFP_DD:
+	case SFF8024_ID_DSFP:
+		cmis4_show_all(page_zero, page_one);
+		break;
 	default:
 		dump_hex(stdout, page_zero->data, page_zero->length, page_zero->offset);
 		break;
diff --git a/qsfp-dd.c b/qsfp-dd.c
index 900bbb5..5c2e4a0 100644
--- a/qsfp-dd.c
+++ b/qsfp-dd.c
@@ -274,6 +274,20 @@ static void qsfp_dd_show_mod_lvl_monitors(const __u8 *id)
 		  OFFSET_TO_U16(QSFP_DD_CURR_CURR_OFFSET));
 }
 
+static void qsfp_dd_show_link_len_from_page(const __u8 *page_one_data)
+{
+	qsfp_dd_print_smf_cbl_len(page_one_data);
+	sff_show_value_with_unit(page_one_data, QSFP_DD_OM5_LEN_OFFSET,
+				 "Length (OM5)", 2, "m");
+	sff_show_value_with_unit(page_one_data, QSFP_DD_OM4_LEN_OFFSET,
+				 "Length (OM4)", 2, "m");
+	sff_show_value_with_unit(page_one_data, QSFP_DD_OM3_LEN_OFFSET,
+				 "Length (OM3 50/125um)", 2, "m");
+	sff_show_value_with_unit(page_one_data, QSFP_DD_OM2_LEN_OFFSET,
+				 "Length (OM2 50/125um)", 1, "m");
+}
+
+
 /**
  * Print relevant info about the maximum supported fiber media length
  * for each type of fiber media at the maximum module-supported bit rate.
@@ -283,15 +297,7 @@ static void qsfp_dd_show_mod_lvl_monitors(const __u8 *id)
  */
 static void qsfp_dd_show_link_len(const __u8 *id)
 {
-	qsfp_dd_print_smf_cbl_len(id);
-	sff_show_value_with_unit(id, QSFP_DD_OM5_LEN_OFFSET,
-				 "Length (OM5)", 2, "m");
-	sff_show_value_with_unit(id, QSFP_DD_OM4_LEN_OFFSET,
-				 "Length (OM4)", 2, "m");
-	sff_show_value_with_unit(id, QSFP_DD_OM3_LEN_OFFSET,
-				 "Length (OM3 50/125um)", 2, "m");
-	sff_show_value_with_unit(id, QSFP_DD_OM2_LEN_OFFSET,
-				 "Length (OM2 50/125um)", 1, "m");
+	qsfp_dd_show_link_len_from_page(id + PAG01H_UPPER_OFFSET);
 }
 
 /**
@@ -331,3 +337,23 @@ void qsfp_dd_show_all(const __u8 *id)
 	qsfp_dd_show_vendor_info(id);
 	qsfp_dd_show_rev_compliance(id);
 }
+
+void cmis4_show_all(const struct ethtool_module_eeprom *page_zero,
+		    const struct ethtool_module_eeprom *page_one)
+{
+	const __u8 *page_zero_data = page_zero->data;
+
+	qsfp_dd_show_identifier(page_zero_data);
+	qsfp_dd_show_power_info(page_zero_data);
+	qsfp_dd_show_connector(page_zero_data);
+	qsfp_dd_show_cbl_asm_len(page_zero_data);
+	qsfp_dd_show_sig_integrity(page_zero_data);
+	qsfp_dd_show_mit_compliance(page_zero_data);
+	qsfp_dd_show_mod_lvl_monitors(page_zero_data);
+
+	if (page_one)
+		qsfp_dd_show_link_len_from_page(page_one->data);
+
+	qsfp_dd_show_vendor_info(page_zero_data);
+	qsfp_dd_show_rev_compliance(page_zero_data);
+}
diff --git a/qsfp-dd.h b/qsfp-dd.h
index f589c4e..fddb188 100644
--- a/qsfp-dd.h
+++ b/qsfp-dd.h
@@ -96,30 +96,33 @@
 /*-----------------------------------------------------------------------
  * Upper Memory Page 0x01: contains advertising fields that define properties
  * that are unique to active modules and cable assemblies.
- * RealOffset = 1 * 0x80 + LocalOffset
+ * GlobalOffset = 2 * 0x80 + LocalOffset
  */
-#define PAG01H_UPPER_OFFSET			(0x01 * 0x80)
+#define PAG01H_UPPER_OFFSET			(0x02 * 0x80)
 
 /* Supported Link Length (Page 1) */
-#define QSFP_DD_SMF_LEN_OFFSET			(PAG01H_UPPER_OFFSET + 0x84)
-#define QSFP_DD_OM5_LEN_OFFSET			(PAG01H_UPPER_OFFSET + 0x85)
-#define QSFP_DD_OM4_LEN_OFFSET			(PAG01H_UPPER_OFFSET + 0x86)
-#define QSFP_DD_OM3_LEN_OFFSET			(PAG01H_UPPER_OFFSET + 0x87)
-#define QSFP_DD_OM2_LEN_OFFSET			(PAG01H_UPPER_OFFSET + 0x88)
+#define QSFP_DD_SMF_LEN_OFFSET			0x4
+#define QSFP_DD_OM5_LEN_OFFSET			0x5
+#define QSFP_DD_OM4_LEN_OFFSET			0x6
+#define QSFP_DD_OM3_LEN_OFFSET			0x7
+#define QSFP_DD_OM2_LEN_OFFSET			0x8
 
 /* Wavelength (Page 1) */
-#define QSFP_DD_NOM_WAVELENGTH_MSB		(PAG01H_UPPER_OFFSET + 0x8A)
-#define QSFP_DD_NOM_WAVELENGTH_LSB		(PAG01H_UPPER_OFFSET + 0x8B)
-#define QSFP_DD_WAVELENGTH_TOL_MSB		(PAG01H_UPPER_OFFSET + 0x8C)
-#define QSFP_DD_WAVELENGTH_TOL_LSB		(PAG01H_UPPER_OFFSET + 0x8D)
+#define QSFP_DD_NOM_WAVELENGTH_MSB		0xA
+#define QSFP_DD_NOM_WAVELENGTH_LSB		0xB
+#define QSFP_DD_WAVELENGTH_TOL_MSB		0xC
+#define QSFP_DD_WAVELENGTH_TOL_LSB		0xD
 
 /* Signal integrity controls */
-#define QSFP_DD_SIG_INTEG_TX_OFFSET		(PAG01H_UPPER_OFFSET + 0xA1)
-#define QSFP_DD_SIG_INTEG_RX_OFFSET		(PAG01H_UPPER_OFFSET + 0xA2)
+#define QSFP_DD_SIG_INTEG_TX_OFFSET		0xA1
+#define QSFP_DD_SIG_INTEG_RX_OFFSET		0xA2
 
 #define YESNO(x) (((x) != 0) ? "Yes" : "No")
 #define ONOFF(x) (((x) != 0) ? "On" : "Off")
 
 void qsfp_dd_show_all(const __u8 *id);
 
+void cmis4_show_all(const struct ethtool_module_eeprom *page_zero,
+		    const struct ethtool_module_eeprom *page_one);
+
 #endif /* QSFP_DD_H__ */
diff --git a/qsfp.c b/qsfp.c
index 3c1a300..a4d7e08 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -672,38 +672,39 @@ static void sff8636_show_revision_compliance(const __u8 *id)
  * Second byte are 1/256th of degree, which are added to the dec part.
  */
 #define SFF8636_OFFSET_TO_TEMP(offset) ((__s16)OFFSET_TO_U16(offset))
+#define OFFSET_TO_U16_PTR(ptr, offset) (ptr[offset] << 8 | ptr[(offset) + 1])
 
-static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd)
+static void sff8636_dom_parse(const __u8 *id, const __u8 *page_three, struct sff_diags *sd)
 {
 	int i = 0;
 
 	/* Monitoring Thresholds for Alarms and Warnings */
-	sd->sfp_voltage[MCURR] = OFFSET_TO_U16(SFF8636_VCC_CURR);
-	sd->sfp_voltage[HALRM] = OFFSET_TO_U16(SFF8636_VCC_HALRM);
-	sd->sfp_voltage[LALRM] = OFFSET_TO_U16(SFF8636_VCC_LALRM);
-	sd->sfp_voltage[HWARN] = OFFSET_TO_U16(SFF8636_VCC_HWARN);
-	sd->sfp_voltage[LWARN] = OFFSET_TO_U16(SFF8636_VCC_LWARN);
+	sd->sfp_voltage[MCURR] = OFFSET_TO_U16_PTR(id, SFF8636_VCC_CURR);
+	sd->sfp_voltage[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_HALRM);
+	sd->sfp_voltage[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_LALRM);
+	sd->sfp_voltage[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_HWARN);
+	sd->sfp_voltage[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_LWARN);
 
 	sd->sfp_temp[MCURR] = SFF8636_OFFSET_TO_TEMP(SFF8636_TEMP_CURR);
-	sd->sfp_temp[HALRM] = SFF8636_OFFSET_TO_TEMP(SFF8636_TEMP_HALRM);
-	sd->sfp_temp[LALRM] = SFF8636_OFFSET_TO_TEMP(SFF8636_TEMP_LALRM);
-	sd->sfp_temp[HWARN] = SFF8636_OFFSET_TO_TEMP(SFF8636_TEMP_HWARN);
-	sd->sfp_temp[LWARN] = SFF8636_OFFSET_TO_TEMP(SFF8636_TEMP_LWARN);
+	sd->sfp_temp[HALRM] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_HALRM);
+	sd->sfp_temp[LALRM] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_LALRM);
+	sd->sfp_temp[HWARN] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_HWARN);
+	sd->sfp_temp[LWARN] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_LWARN);
 
-	sd->bias_cur[HALRM] = OFFSET_TO_U16(SFF8636_TX_BIAS_HALRM);
-	sd->bias_cur[LALRM] = OFFSET_TO_U16(SFF8636_TX_BIAS_LALRM);
-	sd->bias_cur[HWARN] = OFFSET_TO_U16(SFF8636_TX_BIAS_HWARN);
-	sd->bias_cur[LWARN] = OFFSET_TO_U16(SFF8636_TX_BIAS_LWARN);
+	sd->bias_cur[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_HALRM);
+	sd->bias_cur[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_LALRM);
+	sd->bias_cur[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_HWARN);
+	sd->bias_cur[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_LWARN);
 
-	sd->tx_power[HALRM] = OFFSET_TO_U16(SFF8636_TX_PWR_HALRM);
-	sd->tx_power[LALRM] = OFFSET_TO_U16(SFF8636_TX_PWR_LALRM);
-	sd->tx_power[HWARN] = OFFSET_TO_U16(SFF8636_TX_PWR_HWARN);
-	sd->tx_power[LWARN] = OFFSET_TO_U16(SFF8636_TX_PWR_LWARN);
+	sd->tx_power[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_HALRM);
+	sd->tx_power[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_LALRM);
+	sd->tx_power[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_HWARN);
+	sd->tx_power[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_LWARN);
 
-	sd->rx_power[HALRM] = OFFSET_TO_U16(SFF8636_RX_PWR_HALRM);
-	sd->rx_power[LALRM] = OFFSET_TO_U16(SFF8636_RX_PWR_LALRM);
-	sd->rx_power[HWARN] = OFFSET_TO_U16(SFF8636_RX_PWR_HWARN);
-	sd->rx_power[LWARN] = OFFSET_TO_U16(SFF8636_RX_PWR_LWARN);
+	sd->rx_power[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_HALRM);
+	sd->rx_power[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_LALRM);
+	sd->rx_power[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_HWARN);
+	sd->rx_power[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_LWARN);
 
 
 	/* Channel Specific Data */
@@ -740,7 +741,7 @@ static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd)
 
 }
 
-static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len)
+static void sff8636_show_dom(const __u8 *id, const __u8 *page_three, __u32 eeprom_len)
 {
 	struct sff_diags sd = {0};
 	char *rx_power_string = NULL;
@@ -767,7 +768,7 @@ static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len)
 	sd.tx_power_type = id[SFF8636_DIAG_TYPE_OFFSET] &
 						SFF8636_RX_PWR_TYPE_MASK;
 
-	sff8636_dom_parse(id, &sd);
+	sff8636_dom_parse(id, page_three, &sd);
 
 	PRINT_TEMP("Module temperature", sd.sfp_temp[MCURR]);
 	PRINT_VCC("Module voltage", sd.sfp_voltage[MCURR]);
@@ -818,6 +819,42 @@ static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len)
 	}
 }
 
+
+static void sff6836_show_page_zero(const __u8 *id)
+{
+	sff8636_show_ext_identifier(id);
+	sff8636_show_connector(id);
+	sff8636_show_transceiver(id);
+	sff8636_show_encoding(id);
+	sff_show_value_with_unit(id, SFF8636_BR_NOMINAL_OFFSET,
+			"BR, Nominal", 100, "Mbps");
+	sff8636_show_rate_identifier(id);
+	sff_show_value_with_unit(id, SFF8636_SM_LEN_OFFSET,
+		     "Length (SMF,km)", 1, "km");
+	sff_show_value_with_unit(id, SFF8636_OM3_LEN_OFFSET,
+			"Length (OM3 50um)", 2, "m");
+	sff_show_value_with_unit(id, SFF8636_OM2_LEN_OFFSET,
+			"Length (OM2 50um)", 1, "m");
+	sff_show_value_with_unit(id, SFF8636_OM1_LEN_OFFSET,
+		     "Length (OM1 62.5um)", 1, "m");
+	sff_show_value_with_unit(id, SFF8636_CBL_LEN_OFFSET,
+		     "Length (Copper or Active cable)", 1, "m");
+	sff8636_show_wavelength_or_copper_compliance(id);
+	sff_show_ascii(id, SFF8636_VENDOR_NAME_START_OFFSET,
+		       SFF8636_VENDOR_NAME_END_OFFSET, "Vendor name");
+	sff8636_show_oui(id, SFF8636_VENDOR_OUI_OFFSET);
+	sff_show_ascii(id, SFF8636_VENDOR_PN_START_OFFSET,
+		       SFF8636_VENDOR_PN_END_OFFSET, "Vendor PN");
+	sff_show_ascii(id, SFF8636_VENDOR_REV_START_OFFSET,
+		       SFF8636_VENDOR_REV_END_OFFSET, "Vendor rev");
+	sff_show_ascii(id, SFF8636_VENDOR_SN_START_OFFSET,
+		       SFF8636_VENDOR_SN_END_OFFSET, "Vendor SN");
+	sff_show_ascii(id, SFF8636_DATE_YEAR_OFFSET,
+		       SFF8636_DATE_VENDOR_LOT_OFFSET + 1, "Date code");
+	sff8636_show_revision_compliance(id);
+
+}
+
 void sff8636_show_all(const __u8 *id, __u32 eeprom_len)
 {
 	if (id[SFF8636_ID_OFFSET] == SFF8024_ID_QSFP_DD) {
@@ -829,36 +866,16 @@ void sff8636_show_all(const __u8 *id, __u32 eeprom_len)
 	if ((id[SFF8636_ID_OFFSET] == SFF8024_ID_QSFP) ||
 		(id[SFF8636_ID_OFFSET] == SFF8024_ID_QSFP_PLUS) ||
 		(id[SFF8636_ID_OFFSET] == SFF8024_ID_QSFP28)) {
-		sff8636_show_ext_identifier(id);
-		sff8636_show_connector(id);
-		sff8636_show_transceiver(id);
-		sff8636_show_encoding(id);
-		sff_show_value_with_unit(id, SFF8636_BR_NOMINAL_OFFSET,
-				"BR, Nominal", 100, "Mbps");
-		sff8636_show_rate_identifier(id);
-		sff_show_value_with_unit(id, SFF8636_SM_LEN_OFFSET,
-			     "Length (SMF,km)", 1, "km");
-		sff_show_value_with_unit(id, SFF8636_OM3_LEN_OFFSET,
-				"Length (OM3 50um)", 2, "m");
-		sff_show_value_with_unit(id, SFF8636_OM2_LEN_OFFSET,
-				"Length (OM2 50um)", 1, "m");
-		sff_show_value_with_unit(id, SFF8636_OM1_LEN_OFFSET,
-			     "Length (OM1 62.5um)", 1, "m");
-		sff_show_value_with_unit(id, SFF8636_CBL_LEN_OFFSET,
-			     "Length (Copper or Active cable)", 1, "m");
-		sff8636_show_wavelength_or_copper_compliance(id);
-		sff_show_ascii(id, SFF8636_VENDOR_NAME_START_OFFSET,
-			       SFF8636_VENDOR_NAME_END_OFFSET, "Vendor name");
-		sff8636_show_oui(id, SFF8636_VENDOR_OUI_OFFSET);
-		sff_show_ascii(id, SFF8636_VENDOR_PN_START_OFFSET,
-			       SFF8636_VENDOR_PN_END_OFFSET, "Vendor PN");
-		sff_show_ascii(id, SFF8636_VENDOR_REV_START_OFFSET,
-			       SFF8636_VENDOR_REV_END_OFFSET, "Vendor rev");
-		sff_show_ascii(id, SFF8636_VENDOR_SN_START_OFFSET,
-			       SFF8636_VENDOR_SN_END_OFFSET, "Vendor SN");
-		sff_show_ascii(id, SFF8636_DATE_YEAR_OFFSET,
-			       SFF8636_DATE_VENDOR_LOT_OFFSET + 1, "Date code");
-		sff8636_show_revision_compliance(id);
-		sff8636_show_dom(id, eeprom_len);
+		sff6836_show_page_zero(id);
+		sff8636_show_dom(id, id + SFF8636_PAGE03H_OFFSET, eeprom_len);
 	}
 }
+
+void sff8636_show_all_paged(const struct ethtool_module_eeprom *page_zero,
+			    const struct ethtool_module_eeprom *page_three)
+{
+	sff8636_show_identifier(page_zero->data);
+	sff6836_show_page_zero(page_zero->data);
+	if (page_three)
+		sff8636_show_dom(page_zero->data, page_three->data, ETH_MODULE_SFF_8636_MAX_LEN);
+}
diff --git a/qsfp.h b/qsfp.h
index 9636b0c..23d48f0 100644
--- a/qsfp.h
+++ b/qsfp.h
@@ -592,32 +592,36 @@
  * Offset - Page Num(3) * PageSize(0x80) + Page offset
  */
 
+/* 3 * 128 + Lower page 00h(128) */
+#define SFF8636_PAGE03H_OFFSET (128 * 4)
+
 /* Module Thresholds (48 Bytes) 128-175 */
+/* Offset relative to half page boundary at offset 128 */
 /* MSB at low address, LSB at high address */
-#define	SFF8636_TEMP_HALRM		0x200
-#define	SFF8636_TEMP_LALRM		0x202
-#define	SFF8636_TEMP_HWARN		0x204
-#define	SFF8636_TEMP_LWARN		0x206
-
-#define	SFF8636_VCC_HALRM		0x210
-#define	SFF8636_VCC_LALRM		0x212
-#define	SFF8636_VCC_HWARN		0x214
-#define	SFF8636_VCC_LWARN		0x216
-
-#define	SFF8636_RX_PWR_HALRM		0x230
-#define	SFF8636_RX_PWR_LALRM		0x232
-#define	SFF8636_RX_PWR_HWARN		0x234
-#define	SFF8636_RX_PWR_LWARN		0x236
-
-#define	SFF8636_TX_BIAS_HALRM		0x238
-#define	SFF8636_TX_BIAS_LALRM		0x23A
-#define	SFF8636_TX_BIAS_HWARN		0x23C
-#define	SFF8636_TX_BIAS_LWARN		0x23E
-
-#define	SFF8636_TX_PWR_HALRM		0x240
-#define	SFF8636_TX_PWR_LALRM		0x242
-#define	SFF8636_TX_PWR_HWARN		0x244
-#define	SFF8636_TX_PWR_LWARN		0x246
+#define	SFF8636_TEMP_HALRM	0x0
+#define	SFF8636_TEMP_LALRM	0x2
+#define	SFF8636_TEMP_HWARN	0x4
+#define	SFF8636_TEMP_LWARN	0x6
+
+#define	SFF8636_VCC_HALRM	0x10
+#define	SFF8636_VCC_LALRM	0x12
+#define	SFF8636_VCC_HWARN	0x14
+#define	SFF8636_VCC_LWARN	0x16
+
+#define	SFF8636_RX_PWR_HALRM	0x30
+#define	SFF8636_RX_PWR_LALRM	0x32
+#define	SFF8636_RX_PWR_HWARN	0x34
+#define	SFF8636_RX_PWR_LWARN	0x36
+
+#define	SFF8636_TX_BIAS_HALRM	0x38
+#define	SFF8636_TX_BIAS_LALRM	0x3A
+#define	SFF8636_TX_BIAS_HWARN	0x3C
+#define	SFF8636_TX_BIAS_LWARN	0x3E
+
+#define	SFF8636_TX_PWR_HALRM	0x40
+#define	SFF8636_TX_PWR_LALRM	0x42
+#define	SFF8636_TX_PWR_HWARN	0x44
+#define	SFF8636_TX_PWR_LWARN	0x46
 
 #define	ETH_MODULE_SFF_8636_MAX_LEN	640
 #define	ETH_MODULE_SFF_8436_MAX_LEN	640
diff --git a/sff-common.c b/sff-common.c
index 5285645..2815951 100644
--- a/sff-common.c
+++ b/sff-common.c
@@ -139,6 +139,9 @@ void sff8024_show_identifier(const __u8 *id, int id_offset)
 	case SFF8024_ID_QSFP_DD:
 		printf(" (QSFP-DD Double Density 8X Pluggable Transceiver (INF-8628))\n");
 		break;
+	case SFF8024_ID_DSFP:
+		printf(" (DSFP Dual Small Form Factor Pluggable Transceiver)\n");
+		break;
 	default:
 		printf(" (reserved or unknown)\n");
 		break;
diff --git a/sff-common.h b/sff-common.h
index cfb5d0e..2183f41 100644
--- a/sff-common.h
+++ b/sff-common.h
@@ -62,7 +62,8 @@
 #define  SFF8024_ID_CDFP_S3				0x16
 #define  SFF8024_ID_MICRO_QSFP			0x17
 #define  SFF8024_ID_QSFP_DD				0x18
-#define  SFF8024_ID_LAST				SFF8024_ID_QSFP_DD
+#define  SFF8024_ID_DSFP				0x1B
+#define  SFF8024_ID_LAST				SFF8024_ID_DSFP
 #define  SFF8024_ID_UNALLOCATED_LAST	0x7F
 #define  SFF8024_ID_VENDOR_START		0x80
 #define  SFF8024_ID_VENDOR_LAST			0xFF
-- 
2.26.2

[PATCH ethtool-next 3/4] ethtool: Rename QSFP-DD identifiers to use CMIS 4.0

[Moshe Shemesh]

From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

QSFP-DD and DSFP EEPROM layout complies to CMIS 4.0 specification. As
DSFP support is added, there are currently two standards, which share
the same infrastructure. Rename QSFP_DD and qsfp_dd occurrences to use
CMIS4 or cmis4 respectively to make function names generic for any
module compliant to CMIS 4.0.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
---
 Makefile.am             |   2 +-
 qsfp-dd.c => cmis4.c    | 210 ++++++++++++++++++++--------------------
 cmis4.h                 | 128 ++++++++++++++++++++++++
 netlink/module-eeprom.c |   2 +-
 qsfp-dd.h               | 128 ------------------------
 qsfp.c                  |   2 +-
 6 files changed, 236 insertions(+), 236 deletions(-)
 rename qsfp-dd.c => cmis4.c (56%)
 create mode 100644 cmis4.h
 delete mode 100644 qsfp-dd.h

diff --git a/Makefile.am b/Makefile.am
index 9734bde..1913f84 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -17,7 +17,7 @@ ethtool_SOURCES += \
 		  smsc911x.c at76c50x-usb.c sfc.c stmmac.c	\
 		  sff-common.c sff-common.h sfpid.c sfpdiag.c	\
 		  ixgbevf.c tse.c vmxnet3.c qsfp.c qsfp.h fjes.c lan78xx.c \
-		  igc.c qsfp-dd.c qsfp-dd.h bnxt.c
+		  igc.c cmis4.c cmis4.h bnxt.c
 endif
 
 if ENABLE_BASH_COMPLETION
diff --git a/qsfp-dd.c b/cmis4.c
similarity index 56%
rename from qsfp-dd.c
rename to cmis4.c
index 5c2e4a0..7859217 100644
--- a/qsfp-dd.c
+++ b/cmis4.c
@@ -11,21 +11,21 @@
 #include <math.h>
 #include "internal.h"
 #include "sff-common.h"
-#include "qsfp-dd.h"
+#include "cmis4.h"
 
-static void qsfp_dd_show_identifier(const __u8 *id)
+static void cmis4_show_identifier(const __u8 *id)
 {
-	sff8024_show_identifier(id, QSFP_DD_ID_OFFSET);
+	sff8024_show_identifier(id, CMIS4_ID_OFFSET);
 }
 
-static void qsfp_dd_show_connector(const __u8 *id)
+static void cmis4_show_connector(const __u8 *id)
 {
-	sff8024_show_connector(id, QSFP_DD_CTOR_OFFSET);
+	sff8024_show_connector(id, CMIS4_CTOR_OFFSET);
 }
 
-static void qsfp_dd_show_oui(const __u8 *id)
+static void cmis4_show_oui(const __u8 *id)
 {
-	sff8024_show_oui(id, QSFP_DD_VENDOR_OUI_OFFSET);
+	sff8024_show_oui(id, CMIS4_VENDOR_OUI_OFFSET);
 }
 
 /**
@@ -33,9 +33,9 @@ static void qsfp_dd_show_oui(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 45, section 1.7.2.1, Table 18
  * [2] CMIS Rev. 4, pag. 81, section 8.2.1, Table 8-2
  */
-static void qsfp_dd_show_rev_compliance(const __u8 *id)
+static void cmis4_show_rev_compliance(const __u8 *id)
 {
-	__u8 rev = id[QSFP_DD_REV_COMPLIANCE_OFFSET];
+	__u8 rev = id[CMIS4_REV_COMPLIANCE_OFFSET];
 	int major = (rev >> 4) & 0x0F;
 	int minor = rev & 0x0F;
 
@@ -49,17 +49,17 @@ static void qsfp_dd_show_rev_compliance(const __u8 *id)
  * [2] CMIS Rev. 4, pag. 94, section 8.3.9, Table 8-18
  * [3] QSFP-DD Hardware Rev 5.0, pag. 22, section 4.2.1
  */
-static void qsfp_dd_show_power_info(const __u8 *id)
+static void cmis4_show_power_info(const __u8 *id)
 {
 	float max_power = 0.0f;
 	__u8 base_power = 0;
 	__u8 power_class;
 
 	/* Get the power class (first 3 most significat bytes) */
-	power_class = (id[QSFP_DD_PWR_CLASS_OFFSET] >> 5) & 0x07;
+	power_class = (id[CMIS4_PWR_CLASS_OFFSET] >> 5) & 0x07;
 
 	/* Get the base power in multiples of 0.25W */
-	base_power = id[QSFP_DD_PWR_MAX_POWER_OFFSET];
+	base_power = id[CMIS4_PWR_MAX_POWER_OFFSET];
 	max_power = base_power * 0.25f;
 
 	printf("\t%-41s : %d\n", "Power class", power_class + 1);
@@ -74,30 +74,30 @@ static void qsfp_dd_show_power_info(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 59, section 1.7.3.10, Table 31
  * [2] CMIS Rev. 4, pag. 94, section 8.3.10, Table 8-19
  */
-static void qsfp_dd_show_cbl_asm_len(const __u8 *id)
+static void cmis4_show_cbl_asm_len(const __u8 *id)
 {
 	static const char *fn = "Cable assembly length";
 	float mul = 1.0f;
 	float val = 0.0f;
 
 	/* Check if max length */
-	if (id[QSFP_DD_CBL_ASM_LEN_OFFSET] == QSFP_DD_6300M_MAX_LEN) {
+	if (id[CMIS4_CBL_ASM_LEN_OFFSET] == CMIS4_6300M_MAX_LEN) {
 		printf("\t%-41s : > 6.3km\n", fn);
 		return;
 	}
 
 	/* Get the multiplier from the first two bits */
-	switch (id[QSFP_DD_CBL_ASM_LEN_OFFSET] & QSFP_DD_LEN_MUL_MASK) {
-	case QSFP_DD_MULTIPLIER_00:
+	switch (id[CMIS4_CBL_ASM_LEN_OFFSET] & CMIS4_LEN_MUL_MASK) {
+	case CMIS4_MULTIPLIER_00:
 		mul = 0.1f;
 		break;
-	case QSFP_DD_MULTIPLIER_01:
+	case CMIS4_MULTIPLIER_01:
 		mul = 1.0f;
 		break;
-	case QSFP_DD_MULTIPLIER_10:
+	case CMIS4_MULTIPLIER_10:
 		mul = 10.0f;
 		break;
-	case QSFP_DD_MULTIPLIER_11:
+	case CMIS4_MULTIPLIER_11:
 		mul = 100.0f;
 		break;
 	default:
@@ -105,7 +105,7 @@ static void qsfp_dd_show_cbl_asm_len(const __u8 *id)
 	}
 
 	/* Get base value from first 6 bits and multiply by mul */
-	val = (id[QSFP_DD_CBL_ASM_LEN_OFFSET] & QSFP_DD_LEN_VAL_MASK);
+	val = (id[CMIS4_CBL_ASM_LEN_OFFSET] & CMIS4_LEN_VAL_MASK);
 	val = (float)val * mul;
 	printf("\t%-41s : %0.2fm\n", fn, val);
 }
@@ -117,18 +117,18 @@ static void qsfp_dd_show_cbl_asm_len(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 63, section 1.7.4.2, Table 39
  * [2] CMIS Rev. 4, pag. 99, section 8.4.2, Table 8-27
  */
-static void qsfp_dd_print_smf_cbl_len(const __u8 *id)
+static void cmis4_print_smf_cbl_len(const __u8 *id)
 {
 	static const char *fn = "Length (SMF)";
 	float mul = 1.0f;
 	float val = 0.0f;
 
 	/* Get the multiplier from the first two bits */
-	switch (id[QSFP_DD_SMF_LEN_OFFSET] & QSFP_DD_LEN_MUL_MASK) {
-	case QSFP_DD_MULTIPLIER_00:
+	switch (id[CMIS4_SMF_LEN_OFFSET] & CMIS4_LEN_MUL_MASK) {
+	case CMIS4_MULTIPLIER_00:
 		mul = 0.1f;
 		break;
-	case QSFP_DD_MULTIPLIER_01:
+	case CMIS4_MULTIPLIER_01:
 		mul = 1.0f;
 		break;
 	default:
@@ -136,7 +136,7 @@ static void qsfp_dd_print_smf_cbl_len(const __u8 *id)
 	}
 
 	/* Get base value from first 6 bits and multiply by mul */
-	val = (id[QSFP_DD_SMF_LEN_OFFSET] & QSFP_DD_LEN_VAL_MASK);
+	val = (id[CMIS4_SMF_LEN_OFFSET] & CMIS4_LEN_VAL_MASK);
 	val = (float)val * mul;
 	printf("\t%-41s : %0.2fkm\n", fn, val);
 }
@@ -146,21 +146,21 @@ static void qsfp_dd_print_smf_cbl_len(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 71, section 1.7.4.10, Table 46
  * [2] CMIS Rev. 4, pag. 105, section 8.4.10, Table 8-34
  */
-static void qsfp_dd_show_sig_integrity(const __u8 *id)
+static void cmis4_show_sig_integrity(const __u8 *id)
 {
 	/* CDR Bypass control: 2nd bit from each byte */
 	printf("\t%-41s : ", "Tx CDR bypass control");
-	printf("%s\n", YESNO(id[QSFP_DD_SIG_INTEG_TX_OFFSET] & 0x02));
+	printf("%s\n", YESNO(id[CMIS4_SIG_INTEG_TX_OFFSET] & 0x02));
 
 	printf("\t%-41s : ", "Rx CDR bypass control");
-	printf("%s\n", YESNO(id[QSFP_DD_SIG_INTEG_RX_OFFSET] & 0x02));
+	printf("%s\n", YESNO(id[CMIS4_SIG_INTEG_RX_OFFSET] & 0x02));
 
 	/* CDR Implementation: 1st bit from each byte */
 	printf("\t%-41s : ", "Tx CDR");
-	printf("%s\n", YESNO(id[QSFP_DD_SIG_INTEG_TX_OFFSET] & 0x01));
+	printf("%s\n", YESNO(id[CMIS4_SIG_INTEG_TX_OFFSET] & 0x01));
 
 	printf("\t%-41s : ", "Rx CDR");
-	printf("%s\n", YESNO(id[QSFP_DD_SIG_INTEG_RX_OFFSET] & 0x01));
+	printf("%s\n", YESNO(id[CMIS4_SIG_INTEG_RX_OFFSET] & 0x01));
 }
 
 /**
@@ -173,80 +173,80 @@ static void qsfp_dd_show_sig_integrity(const __u8 *id)
  * --> pag. 98, section 8.4, Table 8-25
  * --> page 100, section 8.4.3, 8.4.4
  */
-static void qsfp_dd_show_mit_compliance(const __u8 *id)
+static void cmis4_show_mit_compliance(const __u8 *id)
 {
 	static const char *cc = " (Copper cable,";
 
 	printf("\t%-41s : 0x%02x", "Transmitter technology",
-	       id[QSFP_DD_MEDIA_INTF_TECH_OFFSET]);
+	       id[CMIS4_MEDIA_INTF_TECH_OFFSET]);
 
-	switch (id[QSFP_DD_MEDIA_INTF_TECH_OFFSET]) {
-	case QSFP_DD_850_VCSEL:
+	switch (id[CMIS4_MEDIA_INTF_TECH_OFFSET]) {
+	case CMIS4_850_VCSEL:
 		printf(" (850 nm VCSEL)\n");
 		break;
-	case QSFP_DD_1310_VCSEL:
+	case CMIS4_1310_VCSEL:
 		printf(" (1310 nm VCSEL)\n");
 		break;
-	case QSFP_DD_1550_VCSEL:
+	case CMIS4_1550_VCSEL:
 		printf(" (1550 nm VCSEL)\n");
 		break;
-	case QSFP_DD_1310_FP:
+	case CMIS4_1310_FP:
 		printf(" (1310 nm FP)\n");
 		break;
-	case QSFP_DD_1310_DFB:
+	case CMIS4_1310_DFB:
 		printf(" (1310 nm DFB)\n");
 		break;
-	case QSFP_DD_1550_DFB:
+	case CMIS4_1550_DFB:
 		printf(" (1550 nm DFB)\n");
 		break;
-	case QSFP_DD_1310_EML:
+	case CMIS4_1310_EML:
 		printf(" (1310 nm EML)\n");
 		break;
-	case QSFP_DD_1550_EML:
+	case CMIS4_1550_EML:
 		printf(" (1550 nm EML)\n");
 		break;
-	case QSFP_DD_OTHERS:
+	case CMIS4_OTHERS:
 		printf(" (Others/Undefined)\n");
 		break;
-	case QSFP_DD_1490_DFB:
+	case CMIS4_1490_DFB:
 		printf(" (1490 nm DFB)\n");
 		break;
-	case QSFP_DD_COPPER_UNEQUAL:
+	case CMIS4_COPPER_UNEQUAL:
 		printf("%s unequalized)\n", cc);
 		break;
-	case QSFP_DD_COPPER_PASS_EQUAL:
+	case CMIS4_COPPER_PASS_EQUAL:
 		printf("%s passive equalized)\n", cc);
 		break;
-	case QSFP_DD_COPPER_NF_EQUAL:
+	case CMIS4_COPPER_NF_EQUAL:
 		printf("%s near and far end limiting active equalizers)\n", cc);
 		break;
-	case QSFP_DD_COPPER_F_EQUAL:
+	case CMIS4_COPPER_F_EQUAL:
 		printf("%s far end limiting active equalizers)\n", cc);
 		break;
-	case QSFP_DD_COPPER_N_EQUAL:
+	case CMIS4_COPPER_N_EQUAL:
 		printf("%s near end limiting active equalizers)\n", cc);
 		break;
-	case QSFP_DD_COPPER_LINEAR_EQUAL:
+	case CMIS4_COPPER_LINEAR_EQUAL:
 		printf("%s linear active equalizers)\n", cc);
 		break;
 	}
 
-	if (id[QSFP_DD_MEDIA_INTF_TECH_OFFSET] >= QSFP_DD_COPPER_UNEQUAL) {
+	if (id[CMIS4_MEDIA_INTF_TECH_OFFSET] >= CMIS4_COPPER_UNEQUAL) {
 		printf("\t%-41s : %udb\n", "Attenuation at 5GHz",
-		       id[QSFP_DD_COPPER_ATT_5GHZ]);
+		       id[CMIS4_COPPER_ATT_5GHZ]);
 		printf("\t%-41s : %udb\n", "Attenuation at 7GHz",
-		       id[QSFP_DD_COPPER_ATT_7GHZ]);
+		       id[CMIS4_COPPER_ATT_7GHZ]);
 		printf("\t%-41s : %udb\n", "Attenuation at 12.9GHz",
-		       id[QSFP_DD_COPPER_ATT_12P9GHZ]);
+		       id[CMIS4_COPPER_ATT_12P9GHZ]);
 		printf("\t%-41s : %udb\n", "Attenuation at 25.8GHz",
-		       id[QSFP_DD_COPPER_ATT_25P8GHZ]);
+		       id[CMIS4_COPPER_ATT_25P8GHZ]);
 	} else {
 		printf("\t%-41s : %.3lfnm\n", "Laser wavelength",
-		       (((id[QSFP_DD_NOM_WAVELENGTH_MSB] << 8) |
-				id[QSFP_DD_NOM_WAVELENGTH_LSB]) * 0.05));
+		       (((id[CMIS4_NOM_WAVELENGTH_MSB] << 8) |
+				id[CMIS4_NOM_WAVELENGTH_LSB]) * 0.05));
 		printf("\t%-41s : %.3lfnm\n", "Laser wavelength tolerance",
-		       (((id[QSFP_DD_WAVELENGTH_TOL_MSB] << 8) |
-		       id[QSFP_DD_WAVELENGTH_TOL_LSB]) * 0.005));
+		       (((id[CMIS4_WAVELENGTH_TOL_MSB] << 8) |
+		       id[CMIS4_WAVELENGTH_TOL_LSB]) * 0.005));
 	}
 }
 
@@ -266,24 +266,24 @@ static void qsfp_dd_show_mit_compliance(const __u8 *id)
  * [2] CMIS Rev. 4:
  * --> pag. 84, section 8.2.4, Table 8-6
  */
-static void qsfp_dd_show_mod_lvl_monitors(const __u8 *id)
+static void cmis4_show_mod_lvl_monitors(const __u8 *id)
 {
 	PRINT_TEMP("Module temperature",
-		   OFFSET_TO_TEMP(QSFP_DD_CURR_TEMP_OFFSET));
+		   OFFSET_TO_TEMP(CMIS4_CURR_TEMP_OFFSET));
 	PRINT_VCC("Module voltage",
-		  OFFSET_TO_U16(QSFP_DD_CURR_CURR_OFFSET));
+		  OFFSET_TO_U16(CMIS4_CURR_CURR_OFFSET));
 }
 
-static void qsfp_dd_show_link_len_from_page(const __u8 *page_one_data)
+static void cmis4_show_link_len_from_page(const __u8 *page_one_data)
 {
-	qsfp_dd_print_smf_cbl_len(page_one_data);
-	sff_show_value_with_unit(page_one_data, QSFP_DD_OM5_LEN_OFFSET,
+	cmis4_print_smf_cbl_len(page_one_data);
+	sff_show_value_with_unit(page_one_data, CMIS4_OM5_LEN_OFFSET,
 				 "Length (OM5)", 2, "m");
-	sff_show_value_with_unit(page_one_data, QSFP_DD_OM4_LEN_OFFSET,
+	sff_show_value_with_unit(page_one_data, CMIS4_OM4_LEN_OFFSET,
 				 "Length (OM4)", 2, "m");
-	sff_show_value_with_unit(page_one_data, QSFP_DD_OM3_LEN_OFFSET,
+	sff_show_value_with_unit(page_one_data, CMIS4_OM3_LEN_OFFSET,
 				 "Length (OM3 50/125um)", 2, "m");
-	sff_show_value_with_unit(page_one_data, QSFP_DD_OM2_LEN_OFFSET,
+	sff_show_value_with_unit(page_one_data, CMIS4_OM2_LEN_OFFSET,
 				 "Length (OM2 50/125um)", 1, "m");
 }
 
@@ -295,9 +295,9 @@ static void qsfp_dd_show_link_len_from_page(const __u8 *page_one_data)
  * [1] CMIS Rev. 3, page 64, section 1.7.4.2, Table 39
  * [2] CMIS Rev. 4, page 99, section 8.4.2, Table 8-27
  */
-static void qsfp_dd_show_link_len(const __u8 *id)
+static void cmis4_show_link_len(const __u8 *id)
 {
-	qsfp_dd_show_link_len_from_page(id + PAG01H_UPPER_OFFSET);
+	cmis4_show_link_len_from_page(id + PAG01H_UPPER_OFFSET);
 }
 
 /**
@@ -305,37 +305,37 @@ static void qsfp_dd_show_link_len(const __u8 *id)
  * [1] CMIS Rev. 3, page 56, section 1.7.3, Table 27
  * [2] CMIS Rev. 4, page 91, section 8.2, Table 8-15
  */
-static void qsfp_dd_show_vendor_info(const __u8 *id)
+static void cmis4_show_vendor_info(const __u8 *id)
 {
-	sff_show_ascii(id, QSFP_DD_VENDOR_NAME_START_OFFSET,
-		       QSFP_DD_VENDOR_NAME_END_OFFSET, "Vendor name");
-	qsfp_dd_show_oui(id);
-	sff_show_ascii(id, QSFP_DD_VENDOR_PN_START_OFFSET,
-		       QSFP_DD_VENDOR_PN_END_OFFSET, "Vendor PN");
-	sff_show_ascii(id, QSFP_DD_VENDOR_REV_START_OFFSET,
-		       QSFP_DD_VENDOR_REV_END_OFFSET, "Vendor rev");
-	sff_show_ascii(id, QSFP_DD_VENDOR_SN_START_OFFSET,
-		       QSFP_DD_VENDOR_SN_END_OFFSET, "Vendor SN");
-	sff_show_ascii(id, QSFP_DD_DATE_YEAR_OFFSET,
-		       QSFP_DD_DATE_VENDOR_LOT_OFFSET + 1, "Date code");
-
-	if (id[QSFP_DD_CLEI_PRESENT_BYTE] & QSFP_DD_CLEI_PRESENT_MASK)
-		sff_show_ascii(id, QSFP_DD_CLEI_START_OFFSET,
-			       QSFP_DD_CLEI_END_OFFSET, "CLEI code");
+	sff_show_ascii(id, CMIS4_VENDOR_NAME_START_OFFSET,
+		       CMIS4_VENDOR_NAME_END_OFFSET, "Vendor name");
+	cmis4_show_oui(id);
+	sff_show_ascii(id, CMIS4_VENDOR_PN_START_OFFSET,
+		       CMIS4_VENDOR_PN_END_OFFSET, "Vendor PN");
+	sff_show_ascii(id, CMIS4_VENDOR_REV_START_OFFSET,
+		       CMIS4_VENDOR_REV_END_OFFSET, "Vendor rev");
+	sff_show_ascii(id, CMIS4_VENDOR_SN_START_OFFSET,
+		       CMIS4_VENDOR_SN_END_OFFSET, "Vendor SN");
+	sff_show_ascii(id, CMIS4_DATE_YEAR_OFFSET,
+		       CMIS4_DATE_VENDOR_LOT_OFFSET + 1, "Date code");
+
+	if (id[CMIS4_CLEI_PRESENT_BYTE] & CMIS4_CLEI_PRESENT_MASK)
+		sff_show_ascii(id, CMIS4_CLEI_START_OFFSET,
+			       CMIS4_CLEI_END_OFFSET, "CLEI code");
 }
 
 void qsfp_dd_show_all(const __u8 *id)
 {
-	qsfp_dd_show_identifier(id);
-	qsfp_dd_show_power_info(id);
-	qsfp_dd_show_connector(id);
-	qsfp_dd_show_cbl_asm_len(id);
-	qsfp_dd_show_sig_integrity(id);
-	qsfp_dd_show_mit_compliance(id);
-	qsfp_dd_show_mod_lvl_monitors(id);
-	qsfp_dd_show_link_len(id);
-	qsfp_dd_show_vendor_info(id);
-	qsfp_dd_show_rev_compliance(id);
+	cmis4_show_identifier(id);
+	cmis4_show_power_info(id);
+	cmis4_show_connector(id);
+	cmis4_show_cbl_asm_len(id);
+	cmis4_show_sig_integrity(id);
+	cmis4_show_mit_compliance(id);
+	cmis4_show_mod_lvl_monitors(id);
+	cmis4_show_link_len(id);
+	cmis4_show_vendor_info(id);
+	cmis4_show_rev_compliance(id);
 }
 
 void cmis4_show_all(const struct ethtool_module_eeprom *page_zero,
@@ -343,17 +343,17 @@ void cmis4_show_all(const struct ethtool_module_eeprom *page_zero,
 {
 	const __u8 *page_zero_data = page_zero->data;
 
-	qsfp_dd_show_identifier(page_zero_data);
-	qsfp_dd_show_power_info(page_zero_data);
-	qsfp_dd_show_connector(page_zero_data);
-	qsfp_dd_show_cbl_asm_len(page_zero_data);
-	qsfp_dd_show_sig_integrity(page_zero_data);
-	qsfp_dd_show_mit_compliance(page_zero_data);
-	qsfp_dd_show_mod_lvl_monitors(page_zero_data);
+	cmis4_show_identifier(page_zero_data);
+	cmis4_show_power_info(page_zero_data);
+	cmis4_show_connector(page_zero_data);
+	cmis4_show_cbl_asm_len(page_zero_data);
+	cmis4_show_sig_integrity(page_zero_data);
+	cmis4_show_mit_compliance(page_zero_data);
+	cmis4_show_mod_lvl_monitors(page_zero_data);
 
 	if (page_one)
-		qsfp_dd_show_link_len_from_page(page_one->data);
+		cmis4_show_link_len_from_page(page_one->data);
 
-	qsfp_dd_show_vendor_info(page_zero_data);
-	qsfp_dd_show_rev_compliance(page_zero_data);
+	cmis4_show_vendor_info(page_zero_data);
+	cmis4_show_rev_compliance(page_zero_data);
 }
diff --git a/cmis4.h b/cmis4.h
new file mode 100644
index 0000000..ba2b2e2
--- /dev/null
+++ b/cmis4.h
@@ -0,0 +1,128 @@
+#ifndef CMIS4_H__
+#define CMIS4_H__
+
+/* Identifier and revision compliance (Page 0) */
+#define CMIS4_ID_OFFSET				0x00
+#define CMIS4_REV_COMPLIANCE_OFFSET		0x01
+
+#define CMIS4_MODULE_TYPE_OFFSET		0x55
+#define CMIS4_MT_MMF				0x01
+#define CMIS4_MT_SMF				0x02
+
+/* Module-Level Monitors (Page 0) */
+#define CMIS4_CURR_TEMP_OFFSET			0x0E
+#define CMIS4_CURR_CURR_OFFSET			0x10
+
+#define CMIS4_CTOR_OFFSET			0xCB
+
+/* Vendor related information (Page 0) */
+#define CMIS4_VENDOR_NAME_START_OFFSET		0x81
+#define CMIS4_VENDOR_NAME_END_OFFSET		0x90
+
+#define CMIS4_VENDOR_OUI_OFFSET			0x91
+
+#define CMIS4_VENDOR_PN_START_OFFSET		0x94
+#define CMIS4_VENDOR_PN_END_OFFSET		0xA3
+
+#define CMIS4_VENDOR_REV_START_OFFSET		0xA4
+#define CMIS4_VENDOR_REV_END_OFFSET		0xA5
+
+#define CMIS4_VENDOR_SN_START_OFFSET		0xA6
+#define CMIS4_VENDOR_SN_END_OFFSET		0xB5
+
+#define CMIS4_DATE_YEAR_OFFSET			0xB6
+#define CMIS4_DATE_VENDOR_LOT_OFFSET		0xBC
+
+/* CLEI Code (Page 0) */
+#define CMIS4_CLEI_PRESENT_BYTE			0x02
+#define CMIS4_CLEI_PRESENT_MASK			0x20
+#define CMIS4_CLEI_START_OFFSET			0xBE
+#define CMIS4_CLEI_END_OFFSET			0xC7
+
+/* Cable assembly length */
+#define CMIS4_CBL_ASM_LEN_OFFSET		0xCA
+#define CMIS4_6300M_MAX_LEN			0xFF
+
+/* Cable length with multiplier */
+#define CMIS4_MULTIPLIER_00			0x00
+#define CMIS4_MULTIPLIER_01			0x40
+#define CMIS4_MULTIPLIER_10			0x80
+#define CMIS4_MULTIPLIER_11			0xC0
+#define CMIS4_LEN_MUL_MASK			0xC0
+#define CMIS4_LEN_VAL_MASK			0x3F
+
+/* Module power characteristics */
+#define CMIS4_PWR_CLASS_OFFSET			0xC8
+#define CMIS4_PWR_MAX_POWER_OFFSET		0xC9
+#define CMIS4_PWR_CLASS_MASK			0xE0
+#define CMIS4_PWR_CLASS_1			0x00
+#define CMIS4_PWR_CLASS_2			0x01
+#define CMIS4_PWR_CLASS_3			0x02
+#define CMIS4_PWR_CLASS_4			0x03
+#define CMIS4_PWR_CLASS_5			0x04
+#define CMIS4_PWR_CLASS_6			0x05
+#define CMIS4_PWR_CLASS_7			0x06
+#define CMIS4_PWR_CLASS_8			0x07
+
+/* Copper cable attenuation */
+#define CMIS4_COPPER_ATT_5GHZ			0xCC
+#define CMIS4_COPPER_ATT_7GHZ			0xCD
+#define CMIS4_COPPER_ATT_12P9GHZ		0xCE
+#define CMIS4_COPPER_ATT_25P8GHZ		0xCF
+
+/* Cable assembly lane */
+#define CMIS4_CABLE_ASM_NEAR_END_OFFSET		0xD2
+#define CMIS4_CABLE_ASM_FAR_END_OFFSET		0xD3
+
+/* Media interface technology */
+#define CMIS4_MEDIA_INTF_TECH_OFFSET		0xD4
+#define CMIS4_850_VCSEL				0x00
+#define CMIS4_1310_VCSEL			0x01
+#define CMIS4_1550_VCSEL			0x02
+#define CMIS4_1310_FP				0x03
+#define CMIS4_1310_DFB				0x04
+#define CMIS4_1550_DFB				0x05
+#define CMIS4_1310_EML				0x06
+#define CMIS4_1550_EML				0x07
+#define CMIS4_OTHERS				0x08
+#define CMIS4_1490_DFB				0x09
+#define CMIS4_COPPER_UNEQUAL			0x0A
+#define CMIS4_COPPER_PASS_EQUAL			0x0B
+#define CMIS4_COPPER_NF_EQUAL			0x0C
+#define CMIS4_COPPER_F_EQUAL			0x0D
+#define CMIS4_COPPER_N_EQUAL			0x0E
+#define CMIS4_COPPER_LINEAR_EQUAL		0x0F
+
+/*-----------------------------------------------------------------------
+ * Upper Memory Page 0x01: contains advertising fields that define properties
+ * that are unique to active modules and cable assemblies.
+ * GlobalOffset = 2 * 0x80 + LocalOffset
+ */
+#define PAG01H_UPPER_OFFSET			(0x02 * 0x80)
+
+/* Supported Link Length (Page 1) */
+#define CMIS4_SMF_LEN_OFFSET			0x4
+#define CMIS4_OM5_LEN_OFFSET			0x5
+#define CMIS4_OM4_LEN_OFFSET			0x6
+#define CMIS4_OM3_LEN_OFFSET			0x7
+#define CMIS4_OM2_LEN_OFFSET			0x8
+
+/* Wavelength (Page 1) */
+#define CMIS4_NOM_WAVELENGTH_MSB		0xA
+#define CMIS4_NOM_WAVELENGTH_LSB		0xB
+#define CMIS4_WAVELENGTH_TOL_MSB		0xC
+#define CMIS4_WAVELENGTH_TOL_LSB		0xD
+
+/* Signal integrity controls */
+#define CMIS4_SIG_INTEG_TX_OFFSET		0xA1
+#define CMIS4_SIG_INTEG_RX_OFFSET		0xA2
+
+#define YESNO(x) (((x) != 0) ? "Yes" : "No")
+#define ONOFF(x) (((x) != 0) ? "On" : "Off")
+
+void qsfp_dd_show_all(const __u8 *id);
+
+void cmis4_show_all(const struct ethtool_module_eeprom *page_zero,
+		    const struct ethtool_module_eeprom *page_one);
+
+#endif /* CMIS4_H__ */
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index 768a261..5f58125 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -11,7 +11,7 @@
 
 #include "../sff-common.h"
 #include "../qsfp.h"
-#include "../qsfp-dd.h"
+#include "../cmis4.h"
 #include "../internal.h"
 #include "../common.h"
 #include "../list.h"
diff --git a/qsfp-dd.h b/qsfp-dd.h
deleted file mode 100644
index fddb188..0000000
--- a/qsfp-dd.h
+++ /dev/null
@@ -1,128 +0,0 @@
-#ifndef QSFP_DD_H__
-#define QSFP_DD_H__
-
-/* Identifier and revision compliance (Page 0) */
-#define QSFP_DD_ID_OFFSET			0x00
-#define QSFP_DD_REV_COMPLIANCE_OFFSET		0x01
-
-#define QSFP_DD_MODULE_TYPE_OFFSET		0x55
-#define QSFP_DD_MT_MMF				0x01
-#define QSFP_DD_MT_SMF				0x02
-
-/* Module-Level Monitors (Page 0) */
-#define QSFP_DD_CURR_TEMP_OFFSET		0x0E
-#define QSFP_DD_CURR_CURR_OFFSET		0x10
-
-#define QSFP_DD_CTOR_OFFSET			0xCB
-
-/* Vendor related information (Page 0) */
-#define QSFP_DD_VENDOR_NAME_START_OFFSET	0x81
-#define QSFP_DD_VENDOR_NAME_END_OFFSET		0x90
-
-#define QSFP_DD_VENDOR_OUI_OFFSET		0x91
-
-#define QSFP_DD_VENDOR_PN_START_OFFSET		0x94
-#define QSFP_DD_VENDOR_PN_END_OFFSET		0xA3
-
-#define QSFP_DD_VENDOR_REV_START_OFFSET		0xA4
-#define QSFP_DD_VENDOR_REV_END_OFFSET		0xA5
-
-#define QSFP_DD_VENDOR_SN_START_OFFSET		0xA6
-#define QSFP_DD_VENDOR_SN_END_OFFSET		0xB5
-
-#define QSFP_DD_DATE_YEAR_OFFSET		0xB6
-#define QSFP_DD_DATE_VENDOR_LOT_OFFSET		0xBC
-
-/* CLEI Code (Page 0) */
-#define QSFP_DD_CLEI_PRESENT_BYTE		0x02
-#define QSFP_DD_CLEI_PRESENT_MASK		0x20
-#define QSFP_DD_CLEI_START_OFFSET		0xBE
-#define QSFP_DD_CLEI_END_OFFSET			0xC7
-
-/* Cable assembly length */
-#define QSFP_DD_CBL_ASM_LEN_OFFSET		0xCA
-#define QSFP_DD_6300M_MAX_LEN			0xFF
-
-/* Cable length with multiplier */
-#define QSFP_DD_MULTIPLIER_00			0x00
-#define QSFP_DD_MULTIPLIER_01			0x40
-#define QSFP_DD_MULTIPLIER_10			0x80
-#define QSFP_DD_MULTIPLIER_11			0xC0
-#define QSFP_DD_LEN_MUL_MASK			0xC0
-#define QSFP_DD_LEN_VAL_MASK			0x3F
-
-/* Module power characteristics */
-#define QSFP_DD_PWR_CLASS_OFFSET		0xC8
-#define QSFP_DD_PWR_MAX_POWER_OFFSET		0xC9
-#define QSFP_DD_PWR_CLASS_MASK			0xE0
-#define QSFP_DD_PWR_CLASS_1			0x00
-#define QSFP_DD_PWR_CLASS_2			0x01
-#define QSFP_DD_PWR_CLASS_3			0x02
-#define QSFP_DD_PWR_CLASS_4			0x03
-#define QSFP_DD_PWR_CLASS_5			0x04
-#define QSFP_DD_PWR_CLASS_6			0x05
-#define QSFP_DD_PWR_CLASS_7			0x06
-#define QSFP_DD_PWR_CLASS_8			0x07
-
-/* Copper cable attenuation */
-#define QSFP_DD_COPPER_ATT_5GHZ			0xCC
-#define QSFP_DD_COPPER_ATT_7GHZ			0xCD
-#define QSFP_DD_COPPER_ATT_12P9GHZ		0xCE
-#define QSFP_DD_COPPER_ATT_25P8GHZ		0xCF
-
-/* Cable assembly lane */
-#define QSFP_DD_CABLE_ASM_NEAR_END_OFFSET	0xD2
-#define QSFP_DD_CABLE_ASM_FAR_END_OFFSET	0xD3
-
-/* Media interface technology */
-#define QSFP_DD_MEDIA_INTF_TECH_OFFSET		0xD4
-#define QSFP_DD_850_VCSEL			0x00
-#define QSFP_DD_1310_VCSEL			0x01
-#define QSFP_DD_1550_VCSEL			0x02
-#define QSFP_DD_1310_FP				0x03
-#define QSFP_DD_1310_DFB			0x04
-#define QSFP_DD_1550_DFB			0x05
-#define QSFP_DD_1310_EML			0x06
-#define QSFP_DD_1550_EML			0x07
-#define QSFP_DD_OTHERS				0x08
-#define QSFP_DD_1490_DFB			0x09
-#define QSFP_DD_COPPER_UNEQUAL			0x0A
-#define QSFP_DD_COPPER_PASS_EQUAL		0x0B
-#define QSFP_DD_COPPER_NF_EQUAL			0x0C
-#define QSFP_DD_COPPER_F_EQUAL			0x0D
-#define QSFP_DD_COPPER_N_EQUAL			0x0E
-#define QSFP_DD_COPPER_LINEAR_EQUAL		0x0F
-
-/*-----------------------------------------------------------------------
- * Upper Memory Page 0x01: contains advertising fields that define properties
- * that are unique to active modules and cable assemblies.
- * GlobalOffset = 2 * 0x80 + LocalOffset
- */
-#define PAG01H_UPPER_OFFSET			(0x02 * 0x80)
-
-/* Supported Link Length (Page 1) */
-#define QSFP_DD_SMF_LEN_OFFSET			0x4
-#define QSFP_DD_OM5_LEN_OFFSET			0x5
-#define QSFP_DD_OM4_LEN_OFFSET			0x6
-#define QSFP_DD_OM3_LEN_OFFSET			0x7
-#define QSFP_DD_OM2_LEN_OFFSET			0x8
-
-/* Wavelength (Page 1) */
-#define QSFP_DD_NOM_WAVELENGTH_MSB		0xA
-#define QSFP_DD_NOM_WAVELENGTH_LSB		0xB
-#define QSFP_DD_WAVELENGTH_TOL_MSB		0xC
-#define QSFP_DD_WAVELENGTH_TOL_LSB		0xD
-
-/* Signal integrity controls */
-#define QSFP_DD_SIG_INTEG_TX_OFFSET		0xA1
-#define QSFP_DD_SIG_INTEG_RX_OFFSET		0xA2
-
-#define YESNO(x) (((x) != 0) ? "Yes" : "No")
-#define ONOFF(x) (((x) != 0) ? "On" : "Off")
-
-void qsfp_dd_show_all(const __u8 *id);
-
-void cmis4_show_all(const struct ethtool_module_eeprom *page_zero,
-		    const struct ethtool_module_eeprom *page_one);
-
-#endif /* QSFP_DD_H__ */
diff --git a/qsfp.c b/qsfp.c
index a4d7e08..5923c0d 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -58,7 +58,7 @@
 #include "internal.h"
 #include "sff-common.h"
 #include "qsfp.h"
-#include "qsfp-dd.h"
+#include "cmis4.h"
 
 #define MAX_DESC_SIZE	42
 
-- 
2.26.2

[PATCH ethtool-next 4/4] ethtool: Update manpages for getmodule (-m) command

[Moshe Shemesh]

From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

Add page, bank and i2c parameters and mention change in offset and
length treatment if either one of new parameters is specified by the
user.

Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
---
 ethtool.8.in | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index fe49b66..9516458 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -359,6 +359,9 @@ ethtool \- query or control network driver and hardware settings
 .B2 hex on off
 .BN offset
 .BN length
+.BN page
+.BN bank
+.BN i2c
 .HP
 .B ethtool \-\-show\-priv\-flags
 .I devname
@@ -1154,6 +1157,17 @@ Changes the number of multi-purpose channels.
 Retrieves and if possible decodes the EEPROM from plugin modules, e.g SFP+, QSFP.
 If the driver and module support it, the optical diagnostic information is also
 read and decoded.
+When either one of
+.I page,
+.I bank
+or
+.I i2c
+parameters is specified, dumps only of a single page or its portion is
+allowed. In such a case
+.I offset
+and
+.I length
+parameters are treated relatively to EEPROM page boundaries.
 .TP
 .B \-\-show\-priv\-flags
 Queries the specified network device for its private flags.  The
-- 
2.26.2

RE: [PATCH ethtool-next 0/4] Extend module EEPROM API

[Don Bollinger]

> From: Moshe Shemesh [mailto:moshe@nvidia.com]
> Sent: Friday, April 23, 2021 12:23 AM
> To: Michal Kubecek <mkubecek@suse.cz>; Andrew Lunn
> <andrew@lunn.ch>; Jakub Kicinski <kuba@kernel.org>; Don Bollinger
> <don@thebollingers.org>; netdev@vger.kernel.org
> Cc: Vladyslav Tarasiuk <vladyslavt@nvidia.com>; Moshe Shemesh
> <moshe@nvidia.com>
> Subject: [PATCH ethtool-next 0/4] Extend module EEPROM API
> 
> Ethtool supports module EEPROM dumps via the `ethtool -m <dev>`
> command.
> But in current state its functionality is limited - offset and length
parameters,
> which are used to specify a linear desired region of EEPROM data to dump,
is
> not enough, considering emergence of complex module EEPROM layouts
> such as CMIS 4.0.
> 
> Moreover, CMIS 4.0 extends the amount of pages that may be accessible by
> introducing another parameter for page addressing - banks.
> Besides, currently module EEPROM is represented as a chunk of
> concatenated pages, where lower 128 bytes of all pages, except page 00h,
> are omitted. Offset and length are used to address parts of this fake
linear
> memory. But in practice drivers, which implement
> get_module_info() and get_module_eeprom() ethtool ops still calculate
> page number and set I2C address on their own.
> 
> This series adds support in `ethtool -m` of dumping an arbitrary page
> specified by page number, bank number and I2C address. Implement netlink
> handler for `ethtool -m` in order to make such requests to the kernel and
> extend CLI by adding corresponding parameters.
> New command line format:
>  ethtool -m <dev> [hex on|off] [raw on|off] [offset N] [length N] [page N]
> [bank N] [i2c N]
> 
> Netlink infrastructure works on per-page basis and allows dumps of a
single
> page at once. But in case user requests human-readable output, which
> currently may require more than one page, userspace can make such
> additional calls to kernel on demand and place pages in a linked list.
> It allows to get pages from cache on demand and pass them to refactored
> SFF decoders.
> 
> Vladyslav Tarasiuk (4):
>   ethtool: Add netlink handler for getmodule (-m)
>   ethtool: Refactor human-readable module EEPROM output for new API
>   ethtool: Rename QSFP-DD identifiers to use CMIS 4.0
>   ethtool: Update manpages to reflect changes to getmodule (-m) command
> 
>  Makefile.am             |   3 +-
>  qsfp-dd.c => cmis4.c    | 220 +++++++++++---------
>  cmis4.h                 | 128 ++++++++++++
>  ethtool.8.in            |  14 ++
>  ethtool.c               |   4 +
>  internal.h              |  12 ++
>  list.h                  |  34 ++++
>  netlink/desc-ethtool.c  |  13 ++
>  netlink/extapi.h        |   2 +
>  netlink/module-eeprom.c | 438
> ++++++++++++++++++++++++++++++++++++++++
>  qsfp-dd.h               | 125 ------------
>  qsfp.c                  | 129 +++++++-----
>  qsfp.h                  |  52 ++---
>  sff-common.c            |   3 +
>  sff-common.h            |   3 +-
>  15 files changed, 876 insertions(+), 304 deletions(-)  rename qsfp-dd.c
=>
> cmis4.c (55%)  create mode 100644 cmis4.h  create mode 100644 list.h
create
> mode 100644 netlink/module-eeprom.c  delete mode 100644 qsfp-dd.h
> 
> --
> 2.26.2

Will there be an ethtool option to write to module eeprom in CMIS format?
There are routine functions to configure the devices that require writing
appropriate values to various registers.  Byte 26 allows software control of
low power mode, squelch and software reset.  Page 10h is full of Lane and
Data Path Control registers.

Beyond the spec, but allowed by the spec, there are vendor specific
capabilities like firmware download that require bulk write (up to 128 bytes
per write).

Using the full capabilities of these devices will require write capability.
Ethtool is the only path that will be allowed.

Don  

RE: [PATCH ethtool-next 2/4] ethtool: Refactor human-readable module EEPROM output

[Don Bollinger]

> From: Moshe Shemesh [mailto:moshe@nvidia.com]
> Sent: Friday, April 23, 2021 12:23 AM
> To: Michal Kubecek <mkubecek@suse.cz>; Andrew Lunn
> <andrew@lunn.ch>; Jakub Kicinski <kuba@kernel.org>; Don Bollinger
> <don@thebollingers.org>; netdev@vger.kernel.org
> Cc: Vladyslav Tarasiuk <vladyslavt@nvidia.com>; Moshe Shemesh
> <moshe@nvidia.com>
> Subject: [PATCH ethtool-next 2/4] ethtool: Refactor human-readable module
> EEPROM output
> 
> From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
> 
> Reuse existing SFF8636 and QSFP-DD infrastructures to implement EEPROM
> decoders, which work with paged memory. Add support for human-readable
> output for QSFP, QSFP28, QSFP Plus, QSFP-DD and DSFP transreceivers from
> netlink 'ethtool -m' handler.
> 
> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
> ---
>  internal.h              |   2 +
>  netlink/module-eeprom.c |  13 ++++
>  qsfp-dd.c               |  44 +++++++++++---
>  qsfp-dd.h               |  29 +++++----
>  qsfp.c                  | 127 +++++++++++++++++++++++-----------------
>  qsfp.h                  |  52 ++++++++--------
>  sff-common.c            |   3 +
>  sff-common.h            |   3 +-
>  8 files changed, 171 insertions(+), 102 deletions(-)
> 
> diff --git a/internal.h b/internal.h
> index 2affebe..33e619b 100644
> --- a/internal.h
> +++ b/internal.h
> @@ -391,6 +391,8 @@ void sff8472_show_all(const __u8 *id);
> 
>  /* QSFP Optics diagnostics */
>  void sff8636_show_all(const __u8 *id, __u32 eeprom_len);
> +void sff8636_show_all_paged(const struct ethtool_module_eeprom
> *page_zero,
> +			    const struct ethtool_module_eeprom
> *page_three);
> 
>  /* FUJITSU Extended Socket network device */  int fjes_dump_regs(struct
> ethtool_drvinfo *info, struct ethtool_regs *regs); diff --git
a/netlink/module-
> eeprom.c b/netlink/module-eeprom.c index 7298087..768a261 100644
> --- a/netlink/module-eeprom.c
> +++ b/netlink/module-eeprom.c
> @@ -291,6 +291,7 @@ static bool page_available(struct
> ethtool_module_eeprom *which)
>  	case SFF8024_ID_QSFP_PLUS:
>  		return (!which->bank && which->page <= 3);
>  	case SFF8024_ID_QSFP_DD:
> +	case SFF8024_ID_DSFP:
>  		return !(flat_mem && which->page);
>  	default:
>  		return true;
> @@ -323,6 +324,7 @@ static int decoder_prefetch(struct nl_context *nlctx)
>  		request.page = 3;
>  		break;
>  	case SFF8024_ID_QSFP_DD:
> +	case SFF8024_ID_DSFP:
>  		memset(&request, 0, sizeof(request));
>  		request.i2c_address = ETH_I2C_ADDRESS_LOW;
>  		request.offset = 128;
> @@ -336,13 +338,24 @@ static int decoder_prefetch(struct nl_context
> *nlctx)
> 
>  static void decoder_print(void)
>  {
> +	struct ethtool_module_eeprom *page_three = cache_get(3, 0,
> +ETH_I2C_ADDRESS_LOW);
>  	struct ethtool_module_eeprom *page_zero = cache_get(0, 0,
> ETH_I2C_ADDRESS_LOW);
> +	struct ethtool_module_eeprom *page_one = cache_get(1, 0,
> +ETH_I2C_ADDRESS_LOW);
>  	u8 module_id = page_zero->data[SFF8636_ID_OFFSET];
> 
>  	switch (module_id) {
>  	case SFF8024_ID_SFP:
>  		sff8079_show_all(page_zero->data);
>  		break;
> +	case SFF8024_ID_QSFP:
> +	case SFF8024_ID_QSFP28:
> +	case SFF8024_ID_QSFP_PLUS:
> +		sff8636_show_all_paged(page_zero, page_three);
> +		break;
> +	case SFF8024_ID_QSFP_DD:
> +	case SFF8024_ID_DSFP:
> +		cmis4_show_all(page_zero, page_one);
> +		break;
>  	default:
>  		dump_hex(stdout, page_zero->data, page_zero->length,
> page_zero->offset);
>  		break;
> diff --git a/qsfp-dd.c b/qsfp-dd.c
> index 900bbb5..5c2e4a0 100644
> --- a/qsfp-dd.c
> +++ b/qsfp-dd.c
> @@ -274,6 +274,20 @@ static void qsfp_dd_show_mod_lvl_monitors(const
> __u8 *id)
>  		  OFFSET_TO_U16(QSFP_DD_CURR_CURR_OFFSET));
>  }
> 
> +static void qsfp_dd_show_link_len_from_page(const __u8
> *page_one_data)
> +{
> +	qsfp_dd_print_smf_cbl_len(page_one_data);
> +	sff_show_value_with_unit(page_one_data,
> QSFP_DD_OM5_LEN_OFFSET,
> +				 "Length (OM5)", 2, "m");
> +	sff_show_value_with_unit(page_one_data,
> QSFP_DD_OM4_LEN_OFFSET,
> +				 "Length (OM4)", 2, "m");
> +	sff_show_value_with_unit(page_one_data,
> QSFP_DD_OM3_LEN_OFFSET,
> +				 "Length (OM3 50/125um)", 2, "m");
> +	sff_show_value_with_unit(page_one_data,
> QSFP_DD_OM2_LEN_OFFSET,
> +				 "Length (OM2 50/125um)", 1, "m");
> +}
> +
> +
>  /**
>   * Print relevant info about the maximum supported fiber media length
>   * for each type of fiber media at the maximum module-supported bit rate.
> @@ -283,15 +297,7 @@ static void qsfp_dd_show_mod_lvl_monitors(const
> __u8 *id)
>   */
>  static void qsfp_dd_show_link_len(const __u8 *id)  {
> -	qsfp_dd_print_smf_cbl_len(id);
> -	sff_show_value_with_unit(id, QSFP_DD_OM5_LEN_OFFSET,
> -				 "Length (OM5)", 2, "m");
> -	sff_show_value_with_unit(id, QSFP_DD_OM4_LEN_OFFSET,
> -				 "Length (OM4)", 2, "m");
> -	sff_show_value_with_unit(id, QSFP_DD_OM3_LEN_OFFSET,
> -				 "Length (OM3 50/125um)", 2, "m");
> -	sff_show_value_with_unit(id, QSFP_DD_OM2_LEN_OFFSET,
> -				 "Length (OM2 50/125um)", 1, "m");
> +	qsfp_dd_show_link_len_from_page(id + PAG01H_UPPER_OFFSET);
>  }
> 
>  /**
> @@ -331,3 +337,23 @@ void qsfp_dd_show_all(const __u8 *id)
>  	qsfp_dd_show_vendor_info(id);
>  	qsfp_dd_show_rev_compliance(id);
>  }
> +
> +void cmis4_show_all(const struct ethtool_module_eeprom *page_zero,
> +		    const struct ethtool_module_eeprom *page_one) {
> +	const __u8 *page_zero_data = page_zero->data;
> +
> +	qsfp_dd_show_identifier(page_zero_data);
> +	qsfp_dd_show_power_info(page_zero_data);
> +	qsfp_dd_show_connector(page_zero_data);
> +	qsfp_dd_show_cbl_asm_len(page_zero_data);
> +	qsfp_dd_show_sig_integrity(page_zero_data);
> +	qsfp_dd_show_mit_compliance(page_zero_data);
> +	qsfp_dd_show_mod_lvl_monitors(page_zero_data);
> +
> +	if (page_one)
> +		qsfp_dd_show_link_len_from_page(page_one->data);
> +
> +	qsfp_dd_show_vendor_info(page_zero_data);
> +	qsfp_dd_show_rev_compliance(page_zero_data);
> +}
> diff --git a/qsfp-dd.h b/qsfp-dd.h
> index f589c4e..fddb188 100644
> --- a/qsfp-dd.h
> +++ b/qsfp-dd.h
> @@ -96,30 +96,33 @@
>  /*-----------------------------------------------------------------------
>   * Upper Memory Page 0x01: contains advertising fields that define
> properties
>   * that are unique to active modules and cable assemblies.
> - * RealOffset = 1 * 0x80 + LocalOffset
> + * GlobalOffset = 2 * 0x80 + LocalOffset
>   */
> -#define PAG01H_UPPER_OFFSET			(0x01 * 0x80)
> +#define PAG01H_UPPER_OFFSET			(0x02 * 0x80)
> 
>  /* Supported Link Length (Page 1) */
> -#define QSFP_DD_SMF_LEN_OFFSET
> 	(PAG01H_UPPER_OFFSET + 0x84)
> -#define QSFP_DD_OM5_LEN_OFFSET
> 	(PAG01H_UPPER_OFFSET + 0x85)
> -#define QSFP_DD_OM4_LEN_OFFSET
> 	(PAG01H_UPPER_OFFSET + 0x86)
> -#define QSFP_DD_OM3_LEN_OFFSET
> 	(PAG01H_UPPER_OFFSET + 0x87)
> -#define QSFP_DD_OM2_LEN_OFFSET
> 	(PAG01H_UPPER_OFFSET + 0x88)
> +#define QSFP_DD_SMF_LEN_OFFSET			0x4
> +#define QSFP_DD_OM5_LEN_OFFSET			0x5
> +#define QSFP_DD_OM4_LEN_OFFSET			0x6
> +#define QSFP_DD_OM3_LEN_OFFSET			0x7
> +#define QSFP_DD_OM2_LEN_OFFSET			0x8

I see here you have switched from offsets from the beginning of flat linear
memory to offsets within the upper half page.  I recommend actually using
the values in the spec, which are offset from the base of the page.  I know
the first 128 bytes are just copies of page 0, and aren't being used.  But
the specs all describe these register offsets in the range 128-255.  This is
also the actual values the driver is sending to the device.  There is
actually no good reason to redefine these values as if they were offsets
from 128.

Thus, QSFP_DD_SMF_LEN_OFFSET should be 0x84, just like the spec says, and
just like the offset the driver would send to the module to get this byte.
It will be MUCH easier to compare hundreds of hard coded values to the spec
if you aren't mentally translating them every time they are copied or
reviewed.

Don

RE: [PATCH ethtool-next 4/4] ethtool: Update manpages for getmodule (-m) command

[Don Bollinger]

> From: Moshe Shemesh [mailto:moshe@nvidia.com]
> Sent: Friday, April 23, 2021 12:23 AM
> To: Michal Kubecek <mkubecek@suse.cz>; Andrew Lunn
> <andrew@lunn.ch>; Jakub Kicinski <kuba@kernel.org>; Don Bollinger
> <don@thebollingers.org>; netdev@vger.kernel.org
> Cc: Vladyslav Tarasiuk <vladyslavt@nvidia.com>; Moshe Shemesh
> <moshe@nvidia.com>
> Subject: [PATCH ethtool-next 4/4] ethtool: Update manpages for getmodule
> (-m) command
> 
> From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
> 
> Add page, bank and i2c parameters and mention change in offset and length
> treatment if either one of new parameters is specified by the user.
> 
> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
> ---
>  ethtool.8.in | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/ethtool.8.in b/ethtool.8.in index fe49b66..9516458 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -359,6 +359,9 @@ ethtool \- query or control network driver and
> hardware settings
>  .B2 hex on off
>  .BN offset
>  .BN length
> +.BN page
> +.BN bank
> +.BN i2c
>  .HP
>  .B ethtool \-\-show\-priv\-flags
>  .I devname
> @@ -1154,6 +1157,17 @@ Changes the number of multi-purpose channels.
>  Retrieves and if possible decodes the EEPROM from plugin modules, e.g
> SFP+, QSFP.
>  If the driver and module support it, the optical diagnostic information
is also
> read and decoded.
> +When either one of
> +.I page,
> +.I bank
> +or
> +.I i2c
> +parameters is specified, dumps only of a single page or its portion is
> +allowed. In such a case .I offset and .I length parameters are treated
> +relatively to EEPROM page boundaries.

You want 'relative', not 'relatively'.

Please spend a few more words on this.  Basically there are two choices.
Assuming lower memory cannot be accessed when page 2 is specified, the
offset of the first byte of page 2 is either 0 or 128.  I can't tell which
of those choices you mean.

Also, based on the code in the other patches, I assume you mean that the
first byte of page 2 is at offset 0.  I recommend against that.  The specs
all assume that the first byte ***of the paged area*** of page 2 (or any
other page) is at 128.  Hundreds of registers are specified in each spec,
all with offsets in the range 128-255.  The ONLY purpose of this option to
ethtool is to manage those registers, per those specs.  Forcing every user
to translate between the spec and the tool is going to be tedious and error
prone.



>  .TP
>  .B \-\-show\-priv\-flags
>  Queries the specified network device for its private flags.  The
> --
> 2.26.2

Don

Re: [PATCH ethtool-next 0/4] Extend module EEPROM API

[Andrew Lunn]

> There are routine functions to configure the devices that require writing
> appropriate values to various registers.  Byte 26 allows software control of
> low power mode, squelch and software reset.  Page 10h is full of Lane and
> Data Path Control registers.

These all sounds like foot canons when in user space control. I would
expect the MAC and or SFP driver to make use of these features, no
need to export them to user space, at least not in a raw format. I
could however imagine ethtool commands to manipulate specific
features, passing the request to the MAC to perform, so it knows what
is going on.
 
> Beyond the spec, but allowed by the spec, there are vendor specific
> capabilities like firmware download that require bulk write (up to 128 bytes
> per write).

This one is not so easy. Since it is vendor specific, we need to
consider how to actually make it vendor generic from Ethtool, or maybe
devlink. Maybe code in the kernel which matches on the vendor string
in the SFP EEPROM, and provides a standardized API towards ethtool,
and does whatever magic is needed towards the SFP. But it gets messy
when you don't have direct access to the SFP, there is a layer of
firmware in the middle, which is often the case.

	 Andrew

RE: [PATCH ethtool-next 0/4] Extend module EEPROM API

[Don Bollinger]

> > There are routine functions to configure the devices that require
> > writing appropriate values to various registers.  Byte 26 allows
> > software control of low power mode, squelch and software reset.  Page
> > 10h is full of Lane and Data Path Control registers.
> 
> These all sounds like foot canons when in user space control. I would
expect
> the MAC and or SFP driver to make use of these features, no need to export
> them to user space, at least not in a raw format. I could however imagine
> ethtool commands to manipulate specific features, passing the request to
> the MAC to perform, so it knows what is going on.
> 
> > Beyond the spec, but allowed by the spec, there are vendor specific
> > capabilities like firmware download that require bulk write (up to 128
> > bytes per write).
> 
> This one is not so easy. Since it is vendor specific, we need to consider
how to
> actually make it vendor generic from Ethtool, or maybe devlink. Maybe code
> in the kernel which matches on the vendor string in the SFP EEPROM, and
> provides a standardized API towards ethtool, and does whatever magic is
> needed towards the SFP. But it gets messy when you don't have direct
> access to the SFP, there is a layer of firmware in the middle, which is
often
> the case.
> 
> 	 Andrew

Here we go again...  It is my experience that there are far more
capabilities in these devices than will ever be captured in ethtool.  Module
vendors can provide additional value to their customers by putting
innovative features into their modules, and providing software applications
to take advantage of those features.  These features don't necessarily
impact the network stack.  They may be used to draw additional diagnostic
data from the devices, or to enable management features like flashing
colored lights built into custom modules.  I've written code to do these and
more things which are unique to one vendor, and valued by their customers.

I'm asking for a generic interface that allows read/write access to
arbitrary registers.  Put the warnings in the documentation, limit access to
it, but make it available.

Don

Re: [PATCH ethtool-next 2/4] ethtool: Refactor human-readable module EEPROM output

[Andrew Lunn]

Hi Don

Please trim the text of the email when replying. Standard netiquette
best practices.

> > -#define PAG01H_UPPER_OFFSET			(0x01 * 0x80)
> > +#define PAG01H_UPPER_OFFSET			(0x02 * 0x80)
> > 
> >  /* Supported Link Length (Page 1) */
> > -#define QSFP_DD_SMF_LEN_OFFSET
> > 	(PAG01H_UPPER_OFFSET + 0x84)
> > -#define QSFP_DD_OM5_LEN_OFFSET
> > 	(PAG01H_UPPER_OFFSET + 0x85)
> > -#define QSFP_DD_OM4_LEN_OFFSET
> > 	(PAG01H_UPPER_OFFSET + 0x86)
> > -#define QSFP_DD_OM3_LEN_OFFSET
> > 	(PAG01H_UPPER_OFFSET + 0x87)
> > -#define QSFP_DD_OM2_LEN_OFFSET
> > 	(PAG01H_UPPER_OFFSET + 0x88)
> > +#define QSFP_DD_SMF_LEN_OFFSET			0x4
> > +#define QSFP_DD_OM5_LEN_OFFSET			0x5
> > +#define QSFP_DD_OM4_LEN_OFFSET			0x6
> > +#define QSFP_DD_OM3_LEN_OFFSET			0x7
> > +#define QSFP_DD_OM2_LEN_OFFSET			0x8
> 
> I see here you have switched from offsets from the beginning of flat linear
> memory to offsets within the upper half page.  I recommend actually using
> the values in the spec, which are offset from the base of the page.

I agree with this. Being able to easily map between spec and code is
important.

	Andrew

Re: [PATCH ethtool-next 0/4] Extend module EEPROM API

[Andrew Lunn]

> Here we go again...  It is my experience that there are far more
> capabilities in these devices than will ever be captured in ethtool.  Module
> vendors can provide additional value to their customers by putting
> innovative features into their modules, and providing software applications
> to take advantage of those features.  These features don't necessarily
> impact the network stack.  They may be used to draw additional diagnostic
> data from the devices, or to enable management features like flashing
> colored lights built into custom modules.  I've written code to do these and
> more things which are unique to one vendor, and valued by their customers.

Sorry, but not going to happen. Ethernet PHYs can have lots of
addition registers on stop of what 802.3 specifies. Vendors do add
additional functionality. And we do support some of it, like
configuring downshift, energy detect power down, cable diagnostics in
a generic manor. And we are open to adding more such features. People
can contribute code which multiple vendors might implement. We then
have well documented APIs which are the same across different vendors.

For LEDs you should be using the Linux LED class drivers. The Ethernet
PHYs are slowly moving that way. Very slowly :-(

Both MAC and Ethernet PHY drivers have tunables. I would suggest using
this concept, but applied to SFPs. This is how most of the additional
PHY features are supported. But first you need to add an SFP driver
framework, which matches on the fields in the EEPROM to load the
driver specific to an SFP. That then gives you a place to add such
code.

	Andrew