[PATCH net-next 0/7] net: dsa: mv88e6xxx: factorize switch info

[PATCH net-next 0/7] net: dsa: mv88e6xxx: factorize switch info

[Vivien Didelot]

This patchset factorizes drastically the mv88e6xxx code by sharing a new
extendable info structure to store static data such as switch family,
product number, number of ports, number of databases and the name.

The next step is to add a "features" bitmap member to the info structure
in order to simplify the shared code with a feature-based logic instead
of checking their family/ID.

This is a step forward having a single mv88e6xxx driver supporting many
similar devices, like any usual Linux driver.

Vivien Didelot (7):
  net: dsa: mv88e6xxx: drop double ds assignment
  net: dsa: mv88e6xxx: drop revision probing
  net: dsa: mv88e6xxx: add switch info
  net: dsa: mv88e6xxx: add family to info
  net: dsa: mv88e6xxx: add number of ports to info
  net: dsa: mv88e6xxx: add number of database to info
  net: dsa: mv88e6xxx: drop switch id

 drivers/net/dsa/mv88e6123.c |  29 +------
 drivers/net/dsa/mv88e6131.c |  32 ++-----
 drivers/net/dsa/mv88e6171.c |  15 ++--
 drivers/net/dsa/mv88e6352.c |  24 ++----
 drivers/net/dsa/mv88e6xxx.c | 201 ++++++++++++--------------------------------
 drivers/net/dsa/mv88e6xxx.h |  83 +++++++-----------
 6 files changed, 106 insertions(+), 278 deletions(-)

-- 
2.8.0

[PATCH net-next 1/7] net: dsa: mv88e6xxx: drop double ds assignment

[Vivien Didelot]

Every driver assigns ps->ds even though it gets assigned in the shared
mv88e6xxx_setup_common function. Kill redundancy.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123.c | 2 --
 drivers/net/dsa/mv88e6131.c | 2 --
 drivers/net/dsa/mv88e6171.c | 2 --
 drivers/net/dsa/mv88e6352.c | 2 --
 4 files changed, 8 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index c34283d..88a812d 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -76,8 +76,6 @@ static int mv88e6123_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
-	ps->ds = ds;
-
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index f5d75fc..6b2bcb0 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -94,8 +94,6 @@ static int mv88e6131_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
-	ps->ds = ds;
-
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index f562250..40222b0 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -72,8 +72,6 @@ static int mv88e6171_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
-	ps->ds = ds;
-
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index e54ee27..dbd920e 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -84,8 +84,6 @@ static int mv88e6352_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
-	ps->ds = ds;
-
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
-- 
2.8.0

[PATCH net-next 2/7] net: dsa: mv88e6xxx: drop revision probing

[Vivien Didelot]

There is no point in having special case for the revision when probing a
switch model. The code gets cluttered with unnecessary defines, and
leads to errors when code such as mv88e6131_setup compares
PORT_SWITCH_ID_6131_B2 to ps->id which mask the revision.

Drop every revision definitions, add a ps->rev variable for eventual
runtime checking and lookup only the product number.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123.c |  6 ------
 drivers/net/dsa/mv88e6131.c |  2 --
 drivers/net/dsa/mv88e6352.c |  6 ------
 drivers/net/dsa/mv88e6xxx.c | 19 ++++++-------------
 drivers/net/dsa/mv88e6xxx.h | 15 ++-------------
 5 files changed, 8 insertions(+), 40 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 88a812d..00c1121 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -19,14 +19,8 @@
 
 static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
 	{ PORT_SWITCH_ID_6123, "Marvell 88E6123" },
-	{ PORT_SWITCH_ID_6123_A1, "Marvell 88E6123 (A1)" },
-	{ PORT_SWITCH_ID_6123_A2, "Marvell 88E6123 (A2)" },
 	{ PORT_SWITCH_ID_6161, "Marvell 88E6161" },
-	{ PORT_SWITCH_ID_6161_A1, "Marvell 88E6161 (A1)" },
-	{ PORT_SWITCH_ID_6161_A2, "Marvell 88E6161 (A2)" },
 	{ PORT_SWITCH_ID_6165, "Marvell 88E6165" },
-	{ PORT_SWITCH_ID_6165_A1, "Marvell 88E6165 (A1)" },
-	{ PORT_SWITCH_ID_6165_A2, "Marvell 88e6165 (A2)" },
 };
 
 static char *mv88e6123_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 6b2bcb0..df534da 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -21,7 +21,6 @@ static const struct mv88e6xxx_switch_id mv88e6131_table[] = {
 	{ PORT_SWITCH_ID_6085, "Marvell 88E6085" },
 	{ PORT_SWITCH_ID_6095, "Marvell 88E6095/88E6095F" },
 	{ PORT_SWITCH_ID_6131, "Marvell 88E6131" },
-	{ PORT_SWITCH_ID_6131_B2, "Marvell 88E6131 (B2)" },
 	{ PORT_SWITCH_ID_6185, "Marvell 88E6185" },
 };
 
@@ -109,7 +108,6 @@ static int mv88e6131_setup(struct dsa_switch *ds)
 		ps->num_ports = 11;
 		break;
 	case PORT_SWITCH_ID_6131:
-	case PORT_SWITCH_ID_6131_B2:
 		ps->num_ports = 8;
 		break;
 	default:
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index dbd920e..30fc5f6 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -27,14 +27,8 @@ static const struct mv88e6xxx_switch_id mv88e6352_table[] = {
 	{ PORT_SWITCH_ID_6176, "Marvell 88E6176" },
 	{ PORT_SWITCH_ID_6240, "Marvell 88E6240" },
 	{ PORT_SWITCH_ID_6320, "Marvell 88E6320" },
-	{ PORT_SWITCH_ID_6320_A1, "Marvell 88E6320 (A1)" },
-	{ PORT_SWITCH_ID_6320_A2, "Marvell 88e6320 (A2)" },
 	{ PORT_SWITCH_ID_6321, "Marvell 88E6321" },
-	{ PORT_SWITCH_ID_6321_A1, "Marvell 88E6321 (A1)" },
-	{ PORT_SWITCH_ID_6321_A2, "Marvell 88e6321 (A2)" },
 	{ PORT_SWITCH_ID_6352, "Marvell 88E6352" },
-	{ PORT_SWITCH_ID_6352_A0, "Marvell 88E6352 (A0)" },
-	{ PORT_SWITCH_ID_6352_A1, "Marvell 88E6352 (A1)" },
 };
 
 static char *mv88e6352_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 9985a0c..ad29040 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2663,11 +2663,15 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
 int mv88e6xxx_setup_common(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int id;
 
 	ps->ds = ds;
 	mutex_init(&ps->smi_mutex);
 
-	ps->id = REG_READ(REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
+	id = REG_READ(REG_PORT(0), PORT_SWITCH_ID);
+
+	ps->id = id & 0xfff0;
+	ps->rev = id & 0xf;
 
 	INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
 
@@ -3083,19 +3087,8 @@ static char *mv88e6xxx_lookup_name(struct mii_bus *bus, int sw_addr,
 
 	/* Look up the exact switch ID */
 	for (i = 0; i < num; ++i)
-		if (table[i].id == ret)
-			return table[i].name;
-
-	/* Look up only the product number */
-	for (i = 0; i < num; ++i) {
-		if (table[i].id == (ret & PORT_SWITCH_ID_PROD_NUM_MASK)) {
-			dev_warn(&bus->dev,
-				 "unknown revision %d, using base switch 0x%x\n",
-				 ret & PORT_SWITCH_ID_REV_MASK,
-				 ret & PORT_SWITCH_ID_PROD_NUM_MASK);
+		if (table[i].id == (ret & 0xfff0))
 			return table[i].name;
-		}
-	}
 
 	return NULL;
 }
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 5d27dec..7304f88 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -84,18 +84,11 @@
 #define PORT_SWITCH_ID_6121	0x1040
 #define PORT_SWITCH_ID_6122	0x1050
 #define PORT_SWITCH_ID_6123	0x1210
-#define PORT_SWITCH_ID_6123_A1	0x1212
-#define PORT_SWITCH_ID_6123_A2	0x1213
 #define PORT_SWITCH_ID_6131	0x1060
-#define PORT_SWITCH_ID_6131_B2	0x1066
 #define PORT_SWITCH_ID_6152	0x1a40
 #define PORT_SWITCH_ID_6155	0x1a50
 #define PORT_SWITCH_ID_6161	0x1610
-#define PORT_SWITCH_ID_6161_A1	0x1612
-#define PORT_SWITCH_ID_6161_A2	0x1613
 #define PORT_SWITCH_ID_6165	0x1650
-#define PORT_SWITCH_ID_6165_A1	0x1652
-#define PORT_SWITCH_ID_6165_A2	0x1653
 #define PORT_SWITCH_ID_6171	0x1710
 #define PORT_SWITCH_ID_6172	0x1720
 #define PORT_SWITCH_ID_6175	0x1750
@@ -104,16 +97,10 @@
 #define PORT_SWITCH_ID_6185	0x1a70
 #define PORT_SWITCH_ID_6240	0x2400
 #define PORT_SWITCH_ID_6320	0x1150
-#define PORT_SWITCH_ID_6320_A1	0x1151
-#define PORT_SWITCH_ID_6320_A2	0x1152
 #define PORT_SWITCH_ID_6321	0x3100
-#define PORT_SWITCH_ID_6321_A1	0x3101
-#define PORT_SWITCH_ID_6321_A2	0x3102
 #define PORT_SWITCH_ID_6350	0x3710
 #define PORT_SWITCH_ID_6351	0x3750
 #define PORT_SWITCH_ID_6352	0x3520
-#define PORT_SWITCH_ID_6352_A0	0x3521
-#define PORT_SWITCH_ID_6352_A1	0x3522
 #define PORT_CONTROL		0x04
 #define PORT_CONTROL_USE_CORE_TAG	BIT(15)
 #define PORT_CONTROL_DROP_ON_LOCK	BIT(14)
@@ -397,6 +384,8 @@ struct mv88e6xxx_priv_port {
 };
 
 struct mv88e6xxx_priv_state {
+	int rev;
+
 	/* The dsa_switch this private structure is related to */
 	struct dsa_switch *ds;
 
-- 
2.8.0

Re: [PATCH net-next 2/7] net: dsa: mv88e6xxx: drop revision probing

[Andrew Lunn]

> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -2663,11 +2663,15 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
>  int mv88e6xxx_setup_common(struct dsa_switch *ds)
>  {
>  	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +	int id;
>  
>  	ps->ds = ds;
>  	mutex_init(&ps->smi_mutex);
>  
> -	ps->id = REG_READ(REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
> +	id = REG_READ(REG_PORT(0), PORT_SWITCH_ID);
> +
> +	ps->id = id & 0xfff0;
> +	ps->rev = id & 0xf;

ps->rev is not actually used anywhere. Drop it.

	Andrew

[PATCH net-next 3/7] net: dsa: mv88e6xxx: add switch info

[Vivien Didelot]

Add a new switch info structure which will be later extended to store
switch models static information, such as product number, name, number
of ports, number of databases, etc.

Merge the lookup function in the probing code, so that we avoid multiple
checking of the MII bus, as well a multiple ID reading.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123.c |  8 +++---
 drivers/net/dsa/mv88e6131.c | 10 +++----
 drivers/net/dsa/mv88e6171.c | 10 +++----
 drivers/net/dsa/mv88e6352.c | 14 +++++-----
 drivers/net/dsa/mv88e6xxx.c | 67 +++++++++++++++++++--------------------------
 drivers/net/dsa/mv88e6xxx.h | 14 ++++++----
 6 files changed, 58 insertions(+), 65 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 00c1121..02bf16c 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -17,10 +17,10 @@
 #include <net/dsa.h>
 #include "mv88e6xxx.h"
 
-static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
-	{ PORT_SWITCH_ID_6123, "Marvell 88E6123" },
-	{ PORT_SWITCH_ID_6161, "Marvell 88E6161" },
-	{ PORT_SWITCH_ID_6165, "Marvell 88E6165" },
+static const struct mv88e6xxx_info mv88e6123_table[] = {
+	{ MV88E6XXX_INFO(0x121, "Marvell 88E6123") },
+	{ MV88E6XXX_INFO(0x161, "Marvell 88E6161") },
+	{ MV88E6XXX_INFO(0x165, "Marvell 88E6165") },
 };
 
 static char *mv88e6123_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index df534da..27dd102 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -17,11 +17,11 @@
 #include <net/dsa.h>
 #include "mv88e6xxx.h"
 
-static const struct mv88e6xxx_switch_id mv88e6131_table[] = {
-	{ PORT_SWITCH_ID_6085, "Marvell 88E6085" },
-	{ PORT_SWITCH_ID_6095, "Marvell 88E6095/88E6095F" },
-	{ PORT_SWITCH_ID_6131, "Marvell 88E6131" },
-	{ PORT_SWITCH_ID_6185, "Marvell 88E6185" },
+static const struct mv88e6xxx_info mv88e6131_table[] = {
+	{ MV88E6XXX_INFO(0x095, "Marvell 88E6095/88E6095F") },
+	{ MV88E6XXX_INFO(0x04a, "Marvell 88E6085") },
+	{ MV88E6XXX_INFO(0x106, "Marvell 88E6131") },
+	{ MV88E6XXX_INFO(0x1a7, "Marvell 88E6185") },
 };
 
 static char *mv88e6131_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 40222b0..334e097 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -17,11 +17,11 @@
 #include <net/dsa.h>
 #include "mv88e6xxx.h"
 
-static const struct mv88e6xxx_switch_id mv88e6171_table[] = {
-	{ PORT_SWITCH_ID_6171, "Marvell 88E6171" },
-	{ PORT_SWITCH_ID_6175, "Marvell 88E6175" },
-	{ PORT_SWITCH_ID_6350, "Marvell 88E6350" },
-	{ PORT_SWITCH_ID_6351, "Marvell 88E6351" },
+static const struct mv88e6xxx_info mv88e6171_table[] = {
+	{ MV88E6XXX_INFO(0x171, "Marvell 88E6171") },
+	{ MV88E6XXX_INFO(0x175, "Marvell 88E6175") },
+	{ MV88E6XXX_INFO(0x371, "Marvell 88E6350") },
+	{ MV88E6XXX_INFO(0x375, "Marvell 88E6351") },
 };
 
 static char *mv88e6171_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 30fc5f6..371edd7 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -22,13 +22,13 @@
 #include <net/dsa.h>
 #include "mv88e6xxx.h"
 
-static const struct mv88e6xxx_switch_id mv88e6352_table[] = {
-	{ PORT_SWITCH_ID_6172, "Marvell 88E6172" },
-	{ PORT_SWITCH_ID_6176, "Marvell 88E6176" },
-	{ PORT_SWITCH_ID_6240, "Marvell 88E6240" },
-	{ PORT_SWITCH_ID_6320, "Marvell 88E6320" },
-	{ PORT_SWITCH_ID_6321, "Marvell 88E6321" },
-	{ PORT_SWITCH_ID_6352, "Marvell 88E6352" },
+static const struct mv88e6xxx_info mv88e6352_table[] = {
+	{ MV88E6XXX_INFO(0x115, "Marvell 88E6320") },
+	{ MV88E6XXX_INFO(0x310, "Marvell 88E6321") },
+	{ MV88E6XXX_INFO(0x172, "Marvell 88E6172") },
+	{ MV88E6XXX_INFO(0x176, "Marvell 88E6176") },
+	{ MV88E6XXX_INFO(0x240, "Marvell 88E6240") },
+	{ MV88E6XXX_INFO(0x352, "Marvell 88E6352") },
 };
 
 static char *mv88e6352_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ad29040..48fd1f4 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2663,16 +2663,10 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
 int mv88e6xxx_setup_common(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int id;
 
 	ps->ds = ds;
 	mutex_init(&ps->smi_mutex);
 
-	id = REG_READ(REG_PORT(0), PORT_SWITCH_ID);
-
-	ps->id = id & 0xfff0;
-	ps->rev = id & 0xf;
-
 	INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
 
 	return 0;
@@ -3072,51 +3066,46 @@ int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
 }
 #endif /* CONFIG_NET_DSA_HWMON */
 
-static char *mv88e6xxx_lookup_name(struct mii_bus *bus, int sw_addr,
-				   const struct mv88e6xxx_switch_id *table,
-				   unsigned int num)
+char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
+			  int sw_addr, void **priv,
+			  const struct mv88e6xxx_info *table, unsigned int num)
 {
-	int i, ret;
+	const struct mv88e6xxx_info *info;
+	struct mv88e6xxx_priv_state *ps;
+	struct mii_bus *bus;
+	int id, i;
 
+	bus = dsa_host_dev_to_mii_bus(host_dev);
 	if (!bus)
 		return NULL;
 
-	ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
-	if (ret < 0)
+	id = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
+	if (id < 0)
 		return NULL;
 
-	/* Look up the exact switch ID */
-	for (i = 0; i < num; ++i)
-		if (table[i].id == (ret & 0xfff0))
-			return table[i].name;
+	for (i = 0, info = &table[i]; i < num; info = &table[++i])
+		if (info->prod_num == (id & 0xfff0) >> 4)
+			goto found;
 
 	return NULL;
-}
 
-char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
-			  int sw_addr, void **priv,
-			  const struct mv88e6xxx_switch_id *table,
-			  unsigned int num)
-{
-	struct mv88e6xxx_priv_state *ps;
-	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
-	char *name;
-
-	if (!bus)
+found:
+	ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
+	if (!ps)
 		return NULL;
 
-	name = mv88e6xxx_lookup_name(bus, sw_addr, table, num);
-	if (name) {
-		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
-		if (!ps)
-			return NULL;
-		*priv = ps;
-		ps->bus = dsa_host_dev_to_mii_bus(host_dev);
-		if (!ps->bus)
-			return NULL;
-		ps->sw_addr = sw_addr;
-	}
-	return name;
+	*priv = ps;
+
+	ps->bus = bus;
+	ps->sw_addr = sw_addr;
+	ps->info = info;
+	ps->id = id & 0xfff0;
+	ps->rev = id & 0xf;
+
+	dev_info(&ps->bus->dev, "found switch %s, revision %u\n",
+		 ps->info->name, ps->rev);
+
+	return (char *) ps->info->name;
 }
 
 static int __init mv88e6xxx_init(void)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 7304f88..5556098 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -354,11 +354,15 @@
 
 #define MV88E6XXX_N_FID		4096
 
-struct mv88e6xxx_switch_id {
-	u16 id;
-	char *name;
+struct mv88e6xxx_info {
+	u16 prod_num;
+	const char *name;
 };
 
+#define MV88E6XXX_INFO(_prod_num, _name) \
+	.prod_num = _prod_num, \
+	.name = _name, \
+
 struct mv88e6xxx_atu_entry {
 	u16	fid;
 	u8	state;
@@ -384,6 +388,7 @@ struct mv88e6xxx_priv_port {
 };
 
 struct mv88e6xxx_priv_state {
+	const struct mv88e6xxx_info *info;
 	int rev;
 
 	/* The dsa_switch this private structure is related to */
@@ -453,8 +458,7 @@ struct mv88e6xxx_hw_stat {
 int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active);
 char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
 			  int sw_addr, void **priv,
-			  const struct mv88e6xxx_switch_id *table,
-			  unsigned int num);
+			  const struct mv88e6xxx_info *table, unsigned int num);
 
 int mv88e6xxx_setup_ports(struct dsa_switch *ds);
 int mv88e6xxx_setup_common(struct dsa_switch *ds);
-- 
2.8.0

Re: [PATCH net-next 3/7] net: dsa: mv88e6xxx: add switch info

[Andrew Lunn]

On Fri, Apr 15, 2016 at 02:25:46PM -0400, Vivien Didelot wrote:
> Add a new switch info structure which will be later extended to store
> switch models static information, such as product number, name, number
> of ports, number of databases, etc.
> 
> Merge the lookup function in the probing code, so that we avoid multiple
> checking of the MII bus, as well a multiple ID reading.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6123.c |  8 +++---
>  drivers/net/dsa/mv88e6131.c | 10 +++----
>  drivers/net/dsa/mv88e6171.c | 10 +++----
>  drivers/net/dsa/mv88e6352.c | 14 +++++-----
>  drivers/net/dsa/mv88e6xxx.c | 67 +++++++++++++++++++--------------------------
>  drivers/net/dsa/mv88e6xxx.h | 14 ++++++----
>  6 files changed, 58 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> index 00c1121..02bf16c 100644
> --- a/drivers/net/dsa/mv88e6123.c
> +++ b/drivers/net/dsa/mv88e6123.c
> @@ -17,10 +17,10 @@
>  #include <net/dsa.h>
>  #include "mv88e6xxx.h"
>  
> -static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
> -	{ PORT_SWITCH_ID_6123, "Marvell 88E6123" },
> -	{ PORT_SWITCH_ID_6161, "Marvell 88E6161" },
> -	{ PORT_SWITCH_ID_6165, "Marvell 88E6165" },
> +static const struct mv88e6xxx_info mv88e6123_table[] = {
> +	{ MV88E6XXX_INFO(0x121, "Marvell 88E6123") },
> +	{ MV88E6XXX_INFO(0x161, "Marvell 88E6161") },
> +	{ MV88E6XXX_INFO(0x165, "Marvell 88E6165") },

Why replace PORT_SWITCH_ID_6123 with the magic number 0x121?

    Andrew

[PATCH net-next 4/7] net: dsa: mv88e6xxx: add family to info

[Vivien Didelot]

Add an mv88e6xxx_family enum to the info structure for better family
indentification.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123.c |  6 ++--
 drivers/net/dsa/mv88e6131.c |  8 ++---
 drivers/net/dsa/mv88e6171.c |  8 ++---
 drivers/net/dsa/mv88e6352.c | 12 ++++----
 drivers/net/dsa/mv88e6xxx.c | 71 +++++----------------------------------------
 drivers/net/dsa/mv88e6xxx.h | 16 +++++++++-
 6 files changed, 40 insertions(+), 81 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 02bf16c..36a0340 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -18,9 +18,9 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6123_table[] = {
-	{ MV88E6XXX_INFO(0x121, "Marvell 88E6123") },
-	{ MV88E6XXX_INFO(0x161, "Marvell 88E6161") },
-	{ MV88E6XXX_INFO(0x165, "Marvell 88E6165") },
+	{ MV88E6XXX_INFO(6165, 0x121, "Marvell 88E6123") },
+	{ MV88E6XXX_INFO(6165, 0x161, "Marvell 88E6161") },
+	{ MV88E6XXX_INFO(6165, 0x165, "Marvell 88E6165") },
 };
 
 static char *mv88e6123_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 27dd102..f75d2dd 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -18,10 +18,10 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6131_table[] = {
-	{ MV88E6XXX_INFO(0x095, "Marvell 88E6095/88E6095F") },
-	{ MV88E6XXX_INFO(0x04a, "Marvell 88E6085") },
-	{ MV88E6XXX_INFO(0x106, "Marvell 88E6131") },
-	{ MV88E6XXX_INFO(0x1a7, "Marvell 88E6185") },
+	{ MV88E6XXX_INFO(6095, 0x095, "Marvell 88E6095/88E6095F") },
+	{ MV88E6XXX_INFO(6097, 0x04a, "Marvell 88E6085") },
+	{ MV88E6XXX_INFO(6185, 0x106, "Marvell 88E6131") },
+	{ MV88E6XXX_INFO(6185, 0x1a7, "Marvell 88E6185") },
 };
 
 static char *mv88e6131_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 334e097..cb5bb19 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -18,10 +18,10 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6171_table[] = {
-	{ MV88E6XXX_INFO(0x171, "Marvell 88E6171") },
-	{ MV88E6XXX_INFO(0x175, "Marvell 88E6175") },
-	{ MV88E6XXX_INFO(0x371, "Marvell 88E6350") },
-	{ MV88E6XXX_INFO(0x375, "Marvell 88E6351") },
+	{ MV88E6XXX_INFO(6351, 0x171, "Marvell 88E6171") },
+	{ MV88E6XXX_INFO(6351, 0x175, "Marvell 88E6175") },
+	{ MV88E6XXX_INFO(6351, 0x371, "Marvell 88E6350") },
+	{ MV88E6XXX_INFO(6351, 0x375, "Marvell 88E6351") },
 };
 
 static char *mv88e6171_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 371edd7..94db0c3 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -23,12 +23,12 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6352_table[] = {
-	{ MV88E6XXX_INFO(0x115, "Marvell 88E6320") },
-	{ MV88E6XXX_INFO(0x310, "Marvell 88E6321") },
-	{ MV88E6XXX_INFO(0x172, "Marvell 88E6172") },
-	{ MV88E6XXX_INFO(0x176, "Marvell 88E6176") },
-	{ MV88E6XXX_INFO(0x240, "Marvell 88E6240") },
-	{ MV88E6XXX_INFO(0x352, "Marvell 88E6352") },
+	{ MV88E6XXX_INFO(6320, 0x115, "Marvell 88E6320") },
+	{ MV88E6XXX_INFO(6320, 0x310, "Marvell 88E6321") },
+	{ MV88E6XXX_INFO(6352, 0x172, "Marvell 88E6172") },
+	{ MV88E6XXX_INFO(6352, 0x176, "Marvell 88E6176") },
+	{ MV88E6XXX_INFO(6352, 0x240, "Marvell 88E6240") },
+	{ MV88E6XXX_INFO(6352, 0x352, "Marvell 88E6352") },
 };
 
 static char *mv88e6352_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 48fd1f4..05d59f9 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -369,111 +369,56 @@ static bool mv88e6xxx_6065_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6031:
-	case PORT_SWITCH_ID_6061:
-	case PORT_SWITCH_ID_6035:
-	case PORT_SWITCH_ID_6065:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6065;
 }
 
 static bool mv88e6xxx_6095_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6092:
-	case PORT_SWITCH_ID_6095:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6095;
 }
 
 static bool mv88e6xxx_6097_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6046:
-	case PORT_SWITCH_ID_6085:
-	case PORT_SWITCH_ID_6096:
-	case PORT_SWITCH_ID_6097:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6097;
 }
 
 static bool mv88e6xxx_6165_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6123:
-	case PORT_SWITCH_ID_6161:
-	case PORT_SWITCH_ID_6165:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6165;
 }
 
 static bool mv88e6xxx_6185_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6121:
-	case PORT_SWITCH_ID_6122:
-	case PORT_SWITCH_ID_6152:
-	case PORT_SWITCH_ID_6155:
-	case PORT_SWITCH_ID_6182:
-	case PORT_SWITCH_ID_6185:
-	case PORT_SWITCH_ID_6108:
-	case PORT_SWITCH_ID_6131:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6185;
 }
 
 static bool mv88e6xxx_6320_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6320:
-	case PORT_SWITCH_ID_6321:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6320;
 }
 
 static bool mv88e6xxx_6351_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6171:
-	case PORT_SWITCH_ID_6175:
-	case PORT_SWITCH_ID_6350:
-	case PORT_SWITCH_ID_6351:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6351;
 }
 
 static bool mv88e6xxx_6352_family(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6172:
-	case PORT_SWITCH_ID_6176:
-	case PORT_SWITCH_ID_6240:
-	case PORT_SWITCH_ID_6352:
-		return true;
-	}
-	return false;
+	return ps->info->family == MV88E6XXX_FAMILY_6352;
 }
 
 static unsigned int mv88e6xxx_num_databases(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 5556098..3d2a186 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -352,14 +352,28 @@
 #define GLOBAL2_QOS_WEIGHT	0x1c
 #define GLOBAL2_MISC		0x1d
 
+enum mv88e6xxx_family {
+	MV88E6XXX_FAMILY_NONE,
+	MV88E6XXX_FAMILY_6065,	/* 6031 6035 6061 6065 */
+	MV88E6XXX_FAMILY_6095,	/* 6092 6095 */
+	MV88E6XXX_FAMILY_6097,	/* 6046 6085 6096 6097 */
+	MV88E6XXX_FAMILY_6165,	/* 6123 6161 6165 */
+	MV88E6XXX_FAMILY_6185,	/* 6108 6121 6122 6131 6152 6155 6182 6185 */
+	MV88E6XXX_FAMILY_6320,	/* 6320 6321 */
+	MV88E6XXX_FAMILY_6351,	/* 6171 6175 6350 6351 */
+	MV88E6XXX_FAMILY_6352,	/* 6172 6176 6240 6352 */
+};
+
 #define MV88E6XXX_N_FID		4096
 
 struct mv88e6xxx_info {
+	enum mv88e6xxx_family family;
 	u16 prod_num;
 	const char *name;
 };
 
-#define MV88E6XXX_INFO(_prod_num, _name) \
+#define MV88E6XXX_INFO(_family, _prod_num, _name) \
+	.family = MV88E6XXX_FAMILY_##_family, \
 	.prod_num = _prod_num, \
 	.name = _name, \
 
-- 
2.8.0

Re: [PATCH net-next 4/7] net: dsa: mv88e6xxx: add family to info

[Andrew Lunn]

On Fri, Apr 15, 2016 at 02:25:47PM -0400, Vivien Didelot wrote:
> Add an mv88e6xxx_family enum to the info structure for better family
> indentification.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6123.c |  6 ++--
>  drivers/net/dsa/mv88e6131.c |  8 ++---
>  drivers/net/dsa/mv88e6171.c |  8 ++---
>  drivers/net/dsa/mv88e6352.c | 12 ++++----
>  drivers/net/dsa/mv88e6xxx.c | 71 +++++----------------------------------------
>  drivers/net/dsa/mv88e6xxx.h | 16 +++++++++-
>  6 files changed, 40 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> index 02bf16c..36a0340 100644
> --- a/drivers/net/dsa/mv88e6123.c
> +++ b/drivers/net/dsa/mv88e6123.c
> @@ -18,9 +18,9 @@
>  #include "mv88e6xxx.h"
>  
>  static const struct mv88e6xxx_info mv88e6123_table[] = {
> -	{ MV88E6XXX_INFO(0x121, "Marvell 88E6123") },
> -	{ MV88E6XXX_INFO(0x161, "Marvell 88E6161") },
> -	{ MV88E6XXX_INFO(0x165, "Marvell 88E6165") },
> +	{ MV88E6XXX_INFO(6165, 0x121, "Marvell 88E6123") },
> +	{ MV88E6XXX_INFO(6165, 0x161, "Marvell 88E6161") },
> +	{ MV88E6XXX_INFO(6165, 0x165, "Marvell 88E6165") },

I think 

> +	{ MV88E6XXX_INFO(MV88E6XXX_FAMILY_6165, 0x165, "Marvell 88E6165") },

is clearer. It is hard to know what these values mean unless you go
look at the macro.

     Andrew

Re: [PATCH net-next 4/7] net: dsa: mv88e6xxx: add family to info

[Vivien Didelot]

Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:

>>> +	{ MV88E6XXX_INFO(6165, 0x165, "Marvell 88E6165") },
>>
>> I think 
>>
>>> +	{ MV88E6XXX_INFO(MV88E6XXX_FAMILY_6165, 0x165, "Marvell 88E6165") },
>>
>> is clearer. It is hard to know what these values mean unless you go
>> look at the macro.
>
> Same goes for the MV88E6XXX_INFO macro... I wanted to avoid long lines
> while keeping the info table clear enough.
>
>     MV88E6XXX_INFO(0x121, "Marvell 88E6123",
>                    MV88E6XXX_FAMILY_6165,) },
>    /*             Family   Prod   Name             */
>    { MV88E6XXX_INFO(6165, 0x121, "Marvell 88E6123") },
>    { MV88E6XXX_INFO(6165, 0x161, "Marvell 88E6161") },
>    { MV88E6XXX_INFO(6165, 0x165, "Marvell 88E6165") },
>    { MV88E6XXX_INFO(6165, 0x165, "Marvell 88E6165") },
>
> But I don't really mind in fact, we'll do as you guys wish.

Oops, sent too fast. Thinking about that, I'll just keep plain struct
mv88e6xxx_info in the tables and we will maybe introduce such macro when
merging everything together.

Thanks,
Vivien

[PATCH net-next 5/7] net: dsa: mv88e6xxx: add number of ports to info

[Vivien Didelot]

Drop the ps->num_ports variable for a new member of the info structure.
This removes the need to assign it at setup time.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123.c | 19 +++----------------
 drivers/net/dsa/mv88e6131.c | 26 +++++---------------------
 drivers/net/dsa/mv88e6171.c | 11 ++++-------
 drivers/net/dsa/mv88e6352.c | 14 ++++++--------
 drivers/net/dsa/mv88e6xxx.c | 38 +++++++++++++++++++-------------------
 drivers/net/dsa/mv88e6xxx.h |  6 +++---
 6 files changed, 40 insertions(+), 74 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 36a0340..2048719 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -18,9 +18,9 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6123_table[] = {
-	{ MV88E6XXX_INFO(6165, 0x121, "Marvell 88E6123") },
-	{ MV88E6XXX_INFO(6165, 0x161, "Marvell 88E6161") },
-	{ MV88E6XXX_INFO(6165, 0x165, "Marvell 88E6165") },
+	{ MV88E6XXX_INFO(6165, 0x121, 3, "Marvell 88E6123") },
+	{ MV88E6XXX_INFO(6165, 0x161, 6, "Marvell 88E6161") },
+	{ MV88E6XXX_INFO(6165, 0x165, 6, "Marvell 88E6165") },
 };
 
 static char *mv88e6123_drv_probe(struct device *dsa_dev,
@@ -67,25 +67,12 @@ static int mv88e6123_setup_global(struct dsa_switch *ds)
 
 static int mv88e6123_setup(struct dsa_switch *ds)
 {
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6123:
-		ps->num_ports = 3;
-		break;
-	case PORT_SWITCH_ID_6161:
-	case PORT_SWITCH_ID_6165:
-		ps->num_ports = 6;
-		break;
-	default:
-		return -ENODEV;
-	}
-
 	ret = mv88e6xxx_switch_reset(ds, false);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index f75d2dd..2eb9fa3 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -18,10 +18,10 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6131_table[] = {
-	{ MV88E6XXX_INFO(6095, 0x095, "Marvell 88E6095/88E6095F") },
-	{ MV88E6XXX_INFO(6097, 0x04a, "Marvell 88E6085") },
-	{ MV88E6XXX_INFO(6185, 0x106, "Marvell 88E6131") },
-	{ MV88E6XXX_INFO(6185, 0x1a7, "Marvell 88E6185") },
+	{ MV88E6XXX_INFO(6095, 0x095, 11, "Marvell 88E6095/88E6095F") },
+	{ MV88E6XXX_INFO(6097, 0x04a, 10, "Marvell 88E6085") },
+	{ MV88E6XXX_INFO(6185, 0x106, 8,  "Marvell 88E6131") },
+	{ MV88E6XXX_INFO(6185, 0x1a7, 10, "Marvell 88E6185") },
 };
 
 static char *mv88e6131_drv_probe(struct device *dsa_dev,
@@ -90,7 +90,6 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
 
 static int mv88e6131_setup(struct dsa_switch *ds)
 {
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
 	ret = mv88e6xxx_setup_common(ds);
@@ -99,21 +98,6 @@ static int mv88e6131_setup(struct dsa_switch *ds)
 
 	mv88e6xxx_ppu_state_init(ds);
 
-	switch (ps->id) {
-	case PORT_SWITCH_ID_6085:
-	case PORT_SWITCH_ID_6185:
-		ps->num_ports = 10;
-		break;
-	case PORT_SWITCH_ID_6095:
-		ps->num_ports = 11;
-		break;
-	case PORT_SWITCH_ID_6131:
-		ps->num_ports = 8;
-		break;
-	default:
-		return -ENODEV;
-	}
-
 	ret = mv88e6xxx_switch_reset(ds, false);
 	if (ret < 0)
 		return ret;
@@ -129,7 +113,7 @@ static int mv88e6131_port_to_phy_addr(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	if (port >= 0 && port < ps->num_ports)
+	if (port >= 0 && port < ps->info->num_ports)
 		return port;
 
 	return -EINVAL;
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index cb5bb19..d5f33d7 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -18,10 +18,10 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6171_table[] = {
-	{ MV88E6XXX_INFO(6351, 0x171, "Marvell 88E6171") },
-	{ MV88E6XXX_INFO(6351, 0x175, "Marvell 88E6175") },
-	{ MV88E6XXX_INFO(6351, 0x371, "Marvell 88E6350") },
-	{ MV88E6XXX_INFO(6351, 0x375, "Marvell 88E6351") },
+	{ MV88E6XXX_INFO(6351, 0x171, 7, "Marvell 88E6171") },
+	{ MV88E6XXX_INFO(6351, 0x175, 7, "Marvell 88E6175") },
+	{ MV88E6XXX_INFO(6351, 0x371, 7, "Marvell 88E6350") },
+	{ MV88E6XXX_INFO(6351, 0x375, 7, "Marvell 88E6351") },
 };
 
 static char *mv88e6171_drv_probe(struct device *dsa_dev,
@@ -69,15 +69,12 @@ static int mv88e6171_setup_global(struct dsa_switch *ds)
 
 static int mv88e6171_setup(struct dsa_switch *ds)
 {
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
 
-	ps->num_ports = 7;
-
 	ret = mv88e6xxx_switch_reset(ds, true);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 94db0c3..e529c18 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -23,12 +23,12 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6352_table[] = {
-	{ MV88E6XXX_INFO(6320, 0x115, "Marvell 88E6320") },
-	{ MV88E6XXX_INFO(6320, 0x310, "Marvell 88E6321") },
-	{ MV88E6XXX_INFO(6352, 0x172, "Marvell 88E6172") },
-	{ MV88E6XXX_INFO(6352, 0x176, "Marvell 88E6176") },
-	{ MV88E6XXX_INFO(6352, 0x240, "Marvell 88E6240") },
-	{ MV88E6XXX_INFO(6352, 0x352, "Marvell 88E6352") },
+	{ MV88E6XXX_INFO(6320, 0x115, 7, "Marvell 88E6320") },
+	{ MV88E6XXX_INFO(6320, 0x310, 7, "Marvell 88E6321") },
+	{ MV88E6XXX_INFO(6352, 0x172, 7, "Marvell 88E6172") },
+	{ MV88E6XXX_INFO(6352, 0x176, 7, "Marvell 88E6176") },
+	{ MV88E6XXX_INFO(6352, 0x240, 7, "Marvell 88E6240") },
+	{ MV88E6XXX_INFO(6352, 0x352, 7, "Marvell 88E6352") },
 };
 
 static char *mv88e6352_drv_probe(struct device *dsa_dev,
@@ -82,8 +82,6 @@ static int mv88e6352_setup(struct dsa_switch *ds)
 	if (ret < 0)
 		return ret;
 
-	ps->num_ports = 7;
-
 	mutex_init(&ps->eeprom_mutex);
 
 	ret = mv88e6xxx_switch_reset(ds, true);
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 05d59f9..f3e8c68 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -518,7 +518,7 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
 		reg |= PORT_PCS_CTRL_DUPLEX_FULL;
 
 	if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) &&
-	    (port >= ps->num_ports - 2)) {
+	    (port >= ps->info->num_ports - 2)) {
 		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
 			reg |= PORT_PCS_CTRL_RGMII_DELAY_RXCLK;
 		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
@@ -1099,7 +1099,7 @@ static int _mv88e6xxx_port_based_vlan_map(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	struct net_device *bridge = ps->ports[port].bridge_dev;
-	const u16 mask = (1 << ps->num_ports) - 1;
+	const u16 mask = (1 << ps->info->num_ports) - 1;
 	u16 output_ports = 0;
 	int reg;
 	int i;
@@ -1108,7 +1108,7 @@ static int _mv88e6xxx_port_based_vlan_map(struct dsa_switch *ds, int port)
 	if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
 		output_ports = mask;
 	} else {
-		for (i = 0; i < ps->num_ports; ++i) {
+		for (i = 0; i < ps->info->num_ports; ++i) {
 			/* allow sending frames to every group member */
 			if (bridge && ps->ports[i].bridge_dev == bridge)
 				output_ports |= BIT(i);
@@ -1249,7 +1249,7 @@ static int _mv88e6xxx_vtu_stu_data_read(struct dsa_switch *ds,
 		regs[i] = ret;
 	}
 
-	for (i = 0; i < ps->num_ports; ++i) {
+	for (i = 0; i < ps->info->num_ports; ++i) {
 		unsigned int shift = (i % 4) * 4 + nibble_offset;
 		u16 reg = regs[i / 4];
 
@@ -1268,7 +1268,7 @@ static int _mv88e6xxx_vtu_stu_data_write(struct dsa_switch *ds,
 	int i;
 	int ret;
 
-	for (i = 0; i < ps->num_ports; ++i) {
+	for (i = 0; i < ps->info->num_ports; ++i) {
 		unsigned int shift = (i % 4) * 4 + nibble_offset;
 		u8 data = entry->data[i];
 
@@ -1600,7 +1600,7 @@ static int _mv88e6xxx_fid_new(struct dsa_switch *ds, u16 *fid)
 	bitmap_zero(fid_bitmap, MV88E6XXX_N_FID);
 
 	/* Set every FID bit used by the (un)bridged ports */
-	for (i = 0; i < ps->num_ports; ++i) {
+	for (i = 0; i < ps->info->num_ports; ++i) {
 		err = _mv88e6xxx_port_fid_get(ds, i, fid);
 		if (err)
 			return err;
@@ -1650,7 +1650,7 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
 		return err;
 
 	/* exclude all ports except the CPU and DSA ports */
-	for (i = 0; i < ps->num_ports; ++i)
+	for (i = 0; i < ps->info->num_ports; ++i)
 		vlan.data[i] = dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i)
 			? GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED
 			: GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
@@ -1739,7 +1739,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 		if (vlan.vid > vid_end)
 			break;
 
-		for (i = 0; i < ps->num_ports; ++i) {
+		for (i = 0; i < ps->info->num_ports; ++i) {
 			if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
 				continue;
 
@@ -1888,7 +1888,7 @@ static int _mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, u16 vid)
 
 	/* keep the VLAN unless all ports are excluded */
 	vlan.valid = false;
-	for (i = 0; i < ps->num_ports; ++i) {
+	for (i = 0; i < ps->info->num_ports; ++i) {
 		if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i))
 			continue;
 
@@ -2197,11 +2197,11 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 	mutex_lock(&ps->smi_mutex);
 
 	/* Get or create the bridge FID and assign it to the port */
-	for (i = 0; i < ps->num_ports; ++i)
+	for (i = 0; i < ps->info->num_ports; ++i)
 		if (ps->ports[i].bridge_dev == bridge)
 			break;
 
-	if (i < ps->num_ports)
+	if (i < ps->info->num_ports)
 		err = _mv88e6xxx_port_fid_get(ds, i, &fid);
 	else
 		err = _mv88e6xxx_fid_new(ds, &fid);
@@ -2215,7 +2215,7 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 	/* Assign the bridge and remap each port's VLANTable */
 	ps->ports[port].bridge_dev = bridge;
 
-	for (i = 0; i < ps->num_ports; ++i) {
+	for (i = 0; i < ps->info->num_ports; ++i) {
 		if (ps->ports[i].bridge_dev == bridge) {
 			err = _mv88e6xxx_port_based_vlan_map(ds, i);
 			if (err)
@@ -2246,7 +2246,7 @@ void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
 	/* Unassign the bridge and remap each port's VLANTable */
 	ps->ports[port].bridge_dev = NULL;
 
-	for (i = 0; i < ps->num_ports; ++i)
+	for (i = 0; i < ps->info->num_ports; ++i)
 		if (i == port || ps->ports[i].bridge_dev == bridge)
 			if (_mv88e6xxx_port_based_vlan_map(ds, i))
 				netdev_warn(ds->ports[i], "failed to remap\n");
@@ -2265,7 +2265,7 @@ static void mv88e6xxx_bridge_work(struct work_struct *work)
 
 	mutex_lock(&ps->smi_mutex);
 
-	for (port = 0; port < ps->num_ports; ++port)
+	for (port = 0; port < ps->info->num_ports; ++port)
 		if (test_and_clear_bit(port, ps->port_state_update_mask) &&
 		    _mv88e6xxx_port_state(ds, port, ps->ports[port].state))
 			netdev_warn(ds->ports[port], "failed to update state to %s\n",
@@ -2597,7 +2597,7 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
 	int ret;
 	int i;
 
-	for (i = 0; i < ps->num_ports; i++) {
+	for (i = 0; i < ps->info->num_ports; i++) {
 		ret = mv88e6xxx_setup_port(ds, i);
 		if (ret < 0)
 			return ret;
@@ -2675,7 +2675,7 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
 	for (i = 0; i < 8; i++)
 		REG_WRITE(REG_GLOBAL2, GLOBAL2_TRUNK_MASK,
 			  0x8000 | (i << GLOBAL2_TRUNK_MASK_NUM_SHIFT) |
-			  ((1 << ps->num_ports) - 1));
+			  ((1 << ps->info->num_ports) - 1));
 
 	/* Clear all trunk mappings. */
 	for (i = 0; i < 16; i++)
@@ -2710,7 +2710,7 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
 		 * ingress rate limit registers to their initial
 		 * state.
 		 */
-		for (i = 0; i < ps->num_ports; i++)
+		for (i = 0; i < ps->info->num_ports; i++)
 			REG_WRITE(REG_GLOBAL2, GLOBAL2_INGRESS_OP,
 				  0x9000 | (i << 8));
 	}
@@ -2747,7 +2747,7 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
 	int i;
 
 	/* Set all ports to the disabled state. */
-	for (i = 0; i < ps->num_ports; i++) {
+	for (i = 0; i < ps->info->num_ports; i++) {
 		ret = REG_READ(REG_PORT(i), PORT_CONTROL);
 		REG_WRITE(REG_PORT(i), PORT_CONTROL, ret & 0xfffc);
 	}
@@ -2815,7 +2815,7 @@ static int mv88e6xxx_port_to_phy_addr(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	if (port >= 0 && port < ps->num_ports)
+	if (port >= 0 && port < ps->info->num_ports)
 		return port;
 	return -EINVAL;
 }
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 3d2a186..6b2d62e 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -369,12 +369,14 @@ enum mv88e6xxx_family {
 struct mv88e6xxx_info {
 	enum mv88e6xxx_family family;
 	u16 prod_num;
+	unsigned int num_ports;
 	const char *name;
 };
 
-#define MV88E6XXX_INFO(_family, _prod_num, _name) \
+#define MV88E6XXX_INFO(_family, _prod_num, _num_ports, _name) \
 	.family = MV88E6XXX_FAMILY_##_family, \
 	.prod_num = _prod_num, \
+	.num_ports = _num_ports, \
 	.name = _name, \
 
 struct mv88e6xxx_atu_entry {
@@ -447,8 +449,6 @@ struct mv88e6xxx_priv_state {
 	struct mutex eeprom_mutex;
 
 	int		id; /* switch product id */
-	int		num_ports;	/* number of switch ports */
-
 	struct mv88e6xxx_priv_port	ports[DSA_MAX_PORTS];
 
 	DECLARE_BITMAP(port_state_update_mask, DSA_MAX_PORTS);
-- 
2.8.0

[PATCH net-next 6/7] net: dsa: mv88e6xxx: add number of database to info

[Vivien Didelot]

Move the number of databases to the info structure.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123.c |  6 +++---
 drivers/net/dsa/mv88e6131.c |  8 ++++----
 drivers/net/dsa/mv88e6171.c |  8 ++++----
 drivers/net/dsa/mv88e6352.c | 12 ++++++------
 drivers/net/dsa/mv88e6xxx.c | 19 +------------------
 drivers/net/dsa/mv88e6xxx.h |  4 +++-
 6 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 2048719..9731e56 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -18,9 +18,9 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6123_table[] = {
-	{ MV88E6XXX_INFO(6165, 0x121, 3, "Marvell 88E6123") },
-	{ MV88E6XXX_INFO(6165, 0x161, 6, "Marvell 88E6161") },
-	{ MV88E6XXX_INFO(6165, 0x165, 6, "Marvell 88E6165") },
+	{ MV88E6XXX_INFO(6165, 0x121, 3, 4096, "Marvell 88E6123") },
+	{ MV88E6XXX_INFO(6165, 0x161, 6, 4096, "Marvell 88E6161") },
+	{ MV88E6XXX_INFO(6165, 0x165, 6, 4096, "Marvell 88E6165") },
 };
 
 static char *mv88e6123_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 2eb9fa3..c507ecb 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -18,10 +18,10 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6131_table[] = {
-	{ MV88E6XXX_INFO(6095, 0x095, 11, "Marvell 88E6095/88E6095F") },
-	{ MV88E6XXX_INFO(6097, 0x04a, 10, "Marvell 88E6085") },
-	{ MV88E6XXX_INFO(6185, 0x106, 8,  "Marvell 88E6131") },
-	{ MV88E6XXX_INFO(6185, 0x1a7, 10, "Marvell 88E6185") },
+	{ MV88E6XXX_INFO(6095, 0x095, 11, 256,  "Marvell 88E6095/88E6095F") },
+	{ MV88E6XXX_INFO(6097, 0x04a, 10, 4096, "Marvell 88E6085") },
+	{ MV88E6XXX_INFO(6185, 0x106, 8,  256,  "Marvell 88E6131") },
+	{ MV88E6XXX_INFO(6185, 0x1a7, 10, 256,  "Marvell 88E6185") },
 };
 
 static char *mv88e6131_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index d5f33d7..bce8f40 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -18,10 +18,10 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6171_table[] = {
-	{ MV88E6XXX_INFO(6351, 0x171, 7, "Marvell 88E6171") },
-	{ MV88E6XXX_INFO(6351, 0x175, 7, "Marvell 88E6175") },
-	{ MV88E6XXX_INFO(6351, 0x371, 7, "Marvell 88E6350") },
-	{ MV88E6XXX_INFO(6351, 0x375, 7, "Marvell 88E6351") },
+	{ MV88E6XXX_INFO(6351, 0x171, 7, 4096, "Marvell 88E6171") },
+	{ MV88E6XXX_INFO(6351, 0x175, 7, 4096, "Marvell 88E6175") },
+	{ MV88E6XXX_INFO(6351, 0x371, 7, 4096, "Marvell 88E6350") },
+	{ MV88E6XXX_INFO(6351, 0x375, 7, 4096, "Marvell 88E6351") },
 };
 
 static char *mv88e6171_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index e529c18..c26d674 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -23,12 +23,12 @@
 #include "mv88e6xxx.h"
 
 static const struct mv88e6xxx_info mv88e6352_table[] = {
-	{ MV88E6XXX_INFO(6320, 0x115, 7, "Marvell 88E6320") },
-	{ MV88E6XXX_INFO(6320, 0x310, 7, "Marvell 88E6321") },
-	{ MV88E6XXX_INFO(6352, 0x172, 7, "Marvell 88E6172") },
-	{ MV88E6XXX_INFO(6352, 0x176, 7, "Marvell 88E6176") },
-	{ MV88E6XXX_INFO(6352, 0x240, 7, "Marvell 88E6240") },
-	{ MV88E6XXX_INFO(6352, 0x352, 7, "Marvell 88E6352") },
+	{ MV88E6XXX_INFO(6320, 0x115, 7, 4096, "Marvell 88E6320") },
+	{ MV88E6XXX_INFO(6320, 0x310, 7, 4096, "Marvell 88E6321") },
+	{ MV88E6XXX_INFO(6352, 0x172, 7, 4096, "Marvell 88E6172") },
+	{ MV88E6XXX_INFO(6352, 0x176, 7, 4096, "Marvell 88E6176") },
+	{ MV88E6XXX_INFO(6352, 0x240, 7, 4096, "Marvell 88E6240") },
+	{ MV88E6XXX_INFO(6352, 0x352, 7, 4096, "Marvell 88E6352") },
 };
 
 static char *mv88e6352_drv_probe(struct device *dsa_dev,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index f3e8c68..d40ac4d 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -425,24 +425,7 @@ static unsigned int mv88e6xxx_num_databases(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	/* The following devices have 4-bit identifiers for 16 databases */
-	if (ps->id == PORT_SWITCH_ID_6061)
-		return 16;
-
-	/* The following devices have 6-bit identifiers for 64 databases */
-	if (ps->id == PORT_SWITCH_ID_6065)
-		return 64;
-
-	/* The following devices have 8-bit identifiers for 256 databases */
-	if (mv88e6xxx_6095_family(ds) || mv88e6xxx_6185_family(ds))
-		return 256;
-
-	/* The following devices have 12-bit identifiers for 4096 databases */
-	if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
-	    mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds))
-		return 4096;
-
-	return 0;
+	return ps->info->num_databases;
 }
 
 static bool mv88e6xxx_has_fid_reg(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 6b2d62e..7e86cc6 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -370,13 +370,15 @@ struct mv88e6xxx_info {
 	enum mv88e6xxx_family family;
 	u16 prod_num;
 	unsigned int num_ports;
+	unsigned int num_databases;
 	const char *name;
 };
 
-#define MV88E6XXX_INFO(_family, _prod_num, _num_ports, _name) \
+#define MV88E6XXX_INFO(_family, _prod_num, _num_ports, _num_db, _name) \
 	.family = MV88E6XXX_FAMILY_##_family, \
 	.prod_num = _prod_num, \
 	.num_ports = _num_ports, \
+	.num_databases = _num_db, \
 	.name = _name, \
 
 struct mv88e6xxx_atu_entry {
-- 
2.8.0

Re: [PATCH net-next 6/7] net: dsa: mv88e6xxx: add number of database to info

[Andrew Lunn]

On Fri, Apr 15, 2016 at 02:25:49PM -0400, Vivien Didelot wrote:
> Move the number of databases to the info structure.

Isn't the number of databases a property of the family?

I would add a table indexed by family.

  Andrew

Re: [PATCH net-next 6/7] net: dsa: mv88e6xxx: add number of database to info

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Fri, Apr 15, 2016 at 02:25:49PM -0400, Vivien Didelot wrote:
>> Move the number of databases to the info structure.
>
> Isn't the number of databases a property of the family?

No.

We've seen [1] for instance that 6061 and 6065 are both part of the 6065
family, but 6061 has 16 databases while 6065 has 64 databases.

> I would add a table indexed by family.

I'm not even sure every Marvell switch has a family. I think we really
want a table of supported *devices*.

To go further, with the coming feature-based logic, checking the family
might not even be that valuable.

[1] https://lkml.org/lkml/2016/3/26/155

Thanks,
Vivien

Re: [PATCH net-next 6/7] net: dsa: mv88e6xxx: add number of database to info

[Andrew Lunn]

On Fri, Apr 15, 2016 at 04:24:26PM -0400, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > On Fri, Apr 15, 2016 at 02:25:49PM -0400, Vivien Didelot wrote:
> >> Move the number of databases to the info structure.
> >
> > Isn't the number of databases a property of the family?
> 
> No.

O.K, then lets put it in the device info table.

     Andrew

[PATCH net-next 7/7] net: dsa: mv88e6xxx: drop switch id

[Vivien Didelot]

We already have the product number and revision stored in the info
structure and the switch private state.

It is not necessary to clutter the header file with shifted product
number for devices that we don't even support yet. Remove them.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c |  1 -
 drivers/net/dsa/mv88e6xxx.h | 34 ----------------------------------
 2 files changed, 35 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index d40ac4d..7ec87df 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3027,7 +3027,6 @@ found:
 	ps->bus = bus;
 	ps->sw_addr = sw_addr;
 	ps->info = info;
-	ps->id = id & 0xfff0;
 	ps->rev = id & 0xf;
 
 	dev_info(&ps->bus->dev, "found switch %s, revision %u\n",
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 7e86cc6..fb81945 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -68,39 +68,6 @@
 #define PORT_PCS_CTRL_UNFORCED		0x03
 #define PORT_PAUSE_CTRL		0x02
 #define PORT_SWITCH_ID		0x03
-#define PORT_SWITCH_ID_PROD_NUM_MASK	0xfff0
-#define PORT_SWITCH_ID_REV_MASK		0x000f
-#define PORT_SWITCH_ID_6031	0x0310
-#define PORT_SWITCH_ID_6035	0x0350
-#define PORT_SWITCH_ID_6046	0x0480
-#define PORT_SWITCH_ID_6061	0x0610
-#define PORT_SWITCH_ID_6065	0x0650
-#define PORT_SWITCH_ID_6085	0x04a0
-#define PORT_SWITCH_ID_6092	0x0970
-#define PORT_SWITCH_ID_6095	0x0950
-#define PORT_SWITCH_ID_6096	0x0980
-#define PORT_SWITCH_ID_6097	0x0990
-#define PORT_SWITCH_ID_6108	0x1070
-#define PORT_SWITCH_ID_6121	0x1040
-#define PORT_SWITCH_ID_6122	0x1050
-#define PORT_SWITCH_ID_6123	0x1210
-#define PORT_SWITCH_ID_6131	0x1060
-#define PORT_SWITCH_ID_6152	0x1a40
-#define PORT_SWITCH_ID_6155	0x1a50
-#define PORT_SWITCH_ID_6161	0x1610
-#define PORT_SWITCH_ID_6165	0x1650
-#define PORT_SWITCH_ID_6171	0x1710
-#define PORT_SWITCH_ID_6172	0x1720
-#define PORT_SWITCH_ID_6175	0x1750
-#define PORT_SWITCH_ID_6176	0x1760
-#define PORT_SWITCH_ID_6182	0x1a60
-#define PORT_SWITCH_ID_6185	0x1a70
-#define PORT_SWITCH_ID_6240	0x2400
-#define PORT_SWITCH_ID_6320	0x1150
-#define PORT_SWITCH_ID_6321	0x3100
-#define PORT_SWITCH_ID_6350	0x3710
-#define PORT_SWITCH_ID_6351	0x3750
-#define PORT_SWITCH_ID_6352	0x3520
 #define PORT_CONTROL		0x04
 #define PORT_CONTROL_USE_CORE_TAG	BIT(15)
 #define PORT_CONTROL_DROP_ON_LOCK	BIT(14)
@@ -450,7 +417,6 @@ struct mv88e6xxx_priv_state {
 	 */
 	struct mutex eeprom_mutex;
 
-	int		id; /* switch product id */
 	struct mv88e6xxx_priv_port	ports[DSA_MAX_PORTS];
 
 	DECLARE_BITMAP(port_state_update_mask, DSA_MAX_PORTS);
-- 
2.8.0

Re: [PATCH net-next 7/7] net: dsa: mv88e6xxx: drop switch id

[Andrew Lunn]

On Fri, Apr 15, 2016 at 02:25:50PM -0400, Vivien Didelot wrote:
> We already have the product number and revision stored in the info
> structure and the switch private state.
> 
> It is not necessary to clutter the header file with shifted product
> number for devices that we don't even support yet. Remove them.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6xxx.c |  1 -
>  drivers/net/dsa/mv88e6xxx.h | 34 ----------------------------------
>  2 files changed, 35 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index d40ac4d..7ec87df 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -3027,7 +3027,6 @@ found:
>  	ps->bus = bus;
>  	ps->sw_addr = sw_addr;
>  	ps->info = info;
> -	ps->id = id & 0xfff0;
>  	ps->rev = id & 0xf;
>  
>  	dev_info(&ps->bus->dev, "found switch %s, revision %u\n",
> diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
> index 7e86cc6..fb81945 100644
> --- a/drivers/net/dsa/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx.h
> @@ -68,39 +68,6 @@
>  #define PORT_PCS_CTRL_UNFORCED		0x03
>  #define PORT_PAUSE_CTRL		0x02
>  #define PORT_SWITCH_ID		0x03
> -#define PORT_SWITCH_ID_PROD_NUM_MASK	0xfff0
> -#define PORT_SWITCH_ID_REV_MASK		0x000f
> -#define PORT_SWITCH_ID_6031	0x0310
> -#define PORT_SWITCH_ID_6035	0x0350
> -#define PORT_SWITCH_ID_6046	0x0480
> -#define PORT_SWITCH_ID_6061	0x0610
> -#define PORT_SWITCH_ID_6065	0x0650
> -#define PORT_SWITCH_ID_6085	0x04a0
> -#define PORT_SWITCH_ID_6092	0x0970
> -#define PORT_SWITCH_ID_6095	0x0950
> -#define PORT_SWITCH_ID_6096	0x0980
> -#define PORT_SWITCH_ID_6097	0x0990
> -#define PORT_SWITCH_ID_6108	0x1070
> -#define PORT_SWITCH_ID_6121	0x1040
> -#define PORT_SWITCH_ID_6122	0x1050
> -#define PORT_SWITCH_ID_6123	0x1210
> -#define PORT_SWITCH_ID_6131	0x1060
> -#define PORT_SWITCH_ID_6152	0x1a40
> -#define PORT_SWITCH_ID_6155	0x1a50
> -#define PORT_SWITCH_ID_6161	0x1610
> -#define PORT_SWITCH_ID_6165	0x1650
> -#define PORT_SWITCH_ID_6171	0x1710
> -#define PORT_SWITCH_ID_6172	0x1720
> -#define PORT_SWITCH_ID_6175	0x1750
> -#define PORT_SWITCH_ID_6176	0x1760
> -#define PORT_SWITCH_ID_6182	0x1a60
> -#define PORT_SWITCH_ID_6185	0x1a70
> -#define PORT_SWITCH_ID_6240	0x2400
> -#define PORT_SWITCH_ID_6320	0x1150
> -#define PORT_SWITCH_ID_6321	0x3100
> -#define PORT_SWITCH_ID_6350	0x3710
> -#define PORT_SWITCH_ID_6351	0x3750
> -#define PORT_SWITCH_ID_6352	0x3520

NACK

These numbers are not obvious. PORT_SWITCH_ID_6320 i can
understand. 0x1150 i have no idea what it is.

	    Andrwe

Re: [PATCH net-next 7/7] net: dsa: mv88e6xxx: drop switch id

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

<snip>

>> -#define PORT_SWITCH_ID_6350	0x3710
>> -#define PORT_SWITCH_ID_6351	0x3750
>> -#define PORT_SWITCH_ID_6352	0x3520
>
> NACK
>
> These numbers are not obvious. PORT_SWITCH_ID_6320 i can
> understand. 0x1150 i have no idea what it is.

0x1150 is not even correct. That's the product number (bits 4:15) masked
with an assumed revision 0 (bits 0:3).

That leads to confusion and error, as seen in the patch 2/7.

These values are now only used in a device description table, where they
seem pretty understandable to me.

This header file is full of inconsistencies. We have masks, offsets,
shifts, shifted and unshifted values, just for the sake of hidding said
magic numbers, while an explicit comment in a function could do the job.

But OK if we really want them defined, I'll introduce 12-bit
PORT_SWITCH_ID_PROD_NUM_* before dropping the 16-bit PORT_SWITCH_ID_*.

Thanks,
Vivien

Re: [PATCH net-next 7/7] net: dsa: mv88e6xxx: drop switch id

[Andrew Lunn]

On Fri, Apr 15, 2016 at 05:00:50PM -0400, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> <snip>
> 
> >> -#define PORT_SWITCH_ID_6350	0x3710
> >> -#define PORT_SWITCH_ID_6351	0x3750
> >> -#define PORT_SWITCH_ID_6352	0x3520
> >
> > NACK
> >
> > These numbers are not obvious. PORT_SWITCH_ID_6320 i can
> > understand. 0x1150 i have no idea what it is.
> 
> 0x1150 is not even correct. That's the product number (bits 4:15) masked
> with an assumed revision 0 (bits 0:3).
> 
> That leads to confusion and error, as seen in the patch 2/7.
> 
> These values are now only used in a device description table, where they
> seem pretty understandable to me.

      { MV88E6XXX_INFO(6320, 0x115, "Marvell 88E6320") },
      { MV88E6XXX_INFO(6320, 0x310, "Marvell 88E6321") },

What does 0x115 have to do with 6320?
What does 0x310 have to do with 6321?

Most do have a pattern, but not all. For a few devices, Marvell has
used /dev/random to pick the ID. Using the macro PORT_SWITCH_ID_6320
documents where these numbers come from, and how to figure out the
correct number of a new device, etc.

> But OK if we really want them defined, I'll introduce 12-bit
> PORT_SWITCH_ID_PROD_NUM_* before dropping the 16-bit
> PORT_SWITCH_ID_*.

I'm O.K. with that.

Thanks
	Andrew