diff mbox series

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

Message ID 1619162596-23846-3-git-send-email-moshe@nvidia.com (mailing list archive)
State Changes Requested
Delegated to: Michal Kubecek
Headers show
Series Extend module EEPROM API | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Moshe Shemesh April 23, 2021, 7:23 a.m. UTC 
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(-)

Comments

Don Bollinger April 30, 2021, 9:38 p.m. UTC | #1 
> 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
Andrew Lunn May 4, 2021, 12:35 p.m. UTC | #2 
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
diff mbox series

Patch

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