| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <871t676dlf.fsf@ketchup.mtl.sfl>
Date: Thu, 14 Apr 2016 18:16:12 -0400
From: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To: Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>
Cc: Florian Fainelli <f.fainelli@...il.com>,
netdev <netdev@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH] dsa: mv88e6xxx: Kill the REG_READ and REG_WRITE macros
Andrew Lunn <andrew@...n.ch> writes:
> 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@...n.ch>
> ---
>
> As requested by Vivien, this patch has been split out of the series.
>
> v2: Use the existing ret variable
> Hold the lock for the whole function, unlock on exit.
> Removed duplicate error test
> Fixed indentation.
> ---
> 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 | 241 +++++++++++++++++++++++++++++++-------------
> drivers/net/dsa/mv88e6xxx.h | 21 ----
> 6 files changed, 224 insertions(+), 123 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> index c34283d929c4..140e44e50e8a 100644
> --- a/drivers/net/dsa/mv88e6123.c
> +++ b/drivers/net/dsa/mv88e6123.c
> @@ -52,7 +52,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
> @@ -61,14 +63,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 f5d75fce1e96..34d297b65040 100644
> --- a/drivers/net/dsa/mv88e6131.c
> +++ b/drivers/net/dsa/mv88e6131.c
> @@ -49,11 +49,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
> @@ -62,31 +67,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 f5622506cdfa..b7af2b78f8ee 100644
> --- a/drivers/net/dsa/mv88e6171.c
> +++ b/drivers/net/dsa/mv88e6171.c
> @@ -46,8 +46,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
> @@ -57,14 +60,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 e54ee27db129..e8cb03fad21a 100644
> --- a/drivers/net/dsa/mv88e6352.c
> +++ b/drivers/net/dsa/mv88e6352.c
> @@ -59,8 +59,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
> @@ -69,14 +72,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 9985a0cf31f1..b018f20829fb 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -180,28 +180,44 @@ int mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
>
> 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]);
> }
>
> int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
> {
> - int i;
> int ret;
> + int i;
Unnecessary.
>
> 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]);
> + ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
> + GLOBAL2_SWITCH_MAC_BUSY |
> + (i << 8) | addr[i]);
> + if (ret)
> + return ret;
>
> /* 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 & GLOBAL2_SWITCH_MAC_BUSY) == 0)
> break;
> }
> @@ -233,13 +249,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)
> @@ -251,15 +275,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;
You could've kept ret here.
>
> 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)
> @@ -2667,7 +2700,9 @@ 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(ds, REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
> + if (ps->id < 0)
> + return ps->id;
>
> INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
>
> @@ -2677,42 +2712,67 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
> int mv88e6xxx_setup_global(struct dsa_switch *ds)
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> - int ret;
> + int err;
> int i;
>
> + mutex_lock(&ps->smi_mutex);
> /* 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)
> + goto unlock;
>
> /* 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)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
> + if (err)
> + goto unlock;
> + err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
> + if (err)
> + goto unlock;
>
> /* 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)
> + goto unlock;
>
> /* 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)
> + goto unlock;
>
> /* 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)
> + goto unlock;
>
> /* Program the DSA routing table. */
> for (i = 0; i < 32; i++) {
> @@ -2722,23 +2782,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)
> + goto unlock;
> }
>
> /* 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)
> + goto unlock;
> + }
>
> /* 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)
> + goto unlock;
> + }
>
> if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> @@ -2746,17 +2818,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)
> + goto unlock;
>
> /* 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)
> + goto unlock;
>
> /* 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)
> + goto unlock;
> + }
> }
>
> if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> @@ -2767,31 +2849,37 @@ 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)
> + goto unlock;
> + }
> }
>
> /* 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)
> + goto unlock;
>
> /* 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;
> }
>
> int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
> @@ -2803,10 +2891,18 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
> int ret;
> int i;
>
> + mutex_lock(&ps->smi_mutex);
> +
> /* 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)
> + goto unlock;
> +
> + ret = _mv88e6xxx_reg_write(ds, REG_PORT(i), PORT_CONTROL,
> + ret & 0xfffc);
> + if (ret)
> + goto unlock;
> }
>
> /* Wait for transmit queues to drain. */
> @@ -2825,22 +2921,31 @@ 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);
> + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc000);
> else
> - REG_WRITE(REG_GLOBAL, 0x04, 0xc400);
> + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc400);
> + if (ret)
> + goto unlock;
>
> /* 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)
> + goto unlock;
> +
> if ((ret & is_reset) == is_reset)
> break;
> usleep_range(1000, 2000);
> }
> if (time_after(jiffies, timeout))
> - return -ETIMEDOUT;
> + ret = -ETIMEDOUT;
> + else
> + ret = 0;
> +unlock:
> + mutex_unlock(&ps->smi_mutex);
>
> - return 0;
> + return ret;
> }
>
> int mv88e6xxx_phy_page_read(struct dsa_switch *ds, int port, int page, int reg)
> diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
> index 5d27decc85cb..0debb9f3cf0a 100644
> --- a/drivers/net/dsa/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx.h
> @@ -542,25 +542,4 @@ 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; \
> - })
> -
> -
> -
> #endif
> --
> 2.7.0
But the idea is here, we need these macro killed.
Tested-by: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Thank you,
Vivien
Powered by blists - more mailing lists