| Message ID | 1460670432-23801-1-git-send-email-andrew@lunn.ch |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Andrew Lunn <andrew@lunn.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@lunn.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@savoirfairelinux.com> Thank you, Vivien
From: Andrew Lunn <andrew@lunn.ch> Date: Thu, 14 Apr 2016 23:47:12 +0200 > 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> Applied, thanks Andrew.
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; 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; 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
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> --- 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(-)