Re: [PATCH net-next 09/16] bnxt_en: Refactor bnxt_dl_register()

On Sun, Jan 26, 2020 at 5:02 PM Jiri Pirko [off-list ref] wrote:

Sun, Jan 26, 2020 at 10:03:03AM CET, michael.chan@broadcom.com wrote:

quoted

From: Vasundhara Volam <redacted>

Define bnxt_dl_params_register() and bnxt_dl_params_unregister()
functions and move params register/unregister code to these newly
defined functions. This patch is in preparation to register
devlink irrespective of firmware spec. version in the next patch.

Signed-off-by: Vasundhara Volam <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 60 ++++++++++++++---------
1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 0c3d224..9253eed 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c

@@ -485,6 +485,38 @@ static const struct devlink_param bnxt_dl_params[] = {

static const struct devlink_param bnxt_dl_port_params[] = {
};

+static int bnxt_dl_params_register(struct bnxt *bp)
+{
+      int rc;
+
+      rc = devlink_params_register(bp->dl, bnxt_dl_params,
+                                   ARRAY_SIZE(bnxt_dl_params));
+      if (rc) {
+              netdev_warn(bp->dev, "devlink_params_register failed. rc=%d",
+                          rc);
+              return rc;
+      }
+      rc = devlink_port_params_register(&bp->dl_port, bnxt_dl_port_params,
+                                        ARRAY_SIZE(bnxt_dl_port_params));

Wait, this assumes that you have 1:1 devlink:devlink_port setup. Is that
correct? Don't you have other devlink_ports for eswitch representors
that have params?

Yes Jiri, this assumes 1:1 setup. Our driver does not register params for VFs.
It will register params only for PFs.

This patch is refactoring of code and moving params_registers to a new function,
which will not be called for VFs.

There is a check for PF in bnxt_dl_register() and return before calling
bnxt_dl_params_register(), if check fails.

quoted

+      if (rc) {
+              netdev_err(bp->dev, "devlink_port_params_register failed");
+              devlink_params_unregister(bp->dl, bnxt_dl_params,
+                                        ARRAY_SIZE(bnxt_dl_params));
+              return rc;
+      }
+      devlink_params_publish(bp->dl);
+
+      return 0;
+}
+
+static void bnxt_dl_params_unregister(struct bnxt *bp)
+{
+      devlink_params_unregister(bp->dl, bnxt_dl_params,
+                                ARRAY_SIZE(bnxt_dl_params));
+      devlink_port_params_unregister(&bp->dl_port, bnxt_dl_port_params,
+                                     ARRAY_SIZE(bnxt_dl_port_params));
+}
+
int bnxt_dl_register(struct bnxt *bp)
{
      struct devlink *dl;

@@ -520,40 +552,24 @@ int bnxt_dl_register(struct bnxt *bp)
      if (!BNXT_PF(bp))
              return 0;

-      rc = devlink_params_register(dl, bnxt_dl_params,
-                                   ARRAY_SIZE(bnxt_dl_params));
-      if (rc) {
-              netdev_warn(bp->dev, "devlink_params_register failed. rc=%d",
-                          rc);
-              goto err_dl_unreg;
-      }
-
      devlink_port_attrs_set(&bp->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
                             bp->pf.port_id, false, 0,
                             bp->switch_id, sizeof(bp->switch_id));
      rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
      if (rc) {
              netdev_err(bp->dev, "devlink_port_register failed");
-              goto err_dl_param_unreg;
+              goto err_dl_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_err(bp->dev, "devlink_port_params_register failed");
+      rc = bnxt_dl_params_register(bp);
+      if (rc)
              goto err_dl_port_unreg;
-      }
-
-      devlink_params_publish(dl);

      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:

@@ -570,12 +586,8 @@ void bnxt_dl_unregister(struct bnxt *bp)
              return;

      if (BNXT_PF(bp)) {
-              devlink_port_params_unregister(&bp->dl_port,
-                                             bnxt_dl_port_params,
-                                             ARRAY_SIZE(bnxt_dl_port_params));
+              bnxt_dl_params_unregister(bp);
              devlink_port_unregister(&bp->dl_port);
-              devlink_params_unregister(dl, bnxt_dl_params,
-                                        ARRAY_SIZE(bnxt_dl_params));
      }
      devlink_unregister(dl);
      devlink_free(dl);
--

2.5.1