[[PATCH net-next RFC] 0/4] MV88E6390 batch two

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [[PATCH net-next RFC] 0/4] MV88E6390 batch two
@ 2016-11-23 23:43 Andrew Lunn
  2016-11-23 23:43 ` [[PATCH net-next RFC] 1/4] net: dsa: mv88e6xxx: Implement mv88e6390 tag remap Andrew Lunn
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Andrew Lunn @ 2016-11-23 23:43 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, Andrew Lunn

RFC only. Not for committing. They will conflict with the mv88e6097
support.

This is the second batch of patches adding support for the
MV88e6390. They are not sufficient to make it work properly.

The mv88e6390 has a much expanded set of priority maps. Refactor the
existing code, and implement basic support for the new device.

Similarly, the monitor control register has been reworked.

The mv88e6390 has something odd in its EDSA tagging implementation,
which means it is not possible to use it. So we need to use DSA
tagging. This is the first device with EDSA support where we need to
use DSA, and the code does not support this. So two patches refactor
the existing code. The two different register definitions are
separated out, and using DSA on an EDSA capable device is added.

Andrew Lunn (4):
  net: dsa: mv88e6xxx: Implement mv88e6390 tag remap
  net: dsa: mv88e6xxx: Monitor and Management tables
  net: dsa: mv88e6xxx: Move the tagging protocol into info
  net: dsa: mv88e6xxx: Refactor CPU and DSA port setup

 drivers/net/dsa/mv88e6xxx/chip.c      | 195 +++++++++++++++++++++-------------
 drivers/net/dsa/mv88e6xxx/global1.c   |  55 ++++++++++
 drivers/net/dsa/mv88e6xxx/global1.h   |   3 +
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  40 ++++---
 drivers/net/dsa/mv88e6xxx/port.c      | 132 +++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h      |   7 ++
 6 files changed, 348 insertions(+), 84 deletions(-)

-- 
2.10.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [[PATCH net-next RFC] 1/4] net: dsa: mv88e6xxx: Implement mv88e6390 tag remap
  2016-11-23 23:43 [[PATCH net-next RFC] 0/4] MV88E6390 batch two Andrew Lunn
@ 2016-11-23 23:43 ` Andrew Lunn
  2016-11-28 20:35   ` Vivien Didelot
  2016-11-23 23:43 ` [[PATCH net-next RFC] 2/4] net: dsa: mv88e6xxx: Monitor and Management tables Andrew Lunn
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2016-11-23 23:43 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, Andrew Lunn

The mv88e6390 does not have the two registers to set the frame
priority map. Instead it has an indirection registers for setting a
number of different priority maps. Refactor the old code into an
function, implement the mv88e6390 version, and use an op to call the
right one.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 37 +++++++++++++++--------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 10 ++++++
 drivers/net/dsa/mv88e6xxx/port.c      | 57 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h      |  2 ++
 4 files changed, 93 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index bada6465af59..880e40288038 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2617,20 +2617,10 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 			if (err)
 				return err;
 		}
+	}
 
-		/* Tag Remap: use an identity 802.1p prio -> switch
-		 * prio mapping.
-		 */
-		err = mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_0123,
-					   0x3210);
-		if (err)
-			return err;
-
-		/* Tag Remap 2: use an identity 802.1p prio -> switch
-		 * prio mapping.
-		 */
-		err = mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_4567,
-					   0x7654);
+	if (chip->info->ops->tag_remap) {
+		err = chip->info->ops->tag_remap(chip, port);
 		if (err)
 			return err;
 	}
@@ -3193,6 +3183,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -3221,6 +3212,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
@@ -3249,6 +3241,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
@@ -3263,6 +3256,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
@@ -3278,6 +3272,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -3295,6 +3290,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -3310,6 +3306,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -3327,6 +3324,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -3357,6 +3355,7 @@ static const struct mv88e6xxx_ops mv88e6190_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
+	.tag_remap = mv88e6390_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6190x_ops = {
@@ -3373,6 +3372,7 @@ static const struct mv88e6xxx_ops mv88e6190x_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
+	.tag_remap = mv88e6390_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6191_ops = {
@@ -3389,6 +3389,7 @@ static const struct mv88e6xxx_ops mv88e6191_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
+	.tag_remap = mv88e6390_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6240_ops = {
@@ -3406,6 +3407,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6290_ops = {
@@ -3422,6 +3424,7 @@ static const struct mv88e6xxx_ops mv88e6290_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
+	.tag_remap = mv88e6390_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
@@ -3438,6 +3441,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6320_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
@@ -3454,6 +3458,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6320_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
@@ -3469,6 +3474,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -3484,6 +3490,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -3501,6 +3508,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.tag_remap = mv88e6095_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
@@ -3517,6 +3525,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
+	.tag_remap = mv88e6390_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -3533,6 +3542,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
+	.tag_remap = mv88e6390_tag_remap,
 };
 
 static const struct mv88e6xxx_ops mv88e6391_ops = {
@@ -3549,6 +3559,7 @@ static const struct mv88e6xxx_ops mv88e6391_ops = {
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
+	.tag_remap = mv88e6390_tag_remap,
 };
 
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 9298faa5878b..241025e0aec7 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -171,6 +171,15 @@
 #define PORT_OUT_FILTERED	0x13
 #define PORT_TAG_REGMAP_0123	0x18
 #define PORT_TAG_REGMAP_4567	0x19
+#define PORT_PRIO_MAP_TABLE	0x18    /* 6390 */
+#define PORT_PRIO_MAP_TABLE_UPDATE		BIT(15)
+#define PORT_PRIO_MAP_TABLE_INGRESS_PCP		(0x0 << 12)
+#define PORT_PRIO_MAP_TABLE_EGRESS_GREEN_PCP	(0x1 << 12)
+#define PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP	(0x2 << 12)
+#define PORT_PRIO_MAP_TABLE_EGRESS_AVB_PCP	(0x3 << 12)
+#define PORT_PRIO_MAP_TABLE_EGRESS_GREEN_DSCP	(0x5 << 12)
+#define PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_DSCP	(0x6 << 12)
+#define PORT_PRIO_MAP_TABLE_EGRESS_AVB_DSCP	(0x7 << 12)
 
 #define GLOBAL_STATUS		0x00
 #define GLOBAL_STATUS_PPU_STATE BIT(15) /* 6351 and 6171 */
@@ -813,6 +822,7 @@ struct mv88e6xxx_ops {
 	void (*stats_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
 	void (*stats_get_stats)(struct mv88e6xxx_chip *chip,  int port,
 				uint64_t *data);
+	int (*tag_remap)(struct mv88e6xxx_chip *chip, int port);
 };
 
 #define STATS_TYPE_PORT		BIT(0)
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index af4772d86086..b7fab70f6cd7 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -496,3 +496,60 @@ int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
 
 	return 0;
 }
+
+int mv88e6095_tag_remap(struct mv88e6xxx_chip *chip, int port)
+{
+	int err;
+
+	/* Tag Remap: use an identity 802.1p prio -> switch prio
+	 * mapping.
+	 */
+	err = mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_0123, 0x3210);
+	if (err)
+		return err;
+
+	/* Tag Remap 2: use an identity 802.1p prio -> switch
+	 * prio mapping.
+	 */
+	return mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_4567, 0x7654);
+}
+
+int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port)
+{
+	int err, reg, i;
+
+	for (i = 0; i <= 7; i++) {
+		reg = i | (i << 4) |
+			PORT_PRIO_MAP_TABLE_INGRESS_PCP |
+			PORT_PRIO_MAP_TABLE_UPDATE;
+		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
+					   reg);
+		if (err)
+			return err;
+
+		reg = i | PORT_PRIO_MAP_TABLE_EGRESS_GREEN_PCP |
+			PORT_PRIO_MAP_TABLE_UPDATE;
+		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
+					   reg);
+		if (err)
+			return err;
+
+		reg = i |
+			PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP |
+			PORT_PRIO_MAP_TABLE_UPDATE;
+		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
+					   reg);
+		if (err)
+			return err;
+
+		reg = i |
+			PORT_PRIO_MAP_TABLE_EGRESS_AVB_PCP |
+			PORT_PRIO_MAP_TABLE_UPDATE;
+		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
+					   reg);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 499129c1489c..99a04cf3d1d6 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -48,5 +48,7 @@ int mv88e6xxx_port_set_pvid(struct mv88e6xxx_chip *chip, int port, u16 pvid);
 
 int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
 				  u16 mode);
+int mv88e6095_tag_remap(struct mv88e6xxx_chip *chip, int port);
+int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port);
 
 #endif /* _MV88E6XXX_PORT_H */
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [[PATCH net-next RFC] 1/4] net: dsa: mv88e6xxx: Implement mv88e6390 tag remap
  2016-11-23 23:43 ` [[PATCH net-next RFC] 1/4] net: dsa: mv88e6xxx: Implement mv88e6390 tag remap Andrew Lunn
@ 2016-11-28 20:35   ` Vivien Didelot
  0 siblings, 0 replies; 8+ messages in thread
From: Vivien Didelot @ 2016-11-28 20:35 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Andrew Lunn

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>  #define PORT_TAG_REGMAP_0123	0x18
>  #define PORT_TAG_REGMAP_4567	0x19
> +#define PORT_PRIO_MAP_TABLE	0x18    /* 6390 */
> +#define PORT_PRIO_MAP_TABLE_UPDATE		BIT(15)
> +#define PORT_PRIO_MAP_TABLE_INGRESS_PCP		(0x0 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_GREEN_PCP	(0x1 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP	(0x2 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_AVB_PCP	(0x3 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_GREEN_DSCP	(0x5 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_DSCP	(0x6 << 12)
> +#define PORT_PRIO_MAP_TABLE_EGRESS_AVB_DSCP	(0x7 << 12)

0x17 is the "IP Priority Mapping Table" register, so I'd define 0x18 as
PORT_IEEE_PRIO_MAP_TABLE to avoid later confusion.

>  
>  #define GLOBAL_STATUS		0x00
>  #define GLOBAL_STATUS_PPU_STATE BIT(15) /* 6351 and 6171 */
> @@ -813,6 +822,7 @@ struct mv88e6xxx_ops {
>  	void (*stats_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
>  	void (*stats_get_stats)(struct mv88e6xxx_chip *chip,  int port,
>  				uint64_t *data);
> +	int (*tag_remap)(struct mv88e6xxx_chip *chip, int port);

I would've prefered an op like .tag_remap(*chip, port, prio, new) and a
wrapper in chip.c which loops over priority 0-7, but that would make the
implementation unnecessarily complex, so let's keep it as is for now ;)

>  };
>  
>  #define STATS_TYPE_PORT		BIT(0)
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index af4772d86086..b7fab70f6cd7 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -496,3 +496,60 @@ int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
>  
>  	return 0;
>  }

Please add an ordered comment:

/* Offset 0x18: Port IEEE Priority Remapping Registers [0-3]
 * Offset 0x19: Port IEEE Priority Remapping Registers [4-7]
 */

> +
> +int mv88e6095_tag_remap(struct mv88e6xxx_chip *chip, int port)
> +{
> +	int err;
> +
> +	/* Tag Remap: use an identity 802.1p prio -> switch prio
> +	 * mapping.
> +	 */
> +	err = mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_0123, 0x3210);
> +	if (err)
> +		return err;
> +
> +	/* Tag Remap 2: use an identity 802.1p prio -> switch
> +	 * prio mapping.
> +	 */

A single comment like this before the 2 writes will be enough:

    /* Use a direct priority mapping for all IEEE tagged frames */

> +	return mv88e6xxx_port_write(chip, port, PORT_TAG_REGMAP_4567, 0x7654);
> +}

Functions of port.c implementing Port Registers features must be
prefixed mv88e6xxx_port_* (6xxx can be a model in case of conflict).
mv88e6xxx_port_tag_remap() seems fine.

> +
> +int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port)
> +{
> +	int err, reg, i;
> +
> +	for (i = 0; i <= 7; i++) {
> +		reg = i | (i << 4) |

The pointer offset is 9, not 4.

> +			PORT_PRIO_MAP_TABLE_INGRESS_PCP |
> +			PORT_PRIO_MAP_TABLE_UPDATE;
> +		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> +					   reg);
> +		if (err)
> +			return err;
> +
> +		reg = i | PORT_PRIO_MAP_TABLE_EGRESS_GREEN_PCP |
> +			PORT_PRIO_MAP_TABLE_UPDATE;
> +		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> +					   reg);
> +		if (err)
> +			return err;
> +
> +		reg = i |
> +			PORT_PRIO_MAP_TABLE_EGRESS_YELLOW_PCP |
> +			PORT_PRIO_MAP_TABLE_UPDATE;
> +		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> +					   reg);
> +		if (err)
> +			return err;
> +
> +		reg = i |
> +			PORT_PRIO_MAP_TABLE_EGRESS_AVB_PCP |
> +			PORT_PRIO_MAP_TABLE_UPDATE;
> +		err = mv88e6xxx_port_write(chip, port, PORT_PRIO_MAP_TABLE,
> +					   reg);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}

Please add a static helper first to write the table, e.g.

    /* Offset 0x18: Port IEEE Priority Mapping Table (88E6190) */

    static int mv88e6xxx_port_ieeepmt_write(struct mv88e6xxx_chip *chip,
                                            int port, u8 table,
                                            u8 pointer, u16 data)

And then provide

    int mv88e6xxx_port_ieeepmt_tag_remap(...)


Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [[PATCH net-next RFC] 2/4] net: dsa: mv88e6xxx: Monitor and Management tables
  2016-11-23 23:43 [[PATCH net-next RFC] 0/4] MV88E6390 batch two Andrew Lunn
  2016-11-23 23:43 ` [[PATCH net-next RFC] 1/4] net: dsa: mv88e6xxx: Implement mv88e6390 tag remap Andrew Lunn
@ 2016-11-23 23:43 ` Andrew Lunn
  2016-11-28 17:42   ` Vivien Didelot
  2016-11-23 23:43 ` [[PATCH net-next RFC] 3/4] net: dsa: mv88e6xxx: Move the tagging protocol into info Andrew Lunn
  2016-11-23 23:43 ` [[PATCH net-next RFC] 4/4] net: dsa: mv88e6xxx: Refactor CPU and DSA port setup Andrew Lunn
  3 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2016-11-23 23:43 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, Andrew Lunn

The mv88e6390 changes the monitor control register into the Monitor
and Management control, which is an indirection register to various
registers. Move the existing code into global1.c, and add new code for
the mv88e6390.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 37 +++++++++++++++++------
 drivers/net/dsa/mv88e6xxx/global1.c   | 55 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/global1.h   |  3 ++
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  7 +++++
 4 files changed, 93 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 880e40288038..a6fa3f81e11b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2747,15 +2747,11 @@ static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
 	if (err)
 		return err;
 
-	/* Configure the upstream port, and configure it as the port to which
-	 * ingress and egress and ARP monitor frames are to be sent.
-	 */
-	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
-		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
-		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
-	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
-	if (err)
-		return err;
+	if (chip->info->ops->monitor_ctrl) {
+		err = chip->info->ops->monitor_ctrl(chip, upstream_port);
+		if (err)
+			return err;
+	}
 
 	/* Disable remote management, and set the switch's DSA device number. */
 	err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL_2,
@@ -3184,6 +3180,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -3198,6 +3195,7 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6123_ops = {
@@ -3213,6 +3211,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
@@ -3227,6 +3226,7 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3242,6 +3242,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
@@ -3257,6 +3258,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
@@ -3273,6 +3275,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -3291,6 +3294,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -3307,6 +3311,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -3325,6 +3330,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -3339,6 +3345,7 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6190_ops = {
@@ -3356,6 +3363,7 @@ static const struct mv88e6xxx_ops mv88e6190_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
+	.monitor_ctrl = mv88e6390_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6190x_ops = {
@@ -3373,6 +3381,7 @@ static const struct mv88e6xxx_ops mv88e6190x_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
+	.monitor_ctrl = mv88e6390_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6191_ops = {
@@ -3390,6 +3399,7 @@ static const struct mv88e6xxx_ops mv88e6191_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
+	.monitor_ctrl = mv88e6390_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6240_ops = {
@@ -3408,6 +3418,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6290_ops = {
@@ -3425,6 +3436,7 @@ static const struct mv88e6xxx_ops mv88e6290_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
+	.monitor_ctrl = mv88e6390_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
@@ -3442,6 +3454,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6320_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
@@ -3459,6 +3472,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6320_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
@@ -3475,6 +3489,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -3491,6 +3506,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -3509,6 +3525,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
+	.monitor_ctrl = mv88e6095_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
@@ -3526,6 +3543,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
+	.monitor_ctrl = mv88e6390_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -3543,6 +3561,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.stats_get_strings = mv88e6320_stats_get_strings,
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
+	.monitor_ctrl = mv88e6390_monitor_ctrl,
 };
 
 static const struct mv88e6xxx_ops mv88e6391_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/global1.c b/drivers/net/dsa/mv88e6xxx/global1.c
index 5fcf23dbf04b..ea4421e1d15c 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.c
+++ b/drivers/net/dsa/mv88e6xxx/global1.c
@@ -33,6 +33,60 @@ int mv88e6xxx_g1_wait(struct mv88e6xxx_chip *chip, int reg, u16 mask)
 	return mv88e6xxx_wait(chip, chip->info->global1_addr, reg, mask);
 }
 
+/* Offset 0x1a: Monitor Control */
+
+int mv88e6095_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port)
+{
+	u16 reg;
+
+	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
+		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
+		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
+
+	return mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
+}
+
+int mv88e6390_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port)
+{
+	u16 reg;
+	int err;
+
+	/* Trap destination */
+	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
+		GLOBAL_MONITOR_CONTROL_CPU_DEST |
+		upstream_port;
+	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
+	if (err)
+		return err;
+
+	/* 01:c2:80:00:00:00:00-01:c2:80:00:00:00:07 are Management */
+	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
+		GLOBAL_MONITOR_CONTROL_0180C280000000XLO | 0xff;
+	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
+	if (err)
+		return err;
+
+	/* 01:c2:80:00:00:00:08-01:c2:80:00:00:00:0f are Management */
+	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
+		GLOBAL_MONITOR_CONTROL_0180C280000000XHI | 0xff;
+	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
+	if (err)
+		return err;
+
+	/* 01:c2:80:00:00:00:20-01:c2:80:00:00:00:27 are Management */
+	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
+		GLOBAL_MONITOR_CONTROL_0180C280000002XLO | 0xff;
+	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
+	if (err)
+		return err;
+
+	/* 01:c2:80:00:00:00:28-01:c2:80:00:00:00:2f are Management */
+	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
+		GLOBAL_MONITOR_CONTROL_0180C280000002XHI | 0xff;
+
+	return mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
+}
+
 /* Offset 0x1c: Global Control 2 */
 
 int mv88e6390_g1_stats_set_histogram(struct mv88e6xxx_chip *chip)
@@ -125,3 +179,4 @@ void mv88e6xxx_g1_stats_read(struct mv88e6xxx_chip *chip, int stat, u32 *val)
 
 	*val = value | reg;
 }
+
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index df3794cdbfb9..4d8e5ddd8d8c 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -25,5 +25,8 @@ int mv88e6320_g1_stats_snapshot(struct mv88e6xxx_chip *chip, int port);
 int mv88e6390_g1_stats_snapshot(struct mv88e6xxx_chip *chip, int port);
 int mv88e6390_g1_stats_set_histogram(struct mv88e6xxx_chip *chip);
 void mv88e6xxx_g1_stats_read(struct mv88e6xxx_chip *chip, int stat, u32 *val);
+int mv88e6095_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port);
+int mv88e6390_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port);
+
 
 #endif /* _MV88E6XXX_GLOBAL1_H */
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 241025e0aec7..6698ec1d22b5 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -289,6 +289,12 @@
 #define GLOBAL_MONITOR_CONTROL_ARP_SHIFT	4
 #define GLOBAL_MONITOR_CONTROL_MIRROR_SHIFT	0
 #define GLOBAL_MONITOR_CONTROL_ARP_DISABLED	(0xf0)
+#define GLOBAL_MONITOR_CONTROL_UPDATE                  BIT(15)
+#define GLOBAL_MONITOR_CONTROL_0180C280000000XLO       (0x00 << 8)
+#define GLOBAL_MONITOR_CONTROL_0180C280000000XHI       (0x01 << 8)
+#define GLOBAL_MONITOR_CONTROL_0180C280000002XLO       (0x02 << 8)
+#define GLOBAL_MONITOR_CONTROL_0180C280000002XHI       (0x03 << 8)
+#define GLOBAL_MONITOR_CONTROL_CPU_DEST		       (0x30 << 8)
 #define GLOBAL_CONTROL_2	0x1c
 #define GLOBAL_CONTROL_2_NO_CASCADE		0xe000
 #define GLOBAL_CONTROL_2_MULTIPLE_CASCADE	0xf000
@@ -823,6 +829,7 @@ struct mv88e6xxx_ops {
 	void (*stats_get_stats)(struct mv88e6xxx_chip *chip,  int port,
 				uint64_t *data);
 	int (*tag_remap)(struct mv88e6xxx_chip *chip, int port);
+	int (*monitor_ctrl)(struct mv88e6xxx_chip *chip, int upstream_port);
 };
 
 #define STATS_TYPE_PORT		BIT(0)
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [[PATCH net-next RFC] 2/4] net: dsa: mv88e6xxx: Monitor and Management tables
  2016-11-23 23:43 ` [[PATCH net-next RFC] 2/4] net: dsa: mv88e6xxx: Monitor and Management tables Andrew Lunn
@ 2016-11-28 17:42   ` Vivien Didelot
  0 siblings, 0 replies; 8+ messages in thread
From: Vivien Didelot @ 2016-11-28 17:42 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Andrew Lunn

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> +/* Offset 0x1a: Monitor Control */

Thanks. We could also optionally add another line Offset 0x1A: Monitor &
MGMT Control, to mention the change on newer chips.

> +
> +int mv88e6095_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port)
> +{
> +	u16 reg;
> +
> +	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> +		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> +		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
> +
> +	return mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +}

Monitor Control is just one of these registers which control several
stuffs.

On 6185, it controls only the monitor destination port for ingress,
egress and ARP. On 6352, it controls the monitor destination port for
ingress and egress, sets the CPU destination port and the mirror port.

Hence using GLOBAL_MONITOR_CONTROL_ARP_SHIFT is bad since bits 7:4
differ in those two (CPU Dest vs. ARP Dest). Unless you tell me that
these are the same.

Please add a .set_monitor_port ops and provide a more explicit function
like mv88e6xxx_g1_set_monitor_port() which configures the ingress/egress
monitor destination port and eventually the CPU/ARP dest if the effect
is similar (otherwise a .set_cpu_port ops is needed).

> +
> +int mv88e6390_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port)
> +{
> +	u16 reg;
> +	int err;
> +
> +	/* Trap destination */
> +	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> +		GLOBAL_MONITOR_CONTROL_CPU_DEST |
> +		upstream_port;
> +	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +	if (err)
> +		return err;
> +
> +	/* 01:c2:80:00:00:00:00-01:c2:80:00:00:00:07 are Management */
> +	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> +		GLOBAL_MONITOR_CONTROL_0180C280000000XLO | 0xff;
> +	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +	if (err)
> +		return err;
> +
> +	/* 01:c2:80:00:00:00:08-01:c2:80:00:00:00:0f are Management */
> +	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> +		GLOBAL_MONITOR_CONTROL_0180C280000000XHI | 0xff;
> +	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +	if (err)
> +		return err;
> +
> +	/* 01:c2:80:00:00:00:20-01:c2:80:00:00:00:27 are Management */
> +	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> +		GLOBAL_MONITOR_CONTROL_0180C280000002XLO | 0xff;
> +	err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +	if (err)
> +		return err;
> +
> +	/* 01:c2:80:00:00:00:28-01:c2:80:00:00:00:2f are Management */
> +	reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> +		GLOBAL_MONITOR_CONTROL_0180C280000002XHI | 0xff;
> +
> +	return mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +}
> +

This function does more that the 6095 implementation.

First please provide a static helper to write the Monitor & MGMT Control
table. Something like:

    mv88e6xxx_g1_monitor_mgmt_write(struct mv88e6xxx_chip *chip,
                                    u8 pointer, u8 data)

Then use it to implement a mv88e6xxx_g1_set_trap_port() function.

Rsvd2CPU is a different thing. 6352 has 2 dedicated registers in Global
2 to configure the management addresses 01:c2:80:00:00:00:2x (offset
0x2) and 01:c2:80:00:00:00:0x (offset 0x3). 6185 only has register 0x3.
Then 6390 stores the Rsvd2CPU addresses in this MGMT table.

I'd expect something like new .set_rsvd2cpu{0,2}(chip, u16 x) ops and
the following implementation:

    int mv88e6xxx_g2_set_rsvd2cpu2(struct mv88e6xxx_chip *chip, u16 x);
    int mv88e6xxx_g2_set_rsvd2cpu0(struct mv88e6xxx_chip *chip, u16 x);

int global2.c, and:

    int mv88e6xxx_g1_set_rsvd2cpu2(struct mv88e6xxx_chip *chip, u16 x);
    int mv88e6xxx_g1_set_rsvd2cpu0(struct mv88e6xxx_chip *chip, u16 x);

in global1.c (which use mv88e6xxx_g1_monitor_mgmt_write()).

Later (not required now), a nice wrapper in chip.c could set a rsvd2cpu
bit for a given address, or fallback to loading it in the ATU as MGMT.

Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [[PATCH net-next RFC] 3/4] net: dsa: mv88e6xxx: Move the tagging protocol into info
  2016-11-23 23:43 [[PATCH net-next RFC] 0/4] MV88E6390 batch two Andrew Lunn
  2016-11-23 23:43 ` [[PATCH net-next RFC] 1/4] net: dsa: mv88e6xxx: Implement mv88e6390 tag remap Andrew Lunn
  2016-11-23 23:43 ` [[PATCH net-next RFC] 2/4] net: dsa: mv88e6xxx: Monitor and Management tables Andrew Lunn
@ 2016-11-23 23:43 ` Andrew Lunn
  2016-11-23 23:43 ` [[PATCH net-next RFC] 4/4] net: dsa: mv88e6xxx: Refactor CPU and DSA port setup Andrew Lunn
  3 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2016-11-23 23:43 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, Andrew Lunn

Older chips support a single tagging protocol, DSA. New chips support
both DSA and EDSA, an enhanced version. Having both as an option
changes the register layouts. Up until now, it has been assumed that
if EDSA is supported, it will be used. Hence the register layout has
been determined by which protocol should be used. However, mv88e6390
has a different implementation of EDSA, which requires we need to use
the DSA tagging. Hence separate the selection of the protocol from the
register layout.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 33 +++++++++++++++++++++++++++------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 17 ++++-------------
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index a6fa3f81e11b..15ea1207b21a 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2482,7 +2482,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 		PORT_CONTROL_USE_TAG | PORT_CONTROL_USE_IP |
 		PORT_CONTROL_STATE_FORWARDING;
 	if (dsa_is_cpu_port(ds, port)) {
-		if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_EDSA))
+		if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA)
 			reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA |
 				PORT_CONTROL_FORWARD_UNKNOWN_MC;
 		else
@@ -2611,7 +2611,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 		/* Port Ethertype: use the Ethertype DSA Ethertype
 		 * value.
 		 */
-		if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_EDSA)) {
+		if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA) {
 			err = mv88e6xxx_port_write(chip, port, PORT_ETH_TYPE,
 						   ETH_P_EDSA);
 			if (err)
@@ -3592,6 +3592,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 8,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6097,
 		.ops = &mv88e6085_ops,
 	},
@@ -3606,6 +3607,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 8,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6095,
 		.ops = &mv88e6095_ops,
 	},
@@ -3620,6 +3622,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 		.ops = &mv88e6123_ops,
 	},
@@ -3634,6 +3637,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
 		.ops = &mv88e6131_ops,
 	},
@@ -3648,6 +3652,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 		.ops = &mv88e6161_ops,
 	},
@@ -3662,6 +3667,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 		.ops = &mv88e6165_ops,
 	},
@@ -3676,6 +3682,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 		.ops = &mv88e6171_ops,
 	},
@@ -3690,6 +3697,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 		.ops = &mv88e6172_ops,
 	},
@@ -3704,6 +3712,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 		.ops = &mv88e6175_ops,
 	},
@@ -3718,6 +3727,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 		.ops = &mv88e6176_ops,
 	},
@@ -3732,6 +3742,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 8,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
 		.ops = &mv88e6185_ops,
 	},
@@ -3744,6 +3755,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.port_base_addr = 0x0,
 		.global1_addr = 0x1b,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
@@ -3760,6 +3772,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
 		.ops = &mv88e6190x_ops,
 	},
@@ -3773,6 +3786,8 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x0,
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
 		.ops = &mv88e6391_ops,
 	},
@@ -3787,6 +3802,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 		.ops = &mv88e6240_ops,
 	},
@@ -3801,6 +3817,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
 		.ops = &mv88e6290_ops,
 	},
@@ -3815,6 +3832,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 8,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
 		.ops = &mv88e6320_ops,
 	},
@@ -3829,6 +3847,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 8,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
 		.ops = &mv88e6321_ops,
 	},
@@ -3843,6 +3862,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 		.ops = &mv88e6350_ops,
 	},
@@ -3857,6 +3877,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 		.ops = &mv88e6351_ops,
 	},
@@ -3871,6 +3892,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_EDSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 		.ops = &mv88e6352_ops,
 	},
@@ -3884,6 +3906,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
 		.ops = &mv88e6390_ops,
 	},
@@ -3897,6 +3920,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.global1_addr = 0x1b,
 		.age_time_coeff = 15000,
 		.g1_irqs = 9,
+		.tag_protocol = DSA_TAG_PROTO_DSA,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
 		.ops = &mv88e6390x_ops,
 	},
@@ -3997,10 +4021,7 @@ static enum dsa_tag_protocol mv88e6xxx_get_tag_protocol(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 
-	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_EDSA))
-		return DSA_TAG_PROTO_EDSA;
-
-	return DSA_TAG_PROTO_DSA;
+	return chip->info->tag_protocol;
 }
 
 static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 6698ec1d22b5..a0d0f79a7de8 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -430,12 +430,6 @@ enum mv88e6xxx_family {
 };
 
 enum mv88e6xxx_cap {
-	/* Two different tag protocols can be used by the driver. All
-	 * switches support DSA, but only later generations support
-	 * EDSA.
-	 */
-	MV88E6XXX_CAP_EDSA,
-
 	/* Energy Efficient Ethernet.
 	 */
 	MV88E6XXX_CAP_EEE,
@@ -498,7 +492,6 @@ enum mv88e6xxx_cap {
 };
 
 /* Bitmask of capabilities */
-#define MV88E6XXX_FLAG_EDSA		BIT_ULL(MV88E6XXX_CAP_EDSA)
 #define MV88E6XXX_FLAG_EEE		BIT_ULL(MV88E6XXX_CAP_EEE)
 
 #define MV88E6XXX_FLAG_SMI_CMD		BIT_ULL(MV88E6XXX_CAP_SMI_CMD)
@@ -593,8 +586,7 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAG_VTU)
 
 #define MV88E6XXX_FLAGS_FAMILY_6320	\
-	(MV88E6XXX_FLAG_EDSA |		\
-	 MV88E6XXX_FLAG_EEE |		\
+	(MV88E6XXX_FLAG_EEE |		\
 	 MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_2X |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
@@ -608,8 +600,7 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAGS_PVT)
 
 #define MV88E6XXX_FLAGS_FAMILY_6351	\
-	(MV88E6XXX_FLAG_EDSA |		\
-	 MV88E6XXX_FLAG_G1_ATU_FID |	\
+	(MV88E6XXX_FLAG_G1_ATU_FID |	\
 	 MV88E6XXX_FLAG_G1_VTU_FID |	\
 	 MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_G2_INT |	\
@@ -625,8 +616,7 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAGS_PVT)
 
 #define MV88E6XXX_FLAGS_FAMILY_6352	\
-	(MV88E6XXX_FLAG_EDSA |		\
-	 MV88E6XXX_FLAG_EEE |		\
+	(MV88E6XXX_FLAG_EEE |		\
 	 MV88E6XXX_FLAG_G1_ATU_FID |	\
 	 MV88E6XXX_FLAG_G1_VTU_FID |	\
 	 MV88E6XXX_FLAG_GLOBAL2 |	\
@@ -668,6 +658,7 @@ struct mv88e6xxx_info {
 	unsigned int global1_addr;
 	unsigned int age_time_coeff;
 	unsigned int g1_irqs;
+	enum dsa_tag_protocol tag_protocol;
 	unsigned long long flags;
 	const struct mv88e6xxx_ops *ops;
 };
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [[PATCH net-next RFC] 4/4] net: dsa: mv88e6xxx: Refactor CPU and DSA port setup
  2016-11-23 23:43 [[PATCH net-next RFC] 0/4] MV88E6390 batch two Andrew Lunn
                   ` (2 preceding siblings ...)
  2016-11-23 23:43 ` [[PATCH net-next RFC] 3/4] net: dsa: mv88e6xxx: Move the tagging protocol into info Andrew Lunn
@ 2016-11-23 23:43 ` Andrew Lunn
  2016-11-28 16:00   ` Vivien Didelot
  3 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2016-11-23 23:43 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, Andrew Lunn

Older chips only support DSA tagging. Newer chips have both DSA and
EDSA tagging. Put these two different implementations into functions
which get called from the ops structure.

This results in the helper mv88e6xxx_6065_family() becoming unused, so
remove it.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 92 ++++++++++++++++++-----------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  6 +++
 drivers/net/dsa/mv88e6xxx/port.c      | 75 ++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h      |  5 ++
 4 files changed, 133 insertions(+), 45 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 15ea1207b21a..28bd10d95750 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -677,11 +677,6 @@ static int mv88e6xxx_phy_ppu_write(struct mv88e6xxx_chip *chip, int addr,
 	return err;
 }
 
-static bool mv88e6xxx_6065_family(struct mv88e6xxx_chip *chip)
-{
-	return chip->info->family == MV88E6XXX_FAMILY_6065;
-}
-
 static bool mv88e6xxx_6095_family(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->family == MV88E6XXX_FAMILY_6095;
@@ -2473,41 +2468,20 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 	 * If this is the upstream port for this switch, enable
 	 * forwarding of unknown unicasts and multicasts.
 	 */
-	reg = 0;
-	if (mv88e6xxx_6352_family(chip) || mv88e6xxx_6351_family(chip) ||
-	    mv88e6xxx_6165_family(chip) || mv88e6xxx_6097_family(chip) ||
-	    mv88e6xxx_6095_family(chip) || mv88e6xxx_6065_family(chip) ||
-	    mv88e6xxx_6185_family(chip) || mv88e6xxx_6320_family(chip))
-		reg = PORT_CONTROL_IGMP_MLD_SNOOP |
+	reg = PORT_CONTROL_IGMP_MLD_SNOOP |
 		PORT_CONTROL_USE_TAG | PORT_CONTROL_USE_IP |
 		PORT_CONTROL_STATE_FORWARDING;
+	err = mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
+	if (err)
+		return err;
+
 	if (dsa_is_cpu_port(ds, port)) {
-		if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA)
-			reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA |
-				PORT_CONTROL_FORWARD_UNKNOWN_MC;
-		else
-			reg |= PORT_CONTROL_DSA_TAG;
-		reg |= PORT_CONTROL_EGRESS_ADD_TAG |
-			PORT_CONTROL_FORWARD_UNKNOWN;
+		err = chip->info->ops->cpu_port_config(chip, port);
+		if (err)
+			return err;
 	}
 	if (dsa_is_dsa_port(ds, port)) {
-		if (mv88e6xxx_6095_family(chip) ||
-		    mv88e6xxx_6185_family(chip))
-			reg |= PORT_CONTROL_DSA_TAG;
-		if (mv88e6xxx_6352_family(chip) ||
-		    mv88e6xxx_6351_family(chip) ||
-		    mv88e6xxx_6165_family(chip) ||
-		    mv88e6xxx_6097_family(chip) ||
-		    mv88e6xxx_6320_family(chip)) {
-			reg |= PORT_CONTROL_FRAME_MODE_DSA;
-		}
-
-		if (port == dsa_upstream_port(ds))
-			reg |= PORT_CONTROL_FORWARD_UNKNOWN |
-				PORT_CONTROL_FORWARD_UNKNOWN_MC;
-	}
-	if (reg) {
-		err = mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
+		err = chip->info->ops->dsa_port_config(chip, port);
 		if (err)
 			return err;
 	}
@@ -2607,16 +2581,6 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 					   0x0000);
 		if (err)
 			return err;
-
-		/* Port Ethertype: use the Ethertype DSA Ethertype
-		 * value.
-		 */
-		if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA) {
-			err = mv88e6xxx_port_write(chip, port, PORT_ETH_TYPE,
-						   ETH_P_EDSA);
-			if (err)
-				return err;
-		}
 	}
 
 	if (chip->info->ops->tag_remap) {
@@ -3181,6 +3145,8 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -3196,6 +3162,8 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6095_cpu_port_config,
+	.dsa_port_config = mv88e6095_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6123_ops = {
@@ -3212,6 +3180,8 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
@@ -3227,6 +3197,8 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6095_cpu_port_config,
+	.dsa_port_config = mv88e6095_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3243,6 +3215,8 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
@@ -3259,6 +3233,8 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
@@ -3276,6 +3252,8 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -3295,6 +3273,8 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -3312,6 +3292,8 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -3331,6 +3313,8 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -3346,6 +3330,8 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 	.stats_get_strings = mv88e6095_stats_get_strings,
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6095_cpu_port_config,
+	.dsa_port_config = mv88e6095_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6190_ops = {
@@ -3419,6 +3405,8 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6290_ops = {
@@ -3455,6 +3443,8 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.stats_get_stats = mv88e6320_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
@@ -3473,6 +3463,8 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.stats_get_stats = mv88e6320_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
@@ -3490,6 +3482,8 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -3507,6 +3501,8 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -3526,6 +3522,8 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.stats_get_stats = mv88e6095_stats_get_stats,
 	.tag_remap = mv88e6095_tag_remap,
 	.monitor_ctrl = mv88e6095_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
@@ -3544,6 +3542,8 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
 	.monitor_ctrl = mv88e6390_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -3562,6 +3562,8 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.stats_get_stats = mv88e6390_stats_get_stats,
 	.tag_remap = mv88e6390_tag_remap,
 	.monitor_ctrl = mv88e6390_monitor_ctrl,
+	.cpu_port_config = mv88e6351_cpu_port_config,
+	.dsa_port_config = mv88e6351_dsa_port_config,
 };
 
 static const struct mv88e6xxx_ops mv88e6391_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index a0d0f79a7de8..b846a33c024c 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -123,6 +123,10 @@
 #define PORT_CONTROL_USE_TAG		BIT(4)
 #define PORT_CONTROL_FORWARD_UNKNOWN_MC	BIT(3)
 #define PORT_CONTROL_FORWARD_UNKNOWN	BIT(2)
+#define PORT_CONTROL_NOT_EGREES_UNKNOWN_DA		(0x0 << 2)
+#define PORT_CONTROL_NOT_EGREES_UNKNOWN_MULTICAST_DA	(0x1 << 2)
+#define PORT_CONTROL_NOT_EGREES_UNKNOWN_UNITCAST_DA	(0x2 << 2)
+#define PORT_CONTROL_EGREES_ALL_UNKNOWN_DA		(0x3 << 2)
 #define PORT_CONTROL_STATE_MASK		0x03
 #define PORT_CONTROL_STATE_DISABLED	0x00
 #define PORT_CONTROL_STATE_BLOCKING	0x01
@@ -821,6 +825,8 @@ struct mv88e6xxx_ops {
 				uint64_t *data);
 	int (*tag_remap)(struct mv88e6xxx_chip *chip, int port);
 	int (*monitor_ctrl)(struct mv88e6xxx_chip *chip, int upstream_port);
+	int (*cpu_port_config)(struct mv88e6xxx_chip *chip, int port);
+	int (*dsa_port_config)(struct mv88e6xxx_chip *chip, int port);
 };
 
 #define STATS_TYPE_PORT		BIT(0)
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index b7fab70f6cd7..a37d7d72df47 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -553,3 +553,78 @@ int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port)
 
 	return 0;
 }
+
+int mv88e6095_cpu_port_config(struct mv88e6xxx_chip *chip, int port)
+{
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, &reg);
+	if (err)
+		return err;
+
+	reg |= PORT_CONTROL_DSA_TAG |
+		PORT_CONTROL_EGRESS_ADD_TAG |
+		PORT_CONTROL_FORWARD_UNKNOWN;
+
+	return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
+}
+
+int mv88e6351_cpu_port_config(struct mv88e6xxx_chip *chip, int port)
+{
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, &reg);
+	if (err)
+		return err;
+
+	if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA) {
+		reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA |
+			PORT_CONTROL_EGRESS_ADD_TAG;
+
+		/* Port Ethertype: use the Ethertype DSA Ethertype
+		 * value.
+		 */
+		err = mv88e6xxx_port_write(chip, port, PORT_ETH_TYPE,
+					   ETH_P_EDSA);
+		if (err)
+			return err;
+	} else {
+		reg |= PORT_CONTROL_FRAME_MODE_DSA;
+	}
+
+	reg |= PORT_CONTROL_EGREES_ALL_UNKNOWN_DA;
+
+	return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
+}
+
+int mv88e6095_dsa_port_config(struct mv88e6xxx_chip *chip, int port)
+{
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, &reg);
+	if (err)
+		return err;
+
+	reg |= PORT_CONTROL_DSA_TAG |
+		PORT_CONTROL_FORWARD_UNKNOWN;
+
+	return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
+}
+
+int mv88e6351_dsa_port_config(struct mv88e6xxx_chip *chip, int port)
+{
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, &reg);
+	if (err)
+		return err;
+
+	reg |= PORT_CONTROL_FRAME_MODE_DSA |
+		PORT_CONTROL_EGREES_ALL_UNKNOWN_DA;
+
+	return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
+}
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 99a04cf3d1d6..18070beae35a 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -51,4 +51,9 @@ int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
 int mv88e6095_tag_remap(struct mv88e6xxx_chip *chip, int port);
 int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port);
 
+int mv88e6095_cpu_port_config(struct mv88e6xxx_chip *chip, int port);
+int mv88e6351_cpu_port_config(struct mv88e6xxx_chip *chip, int port);
+int mv88e6095_dsa_port_config(struct mv88e6xxx_chip *chip, int port);
+int mv88e6351_dsa_port_config(struct mv88e6xxx_chip *chip, int port);
+
 #endif /* _MV88E6XXX_PORT_H */
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [[PATCH net-next RFC] 4/4] net: dsa: mv88e6xxx: Refactor CPU and DSA port setup
  2016-11-23 23:43 ` [[PATCH net-next RFC] 4/4] net: dsa: mv88e6xxx: Refactor CPU and DSA port setup Andrew Lunn
@ 2016-11-28 16:00   ` Vivien Didelot
  0 siblings, 0 replies; 8+ messages in thread
From: Vivien Didelot @ 2016-11-28 16:00 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Andrew Lunn

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>  static const struct mv88e6xxx_ops mv88e6391_ops = {
> diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> index a0d0f79a7de8..b846a33c024c 100644
> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> @@ -123,6 +123,10 @@
>  #define PORT_CONTROL_USE_TAG		BIT(4)
>  #define PORT_CONTROL_FORWARD_UNKNOWN_MC	BIT(3)
>  #define PORT_CONTROL_FORWARD_UNKNOWN	BIT(2)
> +#define PORT_CONTROL_NOT_EGREES_UNKNOWN_DA		(0x0 << 2)
> +#define PORT_CONTROL_NOT_EGREES_UNKNOWN_MULTICAST_DA	(0x1 << 2)
> +#define PORT_CONTROL_NOT_EGREES_UNKNOWN_UNITCAST_DA	(0x2 << 2)

s/EGREES/EGRESS/.

> +#define PORT_CONTROL_EGREES_ALL_UNKNOWN_DA		(0x3 << 2)
>  #define PORT_CONTROL_STATE_MASK		0x03
>  #define PORT_CONTROL_STATE_DISABLED	0x00
>  #define PORT_CONTROL_STATE_BLOCKING	0x01
> @@ -821,6 +825,8 @@ struct mv88e6xxx_ops {
>  				uint64_t *data);
>  	int (*tag_remap)(struct mv88e6xxx_chip *chip, int port);
>  	int (*monitor_ctrl)(struct mv88e6xxx_chip *chip, int upstream_port);
> +	int (*cpu_port_config)(struct mv88e6xxx_chip *chip, int port);
> +	int (*dsa_port_config)(struct mv88e6xxx_chip *chip, int port);
>  };

Hum, I dislike having config wrappers in the library. The functions
stored in the mv88e6xxx_ops structure should reflect the features
exposed by the SMI device (Port, Global X, PHY, etc.) registers.

They should not handle configuration logic, we should see them as an
MV88E6XXX API used to implement a chip (usually wrapped in chip.c).

>  
>  #define STATS_TYPE_PORT		BIT(0)
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index b7fab70f6cd7..a37d7d72df47 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -553,3 +553,78 @@ int mv88e6390_tag_remap(struct mv88e6xxx_chip *chip, int port)
>  
>  	return 0;
>  }
> +
> +int mv88e6095_cpu_port_config(struct mv88e6xxx_chip *chip, int port)
> +{
> +	u16 reg;
> +	int err;
> +
> +	err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, &reg);
> +	if (err)
> +		return err;
> +
> +	reg |= PORT_CONTROL_DSA_TAG |
> +		PORT_CONTROL_EGRESS_ADD_TAG |
> +		PORT_CONTROL_FORWARD_UNKNOWN;
> +
> +	return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
> +}
> +
> +int mv88e6351_cpu_port_config(struct mv88e6xxx_chip *chip, int port)
> +{
> +	u16 reg;
> +	int err;
> +
> +	err = mv88e6xxx_port_read(chip, port, PORT_CONTROL, &reg);
> +	if (err)
> +		return err;
> +
> +	if (chip->info->tag_protocol == DSA_TAG_PROTO_EDSA) {
> +		reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA |
> +			PORT_CONTROL_EGRESS_ADD_TAG;
> +
> +		/* Port Ethertype: use the Ethertype DSA Ethertype
> +		 * value.
> +		 */
> +		err = mv88e6xxx_port_write(chip, port, PORT_ETH_TYPE,
> +					   ETH_P_EDSA);
> +		if (err)
> +			return err;
> +	} else {
> +		reg |= PORT_CONTROL_FRAME_MODE_DSA;
> +	}
> +
> +	reg |= PORT_CONTROL_EGREES_ALL_UNKNOWN_DA;
> +
> +	return mv88e6xxx_port_write(chip, port, PORT_CONTROL, reg);
> +}

The device (Port, Global X, etc.) functions should reflect and implement
the feature described in the register(s) they read from/write to.

Please provide generic functions to configure a port's Frame mode and
Egress mode under the comment /* Offset 0x04: Port Control Register */

mv88e6xxx_port_set_{frame,egress}_mode() would work. The Frame mode bits
changed between chips, but the Egress mode bits are the same since 6065.

A frame mode can be "Normal Network mode", "DSA mode", "Provider mode",
or "Ether Type DSA mode". Maybe use an enum, or switching on the u8
frame_mode value might be enough, as you wish.

Please also provide a generic function to set the Port EType under an
ordered comment:

/* Offset 0x0F: Port E Type */

int mv88e6xxx_port_set_etype(struct mv88e6xxx_chip *chip, int port,
                             u16 etype)
{
    return mv88e6xxx_port_write(chip, port, PORT_ETH_TYPE, etype);
}

Then, some nice wrappers in chip.c can use them depending on the chip's
tag_protocol to configure what net/dsa calls cpu, dsa, or user port.

Thanks,

        Vivien

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-11-28 20:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-23 23:43 [[PATCH net-next RFC] 0/4] MV88E6390 batch two Andrew Lunn
2016-11-23 23:43 ` [[PATCH net-next RFC] 1/4] net: dsa: mv88e6xxx: Implement mv88e6390 tag remap Andrew Lunn
2016-11-28 20:35   ` Vivien Didelot
2016-11-23 23:43 ` [[PATCH net-next RFC] 2/4] net: dsa: mv88e6xxx: Monitor and Management tables Andrew Lunn
2016-11-28 17:42   ` Vivien Didelot
2016-11-23 23:43 ` [[PATCH net-next RFC] 3/4] net: dsa: mv88e6xxx: Move the tagging protocol into info Andrew Lunn
2016-11-23 23:43 ` [[PATCH net-next RFC] 4/4] net: dsa: mv88e6xxx: Refactor CPU and DSA port setup Andrew Lunn
2016-11-28 16:00   ` Vivien Didelot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).