diff mbox series

[net-next,09/11] dsa: move devlink_port_attrs_set() call before register

Message ID 20190321132019.1426-10-jiri@resnulli.us
State Changes Requested
Delegated to: David Miller
Headers show
Series devlink: small spring cleanup | expand

Commit Message

Jiri Pirko March 21, 2019, 1:20 p.m. UTC 
From: Jiri Pirko <jiri@mellanox.com>

Since attrs are static during the existence of devlink port, set the
before registration of the port.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/dsa/dsa2.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

Comments

Andrew Lunn March 21, 2019, 1:41 p.m. UTC | #1 
On Thu, Mar 21, 2019 at 02:20:17PM +0100, Jiri Pirko wrote:
> +	switch (dp->type) {
> +	case DSA_PORT_TYPE_CPU:
> +		flavour = DEVLINK_PORT_FLAVOUR_CPU;
> +		break;
> +	case DSA_PORT_TYPE_DSA:
> +		flavour = DEVLINK_PORT_FLAVOUR_DSA;
> +		break;
> +	case DSA_PORT_TYPE_USER: /* fall-through */
> +	default:
> +		flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
> +		break;
> +	}
> +
> +	/* dp->index is used now as port_number. However
> +	 * CPU ports should have separate numbering
> +	 * independent from front panel port numbers.
> +	 */

Hi Jiri

Given the way you have rearranged this code, i think the text above
should now be:

However CPU ports and DSA ports should have separate numbering.

	Andrew
Jiri Pirko March 21, 2019, 1:44 p.m. UTC | #2 
Thu, Mar 21, 2019 at 02:41:01PM CET, andrew@lunn.ch wrote:
>On Thu, Mar 21, 2019 at 02:20:17PM +0100, Jiri Pirko wrote:
>> +	switch (dp->type) {
>> +	case DSA_PORT_TYPE_CPU:
>> +		flavour = DEVLINK_PORT_FLAVOUR_CPU;
>> +		break;
>> +	case DSA_PORT_TYPE_DSA:
>> +		flavour = DEVLINK_PORT_FLAVOUR_DSA;
>> +		break;
>> +	case DSA_PORT_TYPE_USER: /* fall-through */
>> +	default:
>> +		flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
>> +		break;
>> +	}
>> +
>> +	/* dp->index is used now as port_number. However
>> +	 * CPU ports should have separate numbering
>> +	 * independent from front panel port numbers.
>> +	 */
>
>Hi Jiri
>
>Given the way you have rearranged this code, i think the text above
>should now be:
>
>However CPU ports and DSA ports should have separate numbering.

You are right. Will fix in v2.

>
>	Andrew
diff mbox series

Patch

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4558de672b4f..6d71569afd88 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -258,14 +258,36 @@  static void dsa_tree_teardown_default_cpu(struct dsa_switch_tree *dst)
 
 static int dsa_port_setup(struct dsa_port *dp)
 {
+	enum devlink_port_flavour flavour;
 	struct dsa_switch *ds = dp->ds;
-	int err = 0;
+	int err;
+
+	if (dp->type == DSA_PORT_TYPE_UNUSED)
+		return 0;
 
 	memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
 
-	if (dp->type != DSA_PORT_TYPE_UNUSED)
-		err = devlink_port_register(ds->devlink, &dp->devlink_port,
-					    dp->index);
+	switch (dp->type) {
+	case DSA_PORT_TYPE_CPU:
+		flavour = DEVLINK_PORT_FLAVOUR_CPU;
+		break;
+	case DSA_PORT_TYPE_DSA:
+		flavour = DEVLINK_PORT_FLAVOUR_DSA;
+		break;
+	case DSA_PORT_TYPE_USER: /* fall-through */
+	default:
+		flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
+		break;
+	}
+
+	/* dp->index is used now as port_number. However
+	 * CPU ports should have separate numbering
+	 * independent from front panel port numbers.
+	 */
+	devlink_port_attrs_set(&dp->devlink_port, flavour,
+			       dp->index, false, 0);
+	err = devlink_port_register(ds->devlink, &dp->devlink_port,
+				    dp->index);
 	if (err)
 		return err;
 
@@ -273,13 +295,6 @@  static int dsa_port_setup(struct dsa_port *dp)
 	case DSA_PORT_TYPE_UNUSED:
 		break;
 	case DSA_PORT_TYPE_CPU:
-		/* dp->index is used now as port_number. However
-		 * CPU ports should have separate numbering
-		 * independent from front panel port numbers.
-		 */
-		devlink_port_attrs_set(&dp->devlink_port,
-				       DEVLINK_PORT_FLAVOUR_CPU,
-				       dp->index, false, 0);
 		err = dsa_port_link_register_of(dp);
 		if (err) {
 			dev_err(ds->dev, "failed to setup link for port %d.%d\n",
@@ -288,13 +303,6 @@  static int dsa_port_setup(struct dsa_port *dp)
 		}
 		break;
 	case DSA_PORT_TYPE_DSA:
-		/* dp->index is used now as port_number. However
-		 * DSA ports should have separate numbering
-		 * independent from front panel port numbers.
-		 */
-		devlink_port_attrs_set(&dp->devlink_port,
-				       DEVLINK_PORT_FLAVOUR_DSA,
-				       dp->index, false, 0);
 		err = dsa_port_link_register_of(dp);
 		if (err) {
 			dev_err(ds->dev, "failed to setup link for port %d.%d\n",
@@ -303,9 +311,6 @@  static int dsa_port_setup(struct dsa_port *dp)
 		}
 		break;
 	case DSA_PORT_TYPE_USER:
-		devlink_port_attrs_set(&dp->devlink_port,
-				       DEVLINK_PORT_FLAVOUR_PHYSICAL,
-				       dp->index, false, 0);
 		err = dsa_slave_create(dp);
 		if (err)
 			dev_err(ds->dev, "failed to create slave for port %d.%d\n",