[PATCH v3 net-next v3 00/14] net: dsa: mv88e6xxx: probe compatible

[PATCH v3 net-next v3 00/14] net: dsa: mv88e6xxx: probe compatible

[Vivien Didelot]

This patchset factorizes the legacy DSA probing and the new MDIO probing
functions with a few helpers.

This will allow us to use a compatible chip info to describe how to access the
SMI device and its switch ID register at probe time.

For the legacy probe, we fix the compatible info to 88E6085. For the MDIO
probe, we will use the compatible info from the device node data.

The first 5 patches are already reviewed.
The next 3 patches are cosmetics, with no functional changes.
The next 4 patches add probe helpers and pass the compatible info.
The last 2 patches extend the info structure to store the SMI access mode and
the location of the switch ID register.

Changes since v2 [2]:

  - do not guess compatible model in legacy probe
  - add low level SMI API using a chip structure
  - allocate before probe and detection
  - add 3 cosmetic patches

Changes since v1 [1]:

  - merge style fix from Ben Dooks
  - add Acked-by/Reviewed-by tags
  - drop one compatible string per model
  - detect the SMI device based on the compatible info
  - add an SMI ops structure

[1] https://lkml.org/lkml/2016/6/8/1201
[2] https://lkml.org/lkml/2016/6/14/671

Vivien Didelot (14):
  net: dsa: mv88e6xxx: fix style issues
  net: dsa: mv88e6xxx: remove redundant assignments
  net: dsa: mv88e6xxx: use already declared variables
  net: dsa: mv88e6xxx: do not increment bus refcount
  net: dsa: mv88e6xxx: add switch register helpers
  net: dsa: mv88e6xxx: use gpio get optional variant
  net: dsa: mv88e6xxx: remove table args in info lookup
  net: dsa: mv88e6xxx: rename smi_mutex to reg_lock
  net: dsa: mv88e6xxx: add chip allocation helper
  net: dsa: mv88e6xxx: add SMI init helper
  net: dsa: mv88e6xxx: add detection helper
  net: dsa: mv88e6xxx: pass compatible info
  net: dsa: mv88e6xxx: add addressing mode to info
  net: dsa: mv88e6xxx: add port base address to info

 drivers/net/dsa/mv88e6xxx.c | 508 ++++++++++++++++++++++++++++----------------
 drivers/net/dsa/mv88e6xxx.h |  27 ++-
 2 files changed, 343 insertions(+), 192 deletions(-)

-- 
2.8.3

[PATCH v3 net-next v3 01/14] net: dsa: mv88e6xxx: fix style issues

[Vivien Didelot]

This patch fixes 5 style problems reported by checkpatch:

    WARNING: suspect code indent for conditional statements (8, 24)
    #492: FILE: drivers/net/dsa/mv88e6xxx.c:492:
    +	if (phydev->link)
    +			reg |= PORT_PCS_CTRL_LINK_UP;

    CHECK: Logical continuations should be on the previous line
    #1318: FILE: drivers/net/dsa/mv88e6xxx.c:1318:
    +		     oldstate == PORT_CONTROL_STATE_FORWARDING)
    +		    && (state == PORT_CONTROL_STATE_DISABLED ||

    CHECK: multiple assignments should be avoided
    #1662: FILE: drivers/net/dsa/mv88e6xxx.c:1662:
    +		vlan->vid_begin = vlan->vid_end = next.vid;

    WARNING: line over 80 characters
    #2097: FILE: drivers/net/dsa/mv88e6xxx.c:2097:
    +				       const struct switchdev_obj_port_vlan *vlan,

    WARNING: suspect code indent for conditional statements (16, 32)
    #2734: FILE: drivers/net/dsa/mv88e6xxx.c:2734:
    +		if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
    [...]
    +				reg |= PORT_CONTROL_EGRESS_ADD_TAG;

    total: 0 errors, 3 warnings, 2 checks, 3805 lines checked

It also rebases and integrates changes sent by Ben Dooks [1]:

    The driver has a number of functions that are not exported or
    declared elsewhere, so make them static to avoid the following
    warnings from sparse:

    drivers/net/dsa/mv88e6xxx.c:113:5: warning: symbol 'mv88e6xxx_reg_read' was not declared. Should it be static?
    drivers/net/dsa/mv88e6xxx.c:167:5: warning: symbol 'mv88e6xxx_reg_write' was not declared. Should it be static?
    drivers/net/dsa/mv88e6xxx.c:231:5: warning: symbol 'mv88e6xxx_set_addr' was not declared. Should it be static?
    drivers/net/dsa/mv88e6xxx.c:367:6: warning: symbol 'mv88e6xxx_ppu_state_init' was not declared. Should it be static?
    drivers/net/dsa/mv88e6xxx.c:3157:5: warning: symbol 'mv88e6xxx_phy_page_read' was not declared. Should it be static?
    drivers/net/dsa/mv88e6xxx.c:3169:5: warning: symbol 'mv88e6xxx_phy_page_write' was not declared. Should it be static?
    drivers/net/dsa/mv88e6xxx.c:3583:26: warning: symbol 'mv88e6xxx_switch_driver' was not declared. Should it be static?
    drivers/net/dsa/mv88e6xxx.c:3621:5: warning: symbol 'mv88e6xxx_probe' was not declared. Should it be static?

[1] http://patchwork.ozlabs.org/patch/632708/

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ee06055..1972ec5 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -111,7 +111,8 @@ static int _mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps,
 	return ret;
 }
 
-int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr, int reg)
+static int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr,
+			      int reg)
 {
 	int ret;
 
@@ -165,8 +166,8 @@ static int _mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
 	return __mv88e6xxx_reg_write(ps->bus, ps->sw_addr, addr, reg, val);
 }
 
-int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
-			int reg, u16 val)
+static int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
+			       int reg, u16 val)
 {
 	int ret;
 
@@ -229,7 +230,7 @@ static int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
 	return 0;
 }
 
-int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
+static int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
@@ -370,7 +371,7 @@ static void mv88e6xxx_ppu_access_put(struct mv88e6xxx_priv_state *ps)
 	mutex_unlock(&ps->ppu_mutex);
 }
 
-void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps)
+static void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps)
 {
 	mutex_init(&ps->ppu_mutex);
 	INIT_WORK(&ps->ppu_work, mv88e6xxx_ppu_reenable_work);
@@ -490,7 +491,7 @@ static void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
 
 	reg |= PORT_PCS_CTRL_FORCE_LINK;
 	if (phydev->link)
-			reg |= PORT_PCS_CTRL_LINK_UP;
+		reg |= PORT_PCS_CTRL_LINK_UP;
 
 	if (mv88e6xxx_6065_family(ps) && phydev->speed > SPEED_100)
 		goto out;
@@ -1314,9 +1315,9 @@ static int _mv88e6xxx_port_state(struct mv88e6xxx_priv_state *ps, int port,
 		 * Blocking or Listening state.
 		 */
 		if ((oldstate == PORT_CONTROL_STATE_LEARNING ||
-		     oldstate == PORT_CONTROL_STATE_FORWARDING)
-		    && (state == PORT_CONTROL_STATE_DISABLED ||
-			state == PORT_CONTROL_STATE_BLOCKING)) {
+		     oldstate == PORT_CONTROL_STATE_FORWARDING) &&
+		    (state == PORT_CONTROL_STATE_DISABLED ||
+		     state == PORT_CONTROL_STATE_BLOCKING)) {
 			ret = _mv88e6xxx_atu_remove(ps, 0, port, false);
 			if (ret)
 				return ret;
@@ -1659,7 +1660,8 @@ static int mv88e6xxx_port_vlan_dump(struct dsa_switch *ds, int port,
 			continue;
 
 		/* reinit and dump this VLAN obj */
-		vlan->vid_begin = vlan->vid_end = next.vid;
+		vlan->vid_begin = next.vid;
+		vlan->vid_end = next.vid;
 		vlan->flags = 0;
 
 		if (next.data[port] == GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED)
@@ -2093,9 +2095,10 @@ unlock:
 	return ret;
 }
 
-static int mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
-				       const struct switchdev_obj_port_vlan *vlan,
-				       struct switchdev_trans *trans)
+static int
+mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
+			    const struct switchdev_obj_port_vlan *vlan,
+			    struct switchdev_trans *trans)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int err;
@@ -2735,7 +2738,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_priv_state *ps, int port)
 		    mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
 		    mv88e6xxx_6095_family(ps) || mv88e6xxx_6065_family(ps) ||
 		    mv88e6xxx_6185_family(ps) || mv88e6xxx_6320_family(ps)) {
-				reg |= PORT_CONTROL_EGRESS_ADD_TAG;
+			reg |= PORT_CONTROL_EGRESS_ADD_TAG;
 		}
 	}
 	if (dsa_is_dsa_port(ds, port)) {
@@ -3158,7 +3161,8 @@ unlock:
 	return err;
 }
 
-int mv88e6xxx_mdio_page_read(struct dsa_switch *ds, int port, int page, int reg)
+static int mv88e6xxx_mdio_page_read(struct dsa_switch *ds, int port, int page,
+				    int reg)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
@@ -3170,8 +3174,8 @@ int mv88e6xxx_mdio_page_read(struct dsa_switch *ds, int port, int page, int reg)
 	return ret;
 }
 
-int mv88e6xxx_mdio_page_write(struct dsa_switch *ds, int port, int page,
-			      int reg, int val)
+static int mv88e6xxx_mdio_page_write(struct dsa_switch *ds, int port, int page,
+				     int reg, int val)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
@@ -3650,7 +3654,7 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	return name;
 }
 
-struct dsa_switch_driver mv88e6xxx_switch_driver = {
+static struct dsa_switch_driver mv88e6xxx_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_EDSA,
 	.probe			= mv88e6xxx_drv_probe,
 	.setup			= mv88e6xxx_setup,
@@ -3686,7 +3690,7 @@ struct dsa_switch_driver mv88e6xxx_switch_driver = {
 	.port_fdb_dump          = mv88e6xxx_port_fdb_dump,
 };
 
-int mv88e6xxx_probe(struct mdio_device *mdiodev)
+static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 {
 	struct device *dev = &mdiodev->dev;
 	struct device_node *np = dev->of_node;
-- 
2.8.3

[PATCH v3 net-next v3 02/14] net: dsa: mv88e6xxx: remove redundant assignments

[Vivien Didelot]

The chip->ds and ds->slave_mii_bus assignments are common to both legacy
and new MDIO probing and are already done in the later setup code.

Remove the duplicated assignments from the MDIO probing code.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 1972ec5..673283a 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3708,7 +3708,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	ds->priv = ps;
 	ds->dev = dev;
 	ps->dev = dev;
-	ps->ds = ds;
 	ps->bus = mdiodev->bus;
 	ps->sw_addr = mdiodev->addr;
 	mutex_init(&ps->smi_mutex);
@@ -3748,8 +3747,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		return err;
 
-	ds->slave_mii_bus = ps->mdio_bus;
-
 	dev_set_drvdata(dev, ds);
 
 	err = dsa_register_switch(ds, mdiodev->dev.of_node);
-- 
2.8.3

[PATCH v3 net-next v3 03/14] net: dsa: mv88e6xxx: use already declared variables

[Vivien Didelot]

In the MDIO probing function, dev is already assigned to &mdiodev->dev
and np is already assigned to mdiodev->dev.of_node, so use them.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 673283a..b3170ea 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3728,7 +3728,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (!ps->info)
 		return -ENODEV;
 
-	ps->reset = devm_gpiod_get(&mdiodev->dev, "reset", GPIOD_ASIS);
+	ps->reset = devm_gpiod_get(dev, "reset", GPIOD_ASIS);
 	if (IS_ERR(ps->reset)) {
 		err = PTR_ERR(ps->reset);
 		if (err == -ENOENT) {
@@ -3743,13 +3743,13 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	    !of_property_read_u32(np, "eeprom-length", &eeprom_len))
 		ps->eeprom_len = eeprom_len;
 
-	err = mv88e6xxx_mdio_register(ps, mdiodev->dev.of_node);
+	err = mv88e6xxx_mdio_register(ps, np);
 	if (err)
 		return err;
 
 	dev_set_drvdata(dev, ds);
 
-	err = dsa_register_switch(ds, mdiodev->dev.of_node);
+	err = dsa_register_switch(ds, np);
 	if (err) {
 		mv88e6xxx_mdio_unregister(ps);
 		return err;
-- 
2.8.3

[PATCH v3 net-next v3 04/14] net: dsa: mv88e6xxx: do not increment bus refcount

[Vivien Didelot]

The MDIO device probe and remove functions are respectively incrementing
and decrementing the bus refcount themselves. Since these bus level
actions are out of the device scope, remove them.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Acked-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index b3170ea..4b4bffc 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3712,8 +3712,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	ps->sw_addr = mdiodev->addr;
 	mutex_init(&ps->smi_mutex);
 
-	get_device(&ps->bus->dev);
-
 	ds->drv = &mv88e6xxx_switch_driver;
 
 	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
@@ -3767,7 +3765,6 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
 	dsa_unregister_switch(ds);
-	put_device(&ps->bus->dev);
 
 	mv88e6xxx_mdio_unregister(ps);
 }
-- 
2.8.3

[PATCH v3 net-next v3 05/14] net: dsa: mv88e6xxx: add switch register helpers

[Vivien Didelot]

The mixed assignments, allocations and registrations in the probe code
make it hard to follow the logic and figure out what is DSA or chip
specific.

Extract the struct dsa_switch related code in a simple
mv88e6xxx_register_switch helper function.

For symmetry in the code, add a mv88e6xxx_unregister_switch function.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 4b4bffc..ad7735d 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3690,30 +3690,48 @@ static struct dsa_switch_driver mv88e6xxx_switch_driver = {
 	.port_fdb_dump          = mv88e6xxx_port_fdb_dump,
 };
 
+static int mv88e6xxx_register_switch(struct mv88e6xxx_priv_state *ps,
+				     struct device_node *np)
+{
+	struct device *dev = ps->dev;
+	struct dsa_switch *ds;
+
+	ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
+	if (!ds)
+		return -ENOMEM;
+
+	ds->dev = dev;
+	ds->priv = ps;
+	ds->drv = &mv88e6xxx_switch_driver;
+
+	dev_set_drvdata(dev, ds);
+
+	return dsa_register_switch(ds, np);
+}
+
+static void mv88e6xxx_unregister_switch(struct mv88e6xxx_priv_state *ps)
+{
+	dsa_unregister_switch(ps->ds);
+}
+
 static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 {
 	struct device *dev = &mdiodev->dev;
 	struct device_node *np = dev->of_node;
 	struct mv88e6xxx_priv_state *ps;
 	int id, prod_num, rev;
-	struct dsa_switch *ds;
 	u32 eeprom_len;
 	int err;
 
-	ds = devm_kzalloc(dev, sizeof(*ds) + sizeof(*ps), GFP_KERNEL);
-	if (!ds)
+	ps = devm_kzalloc(dev, sizeof(*ps), GFP_KERNEL);
+	if (!ps)
 		return -ENOMEM;
 
-	ps = (struct mv88e6xxx_priv_state *)(ds + 1);
-	ds->priv = ps;
-	ds->dev = dev;
 	ps->dev = dev;
 	ps->bus = mdiodev->bus;
 	ps->sw_addr = mdiodev->addr;
 	mutex_init(&ps->smi_mutex);
 
-	ds->drv = &mv88e6xxx_switch_driver;
-
 	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
 	if (id < 0)
 		return id;
@@ -3745,9 +3763,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		return err;
 
-	dev_set_drvdata(dev, ds);
-
-	err = dsa_register_switch(ds, np);
+	err = mv88e6xxx_register_switch(ps, np);
 	if (err) {
 		mv88e6xxx_mdio_unregister(ps);
 		return err;
@@ -3764,8 +3780,7 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
 	struct dsa_switch *ds = dev_get_drvdata(&mdiodev->dev);
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-	dsa_unregister_switch(ds);
-
+	mv88e6xxx_unregister_switch(ps);
 	mv88e6xxx_mdio_unregister(ps);
 }
 
-- 
2.8.3

[PATCH v3 net-next v3 06/14] net: dsa: mv88e6xxx: use gpio get optional variant

[Vivien Didelot]

Use the optional variant to get the reset GPIO line, instead of checking
for the -ENOENT error.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ad7735d..ec28465 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3744,16 +3744,9 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (!ps->info)
 		return -ENODEV;
 
-	ps->reset = devm_gpiod_get(dev, "reset", GPIOD_ASIS);
-	if (IS_ERR(ps->reset)) {
-		err = PTR_ERR(ps->reset);
-		if (err == -ENOENT) {
-			/* Optional, so not an error */
-			ps->reset = NULL;
-		} else {
-			return err;
-		}
-	}
+	ps->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
+	if (IS_ERR(ps->reset))
+		return PTR_ERR(ps->reset);
 
 	if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_EEPROM) &&
 	    !of_property_read_u32(np, "eeprom-length", &eeprom_len))
-- 
2.8.3

Re: [PATCH v3 net-next v3 06/14] net: dsa: mv88e6xxx: use gpio get optional variant

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:28PM -0400, Vivien Didelot wrote:
> Use the optional variant to get the reset GPIO line, instead of checking
> for the -ENOENT error.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v3 net-next v3 07/14] net: dsa: mv88e6xxx: remove table args in info lookup

[Vivien Didelot]

The mv88e6xxx_table array and the mv88e6xxx_lookup_info function are
static, so remove the table and size arguments from the lookup function.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ec28465..75e5408 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3590,15 +3590,13 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 	},
 };
 
-static const struct mv88e6xxx_info *
-mv88e6xxx_lookup_info(unsigned int prod_num, const struct mv88e6xxx_info *table,
-		      unsigned int num)
+static const struct mv88e6xxx_info *mv88e6xxx_lookup_info(unsigned int prod_num)
 {
 	int i;
 
-	for (i = 0; i < num; ++i)
-		if (table[i].prod_num == prod_num)
-			return &table[i];
+	for (i = 0; i < ARRAY_SIZE(mv88e6xxx_table); ++i)
+		if (mv88e6xxx_table[i].prod_num == prod_num)
+			return &mv88e6xxx_table[i];
 
 	return NULL;
 }
@@ -3625,8 +3623,7 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	prod_num = (id & 0xfff0) >> 4;
 	rev = id & 0x000f;
 
-	info = mv88e6xxx_lookup_info(prod_num, mv88e6xxx_table,
-				     ARRAY_SIZE(mv88e6xxx_table));
+	info = mv88e6xxx_lookup_info(prod_num);
 	if (!info)
 		return NULL;
 
@@ -3739,8 +3736,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	prod_num = (id & 0xfff0) >> 4;
 	rev = id & 0x000f;
 
-	ps->info = mv88e6xxx_lookup_info(prod_num, mv88e6xxx_table,
-					 ARRAY_SIZE(mv88e6xxx_table));
+	ps->info = mv88e6xxx_lookup_info(prod_num);
 	if (!ps->info)
 		return -ENODEV;
 
-- 
2.8.3

Re: [PATCH v3 net-next v3 07/14] net: dsa: mv88e6xxx: remove table args in info lookup

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:29PM -0400, Vivien Didelot wrote:
> The mv88e6xxx_table array and the mv88e6xxx_lookup_info function are
> static, so remove the table and size arguments from the lookup function.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v3 net-next v3 08/14] net: dsa: mv88e6xxx: rename smi_mutex to reg_lock

[Vivien Didelot]

The chip smi_mutex mutex is used to protect the access to the internal
switch registers, not only the Multi-chip Addressing Mode, as commented.

Other registers access (like management frames) may use this mutex.

Since we will isolate SMI-specific pieces of code, avoid the confusion
now by renaming smi_mutex to reg_lock. No functional changes here.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 120 ++++++++++++++++++++++----------------------
 drivers/net/dsa/mv88e6xxx.h |   7 +--
 2 files changed, 62 insertions(+), 65 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 75e5408..2c86172 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -29,10 +29,10 @@
 #include <net/switchdev.h>
 #include "mv88e6xxx.h"
 
-static void assert_smi_lock(struct mv88e6xxx_priv_state *ps)
+static void assert_reg_lock(struct mv88e6xxx_priv_state *ps)
 {
-	if (unlikely(!mutex_is_locked(&ps->smi_mutex))) {
-		dev_err(ps->dev, "SMI lock not held!\n");
+	if (unlikely(!mutex_is_locked(&ps->reg_lock))) {
+		dev_err(ps->dev, "Switch registers lock not held!\n");
 		dump_stack();
 	}
 }
@@ -99,7 +99,7 @@ static int _mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps,
 {
 	int ret;
 
-	assert_smi_lock(ps);
+	assert_reg_lock(ps);
 
 	ret = __mv88e6xxx_reg_read(ps->bus, ps->sw_addr, addr, reg);
 	if (ret < 0)
@@ -116,9 +116,9 @@ static int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr,
 {
 	int ret;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	ret = _mv88e6xxx_reg_read(ps, addr, reg);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -158,7 +158,7 @@ static int __mv88e6xxx_reg_write(struct mii_bus *bus, int sw_addr, int addr,
 static int _mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
 				int reg, u16 val)
 {
-	assert_smi_lock(ps);
+	assert_reg_lock(ps);
 
 	dev_dbg(ps->dev, "-> addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
 		addr, reg, val);
@@ -171,9 +171,9 @@ static int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
 {
 	int ret;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	ret = _mv88e6xxx_reg_write(ps, addr, reg, val);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -320,7 +320,7 @@ static void mv88e6xxx_ppu_reenable_work(struct work_struct *ugly)
 
 	ps = container_of(ugly, struct mv88e6xxx_priv_state, ppu_work);
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	if (mutex_trylock(&ps->ppu_mutex)) {
 		if (mv88e6xxx_ppu_enable(ps) == 0)
@@ -328,7 +328,7 @@ static void mv88e6xxx_ppu_reenable_work(struct work_struct *ugly)
 		mutex_unlock(&ps->ppu_mutex);
 	}
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 }
 
 static void mv88e6xxx_ppu_reenable_timer(unsigned long _ps)
@@ -477,7 +477,7 @@ static void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
 	if (!phy_is_pseudo_fixed_link(phydev))
 		return;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_PCS_CTRL);
 	if (ret < 0)
@@ -528,7 +528,7 @@ static void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
 	_mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_PCS_CTRL, reg);
 
 out:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 }
 
 static int _mv88e6xxx_stats_wait(struct mv88e6xxx_priv_state *ps)
@@ -753,11 +753,11 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 	int ret;
 	int i, j;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	ret = _mv88e6xxx_stats_snapshot(ps, port);
 	if (ret < 0) {
-		mutex_unlock(&ps->smi_mutex);
+		mutex_unlock(&ps->reg_lock);
 		return;
 	}
 	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
@@ -768,7 +768,7 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 		}
 	}
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 }
 
 static int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
@@ -787,7 +787,7 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 
 	memset(p, 0xff, 32 * sizeof(u16));
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	for (i = 0; i < 32; i++) {
 		int ret;
@@ -797,7 +797,7 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
 			p[i] = ret;
 	}
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 }
 
 static int _mv88e6xxx_wait(struct mv88e6xxx_priv_state *ps, int reg, int offset,
@@ -824,9 +824,9 @@ static int mv88e6xxx_wait(struct mv88e6xxx_priv_state *ps, int reg,
 {
 	int ret;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	ret = _mv88e6xxx_wait(ps, reg, offset, mask);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -1123,7 +1123,7 @@ static int mv88e6xxx_get_eee(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_EEE))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	reg = mv88e6xxx_mdio_read_indirect(ps, port, 16);
 	if (reg < 0)
@@ -1140,7 +1140,7 @@ static int mv88e6xxx_get_eee(struct dsa_switch *ds, int port,
 	reg = 0;
 
 out:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 	return reg;
 }
 
@@ -1154,7 +1154,7 @@ static int mv88e6xxx_set_eee(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_EEE))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	ret = mv88e6xxx_mdio_read_indirect(ps, port, 16);
 	if (ret < 0)
@@ -1168,7 +1168,7 @@ static int mv88e6xxx_set_eee(struct dsa_switch *ds, int port,
 
 	ret = mv88e6xxx_mdio_write_indirect(ps, port, 16, reg);
 out:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -1402,9 +1402,9 @@ static void mv88e6xxx_port_stp_state_set(struct dsa_switch *ds, int port,
 		break;
 	}
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	err = _mv88e6xxx_port_state(ps, port, stp_state);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	if (err)
 		netdev_err(ds->ports[port].netdev,
@@ -1638,7 +1638,7 @@ static int mv88e6xxx_port_vlan_dump(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_VTU))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	err = _mv88e6xxx_port_pvid_get(ps, port, &pvid);
 	if (err)
@@ -1676,7 +1676,7 @@ static int mv88e6xxx_port_vlan_dump(struct dsa_switch *ds, int port,
 	} while (next.vid < GLOBAL_VTU_VID_MASK);
 
 unlock:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return err;
 }
@@ -2004,7 +2004,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 	if (!vid_begin)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	err = _mv88e6xxx_vtu_vid_write(ps, vid_begin - 1);
 	if (err)
@@ -2043,7 +2043,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 	} while (vlan.vid < vid_end);
 
 unlock:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return err;
 }
@@ -2066,7 +2066,7 @@ static int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_VTU))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_CONTROL_2);
 	if (ret < 0)
@@ -2090,7 +2090,7 @@ static int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
 
 	ret = 0;
 unlock:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -2149,7 +2149,7 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_VTU))
 		return;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
 		if (_mv88e6xxx_port_vlan_add(ps, port, vid, untagged))
@@ -2161,7 +2161,7 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
 		netdev_err(ds->ports[port].netdev, "failed to set PVID %d\n",
 			   vlan->vid_end);
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 }
 
 static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_priv_state *ps,
@@ -2210,7 +2210,7 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_VTU))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	err = _mv88e6xxx_port_pvid_get(ps, port, &pvid);
 	if (err)
@@ -2229,7 +2229,7 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
 	}
 
 unlock:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return err;
 }
@@ -2341,11 +2341,11 @@ static void mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_ATU))
 		return;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	if (_mv88e6xxx_port_fdb_load(ps, port, fdb->addr, fdb->vid, state))
 		netdev_err(ds->ports[port].netdev,
 			   "failed to load MAC address\n");
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 }
 
 static int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
@@ -2357,10 +2357,10 @@ static int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_ATU))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	ret = _mv88e6xxx_port_fdb_load(ps, port, fdb->addr, fdb->vid,
 				       GLOBAL_ATU_DATA_STATE_UNUSED);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -2465,7 +2465,7 @@ static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_ATU))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	/* Dump port's default Filtering Information Database (VLAN ID 0) */
 	err = _mv88e6xxx_port_fid_get(ps, port, &fid);
@@ -2496,7 +2496,7 @@ static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
 	} while (vlan.vid < GLOBAL_VTU_VID_MASK);
 
 unlock:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return err;
 }
@@ -2510,7 +2510,7 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_VLANTABLE))
 		return -EOPNOTSUPP;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	/* Assign the bridge and remap each port's VLANTable */
 	ps->ports[port].bridge_dev = bridge;
@@ -2523,7 +2523,7 @@ static int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 		}
 	}
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return err;
 }
@@ -2537,7 +2537,7 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
 	if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_VLANTABLE))
 		return;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	/* Unassign the bridge and remap each port's VLANTable */
 	ps->ports[port].bridge_dev = NULL;
@@ -2548,7 +2548,7 @@ static void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
 				netdev_warn(ds->ports[i].netdev,
 					    "failed to remap\n");
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 }
 
 static int _mv88e6xxx_mdio_page_write(struct mv88e6xxx_priv_state *ps,
@@ -3139,7 +3139,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_EEPROM))
 		mutex_init(&ps->eeprom_mutex);
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	err = mv88e6xxx_switch_reset(ps);
 	if (err)
@@ -3156,7 +3156,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	}
 
 unlock:
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return err;
 }
@@ -3167,9 +3167,9 @@ static int mv88e6xxx_mdio_page_read(struct dsa_switch *ds, int port, int page,
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	ret = _mv88e6xxx_mdio_page_read(ps, port, page, reg);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -3180,9 +3180,9 @@ static int mv88e6xxx_mdio_page_write(struct dsa_switch *ds, int port, int page,
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 	ret = _mv88e6xxx_mdio_page_write(ps, port, page, reg, val);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 
 	return ret;
 }
@@ -3204,7 +3204,7 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int port, int regnum)
 	if (addr < 0)
 		return 0xffff;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_PPU))
 		ret = mv88e6xxx_mdio_read_ppu(ps, addr, regnum);
@@ -3213,7 +3213,7 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int port, int regnum)
 	else
 		ret = mv88e6xxx_mdio_read_direct(ps, addr, regnum);
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 	return ret;
 }
 
@@ -3227,7 +3227,7 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int port, int regnum,
 	if (addr < 0)
 		return 0xffff;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_PPU))
 		ret = mv88e6xxx_mdio_write_ppu(ps, addr, regnum, val);
@@ -3236,7 +3236,7 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int port, int regnum,
 	else
 		ret = mv88e6xxx_mdio_write_direct(ps, addr, regnum, val);
 
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 	return ret;
 }
 
@@ -3310,7 +3310,7 @@ static int mv88e61xx_get_temp(struct dsa_switch *ds, int *temp)
 
 	*temp = 0;
 
-	mutex_lock(&ps->smi_mutex);
+	mutex_lock(&ps->reg_lock);
 
 	ret = mv88e6xxx_mdio_write_direct(ps, 0x0, 0x16, 0x6);
 	if (ret < 0)
@@ -3343,7 +3343,7 @@ static int mv88e61xx_get_temp(struct dsa_switch *ds, int *temp)
 
 error:
 	mv88e6xxx_mdio_write_direct(ps, 0x0, 0x16, 0x0);
-	mutex_unlock(&ps->smi_mutex);
+	mutex_unlock(&ps->reg_lock);
 	return ret;
 }
 
@@ -3637,7 +3637,7 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	ps->sw_addr = sw_addr;
 	ps->info = info;
 	ps->dev = dsa_dev;
-	mutex_init(&ps->smi_mutex);
+	mutex_init(&ps->reg_lock);
 
 	err = mv88e6xxx_mdio_register(ps, NULL);
 	if (err)
@@ -3727,7 +3727,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	ps->dev = dev;
 	ps->bus = mdiodev->bus;
 	ps->sw_addr = mdiodev->addr;
-	mutex_init(&ps->smi_mutex);
+	mutex_init(&ps->reg_lock);
 
 	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
 	if (id < 0)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 8221c3c..b279f8c 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -554,11 +554,8 @@ struct mv88e6xxx_priv_state {
 	/* The device this structure is associated to */
 	struct device *dev;
 
-	/* When using multi-chip addressing, this mutex protects
-	 * access to the indirect access registers.  (In single-chip
-	 * mode, this mutex is effectively useless.)
-	 */
-	struct mutex	smi_mutex;
+	/* This mutex protects the access to the switch registers */
+	struct mutex reg_lock;
 
 	/* The MII bus and the address on the bus that is used to
 	 * communication with the switch
-- 
2.8.3

Re: [PATCH v3 net-next v3 08/14] net: dsa: mv88e6xxx: rename smi_mutex to reg_lock

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:30PM -0400, Vivien Didelot wrote:
> The chip smi_mutex mutex is used to protect the access to the internal
> switch registers, not only the Multi-chip Addressing Mode, as commented.
> 
> Other registers access (like management frames) may use this mutex.
> 
> Since we will isolate SMI-specific pieces of code, avoid the confusion
> now by renaming smi_mutex to reg_lock. No functional changes here.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v3 net-next v3 09/14] net: dsa: mv88e6xxx: add chip allocation helper

[Vivien Didelot]

Add an helper function to allocate the chip structure at the beginning
of the probe functions. It will be used to initialize the SMI access.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 2c86172..113092a 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3601,6 +3601,21 @@ static const struct mv88e6xxx_info *mv88e6xxx_lookup_info(unsigned int prod_num)
 	return NULL;
 }
 
+static struct mv88e6xxx_priv_state *mv88e6xxx_alloc_chip(struct device *dev)
+{
+	struct mv88e6xxx_priv_state *ps;
+
+	ps = devm_kzalloc(dev, sizeof(*ps), GFP_KERNEL);
+	if (!ps)
+		return NULL;
+
+	ps->dev = dev;
+
+	mutex_init(&ps->reg_lock);
+
+	return ps;
+}
+
 static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 				       struct device *host_dev, int sw_addr,
 				       void **priv)
@@ -3616,32 +3631,30 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	if (!bus)
 		return NULL;
 
+	ps = mv88e6xxx_alloc_chip(dsa_dev);
+	if (!ps)
+		return NULL;
+
 	id = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
 	if (id < 0)
-		return NULL;
+		goto free;
 
 	prod_num = (id & 0xfff0) >> 4;
 	rev = id & 0x000f;
 
 	info = mv88e6xxx_lookup_info(prod_num);
 	if (!info)
-		return NULL;
+		goto free;
 
 	name = 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->dev = dsa_dev;
-	mutex_init(&ps->reg_lock);
 
 	err = mv88e6xxx_mdio_register(ps, NULL);
 	if (err)
-		return NULL;
+		goto free;
 
 	*priv = ps;
 
@@ -3649,6 +3662,10 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 		 prod_num, name, rev);
 
 	return name;
+free:
+	devm_kfree(dsa_dev, ps);
+
+	return NULL;
 }
 
 static struct dsa_switch_driver mv88e6xxx_switch_driver = {
@@ -3720,14 +3737,12 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	u32 eeprom_len;
 	int err;
 
-	ps = devm_kzalloc(dev, sizeof(*ps), GFP_KERNEL);
+	ps = mv88e6xxx_alloc_chip(dev);
 	if (!ps)
 		return -ENOMEM;
 
-	ps->dev = dev;
 	ps->bus = mdiodev->bus;
 	ps->sw_addr = mdiodev->addr;
-	mutex_init(&ps->reg_lock);
 
 	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
 	if (id < 0)
-- 
2.8.3

Re: [PATCH v3 net-next v3 09/14] net: dsa: mv88e6xxx: add chip allocation helper

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:31PM -0400, Vivien Didelot wrote:
> Add an helper function to allocate the chip structure at the beginning
> of the probe functions. It will be used to initialize the SMI access.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v3 net-next v3 10/14] net: dsa: mv88e6xxx: add SMI init helper

[Vivien Didelot]

Add an helper function to isolate SMI specific assignations and checks.

This function will later help choosing the different SMI accesses based
of the compatible info.

Since the chip structure is already allocated in the legacy probe, use
the mv88e6xxx_reg_read access routine instead of __mv88e6xxx_reg_read.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 113092a..4e24ac5 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3616,6 +3616,19 @@ static struct mv88e6xxx_priv_state *mv88e6xxx_alloc_chip(struct device *dev)
 	return ps;
 }
 
+static int mv88e6xxx_smi_init(struct mv88e6xxx_priv_state *ps,
+			      struct mii_bus *bus, int sw_addr)
+{
+	/* ADDR[0] pin is unavailable externally and considered zero */
+	if (sw_addr & 0x1)
+		return -EINVAL;
+
+	ps->bus = bus;
+	ps->sw_addr = sw_addr;
+
+	return 0;
+}
+
 static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 				       struct device *host_dev, int sw_addr,
 				       void **priv)
@@ -3635,7 +3648,11 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	if (!ps)
 		return NULL;
 
-	id = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), PORT_SWITCH_ID);
+	err = mv88e6xxx_smi_init(ps, bus, sw_addr);
+	if (err)
+		goto free;
+
+	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
 	if (id < 0)
 		goto free;
 
@@ -3648,8 +3665,6 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 
 	name = info->name;
 
-	ps->bus = bus;
-	ps->sw_addr = sw_addr;
 	ps->info = info;
 
 	err = mv88e6xxx_mdio_register(ps, NULL);
@@ -3741,8 +3756,9 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (!ps)
 		return -ENOMEM;
 
-	ps->bus = mdiodev->bus;
-	ps->sw_addr = mdiodev->addr;
+	err = mv88e6xxx_smi_init(ps, mdiodev->bus, mdiodev->addr);
+	if (err)
+		return err;
 
 	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
 	if (id < 0)
-- 
2.8.3

Re: [PATCH v3 net-next v3 10/14] net: dsa: mv88e6xxx: add SMI init helper

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:32PM -0400, Vivien Didelot wrote:
> Add an helper function to isolate SMI specific assignations and checks.

I don't think you meant assignations. Assignments?

Otherwise

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v3 net-next v3 11/14] net: dsa: mv88e6xxx: add detection helper

[Vivien Didelot]

Extract the common detection code which assigns the info structure to
the chip given the read switch ID.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 64 +++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 4e24ac5..de92add 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3601,6 +3601,30 @@ static const struct mv88e6xxx_info *mv88e6xxx_lookup_info(unsigned int prod_num)
 	return NULL;
 }
 
+static int mv88e6xxx_detect(struct mv88e6xxx_priv_state *ps)
+{
+	const struct mv88e6xxx_info *info;
+	int id, prod_num, rev;
+
+	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
+	if (id < 0)
+		return id;
+
+	prod_num = (id & 0xfff0) >> 4;
+	rev = id & 0x000f;
+
+	info = mv88e6xxx_lookup_info(prod_num);
+	if (!info)
+		return -ENODEV;
+
+	ps->info = info;
+
+	dev_info(ps->dev, "switch 0x%x detected: %s, revision %u\n",
+		 ps->info->prod_num, ps->info->name, rev);
+
+	return 0;
+}
+
 static struct mv88e6xxx_priv_state *mv88e6xxx_alloc_chip(struct device *dev)
 {
 	struct mv88e6xxx_priv_state *ps;
@@ -3633,11 +3657,8 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 				       struct device *host_dev, int sw_addr,
 				       void **priv)
 {
-	const struct mv88e6xxx_info *info;
 	struct mv88e6xxx_priv_state *ps;
 	struct mii_bus *bus;
-	const char *name;
-	int id, prod_num, rev;
 	int err;
 
 	bus = dsa_host_dev_to_mii_bus(host_dev);
@@ -3652,31 +3673,17 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	if (err)
 		goto free;
 
-	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
-	if (id < 0)
-		goto free;
-
-	prod_num = (id & 0xfff0) >> 4;
-	rev = id & 0x000f;
-
-	info = mv88e6xxx_lookup_info(prod_num);
-	if (!info)
+	err = mv88e6xxx_detect(ps);
+	if (err)
 		goto free;
 
-	name = info->name;
-
-	ps->info = info;
-
 	err = mv88e6xxx_mdio_register(ps, NULL);
 	if (err)
 		goto free;
 
 	*priv = ps;
 
-	dev_info(&ps->bus->dev, "switch 0x%x probed: %s, revision %u\n",
-		 prod_num, name, rev);
-
-	return name;
+	return ps->info->name;
 free:
 	devm_kfree(dsa_dev, ps);
 
@@ -3748,7 +3755,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	struct device *dev = &mdiodev->dev;
 	struct device_node *np = dev->of_node;
 	struct mv88e6xxx_priv_state *ps;
-	int id, prod_num, rev;
 	u32 eeprom_len;
 	int err;
 
@@ -3760,16 +3766,9 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		return err;
 
-	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
-	if (id < 0)
-		return id;
-
-	prod_num = (id & 0xfff0) >> 4;
-	rev = id & 0x000f;
-
-	ps->info = mv88e6xxx_lookup_info(prod_num);
-	if (!ps->info)
-		return -ENODEV;
+	err = mv88e6xxx_detect(ps);
+	if (err)
+		return err;
 
 	ps->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
 	if (IS_ERR(ps->reset))
@@ -3789,9 +3788,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 		return err;
 	}
 
-	dev_info(dev, "switch 0x%x probed: %s, revision %u\n",
-		 prod_num, ps->info->name, rev);
-
 	return 0;
 }
 
-- 
2.8.3

Re: [PATCH v3 net-next v3 11/14] net: dsa: mv88e6xxx: add detection helper

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:33PM -0400, Vivien Didelot wrote:
> Extract the common detection code which assigns the info structure to
> the chip given the read switch ID.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v3 net-next v3 12/14] net: dsa: mv88e6xxx: pass compatible info

[Vivien Didelot]

After allocating the chip structure, pass it a compatible info pointer.

The compatible info structure will be used later to describe how to
access the switch registers and where to read the switch ID.

For the standard MDIO probe, get it from the device node data. For the
legacy DSA driver probing, pass it the 88E6085 info.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index de92add..cadd1e3 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -21,6 +21,7 @@
 #include <linux/list.h>
 #include <linux/mdio.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/of_mdio.h>
 #include <linux/netdevice.h>
 #include <linux/gpio/consumer.h>
@@ -3617,6 +3618,7 @@ static int mv88e6xxx_detect(struct mv88e6xxx_priv_state *ps)
 	if (!info)
 		return -ENODEV;
 
+	/* Update the compatible info with the probed one */
 	ps->info = info;
 
 	dev_info(ps->dev, "switch 0x%x detected: %s, revision %u\n",
@@ -3669,6 +3671,9 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	if (!ps)
 		return NULL;
 
+	/* Legacy SMI probing will only support chips similar to 88E6085 */
+	ps->info = &mv88e6xxx_table[MV88E6085];
+
 	err = mv88e6xxx_smi_init(ps, bus, sw_addr);
 	if (err)
 		goto free;
@@ -3754,14 +3759,21 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 {
 	struct device *dev = &mdiodev->dev;
 	struct device_node *np = dev->of_node;
+	const struct mv88e6xxx_info *compat_info;
 	struct mv88e6xxx_priv_state *ps;
 	u32 eeprom_len;
 	int err;
 
+	compat_info = of_device_get_match_data(dev);
+	if (!compat_info)
+		return -EINVAL;
+
 	ps = mv88e6xxx_alloc_chip(dev);
 	if (!ps)
 		return -ENOMEM;
 
+	ps->info = compat_info;
+
 	err = mv88e6xxx_smi_init(ps, mdiodev->bus, mdiodev->addr);
 	if (err)
 		return err;
@@ -3801,7 +3813,10 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
 }
 
 static const struct of_device_id mv88e6xxx_of_match[] = {
-	{ .compatible = "marvell,mv88e6085" },
+	{
+		.compatible = "marvell,mv88e6085",
+		.data = &mv88e6xxx_table[MV88E6085],
+	},
 	{ /* sentinel */ },
 };
 
-- 
2.8.3

Re: [PATCH v3 net-next v3 12/14] net: dsa: mv88e6xxx: pass compatible info

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:34PM -0400, Vivien Didelot wrote:
> After allocating the chip structure, pass it a compatible info pointer.
> 
> The compatible info structure will be used later to describe how to
> access the switch registers and where to read the switch ID.
> 
> For the standard MDIO probe, get it from the device node data. For the
> legacy DSA driver probing, pass it the 88E6085 info.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v3 net-next v3 13/14] net: dsa: mv88e6xxx: add addressing mode to info

[Vivien Didelot]

When the SMI address of the switch chip is zero, the chip assumes to be
the only one on the SMI master bus and thus responds to all its known
SMI devices addresses (port registers, Global2, etc.)

When its SMI address is not zero, some chips (e.g. 88E6352) use an
indirect access through two SMI Command and Data registers.

Other models (e.g. 88E6060) using less than 16 internal SMI addresses
always use a direct access.

Add a capability flag to describe chips supporting the (indirect)
Multi-chip Addressing Mode, and a low-level API to access the registers
via SMI.

Other accesses (like Ethernet management frames) may be added later.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 135 ++++++++++++++++++++++++++++++++++----------
 drivers/net/dsa/mv88e6xxx.h |  19 ++++++-
 2 files changed, 123 insertions(+), 31 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index cadd1e3..4d32f5a 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -38,15 +38,75 @@ static void assert_reg_lock(struct mv88e6xxx_priv_state *ps)
 	}
 }
 
-/* If the switch's ADDR[4:0] strap pins are strapped to zero, it will
- * use all 32 SMI bus addresses on its SMI bus, and all switch registers
- * will be directly accessible on some {device address,register address}
- * pair.  If the ADDR[4:0] pins are not strapped to zero, the switch
- * will only respond to SMI transactions to that specific address, and
- * an indirect addressing mechanism needs to be used to access its
- * registers.
+/* The switch ADDR[4:1] configuration pins define the chip SMI device address
+ * (ADDR[0] is always zero, thus only even SMI addresses can be strapped).
+ *
+ * When ADDR is all zero, the chip uses Single-chip Addressing Mode, assuming it
+ * is the only device connected to the SMI master. In this mode it responds to
+ * all 32 possible SMI addresses, and thus maps directly the internal devices.
+ *
+ * When ADDR is non-zero, the chip uses Multi-chip Addressing Mode, allowing
+ * multiple devices to share the SMI interface. In this mode it responds to only
+ * 2 registers, used to indirectly access the internal SMI devices.
  */
-static int mv88e6xxx_reg_wait_ready(struct mii_bus *bus, int sw_addr)
+
+struct mv88e6xxx_smi_ops {
+	int (*read)(struct mii_bus *bus, int sw_addr,
+		    int addr, int reg, u16 *val);
+	int (*write)(struct mii_bus *bus, int sw_addr,
+		     int addr, int reg, u16 val);
+};
+
+static int mv88e6xxx_smi_read(struct mv88e6xxx_priv_state *ps,
+			      int addr, int reg, u16 *val)
+{
+	if (!ps->smi_ops)
+		return -EOPNOTSUPP;
+
+	return ps->smi_ops->read(ps->bus, ps->sw_addr, addr, reg, val);
+}
+
+static int mv88e6xxx_smi_write(struct mv88e6xxx_priv_state *ps,
+			       int addr, int reg, u16 val)
+{
+	if (!ps->smi_ops)
+		return -EOPNOTSUPP;
+
+	return ps->smi_ops->write(ps->bus, ps->sw_addr, addr, reg, val);
+}
+
+static int mv88e6xxx_smi_single_chip_read(struct mii_bus *bus, int sw_addr,
+					  int addr, int reg, u16 *val)
+{
+	int ret;
+
+	ret = mdiobus_read_nested(bus, addr, reg);
+	if (ret < 0)
+		return ret;
+
+	*val = ret & 0xffff;
+
+	return 0;
+}
+
+static int mv88e6xxx_smi_single_chip_write(struct mii_bus *bus, int sw_addr,
+					   int addr, int reg, u16 val)
+{
+	int ret;
+
+	ret = mdiobus_write_nested(bus, addr, reg, val);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static const struct mv88e6xxx_smi_ops mv88e6xxx_smi_single_chip_ops = {
+	.read = mv88e6xxx_smi_single_chip_read,
+	.write = mv88e6xxx_smi_single_chip_write,
+};
+
+static int mv88e6xxx_smi_multi_chip_wait(struct mii_bus *bus, int sw_addr)
 {
 	int ret;
 	int i;
@@ -63,16 +123,13 @@ static int mv88e6xxx_reg_wait_ready(struct mii_bus *bus, int sw_addr)
 	return -ETIMEDOUT;
 }
 
-static int __mv88e6xxx_reg_read(struct mii_bus *bus, int sw_addr, int addr,
-				int reg)
+static int mv88e6xxx_smi_multi_chip_read(struct mii_bus *bus, int sw_addr,
+					 int addr, int reg, u16 *val)
 {
 	int ret;
 
-	if (sw_addr == 0)
-		return mdiobus_read_nested(bus, addr, reg);
-
 	/* Wait for the bus to become free. */
-	ret = mv88e6xxx_reg_wait_ready(bus, sw_addr);
+	ret = mv88e6xxx_smi_multi_chip_wait(bus, sw_addr);
 	if (ret < 0)
 		return ret;
 
@@ -83,7 +140,7 @@ static int __mv88e6xxx_reg_read(struct mii_bus *bus, int sw_addr, int addr,
 		return ret;
 
 	/* Wait for the read command to complete. */
-	ret = mv88e6xxx_reg_wait_ready(bus, sw_addr);
+	ret = mv88e6xxx_smi_multi_chip_wait(bus, sw_addr);
 	if (ret < 0)
 		return ret;
 
@@ -92,24 +149,27 @@ static int __mv88e6xxx_reg_read(struct mii_bus *bus, int sw_addr, int addr,
 	if (ret < 0)
 		return ret;
 
-	return ret & 0xffff;
+	*val = ret & 0xffff;
+
+	return 0;
 }
 
 static int _mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps,
 			       int addr, int reg)
 {
-	int ret;
+	u16 val;
+	int err;
 
 	assert_reg_lock(ps);
 
-	ret = __mv88e6xxx_reg_read(ps->bus, ps->sw_addr, addr, reg);
-	if (ret < 0)
-		return ret;
+	err = mv88e6xxx_smi_read(ps, addr, reg, &val);
+	if (err)
+		return err;
 
 	dev_dbg(ps->dev, "<- addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
-		addr, reg, ret);
+		addr, reg, val);
 
-	return ret;
+	return val;
 }
 
 static int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr,
@@ -124,16 +184,13 @@ static int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr,
 	return ret;
 }
 
-static int __mv88e6xxx_reg_write(struct mii_bus *bus, int sw_addr, int addr,
-				 int reg, u16 val)
+static int mv88e6xxx_smi_multi_chip_write(struct mii_bus *bus, int sw_addr,
+					  int addr, int reg, u16 val)
 {
 	int ret;
 
-	if (sw_addr == 0)
-		return mdiobus_write_nested(bus, addr, reg, val);
-
 	/* Wait for the bus to become free. */
-	ret = mv88e6xxx_reg_wait_ready(bus, sw_addr);
+	ret = mv88e6xxx_smi_multi_chip_wait(bus, sw_addr);
 	if (ret < 0)
 		return ret;
 
@@ -149,22 +206,33 @@ static int __mv88e6xxx_reg_write(struct mii_bus *bus, int sw_addr, int addr,
 		return ret;
 
 	/* Wait for the write command to complete. */
-	ret = mv88e6xxx_reg_wait_ready(bus, sw_addr);
+	ret = mv88e6xxx_smi_multi_chip_wait(bus, sw_addr);
 	if (ret < 0)
 		return ret;
 
 	return 0;
 }
 
+static const struct mv88e6xxx_smi_ops mv88e6xxx_smi_multi_chip_ops = {
+	.read = mv88e6xxx_smi_multi_chip_read,
+	.write = mv88e6xxx_smi_multi_chip_write,
+};
+
 static int _mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
 				int reg, u16 val)
 {
+	int err;
+
 	assert_reg_lock(ps);
 
+	err = mv88e6xxx_smi_write(ps, addr, reg, val);
+	if (err)
+		return err;
+
 	dev_dbg(ps->dev, "-> addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
 		addr, reg, val);
 
-	return __mv88e6xxx_reg_write(ps->bus, ps->sw_addr, addr, reg, val);
+	return 0;
 }
 
 static int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
@@ -3649,6 +3717,13 @@ static int mv88e6xxx_smi_init(struct mv88e6xxx_priv_state *ps,
 	if (sw_addr & 0x1)
 		return -EINVAL;
 
+	if (sw_addr == 0)
+		ps->smi_ops = &mv88e6xxx_smi_single_chip_ops;
+	else if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_MULTI_CHIP))
+		ps->smi_ops = &mv88e6xxx_smi_multi_chip_ops;
+	else
+		return -EINVAL;
+
 	ps->bus = bus;
 	ps->sw_addr = sw_addr;
 
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index b279f8c..c82bd18 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -387,6 +387,12 @@ enum mv88e6xxx_cap {
 	 */
 	MV88E6XXX_CAP_EEPROM,
 
+	/* Multi-chip Addressing Mode.
+	 * Some chips require an indirect SMI access when their SMI device
+	 * address is not zero. See SMI_CMD and SMI_DATA.
+	 */
+	MV88E6XXX_CAP_MULTI_CHIP,
+
 	/* Port State Filtering for 802.1D Spanning Tree.
 	 * See PORT_CONTROL_STATE_* values in the PORT_CONTROL register.
 	 */
@@ -439,6 +445,7 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAG_ATU		BIT(MV88E6XXX_CAP_ATU)
 #define MV88E6XXX_FLAG_EEE		BIT(MV88E6XXX_CAP_EEE)
 #define MV88E6XXX_FLAG_EEPROM		BIT(MV88E6XXX_CAP_EEPROM)
+#define MV88E6XXX_FLAG_MULTI_CHIP	BIT(MV88E6XXX_CAP_MULTI_CHIP)
 #define MV88E6XXX_FLAG_PORTSTATE	BIT(MV88E6XXX_CAP_PORTSTATE)
 #define MV88E6XXX_FLAG_PPU		BIT(MV88E6XXX_CAP_PPU)
 #define MV88E6XXX_FLAG_PPU_ACTIVE	BIT(MV88E6XXX_CAP_PPU_ACTIVE)
@@ -452,25 +459,29 @@ enum mv88e6xxx_cap {
 
 #define MV88E6XXX_FLAGS_FAMILY_6095	\
 	(MV88E6XXX_FLAG_ATU |		\
+	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU |		\
 	 MV88E6XXX_FLAG_VLANTABLE |	\
 	 MV88E6XXX_FLAG_VTU)
 
 #define MV88E6XXX_FLAGS_FAMILY_6097	\
 	(MV88E6XXX_FLAG_ATU |		\
+	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU |		\
 	 MV88E6XXX_FLAG_STU |		\
 	 MV88E6XXX_FLAG_VLANTABLE |	\
 	 MV88E6XXX_FLAG_VTU)
 
 #define MV88E6XXX_FLAGS_FAMILY_6165	\
-	(MV88E6XXX_FLAG_STU |		\
+	(MV88E6XXX_FLAG_MULTI_CHIP |	\
+	 MV88E6XXX_FLAG_STU |		\
 	 MV88E6XXX_FLAG_SWITCH_MAC |	\
 	 MV88E6XXX_FLAG_TEMP |		\
 	 MV88E6XXX_FLAG_VTU)
 
 #define MV88E6XXX_FLAGS_FAMILY_6185	\
 	(MV88E6XXX_FLAG_ATU |		\
+	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU |		\
 	 MV88E6XXX_FLAG_VLANTABLE |	\
 	 MV88E6XXX_FLAG_VTU)
@@ -479,6 +490,7 @@ enum mv88e6xxx_cap {
 	(MV88E6XXX_FLAG_ATU |		\
 	 MV88E6XXX_FLAG_EEE |		\
 	 MV88E6XXX_FLAG_EEPROM |	\
+	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PORTSTATE |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
 	 MV88E6XXX_FLAG_SMI_PHY |	\
@@ -490,6 +502,7 @@ enum mv88e6xxx_cap {
 
 #define MV88E6XXX_FLAGS_FAMILY_6351	\
 	(MV88E6XXX_FLAG_ATU |		\
+	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PORTSTATE |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
 	 MV88E6XXX_FLAG_SMI_PHY |	\
@@ -503,6 +516,7 @@ enum mv88e6xxx_cap {
 	(MV88E6XXX_FLAG_ATU |		\
 	 MV88E6XXX_FLAG_EEE |		\
 	 MV88E6XXX_FLAG_EEPROM |	\
+	 MV88E6XXX_FLAG_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PORTSTATE |	\
 	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
 	 MV88E6XXX_FLAG_SMI_PHY |	\
@@ -541,6 +555,8 @@ struct mv88e6xxx_vtu_stu_entry {
 	u8	data[DSA_MAX_PORTS];
 };
 
+struct mv88e6xxx_smi_ops;
+
 struct mv88e6xxx_priv_port {
 	struct net_device *bridge_dev;
 };
@@ -560,6 +576,7 @@ struct mv88e6xxx_priv_state {
 	/* The MII bus and the address on the bus that is used to
 	 * communication with the switch
 	 */
+	const struct mv88e6xxx_smi_ops *smi_ops;
 	struct mii_bus *bus;
 	int sw_addr;
 
-- 
2.8.3

Re: [PATCH v3 net-next v3 13/14] net: dsa: mv88e6xxx: add addressing mode to info

[Andrew Lunn]

> +struct mv88e6xxx_smi_ops {
> +	int (*read)(struct mii_bus *bus, int sw_addr,
> +		    int addr, int reg, u16 *val);
> +	int (*write)(struct mii_bus *bus, int sw_addr,
> +		     int addr, int reg, u16 val);
> +};

Hi Vivien

I still think this API should be based on ps.

With the way you have restructured probe, this now also works, there
is no longer a read without PS in order to get the device ID.

Also, think about the case of reading/writing registers via Ethernet
frames. Such functions would need ps, bus and sw_addr is not useful.

	Andrew

Re: [PATCH v3 net-next v3 13/14] net: dsa: mv88e6xxx: add addressing mode to info

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>> +struct mv88e6xxx_smi_ops {
>> +	int (*read)(struct mii_bus *bus, int sw_addr,
>> +		    int addr, int reg, u16 *val);
>> +	int (*write)(struct mii_bus *bus, int sw_addr,
>> +		     int addr, int reg, u16 val);
>> +};
>
> Hi Vivien
>
> I still think this API should be based on ps.
>
> With the way you have restructured probe, this now also works, there
> is no longer a read without PS in order to get the device ID.
>
> Also, think about the case of reading/writing registers via Ethernet
> frames. Such functions would need ps, bus and sw_addr is not useful.

Yes, I do RMU in mind, that was one motivation behind isolating
SMI-specific pieces of code.

I considered mv88e6xxx_smi_ops private to the SMI (MDIO) access, needing
an additional abstraction for regs access operations.

But I can indeed rename mv88e6xxx_smi_ops to mv88e6xxx_ops and make them
use a ps before one day, implementing an optional ps->rmu_ops.

Thanks,

        Vivien

[PATCH v3 net-next v3 14/14] net: dsa: mv88e6xxx: add port base address to info

[Vivien Didelot]

The switch ID is located at address 0x3 of every Port Registers bank.

But not all Marvell switches have their Port Registers SMI Addresses
starting at 0x10. 88E6060 starts at 0x8 and 88E6390 starts at 0x0.

Add this data in the info structure and use it in the detection code.

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

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 4d32f5a..aeb96db 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3511,6 +3511,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6085",
 		.num_databases = 4096,
 		.num_ports = 10,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6097,
 	},
 
@@ -3520,6 +3521,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6095/88E6095F",
 		.num_databases = 256,
 		.num_ports = 11,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6095,
 	},
 
@@ -3529,6 +3531,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6123",
 		.num_databases = 4096,
 		.num_ports = 3,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 	},
 
@@ -3538,6 +3541,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6131",
 		.num_databases = 256,
 		.num_ports = 8,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
 	},
 
@@ -3547,6 +3551,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6161",
 		.num_databases = 4096,
 		.num_ports = 6,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 	},
 
@@ -3556,6 +3561,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6165",
 		.num_databases = 4096,
 		.num_ports = 6,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 	},
 
@@ -3565,6 +3571,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6171",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	},
 
@@ -3574,6 +3581,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6172",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	},
 
@@ -3583,6 +3591,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6175",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	},
 
@@ -3592,6 +3601,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6176",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	},
 
@@ -3601,6 +3611,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6185",
 		.num_databases = 256,
 		.num_ports = 10,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
 	},
 
@@ -3610,6 +3621,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6240",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	},
 
@@ -3619,6 +3631,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6320",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
 	},
 
@@ -3628,6 +3641,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6321",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
 	},
 
@@ -3637,6 +3651,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6350",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	},
 
@@ -3646,6 +3661,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6351",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	},
 
@@ -3655,6 +3671,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.name = "Marvell 88E6352",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.port_base_addr = 0x10,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	},
 };
@@ -3675,7 +3692,7 @@ static int mv88e6xxx_detect(struct mv88e6xxx_priv_state *ps)
 	const struct mv88e6xxx_info *info;
 	int id, prod_num, rev;
 
-	id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
+	id = mv88e6xxx_reg_read(ps, ps->info->port_base_addr, PORT_SWITCH_ID);
 	if (id < 0)
 		return id;
 
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index c82bd18..b0b5a84 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -533,6 +533,7 @@ struct mv88e6xxx_info {
 	const char *name;
 	unsigned int num_databases;
 	unsigned int num_ports;
+	unsigned int port_base_addr;
 	unsigned long flags;
 };
 
-- 
2.8.3

Re: [PATCH v3 net-next v3 14/14] net: dsa: mv88e6xxx: add port base address to info

[Andrew Lunn]

On Fri, Jun 17, 2016 at 08:07:36PM -0400, Vivien Didelot wrote:
> The switch ID is located at address 0x3 of every Port Registers bank.
> 
> But not all Marvell switches have their Port Registers SMI Addresses
> starting at 0x10. 88E6060 starts at 0x8 and 88E6390 starts at 0x0.
> 
> Add this data in the info structure and use it in the detection code.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew