2016-04-16 22:42:08

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 0/8] net: dsa: mv88e6xxx: factorize switch info

This patchset factorizes 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 "flags" 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.

Changes v2 -> v3:
- update commit messages and add Andrew's tags
- keep the info lookup code in a separated function
- split the single switch ID reading in probe in a new commit

Changes v1 -> v2:
- define PORT_SWITCH_ID_PROD_NUM_* values
- use plain struct mv88e6xxx_info
- remove non used yet ps->rev

Vivien Didelot (8):
net: dsa: mv88e6xxx: drop double ds assignment
net: dsa: mv88e6xxx: drop revision probing
net: dsa: mv88e6xxx: read switch ID in probe
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 db to info
net: dsa: mv88e6xxx: remove switch ID from ps

drivers/net/dsa/mv88e6123.c | 45 +++++-----
drivers/net/dsa/mv88e6131.c | 53 ++++++------
drivers/net/dsa/mv88e6171.c | 36 +++++---
drivers/net/dsa/mv88e6352.c | 55 ++++++++----
drivers/net/dsa/mv88e6xxx.c | 206 ++++++++++++++------------------------------
drivers/net/dsa/mv88e6xxx.h | 92 ++++++++------------
6 files changed, 214 insertions(+), 273 deletions(-)

--
2.8.0


2016-04-16 22:42:10

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 5/8] net: dsa: mv88e6xxx: add family to info

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

Signed-off-by: Vivien Didelot <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
drivers/net/dsa/mv88e6123.c | 3 ++
drivers/net/dsa/mv88e6131.c | 4 +++
drivers/net/dsa/mv88e6171.c | 4 +++
drivers/net/dsa/mv88e6352.c | 6 ++++
drivers/net/dsa/mv88e6xxx.c | 71 +++++----------------------------------------
drivers/net/dsa/mv88e6xxx.h | 13 +++++++++
6 files changed, 38 insertions(+), 63 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 9120fcf..93429d2 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -20,12 +20,15 @@
static const struct mv88e6xxx_info mv88e6123_table[] = {
{
.prod_num = PORT_SWITCH_ID_PROD_NUM_6123,
+ .family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6123",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6161,
+ .family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6161",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6165,
+ .family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6165",
}
};
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 82ff0c4..ec06321 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -20,15 +20,19 @@
static const struct mv88e6xxx_info mv88e6131_table[] = {
{
.prod_num = PORT_SWITCH_ID_PROD_NUM_6095,
+ .family = MV88E6XXX_FAMILY_6095,
.name = "Marvell 88E6095/88E6095F",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6085,
+ .family = MV88E6XXX_FAMILY_6097,
.name = "Marvell 88E6085",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6131,
+ .family = MV88E6XXX_FAMILY_6185,
.name = "Marvell 88E6131",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6185,
+ .family = MV88E6XXX_FAMILY_6185,
.name = "Marvell 88E6185",
}
};
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 5e9eb8c..efe3c0f 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -20,15 +20,19 @@
static const struct mv88e6xxx_info mv88e6171_table[] = {
{
.prod_num = PORT_SWITCH_ID_PROD_NUM_6171,
+ .family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6171",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6175,
+ .family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6175",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6350,
+ .family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6350",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6351,
+ .family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6351",
}
};
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 3a53cfb..c051c02 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -25,21 +25,27 @@
static const struct mv88e6xxx_info mv88e6352_table[] = {
{
.prod_num = PORT_SWITCH_ID_PROD_NUM_6320,
+ .family = MV88E6XXX_FAMILY_6320,
.name = "Marvell 88E6320",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6321,
+ .family = MV88E6XXX_FAMILY_6320,
.name = "Marvell 88E6321",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6172,
+ .family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6172",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6176,
+ .family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6176",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6240,
+ .family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6240",
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6352,
+ .family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6352",
}
};
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 1f69b28..b3f0fd5 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 52c7e03..8bb2586 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -367,9 +367,22 @@
#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;
};
--
2.8.0

2016-04-16 22:42:25

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 7/8] net: dsa: mv88e6xxx: add number of db to info

Add the number of databases to the info structure.

Signed-off-by: Vivien Didelot <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
drivers/net/dsa/mv88e6123.c | 3 +++
drivers/net/dsa/mv88e6131.c | 4 ++++
drivers/net/dsa/mv88e6171.c | 4 ++++
drivers/net/dsa/mv88e6352.c | 6 ++++++
drivers/net/dsa/mv88e6xxx.c | 19 +------------------
drivers/net/dsa/mv88e6xxx.h | 1 +
6 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index e3802b4..b3cbf8e 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -22,16 +22,19 @@ static const struct mv88e6xxx_info mv88e6123_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6123,
.family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6123",
+ .num_databases = 4096,
.num_ports = 3,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6161,
.family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6161",
+ .num_databases = 4096,
.num_ports = 6,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6165,
.family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6165",
+ .num_databases = 4096,
.num_ports = 6,
}
};
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index f6b465b..8a161c5 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -22,21 +22,25 @@ static const struct mv88e6xxx_info mv88e6131_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6095,
.family = MV88E6XXX_FAMILY_6095,
.name = "Marvell 88E6095/88E6095F",
+ .num_databases = 256,
.num_ports = 11,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6085,
.family = MV88E6XXX_FAMILY_6097,
.name = "Marvell 88E6085",
+ .num_databases = 4096,
.num_ports = 10,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6131,
.family = MV88E6XXX_FAMILY_6185,
.name = "Marvell 88E6131",
+ .num_databases = 256,
.num_ports = 8,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6185,
.family = MV88E6XXX_FAMILY_6185,
.name = "Marvell 88E6185",
+ .num_databases = 256,
.num_ports = 10,
}
};
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index cdfa899..689824f 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -22,21 +22,25 @@ static const struct mv88e6xxx_info mv88e6171_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6171,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6171",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6175,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6175",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6350,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6350",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6351,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6351",
+ .num_databases = 4096,
.num_ports = 7,
}
};
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 9aaddca..abd9113 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -27,31 +27,37 @@ static const struct mv88e6xxx_info mv88e6352_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6320,
.family = MV88E6XXX_FAMILY_6320,
.name = "Marvell 88E6320",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6321,
.family = MV88E6XXX_FAMILY_6320,
.name = "Marvell 88E6321",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6172,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6172",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6176,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6176",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6240,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6240",
+ .num_databases = 4096,
.num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6352,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6352",
+ .num_databases = 4096,
.num_ports = 7,
}
};
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index e9985dad..e6e657b 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 1f3ab6f..4b3121b 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -385,6 +385,7 @@ struct mv88e6xxx_info {
enum mv88e6xxx_family family;
u16 prod_num;
const char *name;
+ unsigned int num_databases;
unsigned int num_ports;
};

--
2.8.0

2016-04-16 22:42:24

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 6/8] net: dsa: mv88e6xxx: add number of ports to info

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

Signed-off-by: Vivien Didelot <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
drivers/net/dsa/mv88e6123.c | 16 +++-------------
drivers/net/dsa/mv88e6131.c | 22 +++++-----------------
drivers/net/dsa/mv88e6171.c | 7 ++++---
drivers/net/dsa/mv88e6352.c | 8 ++++++--
drivers/net/dsa/mv88e6xxx.c | 38 +++++++++++++++++++-------------------
drivers/net/dsa/mv88e6xxx.h | 3 +--
6 files changed, 38 insertions(+), 56 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 93429d2..e3802b4 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -22,14 +22,17 @@ static const struct mv88e6xxx_info mv88e6123_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6123,
.family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6123",
+ .num_ports = 3,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6161,
.family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6161",
+ .num_ports = 6,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6165,
.family = MV88E6XXX_FAMILY_6165,
.name = "Marvell 88E6165",
+ .num_ports = 6,
}
};

@@ -77,25 +80,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 ec06321..f6b465b 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -22,18 +22,22 @@ static const struct mv88e6xxx_info mv88e6131_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6095,
.family = MV88E6XXX_FAMILY_6095,
.name = "Marvell 88E6095/88E6095F",
+ .num_ports = 11,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6085,
.family = MV88E6XXX_FAMILY_6097,
.name = "Marvell 88E6085",
+ .num_ports = 10,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6131,
.family = MV88E6XXX_FAMILY_6185,
.name = "Marvell 88E6131",
+ .num_ports = 8,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6185,
.family = MV88E6XXX_FAMILY_6185,
.name = "Marvell 88E6185",
+ .num_ports = 10,
}
};

@@ -103,7 +107,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);
@@ -112,21 +115,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;
@@ -142,7 +130,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 efe3c0f..cdfa899 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -22,18 +22,22 @@ static const struct mv88e6xxx_info mv88e6171_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6171,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6171",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6175,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6175",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6350,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6350",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6351,
.family = MV88E6XXX_FAMILY_6351,
.name = "Marvell 88E6351",
+ .num_ports = 7,
}
};

@@ -82,15 +86,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 c051c02..9aaddca 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -27,26 +27,32 @@ static const struct mv88e6xxx_info mv88e6352_table[] = {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6320,
.family = MV88E6XXX_FAMILY_6320,
.name = "Marvell 88E6320",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6321,
.family = MV88E6XXX_FAMILY_6320,
.name = "Marvell 88E6321",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6172,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6172",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6176,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6176",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6240,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6240",
+ .num_ports = 7,
}, {
.prod_num = PORT_SWITCH_ID_PROD_NUM_6352,
.family = MV88E6XXX_FAMILY_6352,
.name = "Marvell 88E6352",
+ .num_ports = 7,
}
};

@@ -101,8 +107,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 b3f0fd5..e9985dad 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 8bb2586..1f3ab6f 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -385,6 +385,7 @@ struct mv88e6xxx_info {
enum mv88e6xxx_family family;
u16 prod_num;
const char *name;
+ unsigned int num_ports;
};

struct mv88e6xxx_atu_entry {
@@ -456,8 +457,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

2016-04-16 22:42:21

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 8/8] net: dsa: mv88e6xxx: remove switch ID from ps

ps->id is not needed anymore, so remove it as well as the related
defined values.

Signed-off-by: Vivien Didelot <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
drivers/net/dsa/mv88e6xxx.c | 1 -
drivers/net/dsa/mv88e6xxx.h | 32 --------------------------------
2 files changed, 33 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index e6e657b..e8a198c 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3041,7 +3041,6 @@ char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
ps->bus = bus;
ps->sw_addr = sw_addr;
ps->info = info;
- ps->id = id & 0xfff0;

*priv = ps;

diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 4b3121b..4c5f434 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -85,37 +85,6 @@
#define PORT_SWITCH_ID_PROD_NUM_6352 0x352
#define PORT_SWITCH_ID_PROD_NUM_6350 0x371
#define PORT_SWITCH_ID_PROD_NUM_6351 0x375
-#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)
@@ -457,7 +426,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

2016-04-16 22:43:21

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 1/8] net: dsa: mv88e6xxx: drop double ds assignment

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

Signed-off-by: Vivien Didelot <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
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

2016-04-16 22:42:09

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 3/8] net: dsa: mv88e6xxx: read switch ID in probe

Read the switch ID only once, at probe time, to avoid multiple read
accesses and MII bus checking.

Signed-off-by: Vivien Didelot <[email protected]>
---
drivers/net/dsa/mv88e6xxx.c | 54 +++++++++++++++++++++++++--------------------
1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index b63e985..af5ae70 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2667,8 +2667,6 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
ps->ds = ds;
mutex_init(&ps->smi_mutex);

- ps->id = REG_READ(REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
-
INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);

return 0;
@@ -3068,21 +3066,14 @@ 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,
+static char *mv88e6xxx_lookup_name(unsigned int id,
const struct mv88e6xxx_switch_id *table,
unsigned int num)
{
- int i, ret;
-
- if (!bus)
- return NULL;
-
- ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
- if (ret < 0)
- return NULL;
+ int i;

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

return NULL;
@@ -3094,23 +3085,38 @@ char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
unsigned int num)
{
struct mv88e6xxx_priv_state *ps;
- struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
+ struct mii_bus *bus;
+ int id, prod_num, rev;
char *name;

+ bus = dsa_host_dev_to_mii_bus(host_dev);
if (!bus)
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;
- }
+ id = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
+ if (id < 0)
+ return NULL;
+
+ prod_num = (id & 0xfff0) >> 4;
+ rev = id & 0x000f;
+
+ name = mv88e6xxx_lookup_name(id, table, num);
+ if (!name)
+ return NULL;
+
+ ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
+ if (!ps)
+ return NULL;
+
+ ps->bus = bus;
+ ps->sw_addr = sw_addr;
+ ps->id = id & 0xfff0;
+
+ *priv = ps;
+
+ dev_info(&ps->bus->dev, "switch 0x%x probed: %s, revision %u\n",
+ prod_num, name, rev);
+
return name;
}

--
2.8.0

2016-04-16 22:43:41

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 2/8] net: dsa: mv88e6xxx: drop revision probing

There is no point in having a 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 masks the revision.

Drop every revision definition, and lookup only the product number.

Signed-off-by: Vivien Didelot <[email protected]>
---
drivers/net/dsa/mv88e6123.c | 6 ------
drivers/net/dsa/mv88e6131.c | 2 --
drivers/net/dsa/mv88e6352.c | 6 ------
drivers/net/dsa/mv88e6xxx.c | 14 +-------------
drivers/net/dsa/mv88e6xxx.h | 15 ---------------
5 files changed, 1 insertion(+), 42 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..b63e985 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3081,22 +3081,10 @@ static char *mv88e6xxx_lookup_name(struct mii_bus *bus, int sw_addr,
if (ret < 0)
return NULL;

- /* 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(&bus->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_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
@@ -84,18 +82,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 +95,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)
--
2.8.0

2016-04-16 22:44:03

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next v3 4/8] net: dsa: mv88e6xxx: add switch info

Add a new switch info structure which is meant to store switch models
static information, such as product number, name, number of ports,
number of databases, etc.

Signed-off-by: Vivien Didelot <[email protected]>
---
drivers/net/dsa/mv88e6123.c | 15 +++++++++++----
drivers/net/dsa/mv88e6131.c | 19 ++++++++++++++-----
drivers/net/dsa/mv88e6171.c | 19 ++++++++++++++-----
drivers/net/dsa/mv88e6352.c | 27 ++++++++++++++++++++-------
drivers/net/dsa/mv88e6xxx.c | 21 ++++++++++++---------
drivers/net/dsa/mv88e6xxx.h | 28 +++++++++++++++++++++++-----
6 files changed, 94 insertions(+), 35 deletions(-)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 00c1121..9120fcf 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -17,10 +17,17 @@
#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[] = {
+ {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6123,
+ .name = "Marvell 88E6123",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6161,
+ .name = "Marvell 88E6161",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6165,
+ .name = "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..82ff0c4 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -17,11 +17,20 @@
#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[] = {
+ {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6095,
+ .name = "Marvell 88E6095/88E6095F",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6085,
+ .name = "Marvell 88E6085",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6131,
+ .name = "Marvell 88E6131",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6185,
+ .name = "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..5e9eb8c 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -17,11 +17,20 @@
#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[] = {
+ {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6171,
+ .name = "Marvell 88E6171",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6175,
+ .name = "Marvell 88E6175",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6350,
+ .name = "Marvell 88E6350",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6351,
+ .name = "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..3a53cfb 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -22,13 +22,26 @@
#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[] = {
+ {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6320,
+ .name = "Marvell 88E6320",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6321,
+ .name = "Marvell 88E6321",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6172,
+ .name = "Marvell 88E6172",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6176,
+ .name = "Marvell 88E6176",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6240,
+ .name = "Marvell 88E6240",
+ }, {
+ .prod_num = PORT_SWITCH_ID_PROD_NUM_6352,
+ .name = "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 af5ae70..1f69b28 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3066,24 +3066,24 @@ int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
}
#endif /* CONFIG_NET_DSA_HWMON */

-static char *mv88e6xxx_lookup_name(unsigned int id,
- const struct mv88e6xxx_switch_id *table,
- unsigned int num)
+static const struct mv88e6xxx_info *
+mv88e6xxx_lookup_info(unsigned int prod_num, const struct mv88e6xxx_info *table,
+ unsigned int num)
{
int i;

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

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)
+ const struct mv88e6xxx_info *table, unsigned int num)
{
+ const struct mv88e6xxx_info *info;
struct mv88e6xxx_priv_state *ps;
struct mii_bus *bus;
int id, prod_num, rev;
@@ -3100,16 +3100,19 @@ char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
prod_num = (id & 0xfff0) >> 4;
rev = id & 0x000f;

- name = mv88e6xxx_lookup_name(id, table, num);
- if (!name)
+ info = mv88e6xxx_lookup_info(prod_num, table, num);
+ if (!info)
return NULL;

+ name = (char *) info->name;
+
ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
if (!ps)
return NULL;

ps->bus = bus;
ps->sw_addr = sw_addr;
+ ps->info = info;
ps->id = id & 0xfff0;

*priv = ps;
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 9242aea..52c7e03 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -68,6 +68,23 @@
#define PORT_PCS_CTRL_UNFORCED 0x03
#define PORT_PAUSE_CTRL 0x02
#define PORT_SWITCH_ID 0x03
+#define PORT_SWITCH_ID_PROD_NUM_6085 0x04a
+#define PORT_SWITCH_ID_PROD_NUM_6095 0x095
+#define PORT_SWITCH_ID_PROD_NUM_6131 0x106
+#define PORT_SWITCH_ID_PROD_NUM_6320 0x115
+#define PORT_SWITCH_ID_PROD_NUM_6123 0x121
+#define PORT_SWITCH_ID_PROD_NUM_6161 0x161
+#define PORT_SWITCH_ID_PROD_NUM_6165 0x165
+#define PORT_SWITCH_ID_PROD_NUM_6171 0x171
+#define PORT_SWITCH_ID_PROD_NUM_6172 0x172
+#define PORT_SWITCH_ID_PROD_NUM_6175 0x175
+#define PORT_SWITCH_ID_PROD_NUM_6176 0x176
+#define PORT_SWITCH_ID_PROD_NUM_6185 0x1a7
+#define PORT_SWITCH_ID_PROD_NUM_6240 0x240
+#define PORT_SWITCH_ID_PROD_NUM_6321 0x310
+#define PORT_SWITCH_ID_PROD_NUM_6352 0x352
+#define PORT_SWITCH_ID_PROD_NUM_6350 0x371
+#define PORT_SWITCH_ID_PROD_NUM_6351 0x375
#define PORT_SWITCH_ID_6031 0x0310
#define PORT_SWITCH_ID_6035 0x0350
#define PORT_SWITCH_ID_6046 0x0480
@@ -352,9 +369,9 @@

#define MV88E6XXX_N_FID 4096

-struct mv88e6xxx_switch_id {
- u16 id;
- char *name;
+struct mv88e6xxx_info {
+ u16 prod_num;
+ const char *name;
};

struct mv88e6xxx_atu_entry {
@@ -382,6 +399,8 @@ struct mv88e6xxx_priv_port {
};

struct mv88e6xxx_priv_state {
+ const struct mv88e6xxx_info *info;
+
/* The dsa_switch this private structure is related to */
struct dsa_switch *ds;

@@ -449,8 +468,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

2016-04-17 15:24:36

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v3 2/8] net: dsa: mv88e6xxx: drop revision probing

On Sat, Apr 16, 2016 at 06:41:39PM -0400, Vivien Didelot wrote:
> There is no point in having a 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 masks the revision.
>
> Drop every revision definition, and lookup only the product number.
>
> Signed-off-by: Vivien Didelot <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Thanks
Andrew

> ---
> drivers/net/dsa/mv88e6123.c | 6 ------
> drivers/net/dsa/mv88e6131.c | 2 --
> drivers/net/dsa/mv88e6352.c | 6 ------
> drivers/net/dsa/mv88e6xxx.c | 14 +-------------
> drivers/net/dsa/mv88e6xxx.h | 15 ---------------
> 5 files changed, 1 insertion(+), 42 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..b63e985 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -3081,22 +3081,10 @@ static char *mv88e6xxx_lookup_name(struct mii_bus *bus, int sw_addr,
> if (ret < 0)
> return NULL;
>
> - /* 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(&bus->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_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
> @@ -84,18 +82,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 +95,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)
> --
> 2.8.0
>

2016-04-17 15:26:31

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v3 3/8] net: dsa: mv88e6xxx: read switch ID in probe

On Sat, Apr 16, 2016 at 06:41:40PM -0400, Vivien Didelot wrote:
> Read the switch ID only once, at probe time, to avoid multiple read
> accesses and MII bus checking.
>
> Signed-off-by: Vivien Didelot <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Thanks
Andrew

> ---
> drivers/net/dsa/mv88e6xxx.c | 54 +++++++++++++++++++++++++--------------------
> 1 file changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index b63e985..af5ae70 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -2667,8 +2667,6 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
> ps->ds = ds;
> mutex_init(&ps->smi_mutex);
>
> - ps->id = REG_READ(REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
> -
> INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
>
> return 0;
> @@ -3068,21 +3066,14 @@ 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,
> +static char *mv88e6xxx_lookup_name(unsigned int id,
> const struct mv88e6xxx_switch_id *table,
> unsigned int num)
> {
> - int i, ret;
> -
> - if (!bus)
> - return NULL;
> -
> - ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
> - if (ret < 0)
> - return NULL;
> + int i;
>
> for (i = 0; i < num; ++i)
> - if (table[i].id == (ret & 0xfff0))
> + if (table[i].id == (id & 0xfff0))
> return table[i].name;
>
> return NULL;
> @@ -3094,23 +3085,38 @@ char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
> unsigned int num)
> {
> struct mv88e6xxx_priv_state *ps;
> - struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
> + struct mii_bus *bus;
> + int id, prod_num, rev;
> char *name;
>
> + bus = dsa_host_dev_to_mii_bus(host_dev);
> if (!bus)
> 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;
> - }
> + id = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
> + if (id < 0)
> + return NULL;
> +
> + prod_num = (id & 0xfff0) >> 4;
> + rev = id & 0x000f;
> +
> + name = mv88e6xxx_lookup_name(id, table, num);
> + if (!name)
> + return NULL;
> +
> + ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
> + if (!ps)
> + return NULL;
> +
> + ps->bus = bus;
> + ps->sw_addr = sw_addr;
> + ps->id = id & 0xfff0;
> +
> + *priv = ps;
> +
> + dev_info(&ps->bus->dev, "switch 0x%x probed: %s, revision %u\n",
> + prod_num, name, rev);
> +
> return name;
> }
>
> --
> 2.8.0
>

2016-04-17 15:35:45

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v3 4/8] net: dsa: mv88e6xxx: add switch info

> @@ -3100,16 +3100,19 @@ char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
> prod_num = (id & 0xfff0) >> 4;
> rev = id & 0x000f;
>
> - name = mv88e6xxx_lookup_name(id, table, num);
> - if (!name)
> + info = mv88e6xxx_lookup_info(prod_num, table, num);
> + if (!info)
> return NULL;
>
> + name = (char *) info->name;
> +

Hi Vivien

Casting off the const is not so nice. How about adding in another
patch to change the API to return a const char *.

Andrew

2016-04-17 16:38:00

by Vivien Didelot

[permalink] [raw]
Subject: Re: [PATCH net-next v3 4/8] net: dsa: mv88e6xxx: add switch info

Hi Andrew,

Andrew Lunn <[email protected]> writes:

>> @@ -3100,16 +3100,19 @@ char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
>> prod_num = (id & 0xfff0) >> 4;
>> rev = id & 0x000f;
>>
>> - name = mv88e6xxx_lookup_name(id, table, num);
>> - if (!name)
>> + info = mv88e6xxx_lookup_info(prod_num, table, num);
>> + if (!info)
>> return NULL;
>>
>> + name = (char *) info->name;
>> +
>
> Hi Vivien
>
> Casting off the const is not so nice. How about adding in another
> patch to change the API to return a const char *.

OK.

Thanks,
Vivien