[PATCH net-next 0/9] bnxt_en: Updates for net-next.

[PATCH net-next 0/9] bnxt_en: Updates for net-next.

[Michael Chan]

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

This series contains these main changes:

1. Change of default message level to enable more logging.
2. Some cleanups related to processing async events from firmware.
3. Allow online ethtool selftest on multi-function PFs.
4. Return stored firmware version information to devlink.

Michael Chan (4):
  bnxt_en: Set driver default message level.
  bnxt_en: Simplify bnxt_async_event_process().
  bnxt_en: Log event_data1 and event_data2 when handling RESET_NOTIFY
    event.
  bnxt_en: Log unknown link speed appropriately.

Vasundhara Volam (5):
  bnxt_en: Return -EROFS to user space, if NVM writes are not permitted.
  bnxt_en: Enable online self tests for multi-host/NPAR mode.
  bnxt_en: Add bnxt_hwrm_nvm_get_dev_info() to query NVM info.
  bnxt_en: Refactor bnxt_dl_info_get().
  bnxt_en: Add stored FW version info to devlink info_get cb.

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  37 +++--
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   1 +
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 153 ++++++++++++------
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.h |   6 +
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  24 ++-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.h |   2 +
 6 files changed, 162 insertions(+), 61 deletions(-)

-- 
2.18.1


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

[PATCH net-next 1/9] bnxt_en: Return -EROFS to user space, if NVM writes are not permitted.

[Michael Chan]

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

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

If NVRAM resources are locked, NVM writes are not permitted. In such
scenarios, firmware returns HWRM_ERR_CODE_RESOURCE_LOCKED error to
firmware commands.

Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 5e4b7fbeef06..d4402a2cd07f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -4325,6 +4325,8 @@ static int bnxt_hwrm_to_stderr(u32 hwrm_err)
 	switch (hwrm_err) {
 	case HWRM_ERR_CODE_SUCCESS:
 		return 0;
+	case HWRM_ERR_CODE_RESOURCE_LOCKED:
+		return -EROFS;
 	case HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED:
 		return -EACCES;
 	case HWRM_ERR_CODE_RESOURCE_ALLOC_ERROR:
-- 
2.18.1


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

[PATCH net-next 2/9] bnxt_en: Enable online self tests for multi-host/NPAR mode.

[Michael Chan]

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

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

Online self tests are not disruptive and can be run in NPAR mode
and in multi-host NIC as well.

Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 52b4ea6ef8c9..dcbb7b70d60a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3298,7 +3298,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 	u8 test_mask = 0;
 	int rc = 0, i;
 
-	if (!bp->num_tests || !BNXT_SINGLE_PF(bp))
+	if (!bp->num_tests || !BNXT_PF(bp))
 		return;
 	memset(buf, 0, sizeof(u64) * bp->num_tests);
 	if (!netif_running(dev)) {
@@ -3311,9 +3311,9 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 		do_ext_lpbk = true;
 
 	if (etest->flags & ETH_TEST_FL_OFFLINE) {
-		if (bp->pf.active_vfs) {
+		if (bp->pf.active_vfs || !BNXT_SINGLE_PF(bp)) {
 			etest->flags |= ETH_TEST_FL_FAILED;
-			netdev_warn(dev, "Offline tests cannot be run with active VFs\n");
+			netdev_warn(dev, "Offline tests cannot be run with active VFs or on shared PF\n");
 			return;
 		}
 		offline = true;
@@ -3829,7 +3829,7 @@ void bnxt_ethtool_init(struct bnxt *bp)
 		bnxt_get_pkgver(dev);
 
 	bp->num_tests = 0;
-	if (bp->hwrm_spec_code < 0x10704 || !BNXT_SINGLE_PF(bp))
+	if (bp->hwrm_spec_code < 0x10704 || !BNXT_PF(bp))
 		return;
 
 	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_SELFTEST_QLIST, -1, -1);
-- 
2.18.1


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

[PATCH net-next 3/9] bnxt_en: Set driver default message level.

[Michael Chan]

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

Currently, bp->msg_enable has default value of 0.  It is more useful
to have the commonly used NETIF_MSG_DRV and NETIF_MSG_HW enabled by
default.

Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d4402a2cd07f..0da6a1138eee 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -69,6 +69,7 @@
 #include "bnxt_debugfs.h"
 
 #define BNXT_TX_TIMEOUT		(5 * HZ)
+#define BNXT_DEF_MSG_ENABLE	(NETIF_MSG_DRV | NETIF_MSG_HW)
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Broadcom BCM573xx network driver");
@@ -12510,6 +12511,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return -ENOMEM;
 
 	bp = netdev_priv(dev);
+	bp->msg_enable = BNXT_DEF_MSG_ENABLE;
 	bnxt_set_max_func_irqs(bp, max_irqs);
 
 	if (bnxt_vf_pciid(ent->driver_data))
-- 
2.18.1


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

Re: [PATCH net-next 3/9] bnxt_en: Set driver default message level.

[Jakub Kicinski]

On Sun, 11 Oct 2020 06:22:55 -0400 Michael Chan wrote:
> Currently, bp->msg_enable has default value of 0.  It is more useful
> to have the commonly used NETIF_MSG_DRV and NETIF_MSG_HW enabled by
> default.
> 
> Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
> Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

This will add whole bunch of output for "RX buffer error 4000[45]", no?
That one needs to switch to a silent reset first, I'd think.

Re: [PATCH net-next 3/9] bnxt_en: Set driver default message level.

[Michael Chan]

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

On Sun, Oct 11, 2020 at 12:34 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun, 11 Oct 2020 06:22:55 -0400 Michael Chan wrote:
> > Currently, bp->msg_enable has default value of 0.  It is more useful
> > to have the commonly used NETIF_MSG_DRV and NETIF_MSG_HW enabled by
> > default.
> >
> > Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
> > Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> > Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>
> This will add whole bunch of output for "RX buffer error 4000[45]", no?
> That one needs to switch to a silent reset first, I'd think.

The last round of net-next patches have made changes to reduce this
noise already.  If the firmware supports the new  RING_MONITOR scheme,
There will be no more messages.  The reset will be counted in a new
ethtool -S counter only.

If the firmware is older and does not support the new scheme, we've
changed the logging to warn_once() in addition to the ethtool -S
counter.  There is no point in warning more than once.

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

Re: [PATCH net-next 3/9] bnxt_en: Set driver default message level.

[Jakub Kicinski]

On Sun, 11 Oct 2020 14:13:04 -0700 Michael Chan wrote:
> On Sun, Oct 11, 2020 at 12:34 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Sun, 11 Oct 2020 06:22:55 -0400 Michael Chan wrote:  
> > > Currently, bp->msg_enable has default value of 0.  It is more useful
> > > to have the commonly used NETIF_MSG_DRV and NETIF_MSG_HW enabled by
> > > default.
> > >
> > > Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
> > > Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> > > Signed-off-by: Michael Chan <michael.chan@broadcom.com>  
> >
> > This will add whole bunch of output for "RX buffer error 4000[45]", no?
> > That one needs to switch to a silent reset first, I'd think.  
> 
> The last round of net-next patches have made changes to reduce this
> noise already.  If the firmware supports the new  RING_MONITOR scheme,
> There will be no more messages.  The reset will be counted in a new
> ethtool -S counter only.
> 
> If the firmware is older and does not support the new scheme, we've
> changed the logging to warn_once() in addition to the ethtool -S
> counter.  There is no point in warning more than once.

I'm talking about bnxt_dbg_dump_states specifically. 

I'm looking at net-next:

 bnxt_rx_pkt
   bnxt_sched_reset
     set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);


 bnxt_sp_task
   bnxt_reset
     bnxt_reset_task
       bnxt_dbg_dump_states

Re: [PATCH net-next 3/9] bnxt_en: Set driver default message level.

[Michael Chan]

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

On Sun, Oct 11, 2020 at 2:53 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> I'm talking about bnxt_dbg_dump_states specifically.
>
> I'm looking at net-next:
>
>  bnxt_rx_pkt
>    bnxt_sched_reset
>      set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
>
>
>  bnxt_sp_task
>    bnxt_reset
>      bnxt_reset_task
>        bnxt_dbg_dump_states
>
Yes, you are right.  Although, we actually go through a slightly
different path with a different event when we encounter these RX
buffer errors:

bnxt_sched_reset()
  set_bit(BNXT_RST_RING_SP_EVENT, &bp->sp_event)

bnxt_rx_ring_reset()
   bnxt_hwrm_rx_ring_reset()

But with the oldest firmware, this call would fail and we would fall back to

bnxt_reset_task() with non-silent mode.

I will respin and change this call to silent mode.

Thanks.

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

[PATCH net-next 4/9] bnxt_en: Simplify bnxt_async_event_process().

[Michael Chan]

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

event_data1 and event_data2 are used when processing most events.
Store these in local variables at the beginning of the function to
simplify many of the case statements.

Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 0da6a1138eee..f093cdd61426 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1980,11 +1980,12 @@ static int bnxt_async_event_process(struct bnxt *bp,
 				    struct hwrm_async_event_cmpl *cmpl)
 {
 	u16 event_id = le16_to_cpu(cmpl->event_id);
+	u32 data1 = le32_to_cpu(cmpl->event_data1);
+	u32 data2 = le32_to_cpu(cmpl->event_data2);
 
 	/* TODO CHIMP_FW: Define event id's for link change, error etc */
 	switch (event_id) {
 	case ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE: {
-		u32 data1 = le32_to_cpu(cmpl->event_data1);
 		struct bnxt_link_info *link_info = &bp->link_info;
 
 		if (BNXT_VF(bp))
@@ -2014,7 +2015,6 @@ static int bnxt_async_event_process(struct bnxt *bp,
 		set_bit(BNXT_HWRM_PF_UNLOAD_SP_EVENT, &bp->sp_event);
 		break;
 	case ASYNC_EVENT_CMPL_EVENT_ID_PORT_CONN_NOT_ALLOWED: {
-		u32 data1 = le32_to_cpu(cmpl->event_data1);
 		u16 port_id = BNXT_GET_EVENT_PORT(data1);
 
 		if (BNXT_VF(bp))
@@ -2031,9 +2031,7 @@ static int bnxt_async_event_process(struct bnxt *bp,
 			goto async_event_process_exit;
 		set_bit(BNXT_RESET_TASK_SILENT_SP_EVENT, &bp->sp_event);
 		break;
-	case ASYNC_EVENT_CMPL_EVENT_ID_RESET_NOTIFY: {
-		u32 data1 = le32_to_cpu(cmpl->event_data1);
-
+	case ASYNC_EVENT_CMPL_EVENT_ID_RESET_NOTIFY:
 		if (!bp->fw_health)
 			goto async_event_process_exit;
 
@@ -2053,10 +2051,8 @@ static int bnxt_async_event_process(struct bnxt *bp,
 		}
 		set_bit(BNXT_FW_RESET_NOTIFY_SP_EVENT, &bp->sp_event);
 		break;
-	}
 	case ASYNC_EVENT_CMPL_EVENT_ID_ERROR_RECOVERY: {
 		struct bnxt_fw_health *fw_health = bp->fw_health;
-		u32 data1 = le32_to_cpu(cmpl->event_data1);
 
 		if (!fw_health)
 			goto async_event_process_exit;
@@ -2084,8 +2080,6 @@ static int bnxt_async_event_process(struct bnxt *bp,
 		goto async_event_process_exit;
 	}
 	case ASYNC_EVENT_CMPL_EVENT_ID_RING_MONITOR_MSG: {
-		u32 data1 = le32_to_cpu(cmpl->event_data1);
-		u32 data2 = le32_to_cpu(cmpl->event_data2);
 		struct bnxt_rx_ring_info *rxr;
 		u16 grp_idx;
 
-- 
2.18.1


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

[PATCH net-next 5/9] bnxt_en: Log event_data1 and event_data2 when handling RESET_NOTIFY event.

[Michael Chan]

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

Log these values that contain useful firmware state information.

Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index f093cdd61426..ca35ec06abf0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2032,6 +2032,9 @@ static int bnxt_async_event_process(struct bnxt *bp,
 		set_bit(BNXT_RESET_TASK_SILENT_SP_EVENT, &bp->sp_event);
 		break;
 	case ASYNC_EVENT_CMPL_EVENT_ID_RESET_NOTIFY:
+		if (netif_msg_hw(bp))
+			netdev_warn(bp->dev, "Received RESET_NOTIFY event, data1: 0x%x, data2: 0x%x\n",
+				    data1, data2);
 		if (!bp->fw_health)
 			goto async_event_process_exit;
 
-- 
2.18.1


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

[PATCH net-next 6/9] bnxt_en: Log unknown link speed appropriately.

[Michael Chan]

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

If the VF virtual link is set to always enabled, the speed may be
unknown when the physical link is down.  The driver currently logs
the link speed as 4294967295 Mbps which is SPEED_UNKNOWN.  Modify
the link up log message as "speed unknown" which makes more sense.

Reviewed-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index ca35ec06abf0..2d6ecf1adbc5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -8902,6 +8902,11 @@ static void bnxt_report_link(struct bnxt *bp)
 		u16 fec;
 
 		netif_carrier_on(bp->dev);
+		speed = bnxt_fw_to_ethtool_speed(bp->link_info.link_speed);
+		if (speed == SPEED_UNKNOWN) {
+			netdev_info(bp->dev, "NIC Link is Up, speed unknown\n");
+			return;
+		}
 		if (bp->link_info.duplex == BNXT_LINK_DUPLEX_FULL)
 			duplex = "full";
 		else
@@ -8914,7 +8919,6 @@ static void bnxt_report_link(struct bnxt *bp)
 			flow_ctrl = "ON - receive";
 		else
 			flow_ctrl = "none";
-		speed = bnxt_fw_to_ethtool_speed(bp->link_info.link_speed);
 		netdev_info(bp->dev, "NIC Link is Up, %u Mbps %s duplex, Flow control: %s\n",
 			    speed, duplex, flow_ctrl);
 		if (bp->flags & BNXT_FLAG_EEE_CAP)
-- 
2.18.1


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

[PATCH net-next 7/9] bnxt_en: Add bnxt_hwrm_nvm_get_dev_info() to query NVM info.

[Michael Chan]

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

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

Add a new bnxt_hwrm_nvm_get_dev_info() to query firmware version
information via NVM_GET_DEV_INFO firmware command.  Use it to
get the running version of the NVM configuration information.

This new function will also be used in subsequent patches to get the
stored firmware versions.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c        | 12 ++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt.h        |  1 +
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c    | 16 ++++++++++++++++
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.h    |  2 ++
 4 files changed, 31 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 2d6ecf1adbc5..55d96e3a69da 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7562,6 +7562,16 @@ static int bnxt_hwrm_func_reset(struct bnxt *bp)
 	return hwrm_send_message(bp, &req, sizeof(req), HWRM_RESET_TIMEOUT);
 }
 
+static void bnxt_nvm_cfg_ver_get(struct bnxt *bp)
+{
+	struct hwrm_nvm_get_dev_info_output nvm_info;
+
+	if (!bnxt_hwrm_nvm_get_dev_info(bp, &nvm_info))
+		snprintf(bp->nvm_cfg_ver, FW_VER_STR_LEN, "%d.%d.%d",
+			 nvm_info.nvm_cfg_ver_maj, nvm_info.nvm_cfg_ver_min,
+			 nvm_info.nvm_cfg_ver_upd);
+}
+
 static int bnxt_hwrm_queue_qportcfg(struct bnxt *bp)
 {
 	int rc = 0;
@@ -11223,6 +11233,8 @@ static int bnxt_fw_init_one_p1(struct bnxt *bp)
 		if (rc)
 			return rc;
 	}
+	bnxt_nvm_cfg_ver_get(bp);
+
 	rc = bnxt_hwrm_func_reset(bp);
 	if (rc)
 		return -ENODEV;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index b208ff7c5d14..21ef1c21f602 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1856,6 +1856,7 @@ struct bnxt {
 #define PHY_VER_STR_LEN         (FW_VER_STR_LEN - BC_HWRM_STR_LEN)
 	char			fw_ver_str[FW_VER_STR_LEN];
 	char			hwrm_ver_supp[FW_VER_STR_LEN];
+	char			nvm_cfg_ver[FW_VER_STR_LEN];
 	u64			fw_ver_code;
 #define BNXT_FW_VER_CODE(maj, min, bld, rsv)			\
 	((u64)(maj) << 48 | (u64)(min) << 32 | (u64)(bld) << 16 | (rsv))
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index dcbb7b70d60a..53687bc7fcf5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2072,6 +2072,22 @@ static u32 bnxt_get_link(struct net_device *dev)
 	return bp->link_info.link_up;
 }
 
+int bnxt_hwrm_nvm_get_dev_info(struct bnxt *bp,
+			       struct hwrm_nvm_get_dev_info_output *nvm_dev_info)
+{
+	struct hwrm_nvm_get_dev_info_output *resp = bp->hwrm_cmd_resp_addr;
+	struct hwrm_nvm_get_dev_info_input req = {0};
+	int rc;
+
+	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_DEV_INFO, -1, -1);
+	mutex_lock(&bp->hwrm_cmd_lock);
+	rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+	if (!rc)
+		memcpy(nvm_dev_info, resp, sizeof(*resp));
+	mutex_unlock(&bp->hwrm_cmd_lock);
+	return rc;
+}
+
 static void bnxt_print_admin_err(struct bnxt *bp)
 {
 	netdev_info(bp->dev, "PF does not have admin privileges to flash or reset the device\n");
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
index 34f44ddfad79..fa6fbde52bea 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
@@ -92,6 +92,8 @@ u32 bnxt_get_rxfh_indir_size(struct net_device *dev);
 u32 _bnxt_fw_to_ethtool_adv_spds(u16, u8);
 u32 bnxt_fw_to_ethtool_speed(u16);
 u16 bnxt_get_fw_auto_link_speeds(u32);
+int bnxt_hwrm_nvm_get_dev_info(struct bnxt *bp,
+			       struct hwrm_nvm_get_dev_info_output *nvm_dev_info);
 int bnxt_flash_package_from_file(struct net_device *dev, const char *filename,
 				 u32 install_type);
 void bnxt_ethtool_init(struct bnxt *bp);
-- 
2.18.1


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

[PATCH net-next 8/9] bnxt_en: Refactor bnxt_dl_info_get().

[Michael Chan]

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

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

Add a new function bnxt_dl_info_put() to simplify the code, as there
are more stored firmware version fields to be added in the next patch.

Also, rename fw_ver variable name to ncsi_ver for better naming while
copying to devlink info_get cb.

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 112 ++++++++++--------
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.h |   6 +
 2 files changed, 70 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index d436134bdc40..65e8da1f7a01 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -382,6 +382,29 @@ static int bnxt_hwrm_get_nvm_cfg_ver(struct bnxt *bp,
 	return rc;
 }
 
+static int bnxt_dl_info_put(struct bnxt *bp, struct devlink_info_req *req,
+			    enum bnxt_dl_version_type type, const char *key,
+			    char *buf)
+{
+	if (!strlen(buf))
+		return 0;
+
+	if ((bp->flags & BNXT_FLAG_CHIP_P5) &&
+	    (!strcmp(key, DEVLINK_INFO_VERSION_GENERIC_FW_NCSI) ||
+	     !strcmp(key, DEVLINK_INFO_VERSION_GENERIC_FW_ROCE)))
+		return 0;
+
+	switch (type) {
+	case BNXT_VERSION_FIXED:
+		return devlink_info_version_fixed_put(req, key, buf);
+	case BNXT_VERSION_RUNNING:
+		return devlink_info_version_running_put(req, key, buf);
+	case BNXT_VERSION_STORED:
+		return devlink_info_version_stored_put(req, key, buf);
+	}
+	return 0;
+}
+
 static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 			    struct netlink_ext_ack *extack)
 {
@@ -390,7 +413,7 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	struct hwrm_ver_get_output *ver_resp;
 	char mgmt_ver[FW_VER_STR_LEN];
 	char roce_ver[FW_VER_STR_LEN];
-	char fw_ver[FW_VER_STR_LEN];
+	char ncsi_ver[FW_VER_STR_LEN];
 	char buf[32];
 	int rc;
 
@@ -398,10 +421,11 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	if (rc)
 		return rc;
 
-	if (strlen(bp->board_partno)) {
-		rc = devlink_info_version_fixed_put(req,
-			DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
-			bp->board_partno);
+	if (BNXT_PF(bp) && (bp->flags & BNXT_FLAG_DSN_VALID)) {
+		sprintf(buf, "%02X-%02X-%02X-%02X-%02X-%02X-%02X-%02X",
+			bp->dsn[7], bp->dsn[6], bp->dsn[5], bp->dsn[4],
+			bp->dsn[3], bp->dsn[2], bp->dsn[1], bp->dsn[0]);
+		rc = devlink_info_serial_number_put(req, buf);
 		if (rc)
 			return rc;
 	}
@@ -412,54 +436,49 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 			return rc;
 	}
 
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_FIXED,
+			      DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
+			      bp->board_partno);
+	if (rc)
+		return rc;
+
 	sprintf(buf, "%X", bp->chip_num);
-	rc = devlink_info_version_fixed_put(req,
-			DEVLINK_INFO_VERSION_GENERIC_ASIC_ID, buf);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_FIXED,
+			      DEVLINK_INFO_VERSION_GENERIC_ASIC_ID, buf);
 	if (rc)
 		return rc;
 
 	ver_resp = &bp->ver_resp;
 	sprintf(buf, "%X", ver_resp->chip_rev);
-	rc = devlink_info_version_fixed_put(req,
-			DEVLINK_INFO_VERSION_GENERIC_ASIC_REV, buf);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_FIXED,
+			      DEVLINK_INFO_VERSION_GENERIC_ASIC_REV, buf);
 	if (rc)
 		return rc;
 
-	if (BNXT_PF(bp)) {
-		sprintf(buf, "%02X-%02X-%02X-%02X-%02X-%02X-%02X-%02X",
-			bp->dsn[7], bp->dsn[6], bp->dsn[5], bp->dsn[4],
-			bp->dsn[3], bp->dsn[2], bp->dsn[1], bp->dsn[0]);
-		rc = devlink_info_serial_number_put(req, buf);
-		if (rc)
-			return rc;
-	}
-
-	if (strlen(ver_resp->active_pkg_name)) {
-		rc =
-		    devlink_info_version_running_put(req,
-					DEVLINK_INFO_VERSION_GENERIC_FW,
-					ver_resp->active_pkg_name);
-		if (rc)
-			return rc;
-	}
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+			      DEVLINK_INFO_VERSION_GENERIC_FW,
+			      ver_resp->active_pkg_name);
+	if (rc)
+		return rc;
 
 	if (BNXT_PF(bp) && !bnxt_hwrm_get_nvm_cfg_ver(bp, &nvm_cfg_ver)) {
 		u32 ver = nvm_cfg_ver.vu32;
 
 		sprintf(buf, "%X.%X.%X", (ver >> 16) & 0xF, (ver >> 8) & 0xF,
 			ver & 0xF);
-		rc = devlink_info_version_running_put(req,
-				DEVLINK_INFO_VERSION_GENERIC_FW_PSID, buf);
+		rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+				      DEVLINK_INFO_VERSION_GENERIC_FW_PSID,
+				      buf);
 		if (rc)
 			return rc;
 	}
 
 	if (ver_resp->flags & VER_GET_RESP_FLAGS_EXT_VER_AVAIL) {
-		snprintf(fw_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
+		snprintf(mgmt_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
 			 ver_resp->hwrm_fw_major, ver_resp->hwrm_fw_minor,
 			 ver_resp->hwrm_fw_build, ver_resp->hwrm_fw_patch);
 
-		snprintf(mgmt_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
+		snprintf(ncsi_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
 			 ver_resp->mgmt_fw_major, ver_resp->mgmt_fw_minor,
 			 ver_resp->mgmt_fw_build, ver_resp->mgmt_fw_patch);
 
@@ -467,11 +486,11 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 			 ver_resp->roce_fw_major, ver_resp->roce_fw_minor,
 			 ver_resp->roce_fw_build, ver_resp->roce_fw_patch);
 	} else {
-		snprintf(fw_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
+		snprintf(mgmt_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
 			 ver_resp->hwrm_fw_maj_8b, ver_resp->hwrm_fw_min_8b,
 			 ver_resp->hwrm_fw_bld_8b, ver_resp->hwrm_fw_rsvd_8b);
 
-		snprintf(mgmt_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
+		snprintf(ncsi_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
 			 ver_resp->mgmt_fw_maj_8b, ver_resp->mgmt_fw_min_8b,
 			 ver_resp->mgmt_fw_bld_8b, ver_resp->mgmt_fw_rsvd_8b);
 
@@ -479,29 +498,26 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 			 ver_resp->roce_fw_maj_8b, ver_resp->roce_fw_min_8b,
 			 ver_resp->roce_fw_bld_8b, ver_resp->roce_fw_rsvd_8b);
 	}
-	rc = devlink_info_version_running_put(req,
-			DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, fw_ver);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, mgmt_ver);
 	if (rc)
 		return rc;
 
-	rc = devlink_info_version_running_put(req,
-				DEVLINK_INFO_VERSION_GENERIC_FW_MGMT_API,
-				bp->hwrm_ver_supp);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_MGMT_API,
+			      bp->hwrm_ver_supp);
 	if (rc)
 		return rc;
 
-	if (!(bp->flags & BNXT_FLAG_CHIP_P5)) {
-		rc = devlink_info_version_running_put(req,
-			DEVLINK_INFO_VERSION_GENERIC_FW_NCSI, mgmt_ver);
-		if (rc)
-			return rc;
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_NCSI, ncsi_ver);
+	if (rc)
+		return rc;
 
-		rc = devlink_info_version_running_put(req,
-			DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);
-		if (rc)
-			return rc;
-	}
-	return 0;
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);
+
+	return rc;
 }
 
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index d5c8bd49383a..d22cab5d6856 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -60,6 +60,12 @@ struct bnxt_dl_nvm_param {
 	u8 dl_num_bytes;
 };
 
+enum bnxt_dl_version_type {
+	BNXT_VERSION_FIXED,
+	BNXT_VERSION_RUNNING,
+	BNXT_VERSION_STORED,
+};
+
 void bnxt_devlink_health_report(struct bnxt *bp, unsigned long event);
 void bnxt_dl_health_status_update(struct bnxt *bp, bool healthy);
 void bnxt_dl_health_recovery_done(struct bnxt *bp);
-- 
2.18.1


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

Re: [PATCH net-next 8/9] bnxt_en: Refactor bnxt_dl_info_get().

[Jakub Kicinski]

On Sun, 11 Oct 2020 06:23:00 -0400 Michael Chan wrote:
> +static int bnxt_dl_info_put(struct bnxt *bp, struct devlink_info_req *req,
> +			    enum bnxt_dl_version_type type, const char *key,
> +			    char *buf)
> +{
> +	if (!strlen(buf))
> +		return 0;

I think buf can be directly read from FW in later patches, it'd be good
to see a strnlen(), or bnxt_hwrm_nvm_get_dev_info() ensuring strings
are null-terminated.

Re: [PATCH net-next 8/9] bnxt_en: Refactor bnxt_dl_info_get().

[Michael Chan]

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

On Sun, Oct 11, 2020 at 2:59 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun, 11 Oct 2020 06:23:00 -0400 Michael Chan wrote:
> > +static int bnxt_dl_info_put(struct bnxt *bp, struct devlink_info_req *req,
> > +                         enum bnxt_dl_version_type type, const char *key,
> > +                         char *buf)
> > +{
> > +     if (!strlen(buf))
> > +             return 0;
>
> I think buf can be directly read from FW in later patches, it'd be good
> to see a strnlen(), or bnxt_hwrm_nvm_get_dev_info() ensuring strings
> are null-terminated.

Our fw interface spec specifies a NULL terminated string to be
returned by firmware.  But we can probably add an additional check
when we process the firmware response.

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

[PATCH net-next 9/9] bnxt_en: Add stored FW version info to devlink info_get cb.

[Michael Chan]

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

From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

This patch adds FW versions stored in the flash to devlink info_get
callback.  Return the correct fw.psid running version using the
newly added bp->nvm_cfg_ver.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 45 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 65e8da1f7a01..7631c2d4c19e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -408,6 +408,7 @@ static int bnxt_dl_info_put(struct bnxt *bp, struct devlink_info_req *req,
 static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 			    struct netlink_ext_ack *extack)
 {
+	struct hwrm_nvm_get_dev_info_output nvm_dev_info;
 	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
 	union devlink_param_value nvm_cfg_ver;
 	struct hwrm_ver_get_output *ver_resp;
@@ -455,6 +456,12 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	if (rc)
 		return rc;
 
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_PSID,
+			      bp->nvm_cfg_ver);
+	if (rc)
+		return rc;
+
 	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
 			      DEVLINK_INFO_VERSION_GENERIC_FW,
 			      ver_resp->active_pkg_name);
@@ -466,7 +473,7 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 
 		sprintf(buf, "%X.%X.%X", (ver >> 16) & 0xF, (ver >> 8) & 0xF,
 			ver & 0xF);
-		rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
+		rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
 				      DEVLINK_INFO_VERSION_GENERIC_FW_PSID,
 				      buf);
 		if (rc)
@@ -516,7 +523,43 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 
 	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_RUNNING,
 			      DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);
+	if (rc)
+		return rc;
+
+	rc = bnxt_hwrm_nvm_get_dev_info(bp, &nvm_dev_info);
+	if (rc)
+		return rc;
+
+	if (!(nvm_dev_info.flags & NVM_GET_DEV_INFO_RESP_FLAGS_FW_VER_VALID))
+		return 0;
+
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
+			      DEVLINK_INFO_VERSION_GENERIC_FW,
+			      nvm_dev_info.pkg_name);
+	if (rc)
+		return rc;
+
+	snprintf(mgmt_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
+		 nvm_dev_info.hwrm_fw_major, nvm_dev_info.hwrm_fw_minor,
+		 nvm_dev_info.hwrm_fw_build, nvm_dev_info.hwrm_fw_patch);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, mgmt_ver);
+	if (rc)
+		return rc;
+
+	snprintf(ncsi_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
+		 nvm_dev_info.mgmt_fw_major, nvm_dev_info.mgmt_fw_minor,
+		 nvm_dev_info.mgmt_fw_build, nvm_dev_info.mgmt_fw_patch);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_NCSI, ncsi_ver);
+	if (rc)
+		return rc;
 
+	snprintf(roce_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
+		 nvm_dev_info.roce_fw_major, nvm_dev_info.roce_fw_minor,
+		 nvm_dev_info.roce_fw_build, nvm_dev_info.roce_fw_patch);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);
 	return rc;
 }
 
-- 
2.18.1


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

Re: [PATCH net-next 9/9] bnxt_en: Add stored FW version info to devlink info_get cb.

[Jakub Kicinski]

On Sun, 11 Oct 2020 06:23:01 -0400 Michael Chan wrote:
> +	rc = bnxt_hwrm_nvm_get_dev_info(bp, &nvm_dev_info);
> +	if (rc)
> +		return rc;

This will not cause an error to be returned for the entire operation on
older FW or HW, right?

> +	if (!(nvm_dev_info.flags & NVM_GET_DEV_INFO_RESP_FLAGS_FW_VER_VALID))
> +		return 0;


> +	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
> +			      DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);
>  	return rc;

return bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
			DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);

Re: [PATCH net-next 9/9] bnxt_en: Add stored FW version info to devlink info_get cb.

[Michael Chan]

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

On Sun, Oct 11, 2020 at 3:02 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun, 11 Oct 2020 06:23:01 -0400 Michael Chan wrote:
> > +     rc = bnxt_hwrm_nvm_get_dev_info(bp, &nvm_dev_info);
> > +     if (rc)
> > +             return rc;
>
> This will not cause an error to be returned for the entire operation on
> older FW or HW, right?
>

I don't think so.  This firmware call has been around for many years.
The call was recently extended to return the stored version
information but older firmware will still return successfully without
the stored versions.  I'll ask Vasundhara to double check on this.

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

Re: [PATCH net-next 9/9] bnxt_en: Add stored FW version info to devlink info_get cb.

[Vasundhara Volam]

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

On Mon, Oct 12, 2020 at 4:01 AM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Sun, Oct 11, 2020 at 3:02 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Sun, 11 Oct 2020 06:23:01 -0400 Michael Chan wrote:
> > > +     rc = bnxt_hwrm_nvm_get_dev_info(bp, &nvm_dev_info);
> > > +     if (rc)
> > > +             return rc;
> >
> > This will not cause an error to be returned for the entire operation on
> > older FW or HW, right?
> >
>
> I don't think so.  This firmware call has been around for many years.
> The call was recently extended to return the stored version
> information but older firmware will still return successfully without
> the stored versions.  I'll ask Vasundhara to double check on this.
Yes, older firmware will return success and does not set the new flag
support for stored firmware versions. Only in error cases, older or
newer FW will return error. Thanks.

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

[PATCH net-next 0/9] bnxt_en: updates for net-next.

[Michael Chan]

Non-critical bug fixes, improvements, a new ethtool feature, and a new
device ID.

Michael Chan (9):
  bnxt_en: Fix invalid max channel parameter in ethtool -l.
  bnxt_en: Add Support for ETHTOOL_GMODULEINFO and ETHTOOL_GMODULEEEPRO
  bnxt_en: Report PCIe link speed and width during driver load
  bnxt_en: Reduce maximum ring pages if page size is 64K.
  bnxt_en: Improve the delay logic for firmware response.
  bnxt_en: Fix length value in dmesg log firmware error message.
  bnxt_en: Simplify and improve unsupported SFP+ module reporting.
  bnxt_en: Add BCM57314 device ID.
  bnxt_en: Use dma_rmb() instead of rmb().

 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 111 +++++++++++--------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  19 +++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 126 +++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h     |  34 ++++++
 4 files changed, 241 insertions(+), 49 deletions(-)

-- 
1.8.3.1