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

[Andrew Lunn]

On Fri, Apr 15, 2016 at 07:21:23PM -0400, Vivien Didelot wrote:
> 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.

You forgot to update the commit message. ps->rev has been removed in
this version.

>  
>   /* Look up the exact switch ID */

This comment now becomes meaningless. Please delete.

>   for (i = 0; i < num; ++i)
> - if (table[i].id == ret)
> + if (table[i].id == (ret & 0xfff0))
>   return table[i].name;

Andrew

[PATCH net-next v2 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 
---
 drivers/net/dsa/mv88e6123.c |  6 --
 drivers/net/dsa/mv88e6131.c |  2 --
 drivers/net/dsa/mv88e6352.c |  6 --
 drivers/net/dsa/mv88e6xxx.c | 13 +
 drivers/net/dsa/mv88e6xxx.h | 15 ---
 5 files changed, 1 insertion(+), 41 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..d7e309d 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3083,20 +3083,9 @@ 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)
+   if (table[i].id == (ret & 0xfff0))
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(>dev,
-"unknown revision %d, using base switch 
0x%x\n",
-ret & PORT_SWITCH_ID_REV_MASK,
-ret & PORT_SWITCH_ID_PROD_NUM_MASK);
-   return table[i].name;
-   }
-   }
-
return NULL;
 }
 
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 5d27dec..9242aea 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -68,8 +68,6 @@
 #define PORT_PCS_CTRL_UNFORCED 0x03
 #define PORT_PAUSE_CTRL0x02
 #define PORT_SWITCH_ID 0x03
-#define PORT_SWITCH_ID_PROD_NUM_MASK   0xfff0
-#define PORT_SWITCH_ID_REV_MASK0x000f
 #define PORT_SWITCH_ID_60310x0310
 #define PORT_SWITCH_ID_60350x0350
 #define PORT_SWITCH_ID_60460x0480
@@ -84,18 +82,11 @@
 #define PORT_SWITCH_ID_61210x1040
 #define PORT_SWITCH_ID_61220x1050
 #define PORT_SWITCH_ID_61230x1210
-#define PORT_SWITCH_ID_6123_A1 0x1212