[PATCH net-next 0/4] qed: Devlink support for config attributes management.

[PATCH net-next 0/4] qed: Devlink support for config attributes management.

[Sudarsana Reddy Kalluru]

The patch series adds support for managing the config attributes using
devlink interfaces.

Please consider applying it to 'net-next' tree.

Sudarsana Reddy Kalluru (4):
  qed: Add APIs for device attributes configuration.
  qed: Perform devlink registration after the hardware init.
  qed: Add new file for devlink implementation.
  qed: Add devlink support for configuration attributes.

 drivers/net/ethernet/qlogic/qed/Makefile      |   3 +-
 drivers/net/ethernet/qlogic/qed/qed.h         |   1 +
 drivers/net/ethernet/qlogic/qed/qed_devlink.c | 281 ++++++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_devlink.h |  41 ++++
 drivers/net/ethernet/qlogic/qed/qed_hsi.h     |  31 +++
 drivers/net/ethernet/qlogic/qed/qed_main.c    | 118 +----------
 drivers/net/ethernet/qlogic/qed/qed_mcp.c     |  64 ++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h     |  14 ++
 8 files changed, 444 insertions(+), 109 deletions(-)
 create mode 100644 drivers/net/ethernet/qlogic/qed/qed_devlink.c
 create mode 100644 drivers/net/ethernet/qlogic/qed/qed_devlink.h

-- 
1.8.3.1

[PATCH net-next 1/4] qed: Add APIs for device attributes configuration.

[Sudarsana Reddy Kalluru]

The patch adds APIs for reading/configuring the device attributes using
mailbox interfaces.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_hsi.h | 20 ++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 64 +++++++++++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h | 14 +++++++
 3 files changed, 98 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
index e054f6c..5091f5b1 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
@@ -12580,6 +12580,8 @@ struct public_drv_mb {
 #define DRV_MSG_CODE_BW_UPDATE_ACK		0x32000000
 #define DRV_MSG_CODE_NIG_DRAIN			0x30000000
 #define DRV_MSG_CODE_S_TAG_UPDATE_ACK		0x3b000000
+#define DRV_MSG_CODE_GET_NVM_CFG_OPTION		0x003e0000
+#define DRV_MSG_CODE_SET_NVM_CFG_OPTION		0x003f0000
 #define DRV_MSG_CODE_INITIATE_PF_FLR            0x02010000
 #define DRV_MSG_CODE_VF_DISABLED_DONE		0xc0000000
 #define DRV_MSG_CODE_CFG_VF_MSIX		0xc0010000
@@ -12748,6 +12750,21 @@ struct public_drv_mb {
 #define DRV_MB_PARAM_FEATURE_SUPPORT_PORT_EEE		0x00000002
 #define DRV_MB_PARAM_FEATURE_SUPPORT_FUNC_VLINK		0x00010000
 
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_SHIFT		0
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_MASK		0x0000FFFF
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_SHIFT		16
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_MASK		0x00010000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_SHIFT		17
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_MASK		0x00020000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_SHIFT	18
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_MASK		0x00040000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_SHIFT		19
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_MASK		0x00080000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_SHIFT	20
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_MASK	0x00100000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_SHIFT	24
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_MASK	0x0f000000
+
 	u32 fw_mb_header;
 #define FW_MSG_CODE_MASK			0xffff0000
 #define FW_MSG_CODE_UNSUPPORTED                 0x00000000
@@ -13319,6 +13336,9 @@ enum spad_sections {
 	SPAD_SECTION_MAX
 };
 
+#define NVM_CFG_ID_MAC_ADDRESS			1
+#define NVM_CFG_ID_MF_MODE			9
+
 #define MCP_TRACE_SIZE          2048	/* 2kb */
 
 /* This section is located at a fixed location in the beginning of the
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 758702c..573911a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -3750,3 +3750,67 @@ int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 
 	return 0;
 }
+
+int qed_mcp_nvm_get_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 *p_len)
+{
+	u32 mb_param = 0, resp, param;
+	int rc;
+
+	QED_MFW_SET_FIELD(mb_param, DRV_MB_PARAM_NVM_CFG_OPTION_ID, option_id);
+	if (flags & QED_NVM_CFG_OPTION_INIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_INIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_FREE)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_FREE, 1);
+	if (flags & QED_NVM_CFG_OPTION_ENTITY_SEL) {
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL, 1);
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID,
+				  entity_id);
+	}
+
+	rc = qed_mcp_nvm_rd_cmd(p_hwfn, p_ptt,
+				DRV_MSG_CODE_GET_NVM_CFG_OPTION,
+				mb_param, &resp, &param, p_len, (u32 *)p_buf);
+
+	return rc;
+}
+
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 len)
+{
+	u32 mb_param = 0, resp, param;
+	int rc;
+
+	QED_MFW_SET_FIELD(mb_param, DRV_MB_PARAM_NVM_CFG_OPTION_ID, option_id);
+	if (flags & QED_NVM_CFG_OPTION_ALL)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ALL, 1);
+	if (flags & QED_NVM_CFG_OPTION_INIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_INIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_COMMIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_FREE)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_FREE, 1);
+	if (flags & QED_NVM_CFG_OPTION_ENTITY_SEL) {
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL, 1);
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID,
+				  entity_id);
+	}
+
+	rc = qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt,
+				DRV_MSG_CODE_SET_NVM_CFG_OPTION,
+				mb_param, &resp, &param, len, (u32 *)p_buf);
+
+	return rc;
+}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index e4f8fe4..550b4dd 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -251,6 +251,12 @@ struct qed_mfw_tlv_generic {
 	struct qed_mfw_tlv_iscsi iscsi;
 };
 
+#define QED_NVM_CFG_OPTION_ALL		BIT(0)
+#define QED_NVM_CFG_OPTION_INIT		BIT(1)
+#define QED_NVM_CFG_OPTION_COMMIT       BIT(2)
+#define QED_NVM_CFG_OPTION_FREE		BIT(3)
+#define QED_NVM_CFG_OPTION_ENTITY_SEL	BIT(4)
+
 /**
  * @brief - returns the link params of the hw function
  *
@@ -1202,4 +1208,12 @@ void qed_mcp_resc_lock_default_init(struct qed_resc_lock_params *p_lock,
  */
 int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
 
+int qed_mcp_nvm_get_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 *p_len);
+
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 len);
+
 #endif
-- 
1.8.3.1

[PATCH net-next 2/4] qed: Perform devlink registration after the hardware init.

[Sudarsana Reddy Kalluru]

Devlink callbacks need access to device resources such as ptt lock, hence
performing the devlink registration after the device initialization.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_main.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 829dd60..fdd84f5 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -472,22 +472,24 @@ static struct qed_dev *qed_probe(struct pci_dev *pdev,
 	}
 	DP_INFO(cdev, "PCI init completed successfully\n");
 
-	rc = qed_devlink_register(cdev);
+	rc = qed_hw_prepare(cdev, QED_PCI_DEFAULT);
 	if (rc) {
-		DP_INFO(cdev, "Failed to register devlink.\n");
+		DP_ERR(cdev, "hw prepare failed\n");
 		goto err2;
 	}
 
-	rc = qed_hw_prepare(cdev, QED_PCI_DEFAULT);
+	rc = qed_devlink_register(cdev);
 	if (rc) {
-		DP_ERR(cdev, "hw prepare failed\n");
-		goto err2;
+		DP_INFO(cdev, "Failed to register devlink.\n");
+		goto err3;
 	}
 
 	DP_INFO(cdev, "qed_probe completed successfully\n");
 
 	return cdev;
 
+err3:
+	qed_hw_remove(cdev);
 err2:
 	qed_free_pci(cdev);
 err1:
@@ -501,14 +503,14 @@ static void qed_remove(struct qed_dev *cdev)
 	if (!cdev)
 		return;
 
+	qed_devlink_unregister(cdev);
+
 	qed_hw_remove(cdev);
 
 	qed_free_pci(cdev);
 
 	qed_set_power_state(cdev, PCI_D3hot);
 
-	qed_devlink_unregister(cdev);
-
 	qed_free_cdev(cdev);
 }
 
-- 
1.8.3.1

[PATCH net-next 3/4] qed: Add new file for devlink implementation.

[Sudarsana Reddy Kalluru]

The patch introduces new files for qed devlink implementation.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/Makefile      |   3 +-
 drivers/net/ethernet/qlogic/qed/qed_devlink.c |  97 ++++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_devlink.h |  18 +++++
 drivers/net/ethernet/qlogic/qed/qed_main.c    | 102 +-------------------------
 4 files changed, 118 insertions(+), 102 deletions(-)
 create mode 100644 drivers/net/ethernet/qlogic/qed/qed_devlink.c
 create mode 100644 drivers/net/ethernet/qlogic/qed/qed_devlink.h

diff --git a/drivers/net/ethernet/qlogic/qed/Makefile b/drivers/net/ethernet/qlogic/qed/Makefile
index a0acb94..2e9f3cc 100644
--- a/drivers/net/ethernet/qlogic/qed/Makefile
+++ b/drivers/net/ethernet/qlogic/qed/Makefile
@@ -3,7 +3,8 @@ obj-$(CONFIG_QED) := qed.o
 
 qed-y := qed_cxt.o qed_dev.o qed_hw.o qed_init_fw_funcs.o qed_init_ops.o \
 	 qed_int.o qed_main.o qed_mcp.o qed_sp_commands.o qed_spq.o qed_l2.o \
-	 qed_selftest.o qed_dcbx.o qed_debug.o qed_ptp.o qed_mng_tlv.o
+	 qed_selftest.o qed_dcbx.o qed_debug.o qed_ptp.o qed_mng_tlv.o \
+	 qed_devlink.o
 qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o
 qed-$(CONFIG_QED_LL2) += qed_ll2.o
 qed-$(CONFIG_QED_RDMA) += qed_roce.o qed_rdma.o qed_iwarp.o
diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
new file mode 100644
index 0000000..acb6c87
--- /dev/null
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <net/devlink.h>
+#include "qed.h"
+#include "qed_devlink.h"
+#include "qed_mcp.h"
+
+static int qed_dl_param_get(struct devlink *dl, u32 id,
+			    struct devlink_param_gset_ctx *ctx)
+{
+	struct qed_devlink *qed_dl;
+	struct qed_dev *cdev;
+
+	qed_dl = devlink_priv(dl);
+	cdev = qed_dl->cdev;
+	ctx->val.vbool = cdev->iwarp_cmt;
+
+	return 0;
+}
+
+static int qed_dl_param_set(struct devlink *dl, u32 id,
+			    struct devlink_param_gset_ctx *ctx)
+{
+	struct qed_devlink *qed_dl;
+	struct qed_dev *cdev;
+
+	qed_dl = devlink_priv(dl);
+	cdev = qed_dl->cdev;
+	cdev->iwarp_cmt = ctx->val.vbool;
+
+	return 0;
+}
+
+static const struct devlink_param qed_devlink_params[] = {
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PARAM_ID_IWARP_CMT,
+			     "iwarp_cmt", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     qed_dl_param_get, qed_dl_param_set, NULL),
+};
+
+static const struct devlink_ops qed_dl_ops;
+
+int qed_devlink_register(struct qed_dev *cdev)
+{
+	union devlink_param_value value;
+	struct qed_devlink *qed_dl;
+	struct devlink *dl;
+	int rc;
+
+	dl = devlink_alloc(&qed_dl_ops, sizeof(*qed_dl));
+	if (!dl)
+		return -ENOMEM;
+
+	qed_dl = devlink_priv(dl);
+
+	cdev->dl = dl;
+	qed_dl->cdev = cdev;
+
+	rc = devlink_register(dl, &cdev->pdev->dev);
+	if (rc)
+		goto err_free;
+
+	rc = devlink_params_register(dl, qed_devlink_params,
+				     ARRAY_SIZE(qed_devlink_params));
+	if (rc)
+		goto err_unregister;
+
+	value.vbool = false;
+	devlink_param_driverinit_value_set(dl,
+					   QED_DEVLINK_PARAM_ID_IWARP_CMT,
+					   value);
+
+	devlink_params_publish(dl);
+	cdev->iwarp_cmt = false;
+
+	return 0;
+
+err_unregister:
+	devlink_unregister(dl);
+
+err_free:
+	cdev->dl = NULL;
+	devlink_free(dl);
+
+	return rc;
+}
+
+void qed_devlink_unregister(struct qed_dev *cdev)
+{
+	if (!cdev->dl)
+		return;
+
+	devlink_params_unregister(cdev->dl, qed_devlink_params,
+				  ARRAY_SIZE(qed_devlink_params));
+
+	devlink_unregister(cdev->dl);
+	devlink_free(cdev->dl);
+}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.h b/drivers/net/ethernet/qlogic/qed/qed_devlink.h
new file mode 100644
index 0000000..86e1caa
--- /dev/null
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _QED_DEVLINK_H
+#define _QED_DEVLINK_H
+#include "qed.h"
+
+struct qed_devlink {
+	struct qed_dev *cdev;
+};
+
+enum qed_devlink_param_id {
+	QED_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	QED_DEVLINK_PARAM_ID_IWARP_CMT,
+};
+
+int qed_devlink_register(struct qed_dev *cdev);
+void qed_devlink_unregister(struct qed_dev *cdev);
+
+#endif
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index fdd84f5..e3a8754 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -63,6 +63,7 @@
 #include "qed_hw.h"
 #include "qed_selftest.h"
 #include "qed_debug.h"
+#include "qed_devlink.h"
 
 #define QED_ROCE_QPS			(8192)
 #define QED_ROCE_DPIS			(8)
@@ -343,107 +344,6 @@ static int qed_set_power_state(struct qed_dev *cdev, pci_power_t state)
 	return 0;
 }
 
-struct qed_devlink {
-	struct qed_dev *cdev;
-};
-
-enum qed_devlink_param_id {
-	QED_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
-	QED_DEVLINK_PARAM_ID_IWARP_CMT,
-};
-
-static int qed_dl_param_get(struct devlink *dl, u32 id,
-			    struct devlink_param_gset_ctx *ctx)
-{
-	struct qed_devlink *qed_dl;
-	struct qed_dev *cdev;
-
-	qed_dl = devlink_priv(dl);
-	cdev = qed_dl->cdev;
-	ctx->val.vbool = cdev->iwarp_cmt;
-
-	return 0;
-}
-
-static int qed_dl_param_set(struct devlink *dl, u32 id,
-			    struct devlink_param_gset_ctx *ctx)
-{
-	struct qed_devlink *qed_dl;
-	struct qed_dev *cdev;
-
-	qed_dl = devlink_priv(dl);
-	cdev = qed_dl->cdev;
-	cdev->iwarp_cmt = ctx->val.vbool;
-
-	return 0;
-}
-
-static const struct devlink_param qed_devlink_params[] = {
-	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PARAM_ID_IWARP_CMT,
-			     "iwarp_cmt", DEVLINK_PARAM_TYPE_BOOL,
-			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
-			     qed_dl_param_get, qed_dl_param_set, NULL),
-};
-
-static const struct devlink_ops qed_dl_ops;
-
-static int qed_devlink_register(struct qed_dev *cdev)
-{
-	union devlink_param_value value;
-	struct qed_devlink *qed_dl;
-	struct devlink *dl;
-	int rc;
-
-	dl = devlink_alloc(&qed_dl_ops, sizeof(*qed_dl));
-	if (!dl)
-		return -ENOMEM;
-
-	qed_dl = devlink_priv(dl);
-
-	cdev->dl = dl;
-	qed_dl->cdev = cdev;
-
-	rc = devlink_register(dl, &cdev->pdev->dev);
-	if (rc)
-		goto err_free;
-
-	rc = devlink_params_register(dl, qed_devlink_params,
-				     ARRAY_SIZE(qed_devlink_params));
-	if (rc)
-		goto err_unregister;
-
-	value.vbool = false;
-	devlink_param_driverinit_value_set(dl,
-					   QED_DEVLINK_PARAM_ID_IWARP_CMT,
-					   value);
-
-	devlink_params_publish(dl);
-	cdev->iwarp_cmt = false;
-
-	return 0;
-
-err_unregister:
-	devlink_unregister(dl);
-
-err_free:
-	cdev->dl = NULL;
-	devlink_free(dl);
-
-	return rc;
-}
-
-static void qed_devlink_unregister(struct qed_dev *cdev)
-{
-	if (!cdev->dl)
-		return;
-
-	devlink_params_unregister(cdev->dl, qed_devlink_params,
-				  ARRAY_SIZE(qed_devlink_params));
-
-	devlink_unregister(cdev->dl);
-	devlink_free(cdev->dl);
-}
-
 /* probing */
 static struct qed_dev *qed_probe(struct pci_dev *pdev,
 				 struct qed_probe_params *params)
-- 
1.8.3.1

[PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Sudarsana Reddy Kalluru]

This patch adds implementation for devlink callbacks for reading/
configuring the device attributes.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed.h         |   1 +
 drivers/net/ethernet/qlogic/qed/qed_devlink.c | 184 ++++++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_devlink.h |  23 ++++
 drivers/net/ethernet/qlogic/qed/qed_hsi.h     |  11 ++
 4 files changed, 219 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h
index 89fe091..2afd5c7 100644
--- a/drivers/net/ethernet/qlogic/qed/qed.h
+++ b/drivers/net/ethernet/qlogic/qed/qed.h
@@ -866,6 +866,7 @@ struct qed_dev {
 
 	struct devlink			*dl;
 	bool				iwarp_cmt;
+	u8				cfg_entity_id;
 };
 
 #define NUM_OF_VFS(dev)         (QED_IS_BB(dev) ? MAX_NUM_VFS_BB \
diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
index acb6c87..232a8c4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
@@ -4,6 +4,30 @@
 #include "qed_devlink.h"
 #include "qed_mcp.h"
 
+static const struct qed_devlink_cfg_param cfg_params[] = {
+	{DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, NVM_CFG_ID_ENABLE_SRIOV,
+	 DEVLINK_PARAM_TYPE_BOOL},
+	{QED_DEVLINK_ENTITY_ID, 0, DEVLINK_PARAM_TYPE_U8},
+	{QED_DEVLINK_DEVICE_CAPABILITIES, NVM_CFG_ID_DEVICE_CAPABILITIES,
+	 DEVLINK_PARAM_TYPE_U8},
+	{QED_DEVLINK_MF_MODE, NVM_CFG_ID_MF_MODE, DEVLINK_PARAM_TYPE_U8},
+	{QED_DEVLINK_DCBX_MODE, NVM_CFG_ID_DCBX_MODE, DEVLINK_PARAM_TYPE_U8},
+	{QED_DEVLINK_PREBOOT_OPROM, NVM_CFG_ID_PREBOOT_OPROM,
+	 DEVLINK_PARAM_TYPE_BOOL},
+	{QED_DEVLINK_PREBOOT_BOOT_PROTOCOL, NVM_CFG_ID_PREBOOT_BOOT_PROTOCOL,
+	 DEVLINK_PARAM_TYPE_U8},
+	{QED_DEVLINK_PREBOOT_VLAN, NVM_CFG_ID_PREBOOT_VLAN,
+	 DEVLINK_PARAM_TYPE_U16},
+	{QED_DEVLINK_PREBOOT_VLAN_VALUE, NVM_CFG_ID_PREBOOT_VLAN_VALUE,
+	 DEVLINK_PARAM_TYPE_U16},
+	{QED_DEVLINK_MBA_DELAY_TIME, NVM_CFG_ID_MBA_DELAY_TIME,
+	 DEVLINK_PARAM_TYPE_U8},
+	{QED_DEVLINK_MBA_SETUP_HOT_KEY, NVM_CFG_ID_MBA_SETUP_HOT_KEY,
+	 DEVLINK_PARAM_TYPE_U8},
+	{QED_DEVLINK_MBA_HIDE_SETUP_PROMPT, NVM_CFG_ID_MBA_HIDE_SETUP_PROMPT,
+	 DEVLINK_PARAM_TYPE_BOOL},
+};
+
 static int qed_dl_param_get(struct devlink *dl, u32 id,
 			    struct devlink_param_gset_ctx *ctx)
 {
@@ -30,11 +54,171 @@ static int qed_dl_param_set(struct devlink *dl, u32 id,
 	return 0;
 }
 
+static int qed_dl_get_perm_cfg(struct devlink *dl, u32 id,
+			       struct devlink_param_gset_ctx *ctx)
+{
+	u8 buf[QED_DL_PARAM_BUF_LEN];
+	struct qed_devlink *qed_dl;
+	int rc, cfg_idx, len = 0;
+	struct qed_hwfn *hwfn;
+	struct qed_dev *cdev;
+	struct qed_ptt *ptt;
+	u32 flags;
+
+	qed_dl = devlink_priv(dl);
+	cdev = qed_dl->cdev;
+	hwfn = QED_LEADING_HWFN(cdev);
+
+	if (id == QED_DEVLINK_ENTITY_ID) {
+		ctx->val.vu8 = cdev->cfg_entity_id;
+		return 0;
+	}
+
+	for (cfg_idx = 0; cfg_idx < ARRAY_SIZE(cfg_params); cfg_idx++)
+		if (cfg_params[cfg_idx].id == id)
+			break;
+
+	if (cfg_idx == ARRAY_SIZE(cfg_params)) {
+		DP_ERR(cdev, "Invalid command id %d\n", id);
+		return -EINVAL;
+	}
+
+	ptt = qed_ptt_acquire(hwfn);
+	if (!ptt)
+		return -EAGAIN;
+
+	memset(buf, 0, QED_DL_PARAM_BUF_LEN);
+	flags = cdev->cfg_entity_id ? QED_DL_PARAM_PF_GET_FLAGS :
+		QED_DL_PARAM_GET_FLAGS;
+	rc = qed_mcp_nvm_get_cfg(hwfn, ptt, cfg_params[cfg_idx].cmd,
+				 cdev->cfg_entity_id, flags, buf, &len);
+	if (rc)
+		DP_ERR(cdev, "Error = %d\n", rc);
+	else
+		memcpy(&ctx->val, buf, len);
+
+	qed_ptt_release(hwfn, ptt);
+
+	return rc;
+}
+
+static int qed_dl_set_perm_cfg(struct devlink *dl, u32 id,
+			       struct devlink_param_gset_ctx *ctx)
+{
+	u8 buf[QED_DL_PARAM_BUF_LEN];
+	struct qed_devlink *qed_dl;
+	int rc, cfg_idx, len = 0;
+	struct qed_hwfn *hwfn;
+	struct qed_dev *cdev;
+	struct qed_ptt *ptt;
+	u32 flags;
+
+	qed_dl = devlink_priv(dl);
+	cdev = qed_dl->cdev;
+	hwfn = QED_LEADING_HWFN(cdev);
+
+	if (id == QED_DEVLINK_ENTITY_ID) {
+		cdev->cfg_entity_id = ctx->val.vu8;
+		return 0;
+	}
+
+	for (cfg_idx = 0; cfg_idx < ARRAY_SIZE(cfg_params); cfg_idx++)
+		if (cfg_params[cfg_idx].id == id)
+			break;
+
+	if (cfg_idx == ARRAY_SIZE(cfg_params)) {
+		DP_ERR(cdev, "Invalid command id %d\n", id);
+		return -EINVAL;
+	}
+
+	memset(buf, 0, QED_DL_PARAM_BUF_LEN);
+	switch (cfg_params[cfg_idx].type) {
+	case DEVLINK_PARAM_TYPE_BOOL:
+		len = 1;
+		break;
+	case DEVLINK_PARAM_TYPE_U8:
+		len = 1;
+		break;
+	case DEVLINK_PARAM_TYPE_U16:
+		len = 2;
+		break;
+	case DEVLINK_PARAM_TYPE_U32:
+		len = 4;
+		break;
+	case DEVLINK_PARAM_TYPE_STRING:
+		len = strlen(ctx->val.vstr);
+		break;
+	}
+
+	memcpy(buf, &ctx->val, len);
+	flags = cdev->cfg_entity_id ? QED_DL_PARAM_PF_SET_FLAGS :
+		QED_DL_PARAM_SET_FLAGS;
+
+	ptt = qed_ptt_acquire(hwfn);
+	if (!ptt)
+		return -EAGAIN;
+
+	rc = qed_mcp_nvm_set_cfg(hwfn, ptt, cfg_params[cfg_idx].cmd,
+				 cdev->cfg_entity_id, flags, buf, len);
+	if (rc)
+		DP_ERR(cdev, "Error = %d\n", rc);
+
+	qed_ptt_release(hwfn, ptt);
+
+	return rc;
+}
+
 static const struct devlink_param qed_devlink_params[] = {
+	DEVLINK_PARAM_GENERIC(ENABLE_SRIOV, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			      qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
 	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PARAM_ID_IWARP_CMT,
 			     "iwarp_cmt", DEVLINK_PARAM_TYPE_BOOL,
 			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
 			     qed_dl_param_get, qed_dl_param_set, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_ENTITY_ID,
+			     "entity_id", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_DEVICE_CAPABILITIES,
+			     "device_capabilities", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_MF_MODE,
+			     "mf_mode", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_DCBX_MODE,
+			     "dcbx_mode", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PREBOOT_OPROM,
+			     "preboot_oprom", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PREBOOT_BOOT_PROTOCOL,
+			     "preboot_boot_protocol", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PREBOOT_VLAN,
+			     "preboot_vlan", DEVLINK_PARAM_TYPE_U16,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PREBOOT_VLAN_VALUE,
+			     "preboot_vlan_value", DEVLINK_PARAM_TYPE_U16,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_MBA_DELAY_TIME,
+			     "mba_delay_time", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_MBA_SETUP_HOT_KEY,
+			     "mba_setup_hot_key", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QED_DEVLINK_MBA_HIDE_SETUP_PROMPT,
+			     "mba_hide_setup_prompt", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qed_dl_get_perm_cfg, qed_dl_set_perm_cfg, NULL),
 };
 
 static const struct devlink_ops qed_dl_ops;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.h b/drivers/net/ethernet/qlogic/qed/qed_devlink.h
index 86e1caa..ca52a3f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.h
@@ -3,6 +3,12 @@
 #define _QED_DEVLINK_H
 #include "qed.h"
 
+#define QED_DL_PARAM_GET_FLAGS		0xA
+#define QED_DL_PARAM_SET_FLAGS		0xE
+#define QED_DL_PARAM_PF_GET_FLAGS	0x1A
+#define QED_DL_PARAM_PF_SET_FLAGS	0x1E
+#define QED_DL_PARAM_BUF_LEN		32
+
 struct qed_devlink {
 	struct qed_dev *cdev;
 };
@@ -10,6 +16,23 @@ struct qed_devlink {
 enum qed_devlink_param_id {
 	QED_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
 	QED_DEVLINK_PARAM_ID_IWARP_CMT,
+	QED_DEVLINK_ENTITY_ID,
+	QED_DEVLINK_DEVICE_CAPABILITIES,
+	QED_DEVLINK_MF_MODE,
+	QED_DEVLINK_DCBX_MODE,
+	QED_DEVLINK_PREBOOT_OPROM,
+	QED_DEVLINK_PREBOOT_BOOT_PROTOCOL,
+	QED_DEVLINK_PREBOOT_VLAN,
+	QED_DEVLINK_PREBOOT_VLAN_VALUE,
+	QED_DEVLINK_MBA_DELAY_TIME,
+	QED_DEVLINK_MBA_SETUP_HOT_KEY,
+	QED_DEVLINK_MBA_HIDE_SETUP_PROMPT,
+};
+
+struct qed_devlink_cfg_param {
+	u16 id;
+	u16 cmd;
+	enum devlink_param_type type;
 };
 
 int qed_devlink_register(struct qed_dev *cdev);
diff --git a/drivers/net/ethernet/qlogic/qed/qed_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
index 5091f5b1..49b6a6e 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
@@ -13338,6 +13338,17 @@ enum spad_sections {
 
 #define NVM_CFG_ID_MAC_ADDRESS			1
 #define NVM_CFG_ID_MF_MODE			9
+#define NVM_CFG_ID_DCBX_MODE			26
+#define NVM_CFG_ID_PREBOOT_OPROM		59
+#define NVM_CFG_ID_MBA_DELAY_TIME		61
+#define NVM_CFG_ID_MBA_SETUP_HOT_KEY		62
+#define NVM_CFG_ID_MBA_HIDE_SETUP_PROMPT	63
+#define NVM_CFG_ID_PREBOOT_BOOT_PROTOCOL	69
+#define NVM_CFG_ID_ENABLE_SRIOV			70
+#define NVM_CFG_ID_DEVICE_CAPABILITIES		117
+#define NVM_CFG_ID_PREBOOT_VLAN			133
+#define NVM_CFG_ID_PREBOOT_VLAN_VALUE		132
+
 
 #define MCP_TRACE_SIZE          2048	/* 2kb */
 
-- 
1.8.3.1

Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Jakub Kicinski]

On Mon, 17 Jun 2019 04:45:28 -0700, Sudarsana Reddy Kalluru wrote:
> This patch adds implementation for devlink callbacks for reading/
> configuring the device attributes.
> 
> Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>

You need to provide documentation for your parameters, plus some of
them look like they should potentially be port params, not device
params.

RE: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Sudarsana Reddy Kalluru]

> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Tuesday, June 18, 2019 4:24 AM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
> <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; Jiri Pirko
> <jiri@resnulli.us>
> Subject: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> configuration attributes.
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Mon, 17 Jun 2019 04:45:28 -0700, Sudarsana Reddy Kalluru wrote:
> > This patch adds implementation for devlink callbacks for reading/
> > configuring the device attributes.
> >
> > Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> 
> You need to provide documentation for your parameters, plus some of them
> look like they should potentially be port params, not device params.

Thanks a lot for your review. Will add the required documentation. In case of Marvell adapter, any of the device/adapter/port parameters can be read/configurable via any PF (ethdev) on the port. Hence adding the commands at device level. Hope this is fine.

Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Jiri Pirko]

Thu, Jun 20, 2019 at 02:09:29PM CEST, skalluru@marvell.com wrote:
>> -----Original Message-----
>> From: Jakub Kicinski <jakub.kicinski@netronome.com>
>> Sent: Tuesday, June 18, 2019 4:24 AM
>> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
>> Cc: davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
>> <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; Jiri Pirko
>> <jiri@resnulli.us>
>> Subject: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
>> configuration attributes.
>> 
>> External Email
>> 
>> ----------------------------------------------------------------------
>> On Mon, 17 Jun 2019 04:45:28 -0700, Sudarsana Reddy Kalluru wrote:
>> > This patch adds implementation for devlink callbacks for reading/
>> > configuring the device attributes.
>> >
>> > Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
>> > Signed-off-by: Ariel Elior <aelior@marvell.com>
>> 
>> You need to provide documentation for your parameters, plus some of them
>> look like they should potentially be port params, not device params.
>
>Thanks a lot for your review. Will add the required documentation. In case of Marvell adapter, any of the device/adapter/port parameters can be read/configurable via any PF (ethdev) on the port. Hence adding the commands at device level. Hope this is fine.

No it is not. Port param should be port param.

Also please be careful not to add any generic param as driver specific.

Thanks!

RE: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Sudarsana Reddy Kalluru]

> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Thursday, June 20, 2019 7:08 PM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>; davem@davemloft.net;
> netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>; Ariel
> Elior <aelior@marvell.com>
> Subject: Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> configuration attributes.
> 
> Thu, Jun 20, 2019 at 02:09:29PM CEST, skalluru@marvell.com wrote:
> >> -----Original Message-----
> >> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> >> Sent: Tuesday, June 18, 2019 4:24 AM
> >> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
> >> <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; Jiri Pirko
> >> <jiri@resnulli.us>
> >> Subject: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> >> configuration attributes.
> >>
> >> External Email
> >>
> >> ---------------------------------------------------------------------
> >> - On Mon, 17 Jun 2019 04:45:28 -0700, Sudarsana Reddy Kalluru wrote:
> >> > This patch adds implementation for devlink callbacks for reading/
> >> > configuring the device attributes.
> >> >
> >> > Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> >>
> >> You need to provide documentation for your parameters, plus some of
> >> them look like they should potentially be port params, not device params.
> >
> >Thanks a lot for your review. Will add the required documentation. In case
> of Marvell adapter, any of the device/adapter/port parameters can be
> read/configurable via any PF (ethdev) on the port. Hence adding the
> commands at device level. Hope this is fine.
> 
> No it is not. Port param should be port param.
> 
> Also please be careful not to add any generic param as driver specific.
> 
> Thanks!
Hi,
   Could you please with my query on the devlink-port-params implementation. [had sent the same query earlier to jiri@mellanox.com (based on the copyright info)].

Kernel seem to be invoking the driver devlink callbacks (registered via DEVLINK_PARAM_DRIVER) only when the associated parameter is published via devlink_params_publish(). callnback invocation path,
   devlink_nl_param_fill()
   {
                if (!param_item->published)
                         continue;
                 ctx.cmode = i;
                  err = devlink_param_get(devlink, param, &ctx);
   }
The API devlink_params_publish() publishes only the devlink-dev parameters (i.e., registered via devlink_params_register()), not the devlink-port params which are registered via devlink_port_params_register(). I couldn't find any other interface for publishing the devlink-port-params.
I have manually verified setting the published flag for port-params (as in below) and, observed that kernel correctly invokes the callbacks of devlink-port-params.
       list_for_each_entry(param_item, &dl_port.param_list, list) {
                param_item->published = true;
      }
Please let me know if I'm missing something here or, it's a missing functionality in the kernel.

Thanks,
Sudarsana

RE: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Sudarsana Reddy Kalluru]

> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Thursday, June 20, 2019 7:08 PM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>; davem@davemloft.net;
> netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>; Ariel
> Elior <aelior@marvell.com>
> Subject: Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> configuration attributes.
> 
> Thu, Jun 20, 2019 at 02:09:29PM CEST, skalluru@marvell.com wrote:
> >> -----Original Message-----
> >> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> >> Sent: Tuesday, June 18, 2019 4:24 AM
> >> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
> >> <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; Jiri Pirko
> >> <jiri@resnulli.us>
> >> Subject: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> >> configuration attributes.
> >>
> >> External Email
> >>
> >> ---------------------------------------------------------------------
> >> - On Mon, 17 Jun 2019 04:45:28 -0700, Sudarsana Reddy Kalluru wrote:
> >> > This patch adds implementation for devlink callbacks for reading/
> >> > configuring the device attributes.
> >> >
> >> > Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> >>
> >> You need to provide documentation for your parameters, plus some of
> >> them look like they should potentially be port params, not device params.
> >
> >Thanks a lot for your review. Will add the required documentation. In case
> of Marvell adapter, any of the device/adapter/port parameters can be
> read/configurable via any PF (ethdev) on the port. Hence adding the
> commands at device level. Hope this is fine.
> 
> No it is not. Port param should be port param.
> 
> Also please be careful not to add any generic param as driver specific.
> 
> Thanks!
Apologies for bringing this topic again. From the driver(s) code paths/'devlink man pages', I understood that devlink-port object is an entity on top of the PCI bus device.
Some drivers say NFP represents vnics (on pci-dev) as a devlink-ports and, some represents (virtual?) ports on the PF/device as devlink-ports.
In the case of Marvell NIC driver, we don't have [port] partitioning of the PCI device. And the config attributes are specific to PCI-device (not the vports/vnics of PF).
Hence I didn't see a need for creating devlink-port objects in the system for Marvell NICs. And planning to add the config attributes to 'devlink-dev' object.
Please let me know if my understanding and the proposal is ok?

Code paths which use devlink-port:
   mlx4_load_one(struct pci_dev *pdev):  mlx4_init_port_info(1 .. dev->caps.num_ports) -> devlink_port_register()
   nfp_net_pf_init_vnics (1 .. pf->vnics) -> nfp_net_pf_init_vnic() -> nfp_devlink_port_register()
   nsim_dev_probe (1 .. nsim_bus_dev->port_count) -> __nsim_dev_port_add() -> devlink_port_register ()

man page for 'devlink-port':
   devlink port set - change devlink port attributes
       DEV/PORT_INDEX - specifies the devlink port to operate on.

       devlink port show pci/0000:01:00.0/1
           Shows the state of specified devlink port.

Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Jakub Kicinski]

On Wed, 3 Jul 2019 12:56:39 +0000, Sudarsana Reddy Kalluru wrote:
> Apologies for bringing this topic again. From the driver(s) code
> paths/'devlink man pages', I understood that devlink-port object is
> an entity on top of the PCI bus device. Some drivers say NFP
> represents vnics (on pci-dev) as a devlink-ports and, some represents
> (virtual?) ports on the PF/device as devlink-ports. In the case of
> Marvell NIC driver, we don't have [port] partitioning of the PCI
> device. And the config attributes are specific to PCI-device (not the
> vports/vnics of PF). Hence I didn't see a need for creating
> devlink-port objects in the system for Marvell NICs. And planning to
> add the config attributes to 'devlink-dev' object. Please let me know
> if my understanding and the proposal is ok?

I understand where you're coming from.  

We want to make that judgement call on attribute-by-attribute basis.  
We want consistency across vendors (and, frankly, multiple drivers of
the same vendor).  If attribute looks like it belongs to the port,
rather than the entire device/ASIC operation, we should make it a port
attribute.

RE: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.

[Sudarsana Reddy Kalluru]

> -----Original Message-----
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Sent: Wednesday, July 3, 2019 11:13 PM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Jiri Pirko <jiri@resnulli.us>; davem@davemloft.net;
> netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>; Ariel
> Elior <aelior@marvell.com>
> Subject: Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> configuration attributes.
> 
> On Wed, 3 Jul 2019 12:56:39 +0000, Sudarsana Reddy Kalluru wrote:
> > Apologies for bringing this topic again. From the driver(s) code
> > paths/'devlink man pages', I understood that devlink-port object is an
> > entity on top of the PCI bus device. Some drivers say NFP represents
> > vnics (on pci-dev) as a devlink-ports and, some represents
> > (virtual?) ports on the PF/device as devlink-ports. In the case of
> > Marvell NIC driver, we don't have [port] partitioning of the PCI
> > device. And the config attributes are specific to PCI-device (not the
> > vports/vnics of PF). Hence I didn't see a need for creating
> > devlink-port objects in the system for Marvell NICs. And planning to
> > add the config attributes to 'devlink-dev' object. Please let me know
> > if my understanding and the proposal is ok?
> 
> I understand where you're coming from.
> 
> We want to make that judgement call on attribute-by-attribute basis.
> We want consistency across vendors (and, frankly, multiple drivers of the
> same vendor).  If attribute looks like it belongs to the port, rather than the
> entire device/ASIC operation, we should make it a port attribute.

Thanks for your mail. I'll go with creating PCI-dev/0 (i.e., port-id 0) for port based attributes as there's no port partitioning for Marvell NICs.