netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/5] bnxt_en: Bug fixes.
@ 2019-10-21  5:34 Michael Chan
  2019-10-21  5:34 ` [PATCH net 1/5] bnxt_en: Fix the size of devlink MSIX parameters Michael Chan
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Michael Chan @ 2019-10-21  5:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, vasundhara-v.volam

Devlink and error recovery bug fix patches.  Most of the work is by
Vasundhara Volam.  Please queue patch 1 and 2 for -stable also.  Thanks.

Michael Chan (1):
  bnxt_en: Fix devlink NVRAM related byte order related issues.

Vasundhara Volam (4):
  bnxt_en: Fix the size of devlink MSIX parameters.
  bnxt_en: Adjust the time to wait before polling firmware readiness.
  bnxt_en: Minor formatting changes in FW devlink_health_reporter
  bnxt_en: Avoid disabling pci device in bnxt_remove_one() for already
    disabled device.

 drivers/net/ethernet/broadcom/bnxt/bnxt.c         |  10 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 112 +++++++++++++---------
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |   3 +-
 3 files changed, 73 insertions(+), 52 deletions(-)

-- 
2.5.1


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

* [PATCH net 1/5] bnxt_en: Fix the size of devlink MSIX parameters.
  2019-10-21  5:34 [PATCH net 0/5] bnxt_en: Bug fixes Michael Chan
@ 2019-10-21  5:34 ` Michael Chan
  2019-10-21  5:34 ` [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues Michael Chan
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Michael Chan @ 2019-10-21  5:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, vasundhara-v.volam, Jiri Pirko

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

The current code that rounds up the NVRAM parameter bit size to the next
byte size for the devlink parameter is not always correct.  The MSIX
devlink parameters are 4 bytes and we don't get the correct size
using this method.

Fix it by adding a new dl_num_bytes member to the bnxt_dl_nvm_param
structure which statically provides bytesize information according
to the devlink parameter type definition.

Fixes: 782a624d00fa ("bnxt_en: Add bnxt_en initial port params table and register it")
Cc: Jiri Pirko <jiri@mellanox.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_devlink.c | 28 +++++++++++------------
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  3 ++-
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index e664392..68f74f5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -215,15 +215,15 @@ enum bnxt_dl_param_id {
 
 static const struct bnxt_dl_nvm_param nvm_params[] = {
 	{DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, NVM_OFF_ENABLE_SRIOV,
-	 BNXT_NVM_SHARED_CFG, 1},
+	 BNXT_NVM_SHARED_CFG, 1, 1},
 	{DEVLINK_PARAM_GENERIC_ID_IGNORE_ARI, NVM_OFF_IGNORE_ARI,
-	 BNXT_NVM_SHARED_CFG, 1},
+	 BNXT_NVM_SHARED_CFG, 1, 1},
 	{DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
-	 NVM_OFF_MSIX_VEC_PER_PF_MAX, BNXT_NVM_SHARED_CFG, 10},
+	 NVM_OFF_MSIX_VEC_PER_PF_MAX, BNXT_NVM_SHARED_CFG, 10, 4},
 	{DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
-	 NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7},
+	 NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7, 4},
 	{BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK, NVM_OFF_DIS_GRE_VER_CHECK,
-	 BNXT_NVM_SHARED_CFG, 1},
+	 BNXT_NVM_SHARED_CFG, 1, 1},
 };
 
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
@@ -232,8 +232,8 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 	struct hwrm_nvm_get_variable_input *req = msg;
 	void *data_addr = NULL, *buf = NULL;
 	struct bnxt_dl_nvm_param nvm_param;
-	int bytesize, idx = 0, rc, i;
 	dma_addr_t data_dma_addr;
+	int idx = 0, rc, i;
 
 	/* Get/Set NVM CFG parameter is supported only on PFs */
 	if (BNXT_VF(bp))
@@ -254,10 +254,9 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 	else if (nvm_param.dir_type == BNXT_NVM_FUNC_CFG)
 		idx = bp->pf.fw_fid - BNXT_FIRST_PF_FID;
 
-	bytesize = roundup(nvm_param.num_bits, BITS_PER_BYTE) / BITS_PER_BYTE;
-	switch (bytesize) {
+	switch (nvm_param.dl_num_bytes) {
 	case 1:
-		if (nvm_param.num_bits == 1)
+		if (nvm_param.nvm_num_bits == 1)
 			buf = &val->vbool;
 		else
 			buf = &val->vu8;
@@ -272,29 +271,30 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 		return -EFAULT;
 	}
 
-	data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+	data_addr = dma_alloc_coherent(&bp->pdev->dev, nvm_param.dl_num_bytes,
 				       &data_dma_addr, GFP_KERNEL);
 	if (!data_addr)
 		return -ENOMEM;
 
 	req->dest_data_addr = cpu_to_le64(data_dma_addr);
-	req->data_len = cpu_to_le16(nvm_param.num_bits);
+	req->data_len = cpu_to_le16(nvm_param.nvm_num_bits);
 	req->option_num = cpu_to_le16(nvm_param.offset);
 	req->index_0 = cpu_to_le16(idx);
 	if (idx)
 		req->dimensions = cpu_to_le16(1);
 
 	if (req->req_type == cpu_to_le16(HWRM_NVM_SET_VARIABLE)) {
-		memcpy(data_addr, buf, bytesize);
+		memcpy(data_addr, buf, nvm_param.dl_num_bytes);
 		rc = hwrm_send_message(bp, msg, msg_len, HWRM_CMD_TIMEOUT);
 	} else {
 		rc = hwrm_send_message_silent(bp, msg, msg_len,
 					      HWRM_CMD_TIMEOUT);
 	}
 	if (!rc && req->req_type == cpu_to_le16(HWRM_NVM_GET_VARIABLE))
-		memcpy(buf, data_addr, bytesize);
+		memcpy(buf, data_addr, nvm_param.dl_num_bytes);
 
-	dma_free_coherent(&bp->pdev->dev, bytesize, data_addr, data_dma_addr);
+	dma_free_coherent(&bp->pdev->dev, nvm_param.dl_num_bytes, data_addr,
+			  data_dma_addr);
 	if (rc == -EACCES)
 		netdev_err(bp->dev, "PF does not have admin privileges to modify NVM config\n");
 	return rc;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index b97e0ba..2f4fd0a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -52,7 +52,8 @@ struct bnxt_dl_nvm_param {
 	u16 id;
 	u16 offset;
 	u16 dir_type;
-	u16 num_bits;
+	u16 nvm_num_bits;
+	u8 dl_num_bytes;
 };
 
 void bnxt_devlink_health_report(struct bnxt *bp, unsigned long event);
-- 
2.5.1


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

* [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues.
  2019-10-21  5:34 [PATCH net 0/5] bnxt_en: Bug fixes Michael Chan
  2019-10-21  5:34 ` [PATCH net 1/5] bnxt_en: Fix the size of devlink MSIX parameters Michael Chan
@ 2019-10-21  5:34 ` Michael Chan
  2019-10-22  4:14   ` Jakub Kicinski
  2019-10-21  5:34 ` [PATCH net 3/5] bnxt_en: Adjust the time to wait before polling firmware readiness Michael Chan
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Michael Chan @ 2019-10-21  5:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, vasundhara-v.volam, Jiri Pirko

The current code does not do endian swapping between the devlink
parameter and the internal NVRAM representation.  Define a union to
represent the little endian NVRAM data and add 2 helper functions to
copy to and from the NVRAM data with the proper byte swapping.

Fixes: 782a624d00fa ("bnxt_en: Add bnxt_en initial port params table and register it")
Cc: Jiri Pirko <jiri@mellanox.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_devlink.c | 81 +++++++++++++++--------
 1 file changed, 54 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 68f74f5..bd4b9f3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -226,12 +226,55 @@ static const struct bnxt_dl_nvm_param nvm_params[] = {
 	 BNXT_NVM_SHARED_CFG, 1, 1},
 };
 
+union bnxt_nvm_data {
+	u8	val8;
+	__le32	val32;
+};
+
+static void bnxt_copy_to_nvm_data(union bnxt_nvm_data *dst,
+				  union devlink_param_value *src,
+				  int nvm_num_bits, int dl_num_bytes)
+{
+	u32 val32 = 0;
+
+	if (nvm_num_bits == 1) {
+		dst->val8 = src->vbool;
+		return;
+	}
+	if (dl_num_bytes == 4)
+		val32 = src->vu32;
+	else if (dl_num_bytes == 2)
+		val32 = (u32)src->vu16;
+	else if (dl_num_bytes == 1)
+		val32 = (u32)src->vu8;
+	dst->val32 = cpu_to_le32(val32);
+}
+
+static void bnxt_copy_from_nvm_data(union devlink_param_value *dst,
+				    union bnxt_nvm_data *src,
+				    int nvm_num_bits, int dl_num_bytes)
+{
+	u32 val32;
+
+	if (nvm_num_bits == 1) {
+		dst->vbool = src->val8;
+		return;
+	}
+	val32 = le32_to_cpu(src->val32);
+	if (dl_num_bytes == 4)
+		dst->vu32 = val32;
+	else if (dl_num_bytes == 2)
+		dst->vu16 = (u16)val32;
+	else if (dl_num_bytes == 1)
+		dst->vu8 = (u8)val32;
+}
+
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 			     int msg_len, union devlink_param_value *val)
 {
 	struct hwrm_nvm_get_variable_input *req = msg;
-	void *data_addr = NULL, *buf = NULL;
 	struct bnxt_dl_nvm_param nvm_param;
+	union bnxt_nvm_data *data;
 	dma_addr_t data_dma_addr;
 	int idx = 0, rc, i;
 
@@ -254,26 +297,9 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 	else if (nvm_param.dir_type == BNXT_NVM_FUNC_CFG)
 		idx = bp->pf.fw_fid - BNXT_FIRST_PF_FID;
 
-	switch (nvm_param.dl_num_bytes) {
-	case 1:
-		if (nvm_param.nvm_num_bits == 1)
-			buf = &val->vbool;
-		else
-			buf = &val->vu8;
-		break;
-	case 2:
-		buf = &val->vu16;
-		break;
-	case 4:
-		buf = &val->vu32;
-		break;
-	default:
-		return -EFAULT;
-	}
-
-	data_addr = dma_alloc_coherent(&bp->pdev->dev, nvm_param.dl_num_bytes,
-				       &data_dma_addr, GFP_KERNEL);
-	if (!data_addr)
+	data = dma_alloc_coherent(&bp->pdev->dev, sizeof(*data),
+				  &data_dma_addr, GFP_KERNEL);
+	if (!data)
 		return -ENOMEM;
 
 	req->dest_data_addr = cpu_to_le64(data_dma_addr);
@@ -284,17 +310,18 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 		req->dimensions = cpu_to_le16(1);
 
 	if (req->req_type == cpu_to_le16(HWRM_NVM_SET_VARIABLE)) {
-		memcpy(data_addr, buf, nvm_param.dl_num_bytes);
+		bnxt_copy_to_nvm_data(data, val, nvm_param.nvm_num_bits,
+				      nvm_param.dl_num_bytes);
 		rc = hwrm_send_message(bp, msg, msg_len, HWRM_CMD_TIMEOUT);
 	} else {
 		rc = hwrm_send_message_silent(bp, msg, msg_len,
 					      HWRM_CMD_TIMEOUT);
+		if (!rc)
+			bnxt_copy_from_nvm_data(val, data,
+						nvm_param.nvm_num_bits,
+						nvm_param.dl_num_bytes);
 	}
-	if (!rc && req->req_type == cpu_to_le16(HWRM_NVM_GET_VARIABLE))
-		memcpy(buf, data_addr, nvm_param.dl_num_bytes);
-
-	dma_free_coherent(&bp->pdev->dev, nvm_param.dl_num_bytes, data_addr,
-			  data_dma_addr);
+	dma_free_coherent(&bp->pdev->dev, sizeof(*data), data, data_dma_addr);
 	if (rc == -EACCES)
 		netdev_err(bp->dev, "PF does not have admin privileges to modify NVM config\n");
 	return rc;
-- 
2.5.1


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

* [PATCH net 3/5] bnxt_en: Adjust the time to wait before polling firmware readiness.
  2019-10-21  5:34 [PATCH net 0/5] bnxt_en: Bug fixes Michael Chan
  2019-10-21  5:34 ` [PATCH net 1/5] bnxt_en: Fix the size of devlink MSIX parameters Michael Chan
  2019-10-21  5:34 ` [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues Michael Chan
@ 2019-10-21  5:34 ` Michael Chan
  2019-10-21  5:34 ` [PATCH net 4/5] bnxt_en: Minor formatting changes in FW devlink_health_reporter Michael Chan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Michael Chan @ 2019-10-21  5:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, vasundhara-v.volam

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

When firmware indicates that driver needs to invoke firmware reset
which is common for both error recovery and live firmware reset path,
driver needs a different time to wait before polling for firmware
readiness.

Modify the wait time to fw_reset_min_dsecs, which is initialised to
correct timeout for error recovery and firmware reset.

Fixes: 4037eb715680 ("bnxt_en: Add a new BNXT_FW_RESET_STATE_POLL_FW_DOWN state.")
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 | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index b4a8cf6..8492618 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10669,14 +10669,11 @@ static void bnxt_fw_reset_task(struct work_struct *work)
 		bp->fw_reset_state = BNXT_FW_RESET_STATE_RESET_FW;
 	}
 	/* fall through */
-	case BNXT_FW_RESET_STATE_RESET_FW: {
-		u32 wait_dsecs = bp->fw_health->post_reset_wait_dsecs;
-
+	case BNXT_FW_RESET_STATE_RESET_FW:
 		bnxt_reset_all(bp);
 		bp->fw_reset_state = BNXT_FW_RESET_STATE_ENABLE_DEV;
-		bnxt_queue_fw_reset_work(bp, wait_dsecs * HZ / 10);
+		bnxt_queue_fw_reset_work(bp, bp->fw_reset_min_dsecs * HZ / 10);
 		return;
-	}
 	case BNXT_FW_RESET_STATE_ENABLE_DEV:
 		if (test_bit(BNXT_STATE_FW_FATAL_COND, &bp->state) &&
 		    bp->fw_health) {
-- 
2.5.1


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

* [PATCH net 4/5] bnxt_en: Minor formatting changes in FW devlink_health_reporter
  2019-10-21  5:34 [PATCH net 0/5] bnxt_en: Bug fixes Michael Chan
                   ` (2 preceding siblings ...)
  2019-10-21  5:34 ` [PATCH net 3/5] bnxt_en: Adjust the time to wait before polling firmware readiness Michael Chan
@ 2019-10-21  5:34 ` Michael Chan
  2019-10-21  5:34 ` [PATCH net 5/5] bnxt_en: Avoid disabling pci device in bnxt_remove_one() for already disabled device Michael Chan
  2019-10-22 20:29 ` [PATCH net 0/5] bnxt_en: Bug fixes Jakub Kicinski
  5 siblings, 0 replies; 9+ messages in thread
From: Michael Chan @ 2019-10-21  5:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, vasundhara-v.volam, Jiri Pirko

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

Minor formatting changes to diagnose cb for FW devlink health
reporter.

Suggested-by: Jiri Pirko <jiri@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.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_devlink.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index bd4b9f3..7151244 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -29,25 +29,20 @@ static int bnxt_fw_reporter_diagnose(struct devlink_health_reporter *reporter,
 	val = bnxt_fw_health_readl(bp, BNXT_FW_HEALTH_REG);
 	health_status = val & 0xffff;
 
-	if (health_status == BNXT_FW_STATUS_HEALTHY) {
-		rc = devlink_fmsg_string_pair_put(fmsg, "FW status",
-						  "Healthy;");
-		if (rc)
-			return rc;
-	} else if (health_status < BNXT_FW_STATUS_HEALTHY) {
-		rc = devlink_fmsg_string_pair_put(fmsg, "FW status",
-						  "Not yet completed initialization;");
+	if (health_status < BNXT_FW_STATUS_HEALTHY) {
+		rc = devlink_fmsg_string_pair_put(fmsg, "Description",
+						  "Not yet completed initialization");
 		if (rc)
 			return rc;
 	} else if (health_status > BNXT_FW_STATUS_HEALTHY) {
-		rc = devlink_fmsg_string_pair_put(fmsg, "FW status",
-						  "Encountered fatal error and cannot recover;");
+		rc = devlink_fmsg_string_pair_put(fmsg, "Description",
+						  "Encountered fatal error and cannot recover");
 		if (rc)
 			return rc;
 	}
 
 	if (val >> 16) {
-		rc = devlink_fmsg_u32_pair_put(fmsg, "Error", val >> 16);
+		rc = devlink_fmsg_u32_pair_put(fmsg, "Error code", val >> 16);
 		if (rc)
 			return rc;
 	}
-- 
2.5.1


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

* [PATCH net 5/5] bnxt_en: Avoid disabling pci device in bnxt_remove_one() for already disabled device.
  2019-10-21  5:34 [PATCH net 0/5] bnxt_en: Bug fixes Michael Chan
                   ` (3 preceding siblings ...)
  2019-10-21  5:34 ` [PATCH net 4/5] bnxt_en: Minor formatting changes in FW devlink_health_reporter Michael Chan
@ 2019-10-21  5:34 ` Michael Chan
  2019-10-22 20:29 ` [PATCH net 0/5] bnxt_en: Bug fixes Jakub Kicinski
  5 siblings, 0 replies; 9+ messages in thread
From: Michael Chan @ 2019-10-21  5:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, vasundhara-v.volam

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

With the recently added error recovery logic, the device may already
be disabled if the firmware recovery is unsuccessful.  In
bnxt_remove_one(), check that the device is still enabled first
before calling pci_disable_device().

Fixes: 3bc7d4a352ef ("bnxt_en: Add BNXT_STATE_IN_FW_RESET state.")
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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8492618..04ec909 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10382,7 +10382,8 @@ static void bnxt_cleanup_pci(struct bnxt *bp)
 {
 	bnxt_unmap_bars(bp, bp->pdev);
 	pci_release_regions(bp->pdev);
-	pci_disable_device(bp->pdev);
+	if (pci_is_enabled(bp->pdev))
+		pci_disable_device(bp->pdev);
 }
 
 static void bnxt_init_dflt_coal(struct bnxt *bp)
-- 
2.5.1


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

* Re: [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues.
  2019-10-21  5:34 ` [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues Michael Chan
@ 2019-10-22  4:14   ` Jakub Kicinski
  2019-10-22  5:38     ` Michael Chan
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2019-10-22  4:14 UTC (permalink / raw)
  To: Michael Chan; +Cc: davem, netdev, vasundhara-v.volam, Jiri Pirko

On Mon, 21 Oct 2019 01:34:26 -0400, Michael Chan wrote:
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> index 68f74f5..bd4b9f3 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> @@ -226,12 +226,55 @@ static const struct bnxt_dl_nvm_param nvm_params[] = {
>  	 BNXT_NVM_SHARED_CFG, 1, 1},
>  };
>  
> +union bnxt_nvm_data {
> +	u8	val8;
> +	__le32	val32;
> +};
> +
> +static void bnxt_copy_to_nvm_data(union bnxt_nvm_data *dst,
> +				  union devlink_param_value *src,
> +				  int nvm_num_bits, int dl_num_bytes)
> +{
> +	u32 val32 = 0;
> +
> +	if (nvm_num_bits == 1) {
> +		dst->val8 = src->vbool;
> +		return;
> +	}

Why do you special case the num_bits == 1? If val32 is __le32 the low
byte would have landed on the first byte anyway, no? 🤔

just curious

> +	if (dl_num_bytes == 4)
> +		val32 = src->vu32;
> +	else if (dl_num_bytes == 2)
> +		val32 = (u32)src->vu16;
> +	else if (dl_num_bytes == 1)
> +		val32 = (u32)src->vu8;
> +	dst->val32 = cpu_to_le32(val32);
> +}

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

* Re: [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues.
  2019-10-22  4:14   ` Jakub Kicinski
@ 2019-10-22  5:38     ` Michael Chan
  0 siblings, 0 replies; 9+ messages in thread
From: Michael Chan @ 2019-10-22  5:38 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: David Miller, Netdev, Vasundhara Volam, Jiri Pirko

On Mon, Oct 21, 2019 at 9:14 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Mon, 21 Oct 2019 01:34:26 -0400, Michael Chan wrote:
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > index 68f74f5..bd4b9f3 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
> > @@ -226,12 +226,55 @@ static const struct bnxt_dl_nvm_param nvm_params[] = {
> >        BNXT_NVM_SHARED_CFG, 1, 1},
> >  };
> >
> > +union bnxt_nvm_data {
> > +     u8      val8;
> > +     __le32  val32;
> > +};
> > +
> > +static void bnxt_copy_to_nvm_data(union bnxt_nvm_data *dst,
> > +                               union devlink_param_value *src,
> > +                               int nvm_num_bits, int dl_num_bytes)
> > +{
> > +     u32 val32 = 0;
> > +
> > +     if (nvm_num_bits == 1) {
> > +             dst->val8 = src->vbool;
> > +             return;
> > +     }
>
> Why do you special case the num_bits == 1? If val32 is __le32 the low
> byte would have landed on the first byte anyway, no?
>
> just curious

Just so that I don't have to do any casting.  Otherwise if I assign it
to the __le32, I believe I have to cast to avoid the warning.

>
> > +     if (dl_num_bytes == 4)
> > +             val32 = src->vu32;
> > +     else if (dl_num_bytes == 2)
> > +             val32 = (u32)src->vu16;
> > +     else if (dl_num_bytes == 1)
> > +             val32 = (u32)src->vu8;
> > +     dst->val32 = cpu_to_le32(val32);
> > +}

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

* Re: [PATCH net 0/5] bnxt_en: Bug fixes.
  2019-10-21  5:34 [PATCH net 0/5] bnxt_en: Bug fixes Michael Chan
                   ` (4 preceding siblings ...)
  2019-10-21  5:34 ` [PATCH net 5/5] bnxt_en: Avoid disabling pci device in bnxt_remove_one() for already disabled device Michael Chan
@ 2019-10-22 20:29 ` Jakub Kicinski
  5 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2019-10-22 20:29 UTC (permalink / raw)
  To: Michael Chan; +Cc: davem, netdev, vasundhara-v.volam

On Mon, 21 Oct 2019 01:34:24 -0400, Michael Chan wrote:
> Devlink and error recovery bug fix patches.  Most of the work is by
> Vasundhara Volam.  

Thanks, applied.

> Please queue patch 1 and 2 for -stable also.  Thanks.

FWIW these will likely only reach 5.3 since it looks like the bug dates
to 5.1 but 5.1 and 5.2 branches of stable are already EOL.

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

end of thread, other threads:[~2019-10-22 20:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-21  5:34 [PATCH net 0/5] bnxt_en: Bug fixes Michael Chan
2019-10-21  5:34 ` [PATCH net 1/5] bnxt_en: Fix the size of devlink MSIX parameters Michael Chan
2019-10-21  5:34 ` [PATCH net 2/5] bnxt_en: Fix devlink NVRAM related byte order related issues Michael Chan
2019-10-22  4:14   ` Jakub Kicinski
2019-10-22  5:38     ` Michael Chan
2019-10-21  5:34 ` [PATCH net 3/5] bnxt_en: Adjust the time to wait before polling firmware readiness Michael Chan
2019-10-21  5:34 ` [PATCH net 4/5] bnxt_en: Minor formatting changes in FW devlink_health_reporter Michael Chan
2019-10-21  5:34 ` [PATCH net 5/5] bnxt_en: Avoid disabling pci device in bnxt_remove_one() for already disabled device Michael Chan
2019-10-22 20:29 ` [PATCH net 0/5] bnxt_en: Bug fixes Jakub Kicinski

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).