[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAACQVJqcS0frPG1br+nrSfteRtkfnPyYncYFeLKegBk8RN+97Q@mail.gmail.com>
Date: Tue, 11 Dec 2018 16:06:55 +0530
From: Vasundhara Volam <vasundhara-v.volam@...adcom.com>
To: Jiří Pírko <jiri@...nulli.us>
Cc: David Miller <davem@...emloft.net>,
"michael.chan@...adcom.com" <michael.chan@...adcom.com>,
Jiri Pirko <jiri@...lanox.com>, jakub.kicinski@...ronome.com,
Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 RFC 8/8] bnxt_en: Add bnxt_en initial port
params table and register it
On Tue, Dec 11, 2018 at 4:00 PM Jiri Pirko <jiri@...nulli.us> wrote:
>
> Tue, Dec 11, 2018 at 09:46:48AM CET, vasundhara-v.volam@...adcom.com wrote:
> >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@...adcom.com>
> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@...adcom.com>
> >---
> > 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.
>
>
> >+ 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.
>
>
> >+ 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".
>
>
> >+ 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
> >
Powered by blists - more mailing lists