Skip to content

Commit

Permalink
net: mvpp2: let phylink manage the carrier state
Browse files Browse the repository at this point in the history
Net drivers using phylink shouldn't mess with the link carrier
themselves and should let phylink manage it. The mvpp2 driver wasn't
following this best practice as the mac_config() function made calls to
change the link carrier state. This led to wrongly reported carrier link
state which then triggered other issues. This patch fixes this
behaviour.

But the PPv2 driver relied on this misbehaviour in two cases: for fixed
links and when not using phylink (ACPI mode). The later was fixed by
adding an explicit call to link_up(), which when the ACPI mode will use
phylink should be removed.

The fixed link case was relying on the mac_config() function to set the
link up, as we found an issue in phylink_start() which assumes the
carrier is off. If not, the link_up() function is never called. To fix
this, a call to netif_carrier_off() is added just before phylink_start()
so that we do not introduce a regression in the driver.

Fixes: 4bb0432 ("net: mvpp2: phylink support")
Reported-by: Russell King <linux@armlinux.org.uk>
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
atenart authored and davem330 committed Sep 17, 2018
1 parent 8540827 commit 41948cc
Showing 1 changed file with 6 additions and 15 deletions.
21 changes: 6 additions & 15 deletions drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ static struct {
*/
static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
const struct phylink_link_state *state);
static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
phy_interface_t interface, struct phy_device *phy);

/* Queue modes */
#define MVPP2_QDIST_SINGLE_MODE 0
Expand Down Expand Up @@ -3142,6 +3144,7 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
mvpp22_mode_reconfigure(port);

if (port->phylink) {
netif_carrier_off(port->dev);
phylink_start(port->phylink);
} else {
/* Phylink isn't used as of now for ACPI, so the MAC has to be
Expand All @@ -3150,9 +3153,10 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
*/
struct phylink_link_state state = {
.interface = port->phy_interface,
.link = 1,
};
mvpp2_mac_config(port->dev, MLO_AN_INBAND, &state);
mvpp2_mac_link_up(port->dev, MLO_AN_INBAND, port->phy_interface,
NULL);
}

netif_tx_start_all_queues(port->dev);
Expand Down Expand Up @@ -4495,10 +4499,6 @@ static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
return;
}

netif_tx_stop_all_queues(port->dev);
if (!port->has_phy)
netif_carrier_off(port->dev);

/* Make sure the port is disabled when reconfiguring the mode */
mvpp2_port_disable(port);

Expand All @@ -4523,16 +4523,7 @@ static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
if (port->priv->hw_version == MVPP21 && port->flags & MVPP2_F_LOOPBACK)
mvpp2_port_loopback_set(port, state);

/* If the port already was up, make sure it's still in the same state */
if (state->link || !port->has_phy) {
mvpp2_port_enable(port);

mvpp2_egress_enable(port);
mvpp2_ingress_enable(port);
if (!port->has_phy)
netif_carrier_on(dev);
netif_tx_wake_all_queues(dev);
}
mvpp2_port_enable(port);
}

static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
Expand Down

0 comments on commit 41948cc

Please sign in to comment.