Thread (25 messages) 25 messages, 5 authors, 2018-12-14

Re: [PATCH net-next v2 RFC 8/8] bnxt_en: Add bnxt_en initial port params table and register it

From: Vasundhara Volam <hidden>
Date: 2018-12-11 10:37:09

On Tue, Dec 11, 2018 at 4:00 PM Jiri Pirko [off-list ref] wrote:
Tue, Dec 11, 2018 at 09:46:48AM CET, vasundhara-v.volam@broadcom.com wrote:
quoted
Register devlink_port with devlink and create initial port params
table for bnxt_en. The table consists of a generic parameter:

wake-on-lan: Enables Wake on Lan for this port when magic packet
is received with this port's MAC address using ACPI pattern.
If enabled, the controller asserts a wake pin upon reception of
WoL packet.  ACPI (Advanced Configuration and Power Interface) is
an industry specification for the efficient handling of power
consumption in desktop and mobile computers.

Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Vasundhara Volam <redacted>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  1 +
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 44 ++++++++++++++++++++++-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  1 +
3 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 3030931..1113e7c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1586,6 +1586,7 @@ struct bnxt {

      /* devlink interface and vf-rep structs */
      struct devlink          *dl;
+      struct devlink_port     dl_port;
      enum devlink_eswitch_mode eswitch_mode;
      struct bnxt_vf_rep      **vf_reps; /* array of vf-rep ptrs */
      u16                     *cfa_code_map; /* cfa_code -> vf_idx map */
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 140dbd6..7d084c3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -37,6 +37,8 @@ enum bnxt_dl_param_id {
       NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7},
      {BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK, NVM_OFF_DIS_GRE_VER_CHECK,
       BNXT_NVM_SHARED_CFG, 1},
+
+      {DEVLINK_PARAM_GENERIC_ID_WOL, NVM_OFF_WOL, BNXT_NVM_PORT_CFG, 1},
};

static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
@@ -70,7 +72,8 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
      bytesize = roundup(nvm_param.num_bits, BITS_PER_BYTE) / BITS_PER_BYTE;
      switch (bytesize) {
      case 1:
-              if (nvm_param.num_bits == 1)
+              if (nvm_param.num_bits == 1 &&
+                  nvm_param.id != DEVLINK_PARAM_GENERIC_ID_WOL)
                      buf = &val->vbool;
              else
                      buf = &val->vu8;
@@ -164,6 +167,16 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
      return 0;
}

+static int bnxt_dl_wol_validate(struct devlink *dl, u32 id,
+                              union devlink_param_value val,
+                              struct netlink_ext_ack *extack)
+{
+      if (val.vu8 != DEVLINK_PARAM_WOL_MAGIC_PKT ||
+          val.vu8 != DEVLINK_PARAM_WOL_DISABLE)
You have extack here, please fill the message for the user so he knows
what is going on.
Okay Sure.
quoted
+              return -EINVAL;
+      return 0;
+}
+
static const struct devlink_param bnxt_dl_params[] = {
      DEVLINK_PARAM_GENERIC(ENABLE_SRIOV,
                            BIT(DEVLINK_PARAM_CMODE_PERMANENT),
@@ -188,6 +201,12 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
                           NULL),
};

+static const struct devlink_param bnxt_dl_port_params[] = {
+      DEVLINK_PARAM_GENERIC(WOL, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+                            bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+                            bnxt_dl_wol_validate),
+};
+
int bnxt_dl_register(struct bnxt *bp)
{
      struct devlink *dl;
@@ -225,8 +244,28 @@ int bnxt_dl_register(struct bnxt *bp)
              goto err_dl_unreg;
      }

+      rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
+      if (rc) {
+              netdev_warn(bp->dev, "devlink_port_register failed. rc=%d",
Why "warn"? This is "err".
I will fix both of this below to err and remove rc=x in the next
version. Thanks.
quoted
+                          rc);
+              goto err_dl_param_unreg;
+      }
+      devlink_port_type_eth_set(&bp->dl_port, bp->dev);
+
+      rc = devlink_port_params_register(&bp->dl_port, bnxt_dl_port_params,
+                                        ARRAY_SIZE(bnxt_dl_port_params));
+      if (rc) {
+              netdev_warn(bp->dev, "devlink_port_params_register failed. rc=%d",
Same here. Also, no need to print "rc=x".

quoted
+                          rc);
+              goto err_dl_port_unreg;
+      }
      return 0;

+err_dl_port_unreg:
+      devlink_port_unregister(&bp->dl_port);
+err_dl_param_unreg:
+      devlink_params_unregister(dl, bnxt_dl_params,
+                                ARRAY_SIZE(bnxt_dl_params));
err_dl_unreg:
      devlink_unregister(dl);
err_dl_free:
@@ -242,6 +281,9 @@ void bnxt_dl_unregister(struct bnxt *bp)
      if (!dl)
              return;

+      devlink_port_params_unregister(&bp->dl_port, bnxt_dl_port_params,
+                                     ARRAY_SIZE(bnxt_dl_port_params));
+      devlink_port_unregister(&bp->dl_port);
      devlink_params_unregister(dl, bnxt_dl_params,
                                ARRAY_SIZE(bnxt_dl_params));
      devlink_unregister(dl);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index 5b6b2c7..da065ca 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -35,6 +35,7 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
#define NVM_OFF_MSIX_VEC_PER_PF_MAX   108
#define NVM_OFF_MSIX_VEC_PER_PF_MIN   114
+#define NVM_OFF_WOL                   152
#define NVM_OFF_IGNORE_ARI            164
#define NVM_OFF_DIS_GRE_VER_CHECK     171
#define NVM_OFF_ENABLE_SRIOV          401
--
1.8.3.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help