diff mbox series

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

Message ID 1580029390-32760-10-git-send-email-michael.chan@broadcom.com
State Changes Requested
Delegated to: David Miller
Headers show
Series bnxt_en: Updates for net-next. | expand

Commit Message

Michael Chan Jan. 26, 2020, 9:03 a.m. UTC 
From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

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 <vasundhara-v.volam@broadcom.com>
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(-)

Comments

Jiri Pirko Jan. 26, 2020, 11:32 a.m. UTC | #1 
Sun, Jan 26, 2020 at 10:03:03AM CET, michael.chan@broadcom.com wrote:
>From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>
>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 <vasundhara-v.volam@broadcom.com>
>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?
 

>+	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
>
Vasundhara Volam Jan. 27, 2020, 4:51 a.m. UTC | #2 
On Sun, Jan 26, 2020 at 5:02 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Sun, Jan 26, 2020 at 10:03:03AM CET, michael.chan@broadcom.com wrote:
> >From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> >
> >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 <vasundhara-v.volam@broadcom.com>
> >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.
>
>
> >+      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
> >
diff mbox series

Patch

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));
+	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);