netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/6] bnxt_en: Driver updates
@ 2022-09-28  0:58 Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 1/6] bnxt_en: Update firmware interface to 1.10.2.118 Michael Chan
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Michael Chan @ 2022-09-28  0:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

This patchset adds more complete module eeprom support and adds
an NVRAM resize step to allow larger firmware images to be flashed
to older firmware.

Edwin Peer (1):
  bnxt_en: add support for QSFP optional EEPROM data

Michael Chan (1):
  bnxt_en: Update firmware interface to 1.10.2.118

Vikas Gupta (4):
  bnxt_en: Add bank parameter to bnxt_read_sfp_module_eeprom_info()
  bnxt_en: Refactor bnxt_get_module_info()
  bnxt_en: add .get_module_eeprom_by_page() support
  bnxt_en: check and resize NVRAM UPDATE entry before flashing

 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  15 ++
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 248 +++++++++++++++---
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 234 ++++++++++++-----
 3 files changed, 402 insertions(+), 95 deletions(-)

-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next 1/6] bnxt_en: Update firmware interface to 1.10.2.118
  2022-09-28  0:58 [PATCH net-next 0/6] bnxt_en: Driver updates Michael Chan
@ 2022-09-28  0:58 ` Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 2/6] bnxt_en: add support for QSFP optional EEPROM data Michael Chan
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Michael Chan @ 2022-09-28  0:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

[-- Attachment #1: Type: text/plain, Size: 25136 bytes --]

The main changes are PTM timestamp support, CMIS EEPROM support, and
asymmetric CoS queues support.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 234 +++++++++++++-----
 1 file changed, 169 insertions(+), 65 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
index b753032a1047..184dd8d11cd3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
@@ -254,6 +254,8 @@ struct cmd_nums {
 	#define HWRM_PORT_DSC_DUMP                        0xd9UL
 	#define HWRM_PORT_EP_TX_QCFG                      0xdaUL
 	#define HWRM_PORT_EP_TX_CFG                       0xdbUL
+	#define HWRM_PORT_CFG                             0xdcUL
+	#define HWRM_PORT_QCFG                            0xddUL
 	#define HWRM_TEMP_MONITOR_QUERY                   0xe0UL
 	#define HWRM_REG_POWER_QUERY                      0xe1UL
 	#define HWRM_CORE_FREQUENCY_QUERY                 0xe2UL
@@ -379,6 +381,8 @@ struct cmd_nums {
 	#define HWRM_FUNC_BACKING_STORE_QCAPS_V2          0x1a8UL
 	#define HWRM_FUNC_DBR_PACING_NQLIST_QUERY         0x1a9UL
 	#define HWRM_FUNC_DBR_RECOVERY_COMPLETED          0x1aaUL
+	#define HWRM_FUNC_SYNCE_CFG                       0x1abUL
+	#define HWRM_FUNC_SYNCE_QCFG                      0x1acUL
 	#define HWRM_SELFTEST_QLIST                       0x200UL
 	#define HWRM_SELFTEST_EXEC                        0x201UL
 	#define HWRM_SELFTEST_IRQ                         0x202UL
@@ -417,6 +421,8 @@ struct cmd_nums {
 	#define HWRM_TF_SESSION_RESC_FREE                 0x2ceUL
 	#define HWRM_TF_SESSION_RESC_FLUSH                0x2cfUL
 	#define HWRM_TF_SESSION_RESC_INFO                 0x2d0UL
+	#define HWRM_TF_SESSION_HOTUP_STATE_SET           0x2d1UL
+	#define HWRM_TF_SESSION_HOTUP_STATE_GET           0x2d2UL
 	#define HWRM_TF_TBL_TYPE_GET                      0x2daUL
 	#define HWRM_TF_TBL_TYPE_SET                      0x2dbUL
 	#define HWRM_TF_TBL_TYPE_BULK_GET                 0x2dcUL
@@ -440,6 +446,25 @@ struct cmd_nums {
 	#define HWRM_TF_GLOBAL_CFG_GET                    0x2fdUL
 	#define HWRM_TF_IF_TBL_SET                        0x2feUL
 	#define HWRM_TF_IF_TBL_GET                        0x2ffUL
+	#define HWRM_TFC_TBL_SCOPE_QCAPS                  0x380UL
+	#define HWRM_TFC_TBL_SCOPE_ID_ALLOC               0x381UL
+	#define HWRM_TFC_TBL_SCOPE_CONFIG                 0x382UL
+	#define HWRM_TFC_TBL_SCOPE_DECONFIG               0x383UL
+	#define HWRM_TFC_TBL_SCOPE_FID_ADD                0x384UL
+	#define HWRM_TFC_TBL_SCOPE_FID_REM                0x385UL
+	#define HWRM_TFC_TBL_SCOPE_POOL_ALLOC             0x386UL
+	#define HWRM_TFC_TBL_SCOPE_POOL_FREE              0x387UL
+	#define HWRM_TFC_SESSION_ID_ALLOC                 0x388UL
+	#define HWRM_TFC_SESSION_FID_ADD                  0x389UL
+	#define HWRM_TFC_SESSION_FID_REM                  0x38aUL
+	#define HWRM_TFC_IDENT_ALLOC                      0x38bUL
+	#define HWRM_TFC_IDENT_FREE                       0x38cUL
+	#define HWRM_TFC_IDX_TBL_ALLOC                    0x38dUL
+	#define HWRM_TFC_IDX_TBL_ALLOC_SET                0x38eUL
+	#define HWRM_TFC_IDX_TBL_SET                      0x38fUL
+	#define HWRM_TFC_IDX_TBL_GET                      0x390UL
+	#define HWRM_TFC_IDX_TBL_FREE                     0x391UL
+	#define HWRM_TFC_GLOBAL_ID_ALLOC                  0x392UL
 	#define HWRM_SV                                   0x400UL
 	#define HWRM_DBG_READ_DIRECT                      0xff10UL
 	#define HWRM_DBG_READ_INDIRECT                    0xff11UL
@@ -546,8 +571,8 @@ struct hwrm_err_output {
 #define HWRM_VERSION_MAJOR 1
 #define HWRM_VERSION_MINOR 10
 #define HWRM_VERSION_UPDATE 2
-#define HWRM_VERSION_RSVD 95
-#define HWRM_VERSION_STR "1.10.2.95"
+#define HWRM_VERSION_RSVD 118
+#define HWRM_VERSION_STR "1.10.2.118"
 
 /* hwrm_ver_get_input (size:192b/24B) */
 struct hwrm_ver_get_input {
@@ -1657,6 +1682,10 @@ struct hwrm_func_qcaps_output {
 	#define FUNC_QCAPS_RESP_FLAGS_EXT2_DBR_PACING_EXT_SUPPORTED             0x8UL
 	#define FUNC_QCAPS_RESP_FLAGS_EXT2_SW_DBR_DROP_RECOVERY_SUPPORTED       0x10UL
 	#define FUNC_QCAPS_RESP_FLAGS_EXT2_GENERIC_STATS_SUPPORTED              0x20UL
+	#define FUNC_QCAPS_RESP_FLAGS_EXT2_UDP_GSO_SUPPORTED                    0x40UL
+	#define FUNC_QCAPS_RESP_FLAGS_EXT2_SYNCE_SUPPORTED                      0x80UL
+	#define FUNC_QCAPS_RESP_FLAGS_EXT2_DBR_PACING_V0_SUPPORTED              0x100UL
+	#define FUNC_QCAPS_RESP_FLAGS_EXT2_TX_PKT_TS_CMPL_SUPPORTED             0x200UL
 	__le16	tunnel_disable_flag;
 	#define FUNC_QCAPS_RESP_TUNNEL_DISABLE_FLAG_DISABLE_VXLAN      0x1UL
 	#define FUNC_QCAPS_RESP_TUNNEL_DISABLE_FLAG_DISABLE_NGE        0x2UL
@@ -1804,7 +1833,20 @@ struct hwrm_func_qcfg_output {
 	#define FUNC_QCFG_RESP_MPC_CHNLS_TE_CFA_ENABLED      0x4UL
 	#define FUNC_QCFG_RESP_MPC_CHNLS_RE_CFA_ENABLED      0x8UL
 	#define FUNC_QCFG_RESP_MPC_CHNLS_PRIMATE_ENABLED     0x10UL
-	u8	unused_2[3];
+	u8	db_page_size;
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_4KB   0x0UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_8KB   0x1UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_16KB  0x2UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_32KB  0x3UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_64KB  0x4UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_128KB 0x5UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_256KB 0x6UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_512KB 0x7UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_1MB   0x8UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_2MB   0x9UL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_4MB   0xaUL
+	#define FUNC_QCFG_RESP_DB_PAGE_SIZE_LAST FUNC_QCFG_RESP_DB_PAGE_SIZE_4MB
+	u8	unused_2[2];
 	__le32	partition_min_bw;
 	#define FUNC_QCFG_RESP_PARTITION_MIN_BW_BW_VALUE_MASK             0xfffffffUL
 	#define FUNC_QCFG_RESP_PARTITION_MIN_BW_BW_VALUE_SFT              0
@@ -1876,6 +1918,7 @@ struct hwrm_func_cfg_input {
 	#define FUNC_CFG_REQ_FLAGS_PPP_PUSH_MODE_DISABLE          0x10000000UL
 	#define FUNC_CFG_REQ_FLAGS_BD_METADATA_ENABLE             0x20000000UL
 	#define FUNC_CFG_REQ_FLAGS_BD_METADATA_DISABLE            0x40000000UL
+	#define FUNC_CFG_REQ_FLAGS_KEY_CTX_ASSETS_TEST            0x80000000UL
 	__le32	enables;
 	#define FUNC_CFG_REQ_ENABLES_ADMIN_MTU                0x1UL
 	#define FUNC_CFG_REQ_ENABLES_MRU                      0x2UL
@@ -2021,12 +2064,26 @@ struct hwrm_func_cfg_input {
 	__le16	num_tx_key_ctxs;
 	__le16	num_rx_key_ctxs;
 	__le32	enables2;
-	#define FUNC_CFG_REQ_ENABLES2_KDNET     0x1UL
+	#define FUNC_CFG_REQ_ENABLES2_KDNET            0x1UL
+	#define FUNC_CFG_REQ_ENABLES2_DB_PAGE_SIZE     0x2UL
 	u8	port_kdnet_mode;
 	#define FUNC_CFG_REQ_PORT_KDNET_MODE_DISABLED 0x0UL
 	#define FUNC_CFG_REQ_PORT_KDNET_MODE_ENABLED  0x1UL
 	#define FUNC_CFG_REQ_PORT_KDNET_MODE_LAST    FUNC_CFG_REQ_PORT_KDNET_MODE_ENABLED
-	u8	unused_0[7];
+	u8	db_page_size;
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_4KB   0x0UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_8KB   0x1UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_16KB  0x2UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_32KB  0x3UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_64KB  0x4UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_128KB 0x5UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_256KB 0x6UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_512KB 0x7UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_1MB   0x8UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_2MB   0x9UL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_4MB   0xaUL
+	#define FUNC_CFG_REQ_DB_PAGE_SIZE_LAST FUNC_CFG_REQ_DB_PAGE_SIZE_4MB
+	u8	unused_0[6];
 };
 
 /* hwrm_func_cfg_output (size:128b/16B) */
@@ -2060,10 +2117,9 @@ struct hwrm_func_qstats_input {
 	__le64	resp_addr;
 	__le16	fid;
 	u8	flags;
-	#define FUNC_QSTATS_REQ_FLAGS_UNUSED       0x0UL
-	#define FUNC_QSTATS_REQ_FLAGS_ROCE_ONLY    0x1UL
-	#define FUNC_QSTATS_REQ_FLAGS_COUNTER_MASK 0x2UL
-	#define FUNC_QSTATS_REQ_FLAGS_LAST        FUNC_QSTATS_REQ_FLAGS_COUNTER_MASK
+	#define FUNC_QSTATS_REQ_FLAGS_ROCE_ONLY        0x1UL
+	#define FUNC_QSTATS_REQ_FLAGS_COUNTER_MASK     0x2UL
+	#define FUNC_QSTATS_REQ_FLAGS_L2_ONLY          0x4UL
 	u8	unused_0[5];
 };
 
@@ -2093,7 +2149,8 @@ struct hwrm_func_qstats_output {
 	__le64	rx_agg_bytes;
 	__le64	rx_agg_events;
 	__le64	rx_agg_aborts;
-	u8	unused_0[7];
+	u8	clear_seq;
+	u8	unused_0[6];
 	u8	valid;
 };
 
@@ -2106,10 +2163,8 @@ struct hwrm_func_qstats_ext_input {
 	__le64	resp_addr;
 	__le16	fid;
 	u8	flags;
-	#define FUNC_QSTATS_EXT_REQ_FLAGS_UNUSED       0x0UL
-	#define FUNC_QSTATS_EXT_REQ_FLAGS_ROCE_ONLY    0x1UL
-	#define FUNC_QSTATS_EXT_REQ_FLAGS_COUNTER_MASK 0x2UL
-	#define FUNC_QSTATS_EXT_REQ_FLAGS_LAST        FUNC_QSTATS_EXT_REQ_FLAGS_COUNTER_MASK
+	#define FUNC_QSTATS_EXT_REQ_FLAGS_ROCE_ONLY        0x1UL
+	#define FUNC_QSTATS_EXT_REQ_FLAGS_COUNTER_MASK     0x2UL
 	u8	unused_0[1];
 	__le32	enables;
 	#define FUNC_QSTATS_EXT_REQ_ENABLES_SCHQ_ID     0x1UL
@@ -2210,6 +2265,7 @@ struct hwrm_func_drv_rgtr_input {
 	#define FUNC_DRV_RGTR_REQ_FLAGS_FAST_RESET_SUPPORT               0x80UL
 	#define FUNC_DRV_RGTR_REQ_FLAGS_RSS_STRICT_HASH_TYPE_SUPPORT     0x100UL
 	#define FUNC_DRV_RGTR_REQ_FLAGS_NPAR_1_2_SUPPORT                 0x200UL
+	#define FUNC_DRV_RGTR_REQ_FLAGS_ASYM_QUEUE_CFG_SUPPORT           0x400UL
 	__le32	enables;
 	#define FUNC_DRV_RGTR_REQ_ENABLES_OS_TYPE             0x1UL
 	#define FUNC_DRV_RGTR_REQ_ENABLES_VER                 0x2UL
@@ -3155,19 +3211,23 @@ struct hwrm_func_ptp_pin_qcfg_output {
 	#define FUNC_PTP_PIN_QCFG_RESP_PIN1_USAGE_SYNC_OUT 0x4UL
 	#define FUNC_PTP_PIN_QCFG_RESP_PIN1_USAGE_LAST    FUNC_PTP_PIN_QCFG_RESP_PIN1_USAGE_SYNC_OUT
 	u8	pin2_usage;
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_NONE     0x0UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_PPS_IN   0x1UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_PPS_OUT  0x2UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNC_IN  0x3UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNC_OUT 0x4UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_LAST    FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNC_OUT
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_NONE                      0x0UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_PPS_IN                    0x1UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_PPS_OUT                   0x2UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNC_IN                   0x3UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNC_OUT                  0x4UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNCE_PRIMARY_CLOCK_OUT   0x5UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNCE_SECONDARY_CLOCK_OUT 0x6UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_LAST                     FUNC_PTP_PIN_QCFG_RESP_PIN2_USAGE_SYNCE_SECONDARY_CLOCK_OUT
 	u8	pin3_usage;
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_NONE     0x0UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_PPS_IN   0x1UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_PPS_OUT  0x2UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNC_IN  0x3UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNC_OUT 0x4UL
-	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_LAST    FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNC_OUT
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_NONE                      0x0UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_PPS_IN                    0x1UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_PPS_OUT                   0x2UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNC_IN                   0x3UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNC_OUT                  0x4UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNCE_PRIMARY_CLOCK_OUT   0x5UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNCE_SECONDARY_CLOCK_OUT 0x6UL
+	#define FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_LAST                     FUNC_PTP_PIN_QCFG_RESP_PIN3_USAGE_SYNCE_SECONDARY_CLOCK_OUT
 	u8	unused_0;
 	u8	valid;
 };
@@ -3215,23 +3275,27 @@ struct hwrm_func_ptp_pin_cfg_input {
 	#define FUNC_PTP_PIN_CFG_REQ_PIN2_STATE_ENABLED  0x1UL
 	#define FUNC_PTP_PIN_CFG_REQ_PIN2_STATE_LAST    FUNC_PTP_PIN_CFG_REQ_PIN2_STATE_ENABLED
 	u8	pin2_usage;
-	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_NONE     0x0UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_PPS_IN   0x1UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_PPS_OUT  0x2UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNC_IN  0x3UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNC_OUT 0x4UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_LAST    FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNC_OUT
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_NONE                      0x0UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_PPS_IN                    0x1UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_PPS_OUT                   0x2UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNC_IN                   0x3UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNC_OUT                  0x4UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNCE_PRIMARY_CLOCK_OUT   0x5UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNCE_SECONDARY_CLOCK_OUT 0x6UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_LAST                     FUNC_PTP_PIN_CFG_REQ_PIN2_USAGE_SYNCE_SECONDARY_CLOCK_OUT
 	u8	pin3_state;
 	#define FUNC_PTP_PIN_CFG_REQ_PIN3_STATE_DISABLED 0x0UL
 	#define FUNC_PTP_PIN_CFG_REQ_PIN3_STATE_ENABLED  0x1UL
 	#define FUNC_PTP_PIN_CFG_REQ_PIN3_STATE_LAST    FUNC_PTP_PIN_CFG_REQ_PIN3_STATE_ENABLED
 	u8	pin3_usage;
-	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_NONE     0x0UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_PPS_IN   0x1UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_PPS_OUT  0x2UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNC_IN  0x3UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNC_OUT 0x4UL
-	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_LAST    FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNC_OUT
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_NONE                      0x0UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_PPS_IN                    0x1UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_PPS_OUT                   0x2UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNC_IN                   0x3UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNC_OUT                  0x4UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNCE_PRIMARY_CLOCK_OUT   0x5UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNCE_SECONDARY_CLOCK_OUT 0x6UL
+	#define FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_LAST                     FUNC_PTP_PIN_CFG_REQ_PIN3_USAGE_SYNCE_SECONDARY_CLOCK_OUT
 	u8	unused_0[4];
 };
 
@@ -3319,9 +3383,9 @@ struct hwrm_func_ptp_ts_query_output {
 	__le16	seq_id;
 	__le16	resp_len;
 	__le64	pps_event_ts;
-	__le64	ptm_res_local_ts;
-	__le64	ptm_pmstr_ts;
-	__le32	ptm_mstr_prop_dly;
+	__le64	ptm_local_ts;
+	__le64	ptm_system_ts;
+	__le32	ptm_link_delay;
 	u8	unused_0[3];
 	u8	valid;
 };
@@ -3417,7 +3481,9 @@ struct hwrm_func_backing_store_cfg_v2_input {
 	#define FUNC_BACKING_STORE_CFG_V2_REQ_TYPE_LAST         FUNC_BACKING_STORE_CFG_V2_REQ_TYPE_INVALID
 	__le16	instance;
 	__le32	flags;
-	#define FUNC_BACKING_STORE_CFG_V2_REQ_FLAGS_PREBOOT_MODE     0x1UL
+	#define FUNC_BACKING_STORE_CFG_V2_REQ_FLAGS_PREBOOT_MODE        0x1UL
+	#define FUNC_BACKING_STORE_CFG_V2_REQ_FLAGS_BS_CFG_ALL_DONE     0x2UL
+	#define FUNC_BACKING_STORE_CFG_V2_REQ_FLAGS_BS_EXTEND           0x4UL
 	__le64	page_dir;
 	__le32	num_entries;
 	__le16	entry_size;
@@ -3853,7 +3919,7 @@ struct hwrm_port_phy_qcfg_input {
 	u8	unused_0[6];
 };
 
-/* hwrm_port_phy_qcfg_output (size:768b/96B) */
+/* hwrm_port_phy_qcfg_output (size:832b/104B) */
 struct hwrm_port_phy_qcfg_output {
 	__le16	error_code;
 	__le16	req_type;
@@ -4150,6 +4216,9 @@ struct hwrm_port_phy_qcfg_output {
 	#define PORT_PHY_QCFG_RESP_LINK_PARTNER_PAM4_ADV_SPEEDS_50GB      0x1UL
 	#define PORT_PHY_QCFG_RESP_LINK_PARTNER_PAM4_ADV_SPEEDS_100GB     0x2UL
 	#define PORT_PHY_QCFG_RESP_LINK_PARTNER_PAM4_ADV_SPEEDS_200GB     0x4UL
+	u8	link_down_reason;
+	#define PORT_PHY_QCFG_RESP_LINK_DOWN_REASON_RF     0x1UL
+	u8	unused_0[7];
 	u8	valid;
 };
 
@@ -4422,9 +4491,7 @@ struct hwrm_port_qstats_input {
 	__le64	resp_addr;
 	__le16	port_id;
 	u8	flags;
-	#define PORT_QSTATS_REQ_FLAGS_UNUSED       0x0UL
-	#define PORT_QSTATS_REQ_FLAGS_COUNTER_MASK 0x1UL
-	#define PORT_QSTATS_REQ_FLAGS_LAST        PORT_QSTATS_REQ_FLAGS_COUNTER_MASK
+	#define PORT_QSTATS_REQ_FLAGS_COUNTER_MASK     0x1UL
 	u8	unused_0[5];
 	__le64	tx_stat_host_addr;
 	__le64	rx_stat_host_addr;
@@ -4552,9 +4619,7 @@ struct hwrm_port_qstats_ext_input {
 	__le16	tx_stat_size;
 	__le16	rx_stat_size;
 	u8	flags;
-	#define PORT_QSTATS_EXT_REQ_FLAGS_UNUSED       0x0UL
-	#define PORT_QSTATS_EXT_REQ_FLAGS_COUNTER_MASK 0x1UL
-	#define PORT_QSTATS_EXT_REQ_FLAGS_LAST        PORT_QSTATS_EXT_REQ_FLAGS_COUNTER_MASK
+	#define PORT_QSTATS_EXT_REQ_FLAGS_COUNTER_MASK     0x1UL
 	u8	unused_0;
 	__le64	tx_stat_host_addr;
 	__le64	rx_stat_host_addr;
@@ -4613,9 +4678,7 @@ struct hwrm_port_ecn_qstats_input {
 	__le16	port_id;
 	__le16	ecn_stat_buf_size;
 	u8	flags;
-	#define PORT_ECN_QSTATS_REQ_FLAGS_UNUSED       0x0UL
-	#define PORT_ECN_QSTATS_REQ_FLAGS_COUNTER_MASK 0x1UL
-	#define PORT_ECN_QSTATS_REQ_FLAGS_LAST        PORT_ECN_QSTATS_REQ_FLAGS_COUNTER_MASK
+	#define PORT_ECN_QSTATS_REQ_FLAGS_COUNTER_MASK     0x1UL
 	u8	unused_0[3];
 	__le64	ecn_stat_host_addr;
 };
@@ -4814,8 +4877,9 @@ struct hwrm_port_phy_qcaps_output {
 	#define PORT_PHY_QCAPS_RESP_SUPPORTED_PAM4_SPEEDS_FORCE_MODE_100G     0x2UL
 	#define PORT_PHY_QCAPS_RESP_SUPPORTED_PAM4_SPEEDS_FORCE_MODE_200G     0x4UL
 	__le16	flags2;
-	#define PORT_PHY_QCAPS_RESP_FLAGS2_PAUSE_UNSUPPORTED     0x1UL
-	#define PORT_PHY_QCAPS_RESP_FLAGS2_PFC_UNSUPPORTED       0x2UL
+	#define PORT_PHY_QCAPS_RESP_FLAGS2_PAUSE_UNSUPPORTED       0x1UL
+	#define PORT_PHY_QCAPS_RESP_FLAGS2_PFC_UNSUPPORTED         0x2UL
+	#define PORT_PHY_QCAPS_RESP_FLAGS2_BANK_ADDR_SUPPORTED     0x4UL
 	u8	internal_port_cnt;
 	u8	valid;
 };
@@ -4830,9 +4894,10 @@ struct hwrm_port_phy_i2c_read_input {
 	__le32	flags;
 	__le32	enables;
 	#define PORT_PHY_I2C_READ_REQ_ENABLES_PAGE_OFFSET     0x1UL
+	#define PORT_PHY_I2C_READ_REQ_ENABLES_BANK_NUMBER     0x2UL
 	__le16	port_id;
 	u8	i2c_slave_addr;
-	u8	unused_0;
+	u8	bank_number;
 	__le16	page_number;
 	__le16	page_offset;
 	u8	data_length;
@@ -6537,6 +6602,7 @@ struct hwrm_vnic_qcaps_output {
 	#define VNIC_QCAPS_RESP_FLAGS_RSS_IPSEC_ESP_SPI_IPV4_CAP              0x400000UL
 	#define VNIC_QCAPS_RESP_FLAGS_RSS_IPSEC_AH_SPI_IPV6_CAP               0x800000UL
 	#define VNIC_QCAPS_RESP_FLAGS_RSS_IPSEC_ESP_SPI_IPV6_CAP              0x1000000UL
+	#define VNIC_QCAPS_RESP_FLAGS_OUTERMOST_RSS_TRUSTED_VF_CAP            0x2000000UL
 	__le16	max_aggs_supported;
 	u8	unused_1[5];
 	u8	valid;
@@ -6827,6 +6893,7 @@ struct hwrm_ring_alloc_input {
 	#define RING_ALLOC_REQ_FLAGS_RX_SOP_PAD                        0x1UL
 	#define RING_ALLOC_REQ_FLAGS_DISABLE_CQ_OVERFLOW_DETECTION     0x2UL
 	#define RING_ALLOC_REQ_FLAGS_NQ_DBR_PACING                     0x4UL
+	#define RING_ALLOC_REQ_FLAGS_TX_PKT_TS_CMPL_ENABLE             0x8UL
 	__le64	page_tbl_addr;
 	__le32	fbo;
 	u8	page_size;
@@ -7626,7 +7693,10 @@ struct hwrm_cfa_ntuple_filter_alloc_input {
 	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_UNKNOWN 0x0UL
 	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_TCP     0x6UL
 	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_UDP     0x11UL
-	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_LAST   CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_UDP
+	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_ICMP    0x1UL
+	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_ICMPV6  0x3aUL
+	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_RSVD    0xffUL
+	#define CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_LAST   CFA_NTUPLE_FILTER_ALLOC_REQ_IP_PROTOCOL_RSVD
 	__le16	dst_id;
 	__le16	mirror_vnic_id;
 	u8	tunnel_type;
@@ -8337,6 +8407,7 @@ struct hwrm_cfa_adv_flow_mgnt_qcaps_output {
 	#define CFA_ADV_FLOW_MGNT_QCAPS_RESP_FLAGS_LAG_SUPPORTED                                0x20000UL
 	#define CFA_ADV_FLOW_MGNT_QCAPS_RESP_FLAGS_NTUPLE_FLOW_NO_L2CTX_SUPPORTED               0x40000UL
 	#define CFA_ADV_FLOW_MGNT_QCAPS_RESP_FLAGS_NIC_FLOW_STATS_SUPPORTED                     0x80000UL
+	#define CFA_ADV_FLOW_MGNT_QCAPS_RESP_FLAGS_NTUPLE_FLOW_RX_EXT_IP_PROTO_SUPPORTED        0x100000UL
 	u8	unused_0[3];
 	u8	valid;
 };
@@ -8355,7 +8426,9 @@ struct hwrm_tunnel_dst_port_query_input {
 	#define TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_IPGRE_V1     0xaUL
 	#define TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_L2_ETYPE     0xbUL
 	#define TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_VXLAN_GPE_V6 0xcUL
-	#define TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_LAST        TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_VXLAN_GPE_V6
+	#define TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_CUSTOM_GRE   0xdUL
+	#define TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_ECPRI        0xeUL
+	#define TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_LAST        TUNNEL_DST_PORT_QUERY_REQ_TUNNEL_TYPE_ECPRI
 	u8	unused_0[7];
 };
 
@@ -8367,7 +8440,16 @@ struct hwrm_tunnel_dst_port_query_output {
 	__le16	resp_len;
 	__le16	tunnel_dst_port_id;
 	__be16	tunnel_dst_port_val;
-	u8	unused_0[3];
+	u8	upar_in_use;
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR0     0x1UL
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR1     0x2UL
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR2     0x4UL
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR3     0x8UL
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR4     0x10UL
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR5     0x20UL
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR6     0x40UL
+	#define TUNNEL_DST_PORT_QUERY_RESP_UPAR_IN_USE_UPAR7     0x80UL
+	u8	unused_0[2];
 	u8	valid;
 };
 
@@ -8385,7 +8467,9 @@ struct hwrm_tunnel_dst_port_alloc_input {
 	#define TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_IPGRE_V1     0xaUL
 	#define TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_L2_ETYPE     0xbUL
 	#define TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_VXLAN_GPE_V6 0xcUL
-	#define TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_LAST        TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_VXLAN_GPE_V6
+	#define TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_CUSTOM_GRE   0xdUL
+	#define TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_ECPRI        0xeUL
+	#define TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_LAST        TUNNEL_DST_PORT_ALLOC_REQ_TUNNEL_TYPE_ECPRI
 	u8	unused_0;
 	__be16	tunnel_dst_port_val;
 	u8	unused_1[4];
@@ -8398,7 +8482,21 @@ struct hwrm_tunnel_dst_port_alloc_output {
 	__le16	seq_id;
 	__le16	resp_len;
 	__le16	tunnel_dst_port_id;
-	u8	unused_0[5];
+	u8	error_info;
+	#define TUNNEL_DST_PORT_ALLOC_RESP_ERROR_INFO_SUCCESS         0x0UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_ERROR_INFO_ERR_ALLOCATED   0x1UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_ERROR_INFO_ERR_NO_RESOURCE 0x2UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_ERROR_INFO_LAST           TUNNEL_DST_PORT_ALLOC_RESP_ERROR_INFO_ERR_NO_RESOURCE
+	u8	upar_in_use;
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR0     0x1UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR1     0x2UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR2     0x4UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR3     0x8UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR4     0x10UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR5     0x20UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR6     0x40UL
+	#define TUNNEL_DST_PORT_ALLOC_RESP_UPAR_IN_USE_UPAR7     0x80UL
+	u8	unused_0[3];
 	u8	valid;
 };
 
@@ -8416,7 +8514,9 @@ struct hwrm_tunnel_dst_port_free_input {
 	#define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_IPGRE_V1     0xaUL
 	#define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_L2_ETYPE     0xbUL
 	#define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_VXLAN_GPE_V6 0xcUL
-	#define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_LAST        TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_VXLAN_GPE_V6
+	#define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_CUSTOM_GRE   0xdUL
+	#define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_ECPRI        0xeUL
+	#define TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_LAST        TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_ECPRI
 	u8	unused_0;
 	__le16	tunnel_dst_port_id;
 	u8	unused_1[4];
@@ -8428,7 +8528,12 @@ struct hwrm_tunnel_dst_port_free_output {
 	__le16	req_type;
 	__le16	seq_id;
 	__le16	resp_len;
-	u8	unused_1[7];
+	u8	error_info;
+	#define TUNNEL_DST_PORT_FREE_RESP_ERROR_INFO_SUCCESS           0x0UL
+	#define TUNNEL_DST_PORT_FREE_RESP_ERROR_INFO_ERR_NOT_OWNER     0x1UL
+	#define TUNNEL_DST_PORT_FREE_RESP_ERROR_INFO_ERR_NOT_ALLOCATED 0x2UL
+	#define TUNNEL_DST_PORT_FREE_RESP_ERROR_INFO_LAST             TUNNEL_DST_PORT_FREE_RESP_ERROR_INFO_ERR_NOT_ALLOCATED
+	u8	unused_1[6];
 	u8	valid;
 };
 
@@ -8686,9 +8791,7 @@ struct hwrm_stat_generic_qstats_input {
 	__le64	resp_addr;
 	__le16	generic_stat_size;
 	u8	flags;
-	#define STAT_GENERIC_QSTATS_REQ_FLAGS_COUNTER      0x0UL
-	#define STAT_GENERIC_QSTATS_REQ_FLAGS_COUNTER_MASK 0x1UL
-	#define STAT_GENERIC_QSTATS_REQ_FLAGS_LAST        STAT_GENERIC_QSTATS_REQ_FLAGS_COUNTER_MASK
+	#define STAT_GENERIC_QSTATS_REQ_FLAGS_COUNTER_MASK     0x1UL
 	u8	unused_0[5];
 	__le64	generic_stat_host_addr;
 };
@@ -10202,6 +10305,7 @@ struct fw_status_reg {
 	#define FW_STATUS_REG_SHUTDOWN               0x100000UL
 	#define FW_STATUS_REG_CRASHED_NO_MASTER      0x200000UL
 	#define FW_STATUS_REG_RECOVERING             0x400000UL
+	#define FW_STATUS_REG_MANU_DEBUG_STATUS      0x800000UL
 };
 
 /* hcomm_status (size:64b/8B) */
-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 2/6] bnxt_en: add support for QSFP optional EEPROM data
  2022-09-28  0:58 [PATCH net-next 0/6] bnxt_en: Driver updates Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 1/6] bnxt_en: Update firmware interface to 1.10.2.118 Michael Chan
@ 2022-09-28  0:58 ` Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 3/6] bnxt_en: Add bank parameter to bnxt_read_sfp_module_eeprom_info() Michael Chan
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Michael Chan @ 2022-09-28  0:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

[-- Attachment #1: Type: text/plain, Size: 6320 bytes --]

From: Edwin Peer <edwin.peer@broadcom.com>

SFF 8636 defines several optional pages. This patch adds support up to
and including page 3. The ethtool offset needs to be mapped onto the
appropriate device page and I2C address, which is handled differently
depending on module type. The necessary linear offset to raw page
mapping is performed based on a table that is configured according to
the module capabilities.

Signed-off-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   5 +
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 120 +++++++++++++++---
 2 files changed, 108 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index b1b17f911300..c54f8c9ab3ad 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2206,6 +2206,11 @@ struct bnxt {
 #define SFF_MODULE_ID_QSFP			0xc
 #define SFF_MODULE_ID_QSFP_PLUS			0xd
 #define SFF_MODULE_ID_QSFP28			0x11
+#define SFF8636_FLATMEM_OFFSET			0x2
+#define SFF8636_FLATMEM_MASK			0x4
+#define SFF8636_OPT_PAGES_OFFSET		0xc3
+#define SFF8636_PAGE1_MASK			0x40
+#define SFF8636_PAGE2_MASK			0x80
 #define BNXT_MAX_PHY_I2C_RESP_SIZE		64
 
 static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index f57e524c7e30..6596dca94c3d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3220,7 +3220,9 @@ static int bnxt_get_module_info(struct net_device *dev,
 			break;
 		case SFF_MODULE_ID_QSFP28:
 			modinfo->type = ETH_MODULE_SFF_8636;
-			modinfo->eeprom_len = ETH_MODULE_SFF_8636_LEN;
+			modinfo->eeprom_len = ETH_MODULE_SFF_8636_MAX_LEN;
+			if (data[SFF8636_FLATMEM_OFFSET] & SFF8636_FLATMEM_MASK)
+				modinfo->eeprom_len = ETH_MODULE_SFF_8636_LEN;
 			break;
 		default:
 			rc = -EOPNOTSUPP;
@@ -3234,32 +3236,116 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
 				  struct ethtool_eeprom *eeprom,
 				  u8 *data)
 {
+	u8 pg_addr[5] = { I2C_DEV_ADDR_A0, I2C_DEV_ADDR_A0 };
+	u16 offset = eeprom->offset, length = eeprom->len;
+	u8 module_info[SFF_DIAG_SUPPORT_OFFSET + 1];
 	struct bnxt *bp = netdev_priv(dev);
-	u16  start = eeprom->offset, length = eeprom->len;
+	u8 page = offset >> 7;
+	u8 max_pages = 2;
 	int rc = 0;
 
-	memset(data, 0, eeprom->len);
+	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0,
+					      SFF_DIAG_SUPPORT_OFFSET + 1,
+					      module_info);
+	if (rc)
+		return rc;
+
+	switch (module_info[0]) {
+	case SFF_MODULE_ID_SFP:
+		if (module_info[SFF_DIAG_SUPPORT_OFFSET]) {
+			pg_addr[2] = I2C_DEV_ADDR_A2;
+			pg_addr[3] = I2C_DEV_ADDR_A2;
+			max_pages = 4;
+		}
+		break;
+	case SFF_MODULE_ID_QSFP28: {
+		u8 opt_pages;
 
-	/* Read A0 portion of the EEPROM */
-	if (start < ETH_MODULE_SFF_8436_LEN) {
-		if (start + eeprom->len > ETH_MODULE_SFF_8436_LEN)
-			length = ETH_MODULE_SFF_8436_LEN - start;
 		rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0,
-						      start, length, data);
+						      SFF8636_OPT_PAGES_OFFSET,
+						      1, &opt_pages);
 		if (rc)
 			return rc;
-		start += length;
-		data += length;
-		length = eeprom->len - length;
+
+		if (opt_pages & SFF8636_PAGE1_MASK) {
+			pg_addr[2] = I2C_DEV_ADDR_A0;
+			max_pages = 3;
+		}
+		if (opt_pages & SFF8636_PAGE2_MASK) {
+			pg_addr[3] = I2C_DEV_ADDR_A0;
+			max_pages = 4;
+		}
+		if (~module_info[SFF8636_FLATMEM_OFFSET] & SFF8636_FLATMEM_MASK) {
+			pg_addr[4] = I2C_DEV_ADDR_A0;
+			max_pages = 5;
+		}
+		break;
 	}
+	default:
+		break;
+	}
+
+	memset(data, 0, eeprom->len);
 
-	/* Read A2 portion of the EEPROM */
-	if (length) {
-		start -= ETH_MODULE_SFF_8436_LEN;
-		rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A2, 0,
-						      start, length, data);
+	/* Read the two 128B base pages in a single pass, since they are
+	 * always supported and both sourced from I2C_DEV_ADDR_A0. Then,
+	 * read individual 128B or 256B chunks as appropriate, according
+	 * to the mappings defined in pg_addr[], which is setup based on
+	 * module capabilities.
+	 *
+	 * The first two pages are both numbered page zero, lower page 0
+	 * and upper page 0 respectively. Raw device pages are numbered
+	 * sequentially thereafter. For SFP modules, reads are always
+	 * from page zero. In this case, the A2 base address is used in
+	 * lieu of the page number to signal reading the upper 256B, with
+	 * offsets relative to the base of this larger I2C region.
+	 *
+	 * Note there may be gaps in the linear ethtool mapping that are
+	 * not backed by raw module pages. Reads to such pages should not
+	 * be attempted because the HWRM call would fail. The caller will
+	 * simply see the preinitialized zeroes in these holes.
+	 *
+	 * Also note, the implementation below depends on pages mapped as
+	 * I2C_DEV_ADDR_A2 in pg_addr[] appearing as 256B aligned pairs.
+	 * This constraint means that it doesn't matter whether the even
+	 * or odd page is used in determining the I2C base address of a
+	 * given region. This allows for larger chunk sizes to be read
+	 * for A2 pages and happens to correspond nicely with the memory
+	 * maps of all currently supported modules. The optional 128B
+	 * A0 pages need to be read relative to an offset of 128B, which
+	 * is where they appear in module memory maps, while the 256B A2
+	 * page pair regions are interpreted by firmware relative to
+	 * offset 0.
+	 */
+	offset &= 0xff;
+	while (length && page < max_pages) {
+		u8 raw_page = page ? page - 1 : 0;
+		u16 chunk;
+
+		if (pg_addr[page] == I2C_DEV_ADDR_A2)
+			raw_page = 0;
+		else if (page)
+			offset |= 0x80;
+		chunk = min_t(u16, length, 256 - offset);
+
+		if (pg_addr[page]) {
+			rc = bnxt_read_sfp_module_eeprom_info(bp, pg_addr[page],
+							      raw_page, offset,
+							      chunk, data);
+			if (rc)
+				return rc;
+		}
+
+		data += chunk;
+		length -= chunk;
+		offset = 0;
+		page += 1 + (chunk > 128);
 	}
-	return rc;
+
+	if (length)
+		return -EINVAL;
+
+	return 0;
 }
 
 static int bnxt_nway_reset(struct net_device *dev)
-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 3/6] bnxt_en: Add bank parameter to bnxt_read_sfp_module_eeprom_info()
  2022-09-28  0:58 [PATCH net-next 0/6] bnxt_en: Driver updates Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 1/6] bnxt_en: Update firmware interface to 1.10.2.118 Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 2/6] bnxt_en: add support for QSFP optional EEPROM data Michael Chan
@ 2022-09-28  0:58 ` Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 4/6] bnxt_en: Refactor bnxt_get_module_info() Michael Chan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Michael Chan @ 2022-09-28  0:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

[-- Attachment #1: Type: text/plain, Size: 4027 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

Newer firmware supports the bank number parameter in the
HWRM_PORT_PHY_I2C_READ command.  Modify the function to pass in
the optional bank number for eeprom.  Existing callers will not use the
new bank number.  It will be used in subsequent patches.

Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  1 +
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 24 ++++++++++++-------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index c54f8c9ab3ad..0209f7caf490 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2116,6 +2116,7 @@ struct bnxt {
 #define BNXT_PHY_FL_NO_FCS		PORT_PHY_QCAPS_RESP_FLAGS_NO_FCS
 #define BNXT_PHY_FL_NO_PAUSE		(PORT_PHY_QCAPS_RESP_FLAGS2_PAUSE_UNSUPPORTED << 8)
 #define BNXT_PHY_FL_NO_PFC		(PORT_PHY_QCAPS_RESP_FLAGS2_PFC_UNSUPPORTED << 8)
+#define BNXT_PHY_FL_BANK_SEL		(PORT_PHY_QCAPS_RESP_FLAGS2_BANK_ADDR_SUPPORTED << 8)
 
 	u8			num_tests;
 	struct bnxt_test_info	*test_info;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 6596dca94c3d..1c8a92fa2f2c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3146,7 +3146,8 @@ static int bnxt_get_eee(struct net_device *dev, struct ethtool_eee *edata)
 }
 
 static int bnxt_read_sfp_module_eeprom_info(struct bnxt *bp, u16 i2c_addr,
-					    u16 page_number, u16 start_addr,
+					    u16 page_number, u8 bank,
+					    bool bank_sel_en, u16 start_addr,
 					    u16 data_length, u8 *buf)
 {
 	struct hwrm_port_phy_i2c_read_output *output;
@@ -3168,8 +3169,11 @@ static int bnxt_read_sfp_module_eeprom_info(struct bnxt *bp, u16 i2c_addr,
 		data_length -= xfer_size;
 		req->page_offset = cpu_to_le16(start_addr + byte_offset);
 		req->data_length = xfer_size;
-		req->enables = cpu_to_le32(start_addr + byte_offset ?
-				 PORT_PHY_I2C_READ_REQ_ENABLES_PAGE_OFFSET : 0);
+		req->bank_number = bank;
+		req->enables = cpu_to_le32((start_addr + byte_offset ?
+				PORT_PHY_I2C_READ_REQ_ENABLES_PAGE_OFFSET : 0) |
+				(bank_sel_en ?
+				PORT_PHY_I2C_READ_REQ_ENABLES_BANK_NUMBER : 0));
 		rc = hwrm_req_send(bp, req);
 		if (!rc)
 			memcpy(buf + byte_offset, output->data, xfer_size);
@@ -3199,8 +3203,8 @@ static int bnxt_get_module_info(struct net_device *dev,
 	if (bp->hwrm_spec_code < 0x10202)
 		return -EOPNOTSUPP;
 
-	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0,
-					      SFF_DIAG_SUPPORT_OFFSET + 1,
+	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
+					      0, SFF_DIAG_SUPPORT_OFFSET + 1,
 					      data);
 	if (!rc) {
 		u8 module_id = data[0];
@@ -3244,8 +3248,8 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
 	u8 max_pages = 2;
 	int rc = 0;
 
-	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0,
-					      SFF_DIAG_SUPPORT_OFFSET + 1,
+	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
+					      0, SFF_DIAG_SUPPORT_OFFSET + 1,
 					      module_info);
 	if (rc)
 		return rc;
@@ -3261,7 +3265,8 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
 	case SFF_MODULE_ID_QSFP28: {
 		u8 opt_pages;
 
-		rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0,
+		rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0,
+						      false,
 						      SFF8636_OPT_PAGES_OFFSET,
 						      1, &opt_pages);
 		if (rc)
@@ -3330,7 +3335,8 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
 
 		if (pg_addr[page]) {
 			rc = bnxt_read_sfp_module_eeprom_info(bp, pg_addr[page],
-							      raw_page, offset,
+							      raw_page, 0,
+							      false, offset,
 							      chunk, data);
 			if (rc)
 				return rc;
-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 4/6] bnxt_en: Refactor bnxt_get_module_info()
  2022-09-28  0:58 [PATCH net-next 0/6] bnxt_en: Driver updates Michael Chan
                   ` (2 preceding siblings ...)
  2022-09-28  0:58 ` [PATCH net-next 3/6] bnxt_en: Add bank parameter to bnxt_read_sfp_module_eeprom_info() Michael Chan
@ 2022-09-28  0:58 ` Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support Michael Chan
  2022-09-28  0:58 ` [PATCH net-next 6/6] bnxt_en: check and resize NVRAM UPDATE entry before flashing Michael Chan
  5 siblings, 0 replies; 11+ messages in thread
From: Michael Chan @ 2022-09-28  0:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

Add bnxt_module_status_check() helper to check if we can proceed to read
module eeprom data.  This helper will be used in the next patch when
adding get_module_eeprom_by_page().

Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 23 +++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 1c8a92fa2f2c..379afa670494 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3184,25 +3184,34 @@ static int bnxt_read_sfp_module_eeprom_info(struct bnxt *bp, u16 i2c_addr,
 	return rc;
 }
 
-static int bnxt_get_module_info(struct net_device *dev,
-				struct ethtool_modinfo *modinfo)
+static int bnxt_module_status_check(struct bnxt *bp)
 {
-	u8 data[SFF_DIAG_SUPPORT_OFFSET + 1];
-	struct bnxt *bp = netdev_priv(dev);
-	int rc;
-
 	/* No point in going further if phy status indicates
 	 * module is not inserted or if it is powered down or
 	 * if it is of type 10GBase-T
 	 */
 	if (bp->link_info.module_status >
-		PORT_PHY_QCFG_RESP_MODULE_STATUS_WARNINGMSG)
+	    PORT_PHY_QCFG_RESP_MODULE_STATUS_WARNINGMSG)
 		return -EOPNOTSUPP;
 
 	/* This feature is not supported in older firmware versions */
 	if (bp->hwrm_spec_code < 0x10202)
 		return -EOPNOTSUPP;
 
+	return 0;
+}
+
+static int bnxt_get_module_info(struct net_device *dev,
+				struct ethtool_modinfo *modinfo)
+{
+	u8 data[SFF_DIAG_SUPPORT_OFFSET + 1];
+	struct bnxt *bp = netdev_priv(dev);
+	int rc;
+
+	rc = bnxt_module_status_check(bp);
+	if (rc)
+		return rc;
+
 	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
 					      0, SFF_DIAG_SUPPORT_OFFSET + 1,
 					      data);
-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support
  2022-09-28  0:58 [PATCH net-next 0/6] bnxt_en: Driver updates Michael Chan
                   ` (3 preceding siblings ...)
  2022-09-28  0:58 ` [PATCH net-next 4/6] bnxt_en: Refactor bnxt_get_module_info() Michael Chan
@ 2022-09-28  0:58 ` Michael Chan
  2022-09-28  9:34   ` Ido Schimmel
  2022-09-28  0:58 ` [PATCH net-next 6/6] bnxt_en: check and resize NVRAM UPDATE entry before flashing Michael Chan
  5 siblings, 1 reply; 11+ messages in thread
From: Michael Chan @ 2022-09-28  0:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

[-- Attachment #1: Type: text/plain, Size: 3464 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

Add support for .get_module_eeprom_by_page() callback
which enables CMIS for pluggable modules.
In the case that bank select is not enabled by f/w then
all the requests fallback to .get_module_eeprom() callback.

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  9 +++
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 55 +++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 0209f7caf490..03b1a0301a46 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -2207,6 +2207,15 @@ struct bnxt {
 #define SFF_MODULE_ID_QSFP			0xc
 #define SFF_MODULE_ID_QSFP_PLUS			0xd
 #define SFF_MODULE_ID_QSFP28			0x11
+#define SFF_MODULE_ID_QSFP_DD			0x18
+#define SFF_MODULE_ID_DSFP			0x1b
+#define SFF_MODULE_ID_QSFP_PLUS_CMIS		0x1e
+
+#define BNXT_SFF_MODULE_BANK_SUPPORTED(module_id)	\
+	((module_id) == SFF_MODULE_ID_QSFP_DD ||	\
+	 (module_id) == SFF_MODULE_ID_QSFP28 ||		\
+	 (module_id) == SFF_MODULE_ID_QSFP_PLUS_CMIS)
+
 #define SFF8636_FLATMEM_OFFSET			0x2
 #define SFF8636_FLATMEM_MASK			0x4
 #define SFF8636_OPT_PAGES_OFFSET		0xc3
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 379afa670494..2b18af95aacb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3363,6 +3363,60 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
 	return 0;
 }
 
+static int bnxt_get_module_eeprom_by_page(struct net_device *dev,
+					  const struct ethtool_module_eeprom *page_data,
+					  struct netlink_ext_ack *extack)
+{
+	struct bnxt *bp = netdev_priv(dev);
+	u16 length = page_data->length;
+	u8 *data = page_data->data;
+	u8 page = page_data->page;
+	u8 bank = page_data->bank;
+	u16 bytes_copied = 0;
+	u8 module_id;
+	int rc;
+
+	/* Return -EOPNOTSUPP to fallback on .get_module_eeprom */
+	if (!(bp->phy_flags & BNXT_PHY_FL_BANK_SEL))
+		return -EOPNOTSUPP;
+
+	rc = bnxt_module_status_check(bp);
+	if (rc)
+		return rc;
+
+	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
+					      0, 1, &module_id);
+	if (rc)
+		return rc;
+
+	if (!BNXT_SFF_MODULE_BANK_SUPPORTED(module_id))
+		return -EOPNOTSUPP;
+
+	while (length > 0) {
+		u16 chunk = ETH_MODULE_EEPROM_PAGE_LEN;
+		int rc;
+
+		/* Do not access more than required */
+		if (length < ETH_MODULE_EEPROM_PAGE_LEN)
+			chunk = length;
+
+		rc = bnxt_read_sfp_module_eeprom_info(bp,
+						      I2C_DEV_ADDR_A0,
+						      page, bank,
+						      true, page_data->offset,
+						      chunk, data);
+		if (rc)
+			return rc;
+
+		data += chunk;
+		length -= chunk;
+		bytes_copied += chunk;
+		page++;
+	}
+
+	return bytes_copied;
+}
+
 static int bnxt_nway_reset(struct net_device *dev)
 {
 	int rc = 0;
@@ -4172,6 +4226,7 @@ const struct ethtool_ops bnxt_ethtool_ops = {
 	.set_eee		= bnxt_set_eee,
 	.get_module_info	= bnxt_get_module_info,
 	.get_module_eeprom	= bnxt_get_module_eeprom,
+	.get_module_eeprom_by_page = bnxt_get_module_eeprom_by_page,
 	.nway_reset		= bnxt_nway_reset,
 	.set_phys_id		= bnxt_set_phys_id,
 	.self_test		= bnxt_self_test,
-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 6/6] bnxt_en: check and resize NVRAM UPDATE entry before flashing
  2022-09-28  0:58 [PATCH net-next 0/6] bnxt_en: Driver updates Michael Chan
                   ` (4 preceding siblings ...)
  2022-09-28  0:58 ` [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support Michael Chan
@ 2022-09-28  0:58 ` Michael Chan
  5 siblings, 0 replies; 11+ messages in thread
From: Michael Chan @ 2022-09-28  0:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

From: Vikas Gupta <vikas.gupta@broadcom.com>

Resize of the UPDATE entry is required if the image to
be flashed is larger than the available space. Add this step,
otherwise flashing larger firmware images by ethtool or devlink
may fail.

Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 2b18af95aacb..2f97f41408e7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2514,6 +2514,7 @@ static int bnxt_flash_firmware_from_file(struct net_device *dev,
 #define MSG_INTERNAL_ERR "PKG install error : Internal error"
 #define MSG_NO_PKG_UPDATE_AREA_ERR "PKG update area not created in nvram"
 #define MSG_NO_SPACE_ERR "PKG insufficient update area in nvram"
+#define MSG_RESIZE_UPDATE_ERR "Resize UPDATE entry error"
 #define MSG_ANTI_ROLLBACK_ERR "HWRM_NVM_INSTALL_UPDATE failure due to Anti-rollback detected"
 #define MSG_GENERIC_FAILURE_ERR "HWRM_NVM_INSTALL_UPDATE failure"
 
@@ -2564,6 +2565,32 @@ static int nvm_update_err_to_stderr(struct net_device *dev, u8 result,
 #define BNXT_NVM_MORE_FLAG	(cpu_to_le16(NVM_MODIFY_REQ_FLAGS_BATCH_MODE))
 #define BNXT_NVM_LAST_FLAG	(cpu_to_le16(NVM_MODIFY_REQ_FLAGS_BATCH_LAST))
 
+static int bnxt_resize_update_entry(struct net_device *dev, size_t fw_size,
+				    struct netlink_ext_ack *extack)
+{
+	u32 item_len;
+	int rc;
+
+	rc = bnxt_find_nvram_item(dev, BNX_DIR_TYPE_UPDATE,
+				  BNX_DIR_ORDINAL_FIRST, BNX_DIR_EXT_NONE, NULL,
+				  &item_len, NULL);
+	if (rc) {
+		BNXT_NVM_ERR_MSG(dev, extack, MSG_NO_PKG_UPDATE_AREA_ERR);
+		return rc;
+	}
+
+	if (fw_size > item_len) {
+		rc = bnxt_flash_nvram(dev, BNX_DIR_TYPE_UPDATE,
+				      BNX_DIR_ORDINAL_FIRST, 0, 1,
+				      round_up(fw_size, 4096), NULL, 0);
+		if (rc) {
+			BNXT_NVM_ERR_MSG(dev, extack, MSG_RESIZE_UPDATE_ERR);
+			return rc;
+		}
+	}
+	return 0;
+}
+
 int bnxt_flash_package_from_fw_obj(struct net_device *dev, const struct firmware *fw,
 				   u32 install_type, struct netlink_ext_ack *extack)
 {
@@ -2580,6 +2607,11 @@ int bnxt_flash_package_from_fw_obj(struct net_device *dev, const struct firmware
 	u16 index;
 	int rc;
 
+	/* resize before flashing larger image than available space */
+	rc = bnxt_resize_update_entry(dev, fw->size, extack);
+	if (rc)
+		return rc;
+
 	bnxt_hwrm_fw_set_time(bp);
 
 	rc = hwrm_req_init(bp, modify, HWRM_NVM_MODIFY);
-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support
  2022-09-28  0:58 ` [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support Michael Chan
@ 2022-09-28  9:34   ` Ido Schimmel
  2022-09-29  5:35     ` Vikas Gupta
  0 siblings, 1 reply; 11+ messages in thread
From: Ido Schimmel @ 2022-09-28  9:34 UTC (permalink / raw)
  To: Michael Chan; +Cc: davem, netdev, kuba, edumazet, pabeni, gospo, vikas.gupta

On Tue, Sep 27, 2022 at 08:58:43PM -0400, Michael Chan wrote:
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index 0209f7caf490..03b1a0301a46 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2207,6 +2207,15 @@ struct bnxt {
>  #define SFF_MODULE_ID_QSFP			0xc
>  #define SFF_MODULE_ID_QSFP_PLUS			0xd
>  #define SFF_MODULE_ID_QSFP28			0x11
> +#define SFF_MODULE_ID_QSFP_DD			0x18
> +#define SFF_MODULE_ID_DSFP			0x1b
> +#define SFF_MODULE_ID_QSFP_PLUS_CMIS		0x1e
> +
> +#define BNXT_SFF_MODULE_BANK_SUPPORTED(module_id)	\
> +	((module_id) == SFF_MODULE_ID_QSFP_DD ||	\
> +	 (module_id) == SFF_MODULE_ID_QSFP28 ||		\
> +	 (module_id) == SFF_MODULE_ID_QSFP_PLUS_CMIS)

I suggest dropping this check unless you have a good reason to keep it.
There are other modules out there that implement CMIS (e.g., OSFP) and
given bnxt implements ethtool_ops::get_module_eeprom_by_page, it should
be able to support them without kernel changes.

See:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=970adfb76095fa719778d70a6b86030d2feb88dd

The problem there was more severe because the driver returned '-EINVAL'
instead of '-EOPNOTSUPP'.

> +
>  #define SFF8636_FLATMEM_OFFSET			0x2
>  #define SFF8636_FLATMEM_MASK			0x4
>  #define SFF8636_OPT_PAGES_OFFSET		0xc3
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index 379afa670494..2b18af95aacb 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -3363,6 +3363,60 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
>  	return 0;
>  }
>  
> +static int bnxt_get_module_eeprom_by_page(struct net_device *dev,
> +					  const struct ethtool_module_eeprom *page_data,
> +					  struct netlink_ext_ack *extack)
> +{
> +	struct bnxt *bp = netdev_priv(dev);
> +	u16 length = page_data->length;
> +	u8 *data = page_data->data;
> +	u8 page = page_data->page;
> +	u8 bank = page_data->bank;
> +	u16 bytes_copied = 0;
> +	u8 module_id;
> +	int rc;
> +
> +	/* Return -EOPNOTSUPP to fallback on .get_module_eeprom */
> +	if (!(bp->phy_flags & BNXT_PHY_FL_BANK_SEL))
> +		return -EOPNOTSUPP;

Maybe:

if (bank && !(bp->phy_flags & BNXT_PHY_FL_BANK_SEL)) {
	// extack
	return -EINVAL;
}

This should allow you to get rid of patch #2.

> +
> +	rc = bnxt_module_status_check(bp);
> +	if (rc)
> +		return rc;

You can add extack here. I see that this function always returns
'-EOPNOTSUPP' in case of errors, even when it does not make sense to
fallback to ethtool_ops::get_module_eeprom.

> +
> +	rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
> +					      0, 1, &module_id);
> +	if (rc)
> +		return rc;
> +
> +	if (!BNXT_SFF_MODULE_BANK_SUPPORTED(module_id))
> +		return -EOPNOTSUPP;

I believe this hunk can be removed given the first comment.

> +
> +	while (length > 0) {
> +		u16 chunk = ETH_MODULE_EEPROM_PAGE_LEN;
> +		int rc;
> +
> +		/* Do not access more than required */
> +		if (length < ETH_MODULE_EEPROM_PAGE_LEN)
> +			chunk = length;
> +
> +		rc = bnxt_read_sfp_module_eeprom_info(bp,
> +						      I2C_DEV_ADDR_A0,

page_data->i2c_address

> +						      page, bank,
> +						      true, page_data->offset,
> +						      chunk, data);
> +		if (rc)

You can add a meaningful extack here according to the return code.

> +			return rc;
> +
> +		data += chunk;
> +		length -= chunk;
> +		bytes_copied += chunk;
> +		page++;
> +	}

I'm not sure why the loop is required? It seems
bnxt_read_sfp_module_eeprom_info() is able to read
'ETH_MODULE_EEPROM_PAGE_LEN' bytes at once, which is the maximum:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool/eeprom.c#n234

> +
> +	return bytes_copied;
> +}
> +
>  static int bnxt_nway_reset(struct net_device *dev)
>  {
>  	int rc = 0;
> @@ -4172,6 +4226,7 @@ const struct ethtool_ops bnxt_ethtool_ops = {
>  	.set_eee		= bnxt_set_eee,
>  	.get_module_info	= bnxt_get_module_info,
>  	.get_module_eeprom	= bnxt_get_module_eeprom,
> +	.get_module_eeprom_by_page = bnxt_get_module_eeprom_by_page,
>  	.nway_reset		= bnxt_nway_reset,
>  	.set_phys_id		= bnxt_set_phys_id,
>  	.self_test		= bnxt_self_test,
> -- 
> 2.18.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support
  2022-09-28  9:34   ` Ido Schimmel
@ 2022-09-29  5:35     ` Vikas Gupta
  2022-09-29  7:32       ` Ido Schimmel
  0 siblings, 1 reply; 11+ messages in thread
From: Vikas Gupta @ 2022-09-29  5:35 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: Michael Chan, davem, netdev, kuba, edumazet, pabeni, gospo

[-- Attachment #1: Type: text/plain, Size: 6074 bytes --]

Hi Ido,

On Wed, Sep 28, 2022 at 3:04 PM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Tue, Sep 27, 2022 at 08:58:43PM -0400, Michael Chan wrote:
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > index 0209f7caf490..03b1a0301a46 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > @@ -2207,6 +2207,15 @@ struct bnxt {
> >  #define SFF_MODULE_ID_QSFP                   0xc
> >  #define SFF_MODULE_ID_QSFP_PLUS                      0xd
> >  #define SFF_MODULE_ID_QSFP28                 0x11
> > +#define SFF_MODULE_ID_QSFP_DD                        0x18
> > +#define SFF_MODULE_ID_DSFP                   0x1b
> > +#define SFF_MODULE_ID_QSFP_PLUS_CMIS         0x1e
> > +
> > +#define BNXT_SFF_MODULE_BANK_SUPPORTED(module_id)    \
> > +     ((module_id) == SFF_MODULE_ID_QSFP_DD ||        \
> > +      (module_id) == SFF_MODULE_ID_QSFP28 ||         \
> > +      (module_id) == SFF_MODULE_ID_QSFP_PLUS_CMIS)
>
> I suggest dropping this check unless you have a good reason to keep it.
> There are other modules out there that implement CMIS (e.g., OSFP) and
> given bnxt implements ethtool_ops::get_module_eeprom_by_page, it should
> be able to support them without kernel changes.
>
> See:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=970adfb76095fa719778d70a6b86030d2feb88dd
>
> The problem there was more severe because the driver returned '-EINVAL'
> instead of '-EOPNOTSUPP'.
>
We want to fallback on get_module_eeprom callback in case modules do
not implement CMIS and we pass the bank parameter accordingly to the
firmware.

> > +
> >  #define SFF8636_FLATMEM_OFFSET                       0x2
> >  #define SFF8636_FLATMEM_MASK                 0x4
> >  #define SFF8636_OPT_PAGES_OFFSET             0xc3
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > index 379afa670494..2b18af95aacb 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > @@ -3363,6 +3363,60 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
> >       return 0;
> >  }
> >
> > +static int bnxt_get_module_eeprom_by_page(struct net_device *dev,
> > +                                       const struct ethtool_module_eeprom *page_data,
> > +                                       struct netlink_ext_ack *extack)
> > +{
> > +     struct bnxt *bp = netdev_priv(dev);
> > +     u16 length = page_data->length;
> > +     u8 *data = page_data->data;
> > +     u8 page = page_data->page;
> > +     u8 bank = page_data->bank;
> > +     u16 bytes_copied = 0;
> > +     u8 module_id;
> > +     int rc;
> > +
> > +     /* Return -EOPNOTSUPP to fallback on .get_module_eeprom */
> > +     if (!(bp->phy_flags & BNXT_PHY_FL_BANK_SEL))
> > +             return -EOPNOTSUPP;
>
> Maybe:
>
> if (bank && !(bp->phy_flags & BNXT_PHY_FL_BANK_SEL)) {
>         // extack
>         return -EINVAL;
> }

BNXT_PHY_FL_BANK_SEL is a firmware capability to handle CMIS/bank
parameters. It does not tell whether the hooked module is CMIS
compliant.
I think EOPNOTSUPP is an appropriate choice here.


>
> This should allow you to get rid of patch #2.

I am not sure how we can get rid of patch #2. Patch #2 handles non CMIS modules.
Can you please elaborate more.

>
> > +
> > +     rc = bnxt_module_status_check(bp);
> > +     if (rc)
> > +             return rc;
>
> You can add extack here. I see that this function always returns
> '-EOPNOTSUPP' in case of errors, even when it does not make sense to
> fallback to ethtool_ops::get_module_eeprom.
Maybe -EIO can be used in one of these cases. I`ll check.

>
> > +
> > +     rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
> > +                                           0, 1, &module_id);
> > +     if (rc)
> > +             return rc;
> > +
> > +     if (!BNXT_SFF_MODULE_BANK_SUPPORTED(module_id))
> > +             return -EOPNOTSUPP;
>
> I believe this hunk can be removed given the first comment.
  As I mentioned above we need this here to fallback.

Thanks,
Vikas
>
> > +
> > +     while (length > 0) {
> > +             u16 chunk = ETH_MODULE_EEPROM_PAGE_LEN;
> > +             int rc;
> > +
> > +             /* Do not access more than required */
> > +             if (length < ETH_MODULE_EEPROM_PAGE_LEN)
> > +                     chunk = length;
> > +
> > +             rc = bnxt_read_sfp_module_eeprom_info(bp,
> > +                                                   I2C_DEV_ADDR_A0,
>
> page_data->i2c_address
>
> > +                                                   page, bank,
> > +                                                   true, page_data->offset,
> > +                                                   chunk, data);
> > +             if (rc)
>
> You can add a meaningful extack here according to the return code.
>
> > +                     return rc;
> > +
> > +             data += chunk;
> > +             length -= chunk;
> > +             bytes_copied += chunk;
> > +             page++;
> > +     }
>
> I'm not sure why the loop is required? It seems
> bnxt_read_sfp_module_eeprom_info() is able to read
> 'ETH_MODULE_EEPROM_PAGE_LEN' bytes at once, which is the maximum:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool/eeprom.c#n234
>
> > +
> > +     return bytes_copied;
> > +}
> > +
> >  static int bnxt_nway_reset(struct net_device *dev)
> >  {
> >       int rc = 0;
> > @@ -4172,6 +4226,7 @@ const struct ethtool_ops bnxt_ethtool_ops = {
> >       .set_eee                = bnxt_set_eee,
> >       .get_module_info        = bnxt_get_module_info,
> >       .get_module_eeprom      = bnxt_get_module_eeprom,
> > +     .get_module_eeprom_by_page = bnxt_get_module_eeprom_by_page,
> >       .nway_reset             = bnxt_nway_reset,
> >       .set_phys_id            = bnxt_set_phys_id,
> >       .self_test              = bnxt_self_test,
> > --
> > 2.18.1
> >

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4206 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support
  2022-09-29  5:35     ` Vikas Gupta
@ 2022-09-29  7:32       ` Ido Schimmel
  2022-09-30 10:05         ` Vikas Gupta
  0 siblings, 1 reply; 11+ messages in thread
From: Ido Schimmel @ 2022-09-29  7:32 UTC (permalink / raw)
  To: Vikas Gupta; +Cc: Michael Chan, davem, netdev, kuba, edumazet, pabeni, gospo

On Thu, Sep 29, 2022 at 11:05:15AM +0530, Vikas Gupta wrote:
> On Wed, Sep 28, 2022 at 3:04 PM Ido Schimmel <idosch@idosch.org> wrote:
> >
> > On Tue, Sep 27, 2022 at 08:58:43PM -0400, Michael Chan wrote:
> > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > index 0209f7caf490..03b1a0301a46 100644
> > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > @@ -2207,6 +2207,15 @@ struct bnxt {
> > >  #define SFF_MODULE_ID_QSFP                   0xc
> > >  #define SFF_MODULE_ID_QSFP_PLUS                      0xd
> > >  #define SFF_MODULE_ID_QSFP28                 0x11
> > > +#define SFF_MODULE_ID_QSFP_DD                        0x18
> > > +#define SFF_MODULE_ID_DSFP                   0x1b

Does not seem to be used.

> > > +#define SFF_MODULE_ID_QSFP_PLUS_CMIS         0x1e
> > > +
> > > +#define BNXT_SFF_MODULE_BANK_SUPPORTED(module_id)    \
> > > +     ((module_id) == SFF_MODULE_ID_QSFP_DD ||        \
> > > +      (module_id) == SFF_MODULE_ID_QSFP28 ||         \

Did you mean DSFP here? QSFP28 is SFF-8636, not CMIS.

> > > +      (module_id) == SFF_MODULE_ID_QSFP_PLUS_CMIS)
> >
> > I suggest dropping this check unless you have a good reason to keep it.
> > There are other modules out there that implement CMIS (e.g., OSFP) and
> > given bnxt implements ethtool_ops::get_module_eeprom_by_page, it should
> > be able to support them without kernel changes.
> >
> > See:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=970adfb76095fa719778d70a6b86030d2feb88dd
> >
> > The problem there was more severe because the driver returned '-EINVAL'
> > instead of '-EOPNOTSUPP'.
> >
> We want to fallback on get_module_eeprom callback in case modules do
> not implement CMIS and we pass the bank parameter accordingly to the
> firmware.

ethtool_ops::get_module_eeprom_by_page has nothing to do with CMIS. It
is a generic operation to retrieve module EEPROM data based on a "3D
address": bank, page and offset.

The driving motivation behind it was CMIS modules, but it must be
implemented in a way that it can retrieve information from modules that
implement a different management interface such as SFF-8636.

Let's say that tomorrow a user asks to retrieve pages 20h-21h from a
QSFP module that implements SFF-8636, how will you support it? You can't
extend the legacy ethtool::get_module_eeprom operation and your current
implementation of ethtool_ops::get_module_eeprom_by_page has an
artificial limitation to support only CMIS modules.

By making sure that your implementation is generic as possible you will
be able to support all possible requests and will not need to
continuously patch the kernel (and users will not need to continuously
upgrade).

> 
> > > +
> > >  #define SFF8636_FLATMEM_OFFSET                       0x2
> > >  #define SFF8636_FLATMEM_MASK                 0x4
> > >  #define SFF8636_OPT_PAGES_OFFSET             0xc3
> > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > > index 379afa670494..2b18af95aacb 100644
> > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > > @@ -3363,6 +3363,60 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
> > >       return 0;
> > >  }
> > >
> > > +static int bnxt_get_module_eeprom_by_page(struct net_device *dev,
> > > +                                       const struct ethtool_module_eeprom *page_data,
> > > +                                       struct netlink_ext_ack *extack)
> > > +{
> > > +     struct bnxt *bp = netdev_priv(dev);
> > > +     u16 length = page_data->length;
> > > +     u8 *data = page_data->data;
> > > +     u8 page = page_data->page;
> > > +     u8 bank = page_data->bank;
> > > +     u16 bytes_copied = 0;
> > > +     u8 module_id;
> > > +     int rc;
> > > +
> > > +     /* Return -EOPNOTSUPP to fallback on .get_module_eeprom */
> > > +     if (!(bp->phy_flags & BNXT_PHY_FL_BANK_SEL))
> > > +             return -EOPNOTSUPP;
> >
> > Maybe:
> >
> > if (bank && !(bp->phy_flags & BNXT_PHY_FL_BANK_SEL)) {
> >         // extack
> >         return -EINVAL;
> > }
> 
> BNXT_PHY_FL_BANK_SEL is a firmware capability to handle CMIS/bank
> parameters. It does not tell whether the hooked module is CMIS
> compliant.
> I think EOPNOTSUPP is an appropriate choice here.

See my comment above. This operation needs to be able to retrieve EEPROM
data from non-CMIS modules as well.

> 
> 
> >
> > This should allow you to get rid of patch #2.
> 
> I am not sure how we can get rid of patch #2. Patch #2 handles non CMIS modules.
> Can you please elaborate more.

All the complicated parsing performed in patch #2 is already performed
in ethtool(8) that knows to request the available pages. The kernel
will first try to retrieve these pages using
ethtool_ops::get_module_eeprom_by_page and fallback to
ethtool::get_module_eeprom in case of '-EOPNOTSUPP'.

By adjusting your implementation of
ethtool_ops::get_module_eeprom_by_page to handle non-CMIS modules you
will be able to avoid extending the legacy operation with all this
complex parsing.

> 
> >
> > > +
> > > +     rc = bnxt_module_status_check(bp);
> > > +     if (rc)
> > > +             return rc;
> >
> > You can add extack here. I see that this function always returns
> > '-EOPNOTSUPP' in case of errors, even when it does not make sense to
> > fallback to ethtool_ops::get_module_eeprom.
> Maybe -EIO can be used in one of these cases. I`ll check.
> 
> >
> > > +
> > > +     rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
> > > +                                           0, 1, &module_id);
> > > +     if (rc)
> > > +             return rc;
> > > +
> > > +     if (!BNXT_SFF_MODULE_BANK_SUPPORTED(module_id))
> > > +             return -EOPNOTSUPP;
> >
> > I believe this hunk can be removed given the first comment.
>   As I mentioned above we need this here to fallback.

No need to fallback, simply avoid making this operation CMIS specific.

> 
> Thanks,
> Vikas
> >
> > > +
> > > +     while (length > 0) {
> > > +             u16 chunk = ETH_MODULE_EEPROM_PAGE_LEN;
> > > +             int rc;
> > > +
> > > +             /* Do not access more than required */
> > > +             if (length < ETH_MODULE_EEPROM_PAGE_LEN)
> > > +                     chunk = length;
> > > +
> > > +             rc = bnxt_read_sfp_module_eeprom_info(bp,
> > > +                                                   I2C_DEV_ADDR_A0,
> >
> > page_data->i2c_address
> >
> > > +                                                   page, bank,
> > > +                                                   true, page_data->offset,
> > > +                                                   chunk, data);
> > > +             if (rc)
> >
> > You can add a meaningful extack here according to the return code.
> >
> > > +                     return rc;
> > > +
> > > +             data += chunk;
> > > +             length -= chunk;
> > > +             bytes_copied += chunk;
> > > +             page++;
> > > +     }
> >
> > I'm not sure why the loop is required? It seems
> > bnxt_read_sfp_module_eeprom_info() is able to read
> > 'ETH_MODULE_EEPROM_PAGE_LEN' bytes at once, which is the maximum:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool/eeprom.c#n234
> >
> > > +
> > > +     return bytes_copied;
> > > +}
> > > +
> > >  static int bnxt_nway_reset(struct net_device *dev)
> > >  {
> > >       int rc = 0;
> > > @@ -4172,6 +4226,7 @@ const struct ethtool_ops bnxt_ethtool_ops = {
> > >       .set_eee                = bnxt_set_eee,
> > >       .get_module_info        = bnxt_get_module_info,
> > >       .get_module_eeprom      = bnxt_get_module_eeprom,
> > > +     .get_module_eeprom_by_page = bnxt_get_module_eeprom_by_page,
> > >       .nway_reset             = bnxt_nway_reset,
> > >       .set_phys_id            = bnxt_set_phys_id,
> > >       .self_test              = bnxt_self_test,
> > > --
> > > 2.18.1
> > >



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support
  2022-09-29  7:32       ` Ido Schimmel
@ 2022-09-30 10:05         ` Vikas Gupta
  0 siblings, 0 replies; 11+ messages in thread
From: Vikas Gupta @ 2022-09-30 10:05 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: Michael Chan, davem, netdev, kuba, edumazet, pabeni, gospo

[-- Attachment #1: Type: text/plain, Size: 8702 bytes --]

Hi Ido,

On Thu, Sep 29, 2022 at 1:02 PM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Thu, Sep 29, 2022 at 11:05:15AM +0530, Vikas Gupta wrote:
> > On Wed, Sep 28, 2022 at 3:04 PM Ido Schimmel <idosch@idosch.org> wrote:
> > >
> > > On Tue, Sep 27, 2022 at 08:58:43PM -0400, Michael Chan wrote:
> > > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > > index 0209f7caf490..03b1a0301a46 100644
> > > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> > > > @@ -2207,6 +2207,15 @@ struct bnxt {
> > > >  #define SFF_MODULE_ID_QSFP                   0xc
> > > >  #define SFF_MODULE_ID_QSFP_PLUS                      0xd
> > > >  #define SFF_MODULE_ID_QSFP28                 0x11
> > > > +#define SFF_MODULE_ID_QSFP_DD                        0x18
> > > > +#define SFF_MODULE_ID_DSFP                   0x1b
>
> Does not seem to be used.
>
> > > > +#define SFF_MODULE_ID_QSFP_PLUS_CMIS         0x1e
> > > > +
> > > > +#define BNXT_SFF_MODULE_BANK_SUPPORTED(module_id)    \
> > > > +     ((module_id) == SFF_MODULE_ID_QSFP_DD ||        \
> > > > +      (module_id) == SFF_MODULE_ID_QSFP28 ||         \
>
> Did you mean DSFP here? QSFP28 is SFF-8636, not CMIS.
>
> > > > +      (module_id) == SFF_MODULE_ID_QSFP_PLUS_CMIS)
> > >
> > > I suggest dropping this check unless you have a good reason to keep it.
> > > There are other modules out there that implement CMIS (e.g., OSFP) and
> > > given bnxt implements ethtool_ops::get_module_eeprom_by_page, it should
> > > be able to support them without kernel changes.
> > >
> > > See:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=970adfb76095fa719778d70a6b86030d2feb88dd
> > >
> > > The problem there was more severe because the driver returned '-EINVAL'
> > > instead of '-EOPNOTSUPP'.
> > >
> > We want to fallback on get_module_eeprom callback in case modules do
> > not implement CMIS and we pass the bank parameter accordingly to the
> > firmware.
>
> ethtool_ops::get_module_eeprom_by_page has nothing to do with CMIS. It
> is a generic operation to retrieve module EEPROM data based on a "3D
> address": bank, page and offset.
>
> The driving motivation behind it was CMIS modules, but it must be
> implemented in a way that it can retrieve information from modules that
> implement a different management interface such as SFF-8636.
>
> Let's say that tomorrow a user asks to retrieve pages 20h-21h from a
> QSFP module that implements SFF-8636, how will you support it? You can't
> extend the legacy ethtool::get_module_eeprom operation and your current
> implementation of ethtool_ops::get_module_eeprom_by_page has an
> artificial limitation to support only CMIS modules.
>
> By making sure that your implementation is generic as possible you will
> be able to support all possible requests and will not need to
> continuously patch the kernel (and users will not need to continuously
> upgrade).

Thanks for the detailed description. We`ll work on a simpler implementation.

Thanks,
Vikas

>
> >
> > > > +
> > > >  #define SFF8636_FLATMEM_OFFSET                       0x2
> > > >  #define SFF8636_FLATMEM_MASK                 0x4
> > > >  #define SFF8636_OPT_PAGES_OFFSET             0xc3
> > > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > > > index 379afa670494..2b18af95aacb 100644
> > > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> > > > @@ -3363,6 +3363,60 @@ static int bnxt_get_module_eeprom(struct net_device *dev,
> > > >       return 0;
> > > >  }
> > > >
> > > > +static int bnxt_get_module_eeprom_by_page(struct net_device *dev,
> > > > +                                       const struct ethtool_module_eeprom *page_data,
> > > > +                                       struct netlink_ext_ack *extack)
> > > > +{
> > > > +     struct bnxt *bp = netdev_priv(dev);
> > > > +     u16 length = page_data->length;
> > > > +     u8 *data = page_data->data;
> > > > +     u8 page = page_data->page;
> > > > +     u8 bank = page_data->bank;
> > > > +     u16 bytes_copied = 0;
> > > > +     u8 module_id;
> > > > +     int rc;
> > > > +
> > > > +     /* Return -EOPNOTSUPP to fallback on .get_module_eeprom */
> > > > +     if (!(bp->phy_flags & BNXT_PHY_FL_BANK_SEL))
> > > > +             return -EOPNOTSUPP;
> > >
> > > Maybe:
> > >
> > > if (bank && !(bp->phy_flags & BNXT_PHY_FL_BANK_SEL)) {
> > >         // extack
> > >         return -EINVAL;
> > > }
> >
> > BNXT_PHY_FL_BANK_SEL is a firmware capability to handle CMIS/bank
> > parameters. It does not tell whether the hooked module is CMIS
> > compliant.
> > I think EOPNOTSUPP is an appropriate choice here.
>
> See my comment above. This operation needs to be able to retrieve EEPROM
> data from non-CMIS modules as well.
>
> >
> >
> > >
> > > This should allow you to get rid of patch #2.
> >
> > I am not sure how we can get rid of patch #2. Patch #2 handles non CMIS modules.
> > Can you please elaborate more.
>
> All the complicated parsing performed in patch #2 is already performed
> in ethtool(8) that knows to request the available pages. The kernel
> will first try to retrieve these pages using
> ethtool_ops::get_module_eeprom_by_page and fallback to
> ethtool::get_module_eeprom in case of '-EOPNOTSUPP'.
>
> By adjusting your implementation of
> ethtool_ops::get_module_eeprom_by_page to handle non-CMIS modules you
> will be able to avoid extending the legacy operation with all this
> complex parsing.
>
> >
> > >
> > > > +
> > > > +     rc = bnxt_module_status_check(bp);
> > > > +     if (rc)
> > > > +             return rc;
> > >
> > > You can add extack here. I see that this function always returns
> > > '-EOPNOTSUPP' in case of errors, even when it does not make sense to
> > > fallback to ethtool_ops::get_module_eeprom.
> > Maybe -EIO can be used in one of these cases. I`ll check.
> >
> > >
> > > > +
> > > > +     rc = bnxt_read_sfp_module_eeprom_info(bp, I2C_DEV_ADDR_A0, 0, 0, false,
> > > > +                                           0, 1, &module_id);
> > > > +     if (rc)
> > > > +             return rc;
> > > > +
> > > > +     if (!BNXT_SFF_MODULE_BANK_SUPPORTED(module_id))
> > > > +             return -EOPNOTSUPP;
> > >
> > > I believe this hunk can be removed given the first comment.
> >   As I mentioned above we need this here to fallback.
>
> No need to fallback, simply avoid making this operation CMIS specific.
>
> >
> > Thanks,
> > Vikas
> > >
> > > > +
> > > > +     while (length > 0) {
> > > > +             u16 chunk = ETH_MODULE_EEPROM_PAGE_LEN;
> > > > +             int rc;
> > > > +
> > > > +             /* Do not access more than required */
> > > > +             if (length < ETH_MODULE_EEPROM_PAGE_LEN)
> > > > +                     chunk = length;
> > > > +
> > > > +             rc = bnxt_read_sfp_module_eeprom_info(bp,
> > > > +                                                   I2C_DEV_ADDR_A0,
> > >
> > > page_data->i2c_address
> > >
> > > > +                                                   page, bank,
> > > > +                                                   true, page_data->offset,
> > > > +                                                   chunk, data);
> > > > +             if (rc)
> > >
> > > You can add a meaningful extack here according to the return code.
> > >
> > > > +                     return rc;
> > > > +
> > > > +             data += chunk;
> > > > +             length -= chunk;
> > > > +             bytes_copied += chunk;
> > > > +             page++;
> > > > +     }
> > >
> > > I'm not sure why the loop is required? It seems
> > > bnxt_read_sfp_module_eeprom_info() is able to read
> > > 'ETH_MODULE_EEPROM_PAGE_LEN' bytes at once, which is the maximum:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ethtool/eeprom.c#n234
> > >
> > > > +
> > > > +     return bytes_copied;
> > > > +}
> > > > +
> > > >  static int bnxt_nway_reset(struct net_device *dev)
> > > >  {
> > > >       int rc = 0;
> > > > @@ -4172,6 +4226,7 @@ const struct ethtool_ops bnxt_ethtool_ops = {
> > > >       .set_eee                = bnxt_set_eee,
> > > >       .get_module_info        = bnxt_get_module_info,
> > > >       .get_module_eeprom      = bnxt_get_module_eeprom,
> > > > +     .get_module_eeprom_by_page = bnxt_get_module_eeprom_by_page,
> > > >       .nway_reset             = bnxt_nway_reset,
> > > >       .set_phys_id            = bnxt_set_phys_id,
> > > >       .self_test              = bnxt_self_test,
> > > > --
> > > > 2.18.1
> > > >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4206 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-09-30 10:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28  0:58 [PATCH net-next 0/6] bnxt_en: Driver updates Michael Chan
2022-09-28  0:58 ` [PATCH net-next 1/6] bnxt_en: Update firmware interface to 1.10.2.118 Michael Chan
2022-09-28  0:58 ` [PATCH net-next 2/6] bnxt_en: add support for QSFP optional EEPROM data Michael Chan
2022-09-28  0:58 ` [PATCH net-next 3/6] bnxt_en: Add bank parameter to bnxt_read_sfp_module_eeprom_info() Michael Chan
2022-09-28  0:58 ` [PATCH net-next 4/6] bnxt_en: Refactor bnxt_get_module_info() Michael Chan
2022-09-28  0:58 ` [PATCH net-next 5/6] bnxt_en: add .get_module_eeprom_by_page() support Michael Chan
2022-09-28  9:34   ` Ido Schimmel
2022-09-29  5:35     ` Vikas Gupta
2022-09-29  7:32       ` Ido Schimmel
2022-09-30 10:05         ` Vikas Gupta
2022-09-28  0:58 ` [PATCH net-next 6/6] bnxt_en: check and resize NVRAM UPDATE entry before flashing Michael Chan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).