diff mbox

[net-next,5/8] dsa: mv88e6xxx: Kill the REG_READ and REG_WRITE macros

Message ID 1460591998-20598-6-git-send-email-andrew@lunn.ch
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Andrew Lunn April 13, 2016, 11:59 p.m. UTC
These macros hide a ds variable and a return statement on error, which
can lead to locking issues. Kill them off.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6123.c |  13 ++-
 drivers/net/dsa/mv88e6131.c |  41 ++++----
 drivers/net/dsa/mv88e6171.c |  16 +--
 drivers/net/dsa/mv88e6352.c |  15 +--
 drivers/net/dsa/mv88e6xxx.c | 232 +++++++++++++++++++++++++++++++-------------
 drivers/net/dsa/mv88e6xxx.h |  19 ----
 6 files changed, 217 insertions(+), 119 deletions(-)

Comments

Vivien Didelot April 14, 2016, 2:54 p.m. UTC | #1
Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

<snip>

>  int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
>  {
> +	int ret, err;
>  	int i;
> -	int ret;
>  
>  	for (i = 0; i < 6; i++) {
>  		int j;
>  
>  		/* Write the MAC address byte. */
> -		REG_WRITE(REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
> -			  GLOBAL2_SWITCH_MAC_BUSY | (i << 8) | addr[i]);
> +		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
> +					  GLOBAL2_SWITCH_MAC_BUSY |
> +					  (i << 8) | addr[i]);
> +		if (err)
> +			return err;

Just use the ret variable here.

>  
>  		/* Wait for the write to complete. */
>  		for (j = 0; j < 16; j++) {
> -			ret = REG_READ(REG_GLOBAL2, GLOBAL2_SWITCH_MAC);
> +			ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2,
> +						 GLOBAL2_SWITCH_MAC);
> +			if (ret < 0)
> +				return ret;
> +
> +			if (ret < 0)
> +				return ret;

duplicated condition.

>  			if ((ret & GLOBAL2_SWITCH_MAC_BUSY) == 0)
>  				break;
>  		}

<snip>

> @@ -2697,7 +2732,8 @@ 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;
> +	ps->id = __mv88e6xxx_reg_read(ps->bus, ps->sw_addr, REG_PORT(0),
> +				      PORT_SWITCH_ID) & 0xfff0;

Here you dropped the error checking, and the (maybe unnecessary?) mutex
locking.

>  
>  	INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
>  
> @@ -2708,42 +2744,66 @@ EXPORT_SYMBOL_GPL(mv88e6xxx_setup_common);
>  int mv88e6xxx_setup_global(struct dsa_switch *ds)
>  {
>  	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -	int ret;
> +	int err;
>  	int i;
>  
>  	/* Set the default address aging time to 5 minutes, and
>  	 * enable address learn messages to be sent to all message
>  	 * ports.
>  	 */
> -	REG_WRITE(REG_GLOBAL, GLOBAL_ATU_CONTROL,
> -		  0x0140 | GLOBAL_ATU_CONTROL_LEARN2ALL);
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_CONTROL,
> +				  0x0140 | GLOBAL_ATU_CONTROL_LEARN2ALL);
> +	if (err)
> +		return err;
>  
>  	/* Configure the IP ToS mapping registers. */
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
> +	if (err)
> +		return err;
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
> +	if (err)
> +		return err;
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
> +	if (err)
> +		return err;
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
> +	if (err)
> +		return err;
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
> +	if (err)
> +		return err;
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
> +	if (err)
> +		return err;
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
> +	if (err)
> +		return err;
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
> +	if (err)
> +		return err;
>  
>  	/* Configure the IEEE 802.1p priority mapping register. */
> -	REG_WRITE(REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
> +	if (err)
> +		return err;
>  
>  	/* Send all frames with destination addresses matching
>  	 * 01:80:c2:00:00:0x to the CPU port.
>  	 */
> -	REG_WRITE(REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
> +	if (err)
> +		return err;
>  
>  	/* Ignore removed tag data on doubly tagged packets, disable
>  	 * flow control messages, force flow control priority to the
>  	 * highest, and send all special multicast frames to the CPU
>  	 * port at the highest priority.
>  	 */
> -	REG_WRITE(REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
> -		  0x7 | GLOBAL2_SWITCH_MGMT_RSVD2CPU | 0x70 |
> -		  GLOBAL2_SWITCH_MGMT_FORCE_FLOW_CTRL_PRI);
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
> +				  0x7 | GLOBAL2_SWITCH_MGMT_RSVD2CPU | 0x70 |
> +				  GLOBAL2_SWITCH_MGMT_FORCE_FLOW_CTRL_PRI);
> +	if (err)
> +		return err;
>  
>  	/* Program the DSA routing table. */
>  	for (i = 0; i < 32; i++) {
> @@ -2753,23 +2813,35 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
>  		    i != ds->index && i < ds->dst->pd->nr_chips)
>  			nexthop = ds->pd->rtable[i] & 0x1f;
>  
> -		REG_WRITE(REG_GLOBAL2, GLOBAL2_DEVICE_MAPPING,
> -			  GLOBAL2_DEVICE_MAPPING_UPDATE |
> -			  (i << GLOBAL2_DEVICE_MAPPING_TARGET_SHIFT) |
> -			  nexthop);
> +		err = mv88e6xxx_reg_write(
> +			ds, REG_GLOBAL2,
> +			GLOBAL2_DEVICE_MAPPING,
> +			GLOBAL2_DEVICE_MAPPING_UPDATE |
> +			(i << GLOBAL2_DEVICE_MAPPING_TARGET_SHIFT) | nexthop);
> +	if (err)
> +		return err;

Wrong indentation here.

>  	}
>  
>  	/* Clear all trunk masks. */
> -	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));
> +	for (i = 0; i < 8; i++) {
> +		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_TRUNK_MASK,
> +					  0x8000 |
> +					  (i << GLOBAL2_TRUNK_MASK_NUM_SHIFT) |
> +					  ((1 << ps->num_ports) - 1));
> +		if (err)
> +			return err;
> +	}
>  
>  	/* Clear all trunk mappings. */
> -	for (i = 0; i < 16; i++)
> -		REG_WRITE(REG_GLOBAL2, GLOBAL2_TRUNK_MAPPING,
> -			  GLOBAL2_TRUNK_MAPPING_UPDATE |
> -			  (i << GLOBAL2_TRUNK_MAPPING_ID_SHIFT));
> +	for (i = 0; i < 16; i++) {
> +		err = mv88e6xxx_reg_write(
> +			ds, REG_GLOBAL2,
> +			GLOBAL2_TRUNK_MAPPING,
> +			GLOBAL2_TRUNK_MAPPING_UPDATE |
> +			(i << GLOBAL2_TRUNK_MAPPING_ID_SHIFT));
> +		if (err)
> +			return err;
> +	}
>  
>  	if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
>  	    mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> @@ -2777,17 +2849,27 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
>  		/* Send all frames with destination addresses matching
>  		 * 01:80:c2:00:00:2x to the CPU port.
>  		 */
> -		REG_WRITE(REG_GLOBAL2, GLOBAL2_MGMT_EN_2X, 0xffff);
> +		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> +					  GLOBAL2_MGMT_EN_2X, 0xffff);
> +		if (err)
> +			return err;
>  
>  		/* Initialise cross-chip port VLAN table to reset
>  		 * defaults.
>  		 */
> -		REG_WRITE(REG_GLOBAL2, GLOBAL2_PVT_ADDR, 0x9000);
> +		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> +					  GLOBAL2_PVT_ADDR, 0x9000);
> +		if (err)
> +			return err;
>  
>  		/* Clear the priority override table. */
> -		for (i = 0; i < 16; i++)
> -			REG_WRITE(REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
> -				  0x8000 | (i << 8));
> +		for (i = 0; i < 16; i++) {
> +			err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> +						  GLOBAL2_PRIO_OVERRIDE,
> +						  0x8000 | (i << 8));
> +			if (err)
> +				return err;
> +		}
>  	}
>  
>  	if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> @@ -2798,31 +2880,38 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
>  		 * ingress rate limit registers to their initial
>  		 * state.
>  		 */
> -		for (i = 0; i < ps->num_ports; i++)
> -			REG_WRITE(REG_GLOBAL2, GLOBAL2_INGRESS_OP,
> -				  0x9000 | (i << 8));
> +		for (i = 0; i < ps->num_ports; i++) {
> +			err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> +						  GLOBAL2_INGRESS_OP,
> +						  0x9000 | (i << 8));
> +			if (err)
> +				return err;
> +		}
>  	}
>  
>  	/* Clear the statistics counters for all ports */
> -	REG_WRITE(REG_GLOBAL, GLOBAL_STATS_OP, GLOBAL_STATS_OP_FLUSH_ALL);
> +	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_STATS_OP,
> +				  GLOBAL_STATS_OP_FLUSH_ALL);
> +	if (err)
> +		return err;
>  
>  	/* Wait for the flush to complete. */
>  	mutex_lock(&ps->smi_mutex);

This function is ugly. As you are rewriting all read/write access in it,
Please put that mutex_lock at its beginning and use the _mv88e6xxx_reg_*
variant with goto unlock statement instead, so we lock/unlock just once.

> -	ret = _mv88e6xxx_stats_wait(ds);
> -	if (ret < 0)
> +	err = _mv88e6xxx_stats_wait(ds);
> +	if (err < 0)
>  		goto unlock;
>  
>  	/* Clear all ATU entries */
> -	ret = _mv88e6xxx_atu_flush(ds, 0, true);
> -	if (ret < 0)
> +	err = _mv88e6xxx_atu_flush(ds, 0, true);
> +	if (err < 0)
>  		goto unlock;
>  
>  	/* Clear all the VTU and STU entries */
> -	ret = _mv88e6xxx_vtu_stu_flush(ds);
> +	err = _mv88e6xxx_vtu_stu_flush(ds);
>  unlock:
>  	mutex_unlock(&ps->smi_mutex);
>  
> -	return ret;
> +	return err;
>  }
>  EXPORT_SYMBOL_GPL(mv88e6xxx_setup_global);
>  
> @@ -2832,13 +2921,19 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
>  	u16 is_reset = (ppu_active ? 0x8800 : 0xc800);
>  	struct gpio_desc *gpiod = ps->reset;
>  	unsigned long timeout;
> -	int ret;
> +	int ret, err;
>  	int i;
>  
>  	/* Set all ports to the disabled state. */
>  	for (i = 0; i < ps->num_ports; i++) {
> -		ret = REG_READ(REG_PORT(i), PORT_CONTROL);
> -		REG_WRITE(REG_PORT(i), PORT_CONTROL, ret & 0xfffc);
> +		ret = mv88e6xxx_reg_read(ds, REG_PORT(i), PORT_CONTROL);
> +		if (ret < 0)
> +			return ret;
> +
> +		err = mv88e6xxx_reg_write(ds, REG_PORT(i), PORT_CONTROL,
> +					  ret & 0xfffc);
> +		if (err)
> +			return err;
>  	}
>  
>  	/* Wait for transmit queues to drain. */
> @@ -2857,14 +2952,19 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
>  	 * through global registers 0x18 and 0x19.
>  	 */
>  	if (ppu_active)
> -		REG_WRITE(REG_GLOBAL, 0x04, 0xc000);
> +		err = mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc000);
>  	else
> -		REG_WRITE(REG_GLOBAL, 0x04, 0xc400);
> +		err = mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc400);
> +	if (err)
> +		return err;

The ret variable is already there, I'd stick with it instead of adding a
new err variable to the function.

>  
>  	/* Wait up to one second for reset to complete. */
>  	timeout = jiffies + 1 * HZ;
>  	while (time_before(jiffies, timeout)) {
> -		ret = REG_READ(REG_GLOBAL, 0x00);
> +		ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, 0x00);
> +		if (ret < 0)
> +			return ret;
> +
>  		if ((ret & is_reset) == is_reset)
>  			break;
>  		usleep_range(1000, 2000);

This comment is up to you, but as you're adding error conditions to the
function, I'd go with a single mutex_lock/mutex_unlock, use the
_mv88e6xxx_reg_* variants and goto unlock statements here as well.

> diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
> index 739d3ff1bddf..6d1b6207144d 100644
> --- a/drivers/net/dsa/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx.h
> @@ -554,25 +554,6 @@ extern struct dsa_switch_driver mv88e6123_switch_driver;
>  extern struct dsa_switch_driver mv88e6352_switch_driver;
>  extern struct dsa_switch_driver mv88e6171_switch_driver;
>  
> -#define REG_READ(addr, reg)						\
> -	({								\
> -		int __ret;						\
> -									\
> -		__ret = mv88e6xxx_reg_read(ds, addr, reg);		\
> -		if (__ret < 0)						\
> -			return __ret;					\
> -		__ret;							\
> -	})
> -
> -#define REG_WRITE(addr, reg, val)					\
> -	({								\
> -		int __ret;						\
> -									\
> -		__ret = mv88e6xxx_reg_write(ds, addr, reg, val);	\
> -		if (__ret < 0)						\
> -			return __ret;					\
> -	})
> -
>  /**
>   * mv88e6xxx_module_driver() - Helper macro for registering mv88e6xxx drivers
>   *

I'm 200% for this patch. However it is not related to the rest of the
patchset, which I'm not really OK with yet (still reviewing). As this
needs a respin anyway, can you extract and send it as a single patch? It
applies cleanly to net-next once you remove the mv88e6xxx_module_driver
macro. I'll ack it right away.

Thanks,
Vivien
diff mbox

Patch

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 7ef0e3c7b703..450dad5af568 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -54,7 +54,9 @@  static int mv88e6123_setup_global(struct dsa_switch *ds)
 	 * external PHYs to poll), don't discard packets with
 	 * excessive collisions, and mask all interrupt sources.
 	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
+	if (ret)
+		return ret;
 
 	/* Configure the upstream port, and configure the upstream
 	 * port as the port to which ingress and egress monitor frames
@@ -63,14 +65,15 @@  static int mv88e6123_setup_global(struct dsa_switch *ds)
 	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
 		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
 		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
-	REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	if (ret)
+		return ret;
 
 	/* Disable remote management for now, and set the switch's
 	 * DSA device number.
 	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2, ds->index & 0x1f);
-
-	return 0;
+	return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
+				   ds->index & 0x1f);
 }
 
 static int mv88e6123_setup(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 25f35937787e..e1e2410a660c 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -51,11 +51,16 @@  static int mv88e6131_setup_global(struct dsa_switch *ds)
 	 * to arbitrate between packet queues, set the maximum frame
 	 * size to 1632, and mask all interrupt sources.
 	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
-		  GLOBAL_CONTROL_PPU_ENABLE | GLOBAL_CONTROL_MAX_FRAME_1632);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
+				  GLOBAL_CONTROL_PPU_ENABLE |
+				  GLOBAL_CONTROL_MAX_FRAME_1632);
+	if (ret)
+		return ret;
 
 	/* Set the VLAN ethertype to 0x8100. */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CORE_TAG_TYPE, 0x8100);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CORE_TAG_TYPE, 0x8100);
+	if (ret)
+		return ret;
 
 	/* Disable ARP mirroring, and configure the upstream port as
 	 * the port to which ingress and egress monitor frames are to
@@ -64,31 +69,33 @@  static int mv88e6131_setup_global(struct dsa_switch *ds)
 	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
 		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
 		GLOBAL_MONITOR_CONTROL_ARP_DISABLED;
-	REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	if (ret)
+		return ret;
 
 	/* Disable cascade port functionality unless this device
 	 * is used in a cascade configuration, and set the switch's
 	 * DSA device number.
 	 */
 	if (ds->dst->pd->nr_chips > 1)
-		REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2,
-			  GLOBAL_CONTROL_2_MULTIPLE_CASCADE |
-			  (ds->index & 0x1f));
+		ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
+					  GLOBAL_CONTROL_2_MULTIPLE_CASCADE |
+					  (ds->index & 0x1f));
 	else
-		REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2,
-			  GLOBAL_CONTROL_2_NO_CASCADE |
-			  (ds->index & 0x1f));
+		ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
+					  GLOBAL_CONTROL_2_NO_CASCADE |
+					  (ds->index & 0x1f));
+	if (ret)
+		return ret;
 
 	/* Force the priority of IGMP/MLD snoop frames and ARP frames
 	 * to the highest setting.
 	 */
-	REG_WRITE(REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
-		  GLOBAL2_PRIO_OVERRIDE_FORCE_SNOOP |
-		  7 << GLOBAL2_PRIO_OVERRIDE_SNOOP_SHIFT |
-		  GLOBAL2_PRIO_OVERRIDE_FORCE_ARP |
-		  7 << GLOBAL2_PRIO_OVERRIDE_ARP_SHIFT);
-
-	return 0;
+	return mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
+				   GLOBAL2_PRIO_OVERRIDE_FORCE_SNOOP |
+				   7 << GLOBAL2_PRIO_OVERRIDE_SNOOP_SHIFT |
+				   GLOBAL2_PRIO_OVERRIDE_FORCE_ARP |
+				   7 << GLOBAL2_PRIO_OVERRIDE_ARP_SHIFT);
 }
 
 static int mv88e6131_setup(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index abf2a9bbbec5..e3998be13515 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -48,8 +48,11 @@  static int mv88e6171_setup_global(struct dsa_switch *ds)
 	/* Discard packets with excessive collisions, mask all
 	 * interrupt sources, enable PPU.
 	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
-		  GLOBAL_CONTROL_PPU_ENABLE | GLOBAL_CONTROL_DISCARD_EXCESS);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
+				  GLOBAL_CONTROL_PPU_ENABLE |
+				  GLOBAL_CONTROL_DISCARD_EXCESS);
+	if (ret)
+		return ret;
 
 	/* Configure the upstream port, and configure the upstream
 	 * port as the port to which ingress and egress monitor frames
@@ -59,14 +62,15 @@  static int mv88e6171_setup_global(struct dsa_switch *ds)
 		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
 		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT |
 		upstream_port << GLOBAL_MONITOR_CONTROL_MIRROR_SHIFT;
-	REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	if (ret)
+		return ret;
 
 	/* Disable remote management for now, and set the switch's
 	 * DSA device number.
 	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2, ds->index & 0x1f);
-
-	return 0;
+	return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
+				   ds->index & 0x1f);
 }
 
 static int mv88e6171_setup(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 22d42fb51991..baeb594b63cc 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -60,8 +60,11 @@  static int mv88e6352_setup_global(struct dsa_switch *ds)
 	/* Discard packets with excessive collisions,
 	 * mask all interrupt sources, enable PPU (bit 14, undocumented).
 	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
-		  GLOBAL_CONTROL_PPU_ENABLE | GLOBAL_CONTROL_DISCARD_EXCESS);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
+				  GLOBAL_CONTROL_PPU_ENABLE |
+				  GLOBAL_CONTROL_DISCARD_EXCESS);
+	if (ret)
+		return ret;
 
 	/* Configure the upstream port, and configure the upstream
 	 * port as the port to which ingress and egress monitor frames
@@ -70,14 +73,14 @@  static int mv88e6352_setup_global(struct dsa_switch *ds)
 	reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
 		upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
 		upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
-	REG_WRITE(REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
+	if (ret)
+		return ret;
 
 	/* Disable remote management for now, and set the switch's
 	 * DSA device number.
 	 */
-	REG_WRITE(REG_GLOBAL, 0x1c, ds->index & 0x1f);
-
-	return 0;
+	return mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x1c, ds->index & 0x1f);
 }
 
 static int mv88e6352_setup(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ef29a94da975..93110460e0c3 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -182,29 +182,47 @@  EXPORT_SYMBOL_GPL(mv88e6xxx_reg_write);
 
 int mv88e6xxx_set_addr_direct(struct dsa_switch *ds, u8 *addr)
 {
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, (addr[0] << 8) | addr[1]);
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]);
-	REG_WRITE(REG_GLOBAL, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]);
+	int err;
 
-	return 0;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_01,
+				  (addr[0] << 8) | addr[1]);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_23,
+				  (addr[2] << 8) | addr[3]);
+	if (err)
+		return err;
+
+	return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_45,
+				   (addr[4] << 8) | addr[5]);
 }
 EXPORT_SYMBOL_GPL(mv88e6xxx_set_addr_direct);
 
 int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
 {
+	int ret, err;
 	int i;
-	int ret;
 
 	for (i = 0; i < 6; i++) {
 		int j;
 
 		/* Write the MAC address byte. */
-		REG_WRITE(REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
-			  GLOBAL2_SWITCH_MAC_BUSY | (i << 8) | addr[i]);
+		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
+					  GLOBAL2_SWITCH_MAC_BUSY |
+					  (i << 8) | addr[i]);
+		if (err)
+			return err;
 
 		/* Wait for the write to complete. */
 		for (j = 0; j < 16; j++) {
-			ret = REG_READ(REG_GLOBAL2, GLOBAL2_SWITCH_MAC);
+			ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2,
+						 GLOBAL2_SWITCH_MAC);
+			if (ret < 0)
+				return ret;
+
+			if (ret < 0)
+				return ret;
 			if ((ret & GLOBAL2_SWITCH_MAC_BUSY) == 0)
 				break;
 		}
@@ -237,13 +255,21 @@  static int mv88e6xxx_ppu_disable(struct dsa_switch *ds)
 	int ret;
 	unsigned long timeout;
 
-	ret = REG_READ(REG_GLOBAL, GLOBAL_CONTROL);
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL,
-		  ret & ~GLOBAL_CONTROL_PPU_ENABLE);
+	ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_CONTROL);
+	if (ret < 0)
+		return ret;
+
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
+				  ret & ~GLOBAL_CONTROL_PPU_ENABLE);
+	if (ret)
+		return ret;
 
 	timeout = jiffies + 1 * HZ;
 	while (time_before(jiffies, timeout)) {
-		ret = REG_READ(REG_GLOBAL, GLOBAL_STATUS);
+		ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATUS);
+		if (ret < 0)
+			return ret;
+
 		usleep_range(1000, 2000);
 		if ((ret & GLOBAL_STATUS_PPU_MASK) !=
 		    GLOBAL_STATUS_PPU_POLLING)
@@ -255,15 +281,24 @@  static int mv88e6xxx_ppu_disable(struct dsa_switch *ds)
 
 static int mv88e6xxx_ppu_enable(struct dsa_switch *ds)
 {
-	int ret;
+	int ret, err;
 	unsigned long timeout;
 
-	ret = REG_READ(REG_GLOBAL, GLOBAL_CONTROL);
-	REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL, ret | GLOBAL_CONTROL_PPU_ENABLE);
+	ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_CONTROL);
+	if (ret < 0)
+		return ret;
+
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
+				  ret | GLOBAL_CONTROL_PPU_ENABLE);
+	if (err)
+		return err;
 
 	timeout = jiffies + 1 * HZ;
 	while (time_before(jiffies, timeout)) {
-		ret = REG_READ(REG_GLOBAL, GLOBAL_STATUS);
+		ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATUS);
+		if (ret < 0)
+			return ret;
+
 		usleep_range(1000, 2000);
 		if ((ret & GLOBAL_STATUS_PPU_MASK) ==
 		    GLOBAL_STATUS_PPU_POLLING)
@@ -2697,7 +2732,8 @@  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;
+	ps->id = __mv88e6xxx_reg_read(ps->bus, ps->sw_addr, REG_PORT(0),
+				      PORT_SWITCH_ID) & 0xfff0;
 
 	INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
 
@@ -2708,42 +2744,66 @@  EXPORT_SYMBOL_GPL(mv88e6xxx_setup_common);
 int mv88e6xxx_setup_global(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int ret;
+	int err;
 	int i;
 
 	/* Set the default address aging time to 5 minutes, and
 	 * enable address learn messages to be sent to all message
 	 * ports.
 	 */
-	REG_WRITE(REG_GLOBAL, GLOBAL_ATU_CONTROL,
-		  0x0140 | GLOBAL_ATU_CONTROL_LEARN2ALL);
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_CONTROL,
+				  0x0140 | GLOBAL_ATU_CONTROL_LEARN2ALL);
+	if (err)
+		return err;
 
 	/* Configure the IP ToS mapping registers. */
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
-	REG_WRITE(REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
+	if (err)
+		return err;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
+	if (err)
+		return err;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
+	if (err)
+		return err;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
+	if (err)
+		return err;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
+	if (err)
+		return err;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
+	if (err)
+		return err;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
+	if (err)
+		return err;
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
+	if (err)
+		return err;
 
 	/* Configure the IEEE 802.1p priority mapping register. */
-	REG_WRITE(REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
+	if (err)
+		return err;
 
 	/* Send all frames with destination addresses matching
 	 * 01:80:c2:00:00:0x to the CPU port.
 	 */
-	REG_WRITE(REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
+	if (err)
+		return err;
 
 	/* Ignore removed tag data on doubly tagged packets, disable
 	 * flow control messages, force flow control priority to the
 	 * highest, and send all special multicast frames to the CPU
 	 * port at the highest priority.
 	 */
-	REG_WRITE(REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
-		  0x7 | GLOBAL2_SWITCH_MGMT_RSVD2CPU | 0x70 |
-		  GLOBAL2_SWITCH_MGMT_FORCE_FLOW_CTRL_PRI);
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
+				  0x7 | GLOBAL2_SWITCH_MGMT_RSVD2CPU | 0x70 |
+				  GLOBAL2_SWITCH_MGMT_FORCE_FLOW_CTRL_PRI);
+	if (err)
+		return err;
 
 	/* Program the DSA routing table. */
 	for (i = 0; i < 32; i++) {
@@ -2753,23 +2813,35 @@  int mv88e6xxx_setup_global(struct dsa_switch *ds)
 		    i != ds->index && i < ds->dst->pd->nr_chips)
 			nexthop = ds->pd->rtable[i] & 0x1f;
 
-		REG_WRITE(REG_GLOBAL2, GLOBAL2_DEVICE_MAPPING,
-			  GLOBAL2_DEVICE_MAPPING_UPDATE |
-			  (i << GLOBAL2_DEVICE_MAPPING_TARGET_SHIFT) |
-			  nexthop);
+		err = mv88e6xxx_reg_write(
+			ds, REG_GLOBAL2,
+			GLOBAL2_DEVICE_MAPPING,
+			GLOBAL2_DEVICE_MAPPING_UPDATE |
+			(i << GLOBAL2_DEVICE_MAPPING_TARGET_SHIFT) | nexthop);
+	if (err)
+		return err;
 	}
 
 	/* Clear all trunk masks. */
-	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));
+	for (i = 0; i < 8; i++) {
+		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_TRUNK_MASK,
+					  0x8000 |
+					  (i << GLOBAL2_TRUNK_MASK_NUM_SHIFT) |
+					  ((1 << ps->num_ports) - 1));
+		if (err)
+			return err;
+	}
 
 	/* Clear all trunk mappings. */
-	for (i = 0; i < 16; i++)
-		REG_WRITE(REG_GLOBAL2, GLOBAL2_TRUNK_MAPPING,
-			  GLOBAL2_TRUNK_MAPPING_UPDATE |
-			  (i << GLOBAL2_TRUNK_MAPPING_ID_SHIFT));
+	for (i = 0; i < 16; i++) {
+		err = mv88e6xxx_reg_write(
+			ds, REG_GLOBAL2,
+			GLOBAL2_TRUNK_MAPPING,
+			GLOBAL2_TRUNK_MAPPING_UPDATE |
+			(i << GLOBAL2_TRUNK_MAPPING_ID_SHIFT));
+		if (err)
+			return err;
+	}
 
 	if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
 	    mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
@@ -2777,17 +2849,27 @@  int mv88e6xxx_setup_global(struct dsa_switch *ds)
 		/* Send all frames with destination addresses matching
 		 * 01:80:c2:00:00:2x to the CPU port.
 		 */
-		REG_WRITE(REG_GLOBAL2, GLOBAL2_MGMT_EN_2X, 0xffff);
+		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
+					  GLOBAL2_MGMT_EN_2X, 0xffff);
+		if (err)
+			return err;
 
 		/* Initialise cross-chip port VLAN table to reset
 		 * defaults.
 		 */
-		REG_WRITE(REG_GLOBAL2, GLOBAL2_PVT_ADDR, 0x9000);
+		err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
+					  GLOBAL2_PVT_ADDR, 0x9000);
+		if (err)
+			return err;
 
 		/* Clear the priority override table. */
-		for (i = 0; i < 16; i++)
-			REG_WRITE(REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
-				  0x8000 | (i << 8));
+		for (i = 0; i < 16; i++) {
+			err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
+						  GLOBAL2_PRIO_OVERRIDE,
+						  0x8000 | (i << 8));
+			if (err)
+				return err;
+		}
 	}
 
 	if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
@@ -2798,31 +2880,38 @@  int mv88e6xxx_setup_global(struct dsa_switch *ds)
 		 * ingress rate limit registers to their initial
 		 * state.
 		 */
-		for (i = 0; i < ps->num_ports; i++)
-			REG_WRITE(REG_GLOBAL2, GLOBAL2_INGRESS_OP,
-				  0x9000 | (i << 8));
+		for (i = 0; i < ps->num_ports; i++) {
+			err = mv88e6xxx_reg_write(ds, REG_GLOBAL2,
+						  GLOBAL2_INGRESS_OP,
+						  0x9000 | (i << 8));
+			if (err)
+				return err;
+		}
 	}
 
 	/* Clear the statistics counters for all ports */
-	REG_WRITE(REG_GLOBAL, GLOBAL_STATS_OP, GLOBAL_STATS_OP_FLUSH_ALL);
+	err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_STATS_OP,
+				  GLOBAL_STATS_OP_FLUSH_ALL);
+	if (err)
+		return err;
 
 	/* Wait for the flush to complete. */
 	mutex_lock(&ps->smi_mutex);
-	ret = _mv88e6xxx_stats_wait(ds);
-	if (ret < 0)
+	err = _mv88e6xxx_stats_wait(ds);
+	if (err < 0)
 		goto unlock;
 
 	/* Clear all ATU entries */
-	ret = _mv88e6xxx_atu_flush(ds, 0, true);
-	if (ret < 0)
+	err = _mv88e6xxx_atu_flush(ds, 0, true);
+	if (err < 0)
 		goto unlock;
 
 	/* Clear all the VTU and STU entries */
-	ret = _mv88e6xxx_vtu_stu_flush(ds);
+	err = _mv88e6xxx_vtu_stu_flush(ds);
 unlock:
 	mutex_unlock(&ps->smi_mutex);
 
-	return ret;
+	return err;
 }
 EXPORT_SYMBOL_GPL(mv88e6xxx_setup_global);
 
@@ -2832,13 +2921,19 @@  int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
 	u16 is_reset = (ppu_active ? 0x8800 : 0xc800);
 	struct gpio_desc *gpiod = ps->reset;
 	unsigned long timeout;
-	int ret;
+	int ret, err;
 	int i;
 
 	/* Set all ports to the disabled state. */
 	for (i = 0; i < ps->num_ports; i++) {
-		ret = REG_READ(REG_PORT(i), PORT_CONTROL);
-		REG_WRITE(REG_PORT(i), PORT_CONTROL, ret & 0xfffc);
+		ret = mv88e6xxx_reg_read(ds, REG_PORT(i), PORT_CONTROL);
+		if (ret < 0)
+			return ret;
+
+		err = mv88e6xxx_reg_write(ds, REG_PORT(i), PORT_CONTROL,
+					  ret & 0xfffc);
+		if (err)
+			return err;
 	}
 
 	/* Wait for transmit queues to drain. */
@@ -2857,14 +2952,19 @@  int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
 	 * through global registers 0x18 and 0x19.
 	 */
 	if (ppu_active)
-		REG_WRITE(REG_GLOBAL, 0x04, 0xc000);
+		err = mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc000);
 	else
-		REG_WRITE(REG_GLOBAL, 0x04, 0xc400);
+		err = mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc400);
+	if (err)
+		return err;
 
 	/* Wait up to one second for reset to complete. */
 	timeout = jiffies + 1 * HZ;
 	while (time_before(jiffies, timeout)) {
-		ret = REG_READ(REG_GLOBAL, 0x00);
+		ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, 0x00);
+		if (ret < 0)
+			return ret;
+
 		if ((ret & is_reset) == is_reset)
 			break;
 		usleep_range(1000, 2000);
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 739d3ff1bddf..6d1b6207144d 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -554,25 +554,6 @@  extern struct dsa_switch_driver mv88e6123_switch_driver;
 extern struct dsa_switch_driver mv88e6352_switch_driver;
 extern struct dsa_switch_driver mv88e6171_switch_driver;
 
-#define REG_READ(addr, reg)						\
-	({								\
-		int __ret;						\
-									\
-		__ret = mv88e6xxx_reg_read(ds, addr, reg);		\
-		if (__ret < 0)						\
-			return __ret;					\
-		__ret;							\
-	})
-
-#define REG_WRITE(addr, reg, val)					\
-	({								\
-		int __ret;						\
-									\
-		__ret = mv88e6xxx_reg_write(ds, addr, reg, val);	\
-		if (__ret < 0)						\
-			return __ret;					\
-	})
-
 /**
  * mv88e6xxx_module_driver() - Helper macro for registering mv88e6xxx drivers
  *