Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933264AbcCGXYy (ORCPT ); Mon, 7 Mar 2016 18:24:54 -0500 Received: from mail.savoirfairelinux.com ([208.88.110.44]:47990 "EHLO mail.savoirfairelinux.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932974AbcCGXYe (ORCPT ); Mon, 7 Mar 2016 18:24:34 -0500 From: Vivien Didelot To: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com, "David S. Miller" , Andrew Lunn , Kevin Smith , Vivien Didelot Subject: [PATCH net-next] net: dsa: mv88e6xxx: rework port state setter Date: Mon, 7 Mar 2016 18:24:17 -0500 Message-Id: <1457393057-22618-1-git-send-email-vivien.didelot@savoirfairelinux.com> X-Mailer: git-send-email 2.7.2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4357 Lines: 131 Apply a few non-functional changes on the port state setter: * add a dynamic debug message with state names to track changes * explicit states checking instead of assuming their numeric values * lock mutex only once when changing several port states * use bitmap macros to declare and access port_state_update_mask Signed-off-by: Vivien Didelot --- drivers/net/dsa/mv88e6xxx.c | 54 +++++++++++++++++++++++++++------------------ drivers/net/dsa/mv88e6xxx.h | 2 +- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c index d11c9d5..3a58a8a 100644 --- a/drivers/net/dsa/mv88e6xxx.c +++ b/drivers/net/dsa/mv88e6xxx.c @@ -1051,39 +1051,49 @@ static int _mv88e6xxx_atu_remove(struct dsa_switch *ds, u16 fid, int port, return _mv88e6xxx_atu_move(ds, fid, port, 0x0f, static_too); } -static int mv88e6xxx_set_port_state(struct dsa_switch *ds, int port, u8 state) +static const char * const mv88e6xxx_port_state_names[] = { + [PORT_CONTROL_STATE_DISABLED] = "Disabled", + [PORT_CONTROL_STATE_BLOCKING] = "Blocking/Listening", + [PORT_CONTROL_STATE_LEARNING] = "Learning", + [PORT_CONTROL_STATE_FORWARDING] = "Forwarding", +}; + +static int _mv88e6xxx_port_state(struct dsa_switch *ds, int port, u8 state) { - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); int reg, ret = 0; u8 oldstate; - mutex_lock(&ps->smi_mutex); - reg = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_CONTROL); - if (reg < 0) { - ret = reg; - goto abort; - } + if (reg < 0) + return reg; oldstate = reg & PORT_CONTROL_STATE_MASK; + if (oldstate != state) { /* Flush forwarding database if we're moving a port * from Learning or Forwarding state to Disabled or * Blocking or Listening state. */ - if (oldstate >= PORT_CONTROL_STATE_LEARNING && - state <= PORT_CONTROL_STATE_BLOCKING) { + if ((oldstate == PORT_CONTROL_STATE_LEARNING || + oldstate == PORT_CONTROL_STATE_FORWARDING) + && (state == PORT_CONTROL_STATE_DISABLED || + state == PORT_CONTROL_STATE_BLOCKING)) { ret = _mv88e6xxx_atu_remove(ds, 0, port, false); if (ret) - goto abort; + return ret; } + reg = (reg & ~PORT_CONTROL_STATE_MASK) | state; ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_CONTROL, reg); + if (ret) + return ret; + + netdev_dbg(ds->ports[port], "PortState %s (was %s)\n", + mv88e6xxx_port_state_names[state], + mv88e6xxx_port_state_names[oldstate]); } -abort: - mutex_unlock(&ps->smi_mutex); return ret; } @@ -1146,13 +1156,11 @@ int mv88e6xxx_port_stp_update(struct dsa_switch *ds, int port, u8 state) break; } - netdev_dbg(ds->ports[port], "port state %d [%d]\n", state, stp_state); - /* mv88e6xxx_port_stp_update may be called with softirqs disabled, * so we can not update the port state directly but need to schedule it. */ ps->ports[port].state = stp_state; - set_bit(port, &ps->port_state_update_mask); + set_bit(port, ps->port_state_update_mask); schedule_work(&ps->bridge_work); return 0; @@ -2228,11 +2236,15 @@ static void mv88e6xxx_bridge_work(struct work_struct *work) ps = container_of(work, struct mv88e6xxx_priv_state, bridge_work); ds = ((struct dsa_switch *)ps) - 1; - while (ps->port_state_update_mask) { - port = __ffs(ps->port_state_update_mask); - clear_bit(port, &ps->port_state_update_mask); - mv88e6xxx_set_port_state(ds, port, ps->ports[port].state); - } + mutex_lock(&ps->smi_mutex); + + for (port = 0; port < ps->num_ports; ++port) + if (test_and_clear_bit(port, ps->port_state_update_mask) && + _mv88e6xxx_port_state(ds, port, ps->ports[port].state)) + netdev_warn(ds->ports[port], "failed to update state to %s\n", + mv88e6xxx_port_state_names[ps->ports[port].state]); + + mutex_unlock(&ps->smi_mutex); } static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port) diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h index d7b088d..3425616 100644 --- a/drivers/net/dsa/mv88e6xxx.h +++ b/drivers/net/dsa/mv88e6xxx.h @@ -426,7 +426,7 @@ struct mv88e6xxx_priv_state { struct mv88e6xxx_priv_port ports[DSA_MAX_PORTS]; - unsigned long port_state_update_mask; + DECLARE_BITMAP(port_state_update_mask, DSA_MAX_PORTS); struct work_struct bridge_work; }; -- 2.7.2