All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.