* [PATCH net-next] net: mvpp2: phylink support
@ 2017-09-21 13:45 Antoine Tenart
2017-09-22 7:56 ` Marcin Wojtas
2017-09-22 11:07 ` Russell King - ARM Linux
0 siblings, 2 replies; 10+ messages in thread
From: Antoine Tenart @ 2017-09-21 13:45 UTC (permalink / raw)
To: davem, linux
Cc: Antoine Tenart, andrew, gregory.clement, thomas.petazzoni,
miquel.raynal, nadavh, linux-kernel, mw, stefanc, netdev
Convert the PPv2 driver to use phylink, which models the MAC to PHY
link. The phylink support is made such a way the GoP link IRQ can still
be used: the two modes are incompatible and the GoP link IRQ will be
used if no PHY is described in the device tree. This is the same
behaviour as before.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/marvell/Kconfig | 1 +
drivers/net/ethernet/marvell/mvpp2.c | 502 +++++++++++++++++++++--------------
2 files changed, 303 insertions(+), 200 deletions(-)
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index da6fb825afea..991138b8dfba 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -86,6 +86,7 @@ config MVPP2
depends on ARCH_MVEBU || COMPILE_TEST
depends on HAS_DMA
select MVMDIO
+ select PHYLINK
---help---
This driver supports the network interface units in the
Marvell ARMADA 375, 7K and 8K SoCs.
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 8041d692db3c..5fb7e76ee128 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -28,6 +28,7 @@
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/phy.h>
+#include <linux/phylink.h>
#include <linux/phy/phy.h>
#include <linux/clk.h>
#include <linux/hrtimer.h>
@@ -341,6 +342,7 @@
#define MVPP2_GMAC_FORCE_LINK_PASS BIT(1)
#define MVPP2_GMAC_IN_BAND_AUTONEG BIT(2)
#define MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS BIT(3)
+#define MVPP2_GMAC_IN_BAND_RESTART_AN BIT(4)
#define MVPP2_GMAC_CONFIG_MII_SPEED BIT(5)
#define MVPP2_GMAC_CONFIG_GMII_SPEED BIT(6)
#define MVPP2_GMAC_AN_SPEED_EN BIT(7)
@@ -350,6 +352,12 @@
#define MVPP2_GMAC_AN_DUPLEX_EN BIT(13)
#define MVPP2_GMAC_STATUS0 0x10
#define MVPP2_GMAC_STATUS0_LINK_UP BIT(0)
+#define MVPP2_GMAC_STATUS0_GMII_SPEED BIT(1)
+#define MVPP2_GMAC_STATUS0_MII_SPEED BIT(2)
+#define MVPP2_GMAC_STATUS0_FULL_DUPLEX BIT(3)
+#define MVPP2_GMAC_STATUS0_RX_PAUSE BIT(6)
+#define MVPP2_GMAC_STATUS0_TX_PAUSE BIT(7)
+#define MVPP2_GMAC_STATUS0_AN_COMPLETE BIT(11)
#define MVPP2_GMAC_PORT_FIFO_CFG_1_REG 0x1c
#define MVPP2_GMAC_TX_FIFO_MIN_TH_OFFS 6
#define MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK 0x1fc0
@@ -878,12 +886,11 @@ struct mvpp2_port {
u16 rx_ring_size;
struct mvpp2_pcpu_stats __percpu *stats;
+ struct device_node *of_node;
+
phy_interface_t phy_interface;
- struct device_node *phy_node;
+ struct phylink *phylink;
struct phy *comphy;
- unsigned int link;
- unsigned int duplex;
- unsigned int speed;
struct mvpp2_bm_pool *pool_long;
struct mvpp2_bm_pool *pool_short;
@@ -4716,13 +4723,14 @@ static void mvpp2_port_periodic_xon_disable(struct mvpp2_port *port)
}
/* Configure loopback port */
-static void mvpp2_port_loopback_set(struct mvpp2_port *port)
+static void mvpp2_port_loopback_set(struct mvpp2_port *port,
+ const struct phylink_link_state *state)
{
u32 val;
val = readl(port->base + MVPP2_GMAC_CTRL_1_REG);
- if (port->speed == 1000)
+ if (state->speed == 1000)
val |= MVPP2_GMAC_GMII_LB_EN_MASK;
else
val &= ~MVPP2_GMAC_GMII_LB_EN_MASK;
@@ -4778,10 +4786,6 @@ static void mvpp2_defaults_set(struct mvpp2_port *port)
int tx_port_num, val, queue, ptxq, lrxq;
if (port->priv->hw_version == MVPP21) {
- /* Configure port to loopback if needed */
- if (port->flags & MVPP2_F_LOOPBACK)
- mvpp2_port_loopback_set(port);
-
/* Update TX FIFO MIN Threshold */
val = readl(port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
val &= ~MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK;
@@ -5860,111 +5864,6 @@ static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static void mvpp2_gmac_set_autoneg(struct mvpp2_port *port,
- struct phy_device *phydev)
-{
- u32 val;
-
- if (port->phy_interface != PHY_INTERFACE_MODE_RGMII &&
- port->phy_interface != PHY_INTERFACE_MODE_RGMII_ID &&
- port->phy_interface != PHY_INTERFACE_MODE_RGMII_RXID &&
- port->phy_interface != PHY_INTERFACE_MODE_RGMII_TXID &&
- port->phy_interface != PHY_INTERFACE_MODE_SGMII)
- return;
-
- val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
- val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
- MVPP2_GMAC_CONFIG_GMII_SPEED |
- MVPP2_GMAC_CONFIG_FULL_DUPLEX |
- MVPP2_GMAC_AN_SPEED_EN |
- MVPP2_GMAC_AN_DUPLEX_EN);
-
- if (phydev->duplex)
- val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
-
- if (phydev->speed == SPEED_1000)
- val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
- else if (phydev->speed == SPEED_100)
- val |= MVPP2_GMAC_CONFIG_MII_SPEED;
-
- writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-}
-
-/* Adjust link */
-static void mvpp2_link_event(struct net_device *dev)
-{
- struct mvpp2_port *port = netdev_priv(dev);
- struct phy_device *phydev = dev->phydev;
- bool link_reconfigured = false;
- u32 val;
-
- if (phydev->link) {
- if (port->phy_interface != phydev->interface && port->comphy) {
- /* disable current port for reconfiguration */
- mvpp2_interrupts_disable(port);
- netif_carrier_off(port->dev);
- mvpp2_port_disable(port);
- phy_power_off(port->comphy);
-
- /* comphy reconfiguration */
- port->phy_interface = phydev->interface;
- mvpp22_comphy_init(port);
-
- /* gop/mac reconfiguration */
- mvpp22_gop_init(port);
- mvpp2_port_mii_set(port);
-
- link_reconfigured = true;
- }
-
- if ((port->speed != phydev->speed) ||
- (port->duplex != phydev->duplex)) {
- mvpp2_gmac_set_autoneg(port, phydev);
-
- port->duplex = phydev->duplex;
- port->speed = phydev->speed;
- }
- }
-
- if (phydev->link != port->link || link_reconfigured) {
- port->link = phydev->link;
-
- if (phydev->link) {
- if (port->phy_interface == PHY_INTERFACE_MODE_RGMII ||
- port->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
- port->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID ||
- port->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID ||
- port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
- val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
- val |= (MVPP2_GMAC_FORCE_LINK_PASS |
- MVPP2_GMAC_FORCE_LINK_DOWN);
- writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
- }
-
- mvpp2_interrupts_enable(port);
- mvpp2_port_enable(port);
-
- mvpp2_egress_enable(port);
- mvpp2_ingress_enable(port);
- netif_carrier_on(dev);
- netif_tx_wake_all_queues(dev);
- } else {
- port->duplex = -1;
- port->speed = 0;
-
- netif_tx_stop_all_queues(dev);
- netif_carrier_off(dev);
- mvpp2_ingress_disable(port);
- mvpp2_egress_disable(port);
-
- mvpp2_port_disable(port);
- mvpp2_interrupts_disable(port);
- }
-
- phy_print_status(phydev);
- }
-}
-
static void mvpp2_timer_set(struct mvpp2_port_pcpu *port_pcpu)
{
ktime_t interval;
@@ -6582,7 +6481,6 @@ static int mvpp2_poll(struct napi_struct *napi, int budget)
/* Set hw internals when starting port */
static void mvpp2_start_dev(struct mvpp2_port *port)
{
- struct net_device *ndev = port->dev;
int i;
if (port->gop_id == 0 &&
@@ -6607,15 +6505,14 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
mvpp2_port_mii_set(port);
mvpp2_port_enable(port);
- if (ndev->phydev)
- phy_start(ndev->phydev);
+ if (port->phylink)
+ phylink_start(port->phylink);
netif_tx_start_all_queues(port->dev);
}
/* Set hw internals when stopping port */
static void mvpp2_stop_dev(struct mvpp2_port *port)
{
- struct net_device *ndev = port->dev;
int i;
/* Stop new packets from arriving to RXQs */
@@ -6634,8 +6531,8 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
mvpp2_egress_disable(port);
mvpp2_port_disable(port);
- if (ndev->phydev)
- phy_stop(ndev->phydev);
+ if (port->phylink)
+ phylink_stop(port->phylink);
phy_power_off(port->comphy);
}
@@ -6688,40 +6585,6 @@ static void mvpp21_get_mac_address(struct mvpp2_port *port, unsigned char *addr)
addr[5] = (mac_addr_l >> MVPP2_GMAC_SA_LOW_OFFS) & 0xFF;
}
-static int mvpp2_phy_connect(struct mvpp2_port *port)
-{
- struct phy_device *phy_dev;
-
- /* No PHY is attached */
- if (!port->phy_node)
- return 0;
-
- phy_dev = of_phy_connect(port->dev, port->phy_node, mvpp2_link_event, 0,
- port->phy_interface);
- if (!phy_dev) {
- netdev_err(port->dev, "cannot connect to phy\n");
- return -ENODEV;
- }
- phy_dev->supported &= PHY_GBIT_FEATURES;
- phy_dev->advertising = phy_dev->supported;
-
- port->link = 0;
- port->duplex = 0;
- port->speed = 0;
-
- return 0;
-}
-
-static void mvpp2_phy_disconnect(struct mvpp2_port *port)
-{
- struct net_device *ndev = port->dev;
-
- if (!ndev->phydev)
- return;
-
- phy_disconnect(ndev->phydev);
-}
-
static int mvpp2_irqs_init(struct mvpp2_port *port)
{
int err, i;
@@ -6765,7 +6628,6 @@ static void mvpp2_irqs_deinit(struct mvpp2_port *port)
static int mvpp2_open(struct net_device *dev)
{
struct mvpp2_port *port = netdev_priv(dev);
- struct mvpp2 *priv = port->priv;
unsigned char mac_bcast[ETH_ALEN] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
int err;
@@ -6811,7 +6673,16 @@ static int mvpp2_open(struct net_device *dev)
goto err_cleanup_txqs;
}
- if (priv->hw_version == MVPP22 && !port->phy_node && port->link_irq) {
+ /* In default link is down */
+ netif_carrier_off(port->dev);
+
+ if (port->phylink) {
+ err = phylink_of_phy_connect(port->phylink, port->of_node);
+ if (err) {
+ netdev_err(port->dev, "could not attach PHY (%d)\n", err);
+ goto err_free_irq;
+ }
+ } else if (port->link_irq) {
err = request_irq(port->link_irq, mvpp2_link_status_isr, 0,
dev->name, port);
if (err) {
@@ -6821,15 +6692,11 @@ static int mvpp2_open(struct net_device *dev)
}
mvpp22_gop_setup_irq(port);
+ } else {
+ netdev_err(dev, "cannot use phylink or GoP link IRQ\n");
+ goto err_free_irq;
}
- /* In default link is down */
- netif_carrier_off(port->dev);
-
- err = mvpp2_phy_connect(port);
- if (err < 0)
- goto err_free_link_irq;
-
/* Unmask interrupts on all CPUs */
on_each_cpu(mvpp2_interrupts_unmask, port, 1);
mvpp2_shared_interrupt_mask_unmask(port, false);
@@ -6838,9 +6705,6 @@ static int mvpp2_open(struct net_device *dev)
return 0;
-err_free_link_irq:
- if (priv->hw_version == MVPP22 && !port->phy_node && port->link_irq)
- free_irq(port->link_irq, port);
err_free_irq:
mvpp2_irqs_deinit(port);
err_cleanup_txqs:
@@ -6854,17 +6718,17 @@ static int mvpp2_stop(struct net_device *dev)
{
struct mvpp2_port *port = netdev_priv(dev);
struct mvpp2_port_pcpu *port_pcpu;
- struct mvpp2 *priv = port->priv;
int cpu;
mvpp2_stop_dev(port);
- mvpp2_phy_disconnect(port);
+ if (port->phylink)
+ phylink_disconnect_phy(port->phylink);
/* Mask interrupts on all CPUs */
on_each_cpu(mvpp2_interrupts_mask, port, 1);
mvpp2_shared_interrupt_mask_unmask(port, true);
- if (priv->hw_version == MVPP22 && !port->phy_node && port->link_irq)
+ if (port->link_irq)
free_irq(port->link_irq, port);
mvpp2_irqs_deinit(port);
@@ -7029,20 +6893,26 @@ mvpp2_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
static int mvpp2_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
{
- int ret;
+ struct mvpp2_port *port = netdev_priv(dev);
- if (!dev->phydev)
+ if (!port->phylink)
return -ENOTSUPP;
- ret = phy_mii_ioctl(dev->phydev, ifr, cmd);
- if (!ret)
- mvpp2_link_event(dev);
-
- return ret;
+ return phylink_mii_ioctl(port->phylink, ifr, cmd);
}
/* Ethtool methods */
+static int mvpp2_ethtool_nway_reset(struct net_device *dev)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+
+ if (!port->phylink)
+ return -ENOTSUPP;
+
+ return phylink_ethtool_nway_reset(port->phylink);
+}
+
/* Set interrupt coalescing for ethtools */
static int mvpp2_ethtool_set_coalesce(struct net_device *dev,
struct ethtool_coalesce *c)
@@ -7170,6 +7040,50 @@ static int mvpp2_ethtool_set_ringparam(struct net_device *dev,
return err;
}
+static void mvpp2_ethtool_get_pause_param(struct net_device *dev,
+ struct ethtool_pauseparam *pause)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+
+ if (!port->phylink)
+ return;
+
+ phylink_ethtool_get_pauseparam(port->phylink, pause);
+}
+
+static int mvpp2_ethtool_set_pause_param(struct net_device *dev,
+ struct ethtool_pauseparam *pause)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+
+ if (!port->phylink)
+ return -ENOTSUPP;
+
+ return phylink_ethtool_set_pauseparam(port->phylink, pause);
+}
+
+static int mvpp2_ethtool_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+
+ if (!port->phylink)
+ return -ENOTSUPP;
+
+ return phylink_ethtool_ksettings_get(port->phylink, cmd);
+}
+
+static int mvpp2_ethtool_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+
+ if (!port->phylink)
+ return -ENOTSUPP;
+
+ return phylink_ethtool_ksettings_set(port->phylink, cmd);
+}
+
/* Device ops */
static const struct net_device_ops mvpp2_netdev_ops = {
@@ -7184,15 +7098,17 @@ static const struct net_device_ops mvpp2_netdev_ops = {
};
static const struct ethtool_ops mvpp2_eth_tool_ops = {
- .nway_reset = phy_ethtool_nway_reset,
+ .nway_reset = mvpp2_ethtool_nway_reset,
.get_link = ethtool_op_get_link,
.set_coalesce = mvpp2_ethtool_set_coalesce,
.get_coalesce = mvpp2_ethtool_get_coalesce,
.get_drvinfo = mvpp2_ethtool_get_drvinfo,
.get_ringparam = mvpp2_ethtool_get_ringparam,
.set_ringparam = mvpp2_ethtool_set_ringparam,
- .get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = phy_ethtool_set_link_ksettings,
+ .get_pauseparam = mvpp2_ethtool_get_pause_param,
+ .set_pauseparam = mvpp2_ethtool_set_pause_param,
+ .get_link_ksettings = mvpp2_ethtool_get_link_ksettings,
+ .set_link_ksettings = mvpp2_ethtool_set_link_ksettings,
};
/* Used for PPv2.1, or PPv2.2 with the old Device Tree binding that
@@ -7492,17 +7408,185 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
eth_hw_addr_random(dev);
}
+static void mvpp2_phylink_validate(struct net_device *dev,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+ phylink_set_port_modes(mask);
+
+ phylink_set(mask, Autoneg);
+ phylink_set(mask, Pause);
+ phylink_set(mask, Asym_Pause);
+
+ phylink_set(mask, 10baseT_Half);
+ phylink_set(mask, 10baseT_Full);
+ phylink_set(mask, 100baseT_Half);
+ phylink_set(mask, 100baseT_Full);
+ phylink_set(mask, 1000baseT_Half);
+ phylink_set(mask, 1000baseT_Full);
+ phylink_set(mask, 1000baseX_Full);
+ phylink_set(mask, 10000baseKR_Full);
+
+ bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(state->advertising, state->advertising, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
+static int mvpp2_phylink_mac_link_state(struct net_device *dev,
+ struct phylink_link_state *state)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+ u32 val;
+
+ if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
+ port->phy_interface != PHY_INTERFACE_MODE_SGMII)
+ return 0;
+
+ val = readl(port->base + MVPP2_GMAC_STATUS0);
+
+ state->an_complete = !!(val & MVPP2_GMAC_STATUS0_AN_COMPLETE);
+ state->link = !!(val & MVPP2_GMAC_STATUS0_LINK_UP);
+ state->duplex = !!(val & MVPP2_GMAC_STATUS0_FULL_DUPLEX);
+
+ if (val & MVPP2_GMAC_STATUS0_GMII_SPEED)
+ state->speed = SPEED_1000;
+ else
+ state->speed = (val & MVPP2_GMAC_STATUS0_MII_SPEED) ?
+ SPEED_100 : SPEED_10;
+
+ state->pause = 0;
+ if (val & MVPP2_GMAC_STATUS0_RX_PAUSE)
+ state->pause |= MLO_PAUSE_RX;
+ if (val & MVPP2_GMAC_STATUS0_TX_PAUSE)
+ state->pause |= MLO_PAUSE_TX;
+
+ return 1;
+}
+
+static void mvpp2_mac_an_restart(struct net_device *dev)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+ u32 val;
+
+ if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
+ port->phy_interface != PHY_INTERFACE_MODE_SGMII)
+ return;
+
+ val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+ val |= MVPP2_GMAC_IN_BAND_RESTART_AN;
+ writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+}
+
+static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
+ const struct phylink_link_state *state)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+ u32 val;
+
+ /* disable current port for reconfiguration */
+ mvpp2_interrupts_disable(port);
+ netif_carrier_off(port->dev);
+ mvpp2_port_disable(port);
+ phy_power_off(port->comphy);
+
+ /* comphy reconfiguration */
+ port->phy_interface = state->interface;
+ mvpp22_comphy_init(port);
+
+ /* gop/mac reconfiguration */
+ mvpp22_gop_init(port);
+ mvpp2_port_mii_set(port);
+
+ if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
+ port->phy_interface != PHY_INTERFACE_MODE_SGMII)
+ return;
+
+ val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+ val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
+ MVPP2_GMAC_CONFIG_GMII_SPEED |
+ MVPP2_GMAC_CONFIG_FULL_DUPLEX |
+ MVPP2_GMAC_AN_SPEED_EN |
+ MVPP2_GMAC_AN_DUPLEX_EN);
+
+ if (state->duplex)
+ val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
+
+ if (state->speed == SPEED_1000)
+ val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
+ else if (state->speed == SPEED_100)
+ val |= MVPP2_GMAC_CONFIG_MII_SPEED;
+
+ writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+
+ if (port->priv->hw_version == MVPP21 && port->flags & MVPP2_F_LOOPBACK)
+ mvpp2_port_loopback_set(port, state);
+}
+
+static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+ u32 val;
+
+ netif_tx_stop_all_queues(dev);
+ netif_carrier_off(dev);
+ mvpp2_ingress_disable(port);
+ mvpp2_egress_disable(port);
+
+ mvpp2_port_disable(port);
+ mvpp2_interrupts_disable(port);
+
+ if (!phylink_autoneg_inband(mode)) {
+ val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+ val &= ~MVPP2_GMAC_FORCE_LINK_PASS;
+ val |= MVPP2_GMAC_FORCE_LINK_DOWN;
+ writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+ }
+}
+
+static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
+ struct phy_device *phy)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+ u32 val;
+
+ if (!phylink_autoneg_inband(mode)) {
+ val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+ val &= ~MVPP2_GMAC_FORCE_LINK_DOWN;
+ val |= MVPP2_GMAC_FORCE_LINK_PASS;
+ writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+ }
+
+ mvpp2_interrupts_enable(port);
+ mvpp2_port_enable(port);
+
+ mvpp2_egress_enable(port);
+ mvpp2_ingress_enable(port);
+ netif_carrier_on(dev);
+ netif_tx_wake_all_queues(dev);
+}
+
+static const struct phylink_mac_ops mvpp2_phylink_ops = {
+ .validate = mvpp2_phylink_validate,
+ .mac_link_state = mvpp2_phylink_mac_link_state,
+ .mac_an_restart = mvpp2_mac_an_restart,
+ .mac_config = mvpp2_mac_config,
+ .mac_link_down = mvpp2_mac_link_down,
+ .mac_link_up = mvpp2_mac_link_up,
+};
+
/* Ports initialization */
static int mvpp2_port_probe(struct platform_device *pdev,
struct device_node *port_node,
struct mvpp2 *priv)
{
- struct device_node *phy_node;
struct phy *comphy;
struct mvpp2_port *port;
struct mvpp2_port_pcpu *port_pcpu;
struct net_device *dev;
struct resource *res;
+ struct phylink *phylink;
char *mac_from = "";
unsigned int ntxqs, nrxqs;
bool has_tx_irqs;
@@ -7526,7 +7610,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
if (!dev)
return -ENOMEM;
- phy_node = of_parse_phandle(port_node, "phy", 0);
phy_mode = of_get_phy_mode(port_node);
if (phy_mode < 0) {
dev_err(&pdev->dev, "incorrect phy mode\n");
@@ -7565,15 +7648,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
if (err)
goto err_free_netdev;
- port->link_irq = of_irq_get_byname(port_node, "link");
- if (port->link_irq == -EPROBE_DEFER) {
- err = -EPROBE_DEFER;
- goto err_deinit_qvecs;
- }
- if (port->link_irq <= 0)
- /* the link irq is optional */
- port->link_irq = 0;
-
if (of_property_read_bool(port_node, "marvell,loopback"))
port->flags |= MVPP2_F_LOOPBACK;
@@ -7583,7 +7657,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
else
port->first_rxq = port->id * priv->max_port_rxqs;
- port->phy_node = phy_node;
+ port->of_node = port_node;
port->phy_interface = phy_mode;
port->comphy = comphy;
@@ -7592,7 +7666,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
port->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(port->base)) {
err = PTR_ERR(port->base);
- goto err_free_irq;
+ goto err_deinit_qvecs;
}
} else {
if (of_property_read_u32(port_node, "gop-port-id",
@@ -7609,7 +7683,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
port->stats = netdev_alloc_pcpu_stats(struct mvpp2_pcpu_stats);
if (!port->stats) {
err = -ENOMEM;
- goto err_free_irq;
+ goto err_deinit_qvecs;
}
mvpp2_port_copy_mac_addr(dev, priv, port_node, &mac_from);
@@ -7662,16 +7736,47 @@ static int mvpp2_port_probe(struct platform_device *pdev,
/* 9676 == 9700 - 20 and rounding to 8 */
dev->max_mtu = 9676;
+ /* The PHY node is optional. If not present the GoP link IRQ will be
+ * used to handle link updates. Otherwise use phylink.
+ */
+ if (of_find_property(port_node, "phy", NULL)) {
+ phylink = phylink_create(dev, port_node, phy_mode,
+ &mvpp2_phylink_ops);
+ if (IS_ERR(phylink)) {
+ err = PTR_ERR(phylink);
+ goto err_free_port_pcpu;
+ }
+ port->phylink = phylink;
+ port->link_irq = 0;
+ } else {
+ port->phylink = NULL;
+ if (priv->hw_version == MVPP22) {
+ port->link_irq = of_irq_get_byname(port_node, "link");
+ if (port->link_irq == -EPROBE_DEFER) {
+ err = -EPROBE_DEFER;
+ goto err_free_port_pcpu;
+ }
+ if (port->link_irq <= 0)
+ /* the link irq is optional */
+ port->link_irq = 0;
+ }
+ }
+
err = register_netdev(dev);
if (err < 0) {
dev_err(&pdev->dev, "failed to register netdev\n");
- goto err_free_port_pcpu;
+ goto err_phylink_irq;
}
netdev_info(dev, "Using %s mac address %pM\n", mac_from, dev->dev_addr);
priv->port_list[id] = port;
return 0;
+err_phylink_irq:
+ if (port->phylink)
+ phylink_destroy(port->phylink);
+ else if (port->link_irq)
+ irq_dispose_mapping(port->link_irq);
err_free_port_pcpu:
free_percpu(port->pcpu);
err_free_txq_pcpu:
@@ -7679,13 +7784,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
free_percpu(port->txqs[i]->pcpu);
err_free_stats:
free_percpu(port->stats);
-err_free_irq:
- if (port->link_irq)
- irq_dispose_mapping(port->link_irq);
err_deinit_qvecs:
mvpp2_queue_vectors_deinit(port);
err_free_netdev:
- of_node_put(phy_node);
free_netdev(dev);
return err;
}
@@ -7696,7 +7797,8 @@ static void mvpp2_port_remove(struct mvpp2_port *port)
int i;
unregister_netdev(port->dev);
- of_node_put(port->phy_node);
+ if (port->phylink)
+ phylink_destroy(port->phylink);
free_percpu(port->pcpu);
free_percpu(port->stats);
for (i = 0; i < port->ntxqs; i++)
--
2.13.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-21 13:45 [PATCH net-next] net: mvpp2: phylink support Antoine Tenart
@ 2017-09-22 7:56 ` Marcin Wojtas
2017-09-22 11:07 ` Russell King - ARM Linux
1 sibling, 0 replies; 10+ messages in thread
From: Marcin Wojtas @ 2017-09-22 7:56 UTC (permalink / raw)
To: Antoine Tenart
Cc: David S. Miller, Russell King - ARM Linux, Andrew Lunn,
Gregory Clément, Thomas Petazzoni, Miquèl Raynal,
nadavh, linux-kernel, Stefan Chulski, netdev
Hi Antoine,
You can add
Tested-by: Marcin Wojtas <mw@semihalf.com>
Best regards,
Marcin
2017-09-21 15:45 GMT+02:00 Antoine Tenart <antoine.tenart@free-electrons.com>:
> Convert the PPv2 driver to use phylink, which models the MAC to PHY
> link. The phylink support is made such a way the GoP link IRQ can still
> be used: the two modes are incompatible and the GoP link IRQ will be
> used if no PHY is described in the device tree. This is the same
> behaviour as before.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
> drivers/net/ethernet/marvell/Kconfig | 1 +
> drivers/net/ethernet/marvell/mvpp2.c | 502 +++++++++++++++++++++--------------
> 2 files changed, 303 insertions(+), 200 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> index da6fb825afea..991138b8dfba 100644
> --- a/drivers/net/ethernet/marvell/Kconfig
> +++ b/drivers/net/ethernet/marvell/Kconfig
> @@ -86,6 +86,7 @@ config MVPP2
> depends on ARCH_MVEBU || COMPILE_TEST
> depends on HAS_DMA
> select MVMDIO
> + select PHYLINK
> ---help---
> This driver supports the network interface units in the
> Marvell ARMADA 375, 7K and 8K SoCs.
> diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
> index 8041d692db3c..5fb7e76ee128 100644
> --- a/drivers/net/ethernet/marvell/mvpp2.c
> +++ b/drivers/net/ethernet/marvell/mvpp2.c
> @@ -28,6 +28,7 @@
> #include <linux/of_address.h>
> #include <linux/of_device.h>
> #include <linux/phy.h>
> +#include <linux/phylink.h>
> #include <linux/phy/phy.h>
> #include <linux/clk.h>
> #include <linux/hrtimer.h>
> @@ -341,6 +342,7 @@
> #define MVPP2_GMAC_FORCE_LINK_PASS BIT(1)
> #define MVPP2_GMAC_IN_BAND_AUTONEG BIT(2)
> #define MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS BIT(3)
> +#define MVPP2_GMAC_IN_BAND_RESTART_AN BIT(4)
> #define MVPP2_GMAC_CONFIG_MII_SPEED BIT(5)
> #define MVPP2_GMAC_CONFIG_GMII_SPEED BIT(6)
> #define MVPP2_GMAC_AN_SPEED_EN BIT(7)
> @@ -350,6 +352,12 @@
> #define MVPP2_GMAC_AN_DUPLEX_EN BIT(13)
> #define MVPP2_GMAC_STATUS0 0x10
> #define MVPP2_GMAC_STATUS0_LINK_UP BIT(0)
> +#define MVPP2_GMAC_STATUS0_GMII_SPEED BIT(1)
> +#define MVPP2_GMAC_STATUS0_MII_SPEED BIT(2)
> +#define MVPP2_GMAC_STATUS0_FULL_DUPLEX BIT(3)
> +#define MVPP2_GMAC_STATUS0_RX_PAUSE BIT(6)
> +#define MVPP2_GMAC_STATUS0_TX_PAUSE BIT(7)
> +#define MVPP2_GMAC_STATUS0_AN_COMPLETE BIT(11)
> #define MVPP2_GMAC_PORT_FIFO_CFG_1_REG 0x1c
> #define MVPP2_GMAC_TX_FIFO_MIN_TH_OFFS 6
> #define MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK 0x1fc0
> @@ -878,12 +886,11 @@ struct mvpp2_port {
> u16 rx_ring_size;
> struct mvpp2_pcpu_stats __percpu *stats;
>
> + struct device_node *of_node;
> +
> phy_interface_t phy_interface;
> - struct device_node *phy_node;
> + struct phylink *phylink;
> struct phy *comphy;
> - unsigned int link;
> - unsigned int duplex;
> - unsigned int speed;
>
> struct mvpp2_bm_pool *pool_long;
> struct mvpp2_bm_pool *pool_short;
> @@ -4716,13 +4723,14 @@ static void mvpp2_port_periodic_xon_disable(struct mvpp2_port *port)
> }
>
> /* Configure loopback port */
> -static void mvpp2_port_loopback_set(struct mvpp2_port *port)
> +static void mvpp2_port_loopback_set(struct mvpp2_port *port,
> + const struct phylink_link_state *state)
> {
> u32 val;
>
> val = readl(port->base + MVPP2_GMAC_CTRL_1_REG);
>
> - if (port->speed == 1000)
> + if (state->speed == 1000)
> val |= MVPP2_GMAC_GMII_LB_EN_MASK;
> else
> val &= ~MVPP2_GMAC_GMII_LB_EN_MASK;
> @@ -4778,10 +4786,6 @@ static void mvpp2_defaults_set(struct mvpp2_port *port)
> int tx_port_num, val, queue, ptxq, lrxq;
>
> if (port->priv->hw_version == MVPP21) {
> - /* Configure port to loopback if needed */
> - if (port->flags & MVPP2_F_LOOPBACK)
> - mvpp2_port_loopback_set(port);
> -
> /* Update TX FIFO MIN Threshold */
> val = readl(port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
> val &= ~MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK;
> @@ -5860,111 +5864,6 @@ static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void mvpp2_gmac_set_autoneg(struct mvpp2_port *port,
> - struct phy_device *phydev)
> -{
> - u32 val;
> -
> - if (port->phy_interface != PHY_INTERFACE_MODE_RGMII &&
> - port->phy_interface != PHY_INTERFACE_MODE_RGMII_ID &&
> - port->phy_interface != PHY_INTERFACE_MODE_RGMII_RXID &&
> - port->phy_interface != PHY_INTERFACE_MODE_RGMII_TXID &&
> - port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> - return;
> -
> - val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> - val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
> - MVPP2_GMAC_CONFIG_GMII_SPEED |
> - MVPP2_GMAC_CONFIG_FULL_DUPLEX |
> - MVPP2_GMAC_AN_SPEED_EN |
> - MVPP2_GMAC_AN_DUPLEX_EN);
> -
> - if (phydev->duplex)
> - val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
> -
> - if (phydev->speed == SPEED_1000)
> - val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
> - else if (phydev->speed == SPEED_100)
> - val |= MVPP2_GMAC_CONFIG_MII_SPEED;
> -
> - writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> -}
> -
> -/* Adjust link */
> -static void mvpp2_link_event(struct net_device *dev)
> -{
> - struct mvpp2_port *port = netdev_priv(dev);
> - struct phy_device *phydev = dev->phydev;
> - bool link_reconfigured = false;
> - u32 val;
> -
> - if (phydev->link) {
> - if (port->phy_interface != phydev->interface && port->comphy) {
> - /* disable current port for reconfiguration */
> - mvpp2_interrupts_disable(port);
> - netif_carrier_off(port->dev);
> - mvpp2_port_disable(port);
> - phy_power_off(port->comphy);
> -
> - /* comphy reconfiguration */
> - port->phy_interface = phydev->interface;
> - mvpp22_comphy_init(port);
> -
> - /* gop/mac reconfiguration */
> - mvpp22_gop_init(port);
> - mvpp2_port_mii_set(port);
> -
> - link_reconfigured = true;
> - }
> -
> - if ((port->speed != phydev->speed) ||
> - (port->duplex != phydev->duplex)) {
> - mvpp2_gmac_set_autoneg(port, phydev);
> -
> - port->duplex = phydev->duplex;
> - port->speed = phydev->speed;
> - }
> - }
> -
> - if (phydev->link != port->link || link_reconfigured) {
> - port->link = phydev->link;
> -
> - if (phydev->link) {
> - if (port->phy_interface == PHY_INTERFACE_MODE_RGMII ||
> - port->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - port->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> - port->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> - port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
> - val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> - val |= (MVPP2_GMAC_FORCE_LINK_PASS |
> - MVPP2_GMAC_FORCE_LINK_DOWN);
> - writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> - }
> -
> - mvpp2_interrupts_enable(port);
> - mvpp2_port_enable(port);
> -
> - mvpp2_egress_enable(port);
> - mvpp2_ingress_enable(port);
> - netif_carrier_on(dev);
> - netif_tx_wake_all_queues(dev);
> - } else {
> - port->duplex = -1;
> - port->speed = 0;
> -
> - netif_tx_stop_all_queues(dev);
> - netif_carrier_off(dev);
> - mvpp2_ingress_disable(port);
> - mvpp2_egress_disable(port);
> -
> - mvpp2_port_disable(port);
> - mvpp2_interrupts_disable(port);
> - }
> -
> - phy_print_status(phydev);
> - }
> -}
> -
> static void mvpp2_timer_set(struct mvpp2_port_pcpu *port_pcpu)
> {
> ktime_t interval;
> @@ -6582,7 +6481,6 @@ static int mvpp2_poll(struct napi_struct *napi, int budget)
> /* Set hw internals when starting port */
> static void mvpp2_start_dev(struct mvpp2_port *port)
> {
> - struct net_device *ndev = port->dev;
> int i;
>
> if (port->gop_id == 0 &&
> @@ -6607,15 +6505,14 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
>
> mvpp2_port_mii_set(port);
> mvpp2_port_enable(port);
> - if (ndev->phydev)
> - phy_start(ndev->phydev);
> + if (port->phylink)
> + phylink_start(port->phylink);
> netif_tx_start_all_queues(port->dev);
> }
>
> /* Set hw internals when stopping port */
> static void mvpp2_stop_dev(struct mvpp2_port *port)
> {
> - struct net_device *ndev = port->dev;
> int i;
>
> /* Stop new packets from arriving to RXQs */
> @@ -6634,8 +6531,8 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
>
> mvpp2_egress_disable(port);
> mvpp2_port_disable(port);
> - if (ndev->phydev)
> - phy_stop(ndev->phydev);
> + if (port->phylink)
> + phylink_stop(port->phylink);
> phy_power_off(port->comphy);
> }
>
> @@ -6688,40 +6585,6 @@ static void mvpp21_get_mac_address(struct mvpp2_port *port, unsigned char *addr)
> addr[5] = (mac_addr_l >> MVPP2_GMAC_SA_LOW_OFFS) & 0xFF;
> }
>
> -static int mvpp2_phy_connect(struct mvpp2_port *port)
> -{
> - struct phy_device *phy_dev;
> -
> - /* No PHY is attached */
> - if (!port->phy_node)
> - return 0;
> -
> - phy_dev = of_phy_connect(port->dev, port->phy_node, mvpp2_link_event, 0,
> - port->phy_interface);
> - if (!phy_dev) {
> - netdev_err(port->dev, "cannot connect to phy\n");
> - return -ENODEV;
> - }
> - phy_dev->supported &= PHY_GBIT_FEATURES;
> - phy_dev->advertising = phy_dev->supported;
> -
> - port->link = 0;
> - port->duplex = 0;
> - port->speed = 0;
> -
> - return 0;
> -}
> -
> -static void mvpp2_phy_disconnect(struct mvpp2_port *port)
> -{
> - struct net_device *ndev = port->dev;
> -
> - if (!ndev->phydev)
> - return;
> -
> - phy_disconnect(ndev->phydev);
> -}
> -
> static int mvpp2_irqs_init(struct mvpp2_port *port)
> {
> int err, i;
> @@ -6765,7 +6628,6 @@ static void mvpp2_irqs_deinit(struct mvpp2_port *port)
> static int mvpp2_open(struct net_device *dev)
> {
> struct mvpp2_port *port = netdev_priv(dev);
> - struct mvpp2 *priv = port->priv;
> unsigned char mac_bcast[ETH_ALEN] = {
> 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> int err;
> @@ -6811,7 +6673,16 @@ static int mvpp2_open(struct net_device *dev)
> goto err_cleanup_txqs;
> }
>
> - if (priv->hw_version == MVPP22 && !port->phy_node && port->link_irq) {
> + /* In default link is down */
> + netif_carrier_off(port->dev);
> +
> + if (port->phylink) {
> + err = phylink_of_phy_connect(port->phylink, port->of_node);
> + if (err) {
> + netdev_err(port->dev, "could not attach PHY (%d)\n", err);
> + goto err_free_irq;
> + }
> + } else if (port->link_irq) {
> err = request_irq(port->link_irq, mvpp2_link_status_isr, 0,
> dev->name, port);
> if (err) {
> @@ -6821,15 +6692,11 @@ static int mvpp2_open(struct net_device *dev)
> }
>
> mvpp22_gop_setup_irq(port);
> + } else {
> + netdev_err(dev, "cannot use phylink or GoP link IRQ\n");
> + goto err_free_irq;
> }
>
> - /* In default link is down */
> - netif_carrier_off(port->dev);
> -
> - err = mvpp2_phy_connect(port);
> - if (err < 0)
> - goto err_free_link_irq;
> -
> /* Unmask interrupts on all CPUs */
> on_each_cpu(mvpp2_interrupts_unmask, port, 1);
> mvpp2_shared_interrupt_mask_unmask(port, false);
> @@ -6838,9 +6705,6 @@ static int mvpp2_open(struct net_device *dev)
>
> return 0;
>
> -err_free_link_irq:
> - if (priv->hw_version == MVPP22 && !port->phy_node && port->link_irq)
> - free_irq(port->link_irq, port);
> err_free_irq:
> mvpp2_irqs_deinit(port);
> err_cleanup_txqs:
> @@ -6854,17 +6718,17 @@ static int mvpp2_stop(struct net_device *dev)
> {
> struct mvpp2_port *port = netdev_priv(dev);
> struct mvpp2_port_pcpu *port_pcpu;
> - struct mvpp2 *priv = port->priv;
> int cpu;
>
> mvpp2_stop_dev(port);
> - mvpp2_phy_disconnect(port);
> + if (port->phylink)
> + phylink_disconnect_phy(port->phylink);
>
> /* Mask interrupts on all CPUs */
> on_each_cpu(mvpp2_interrupts_mask, port, 1);
> mvpp2_shared_interrupt_mask_unmask(port, true);
>
> - if (priv->hw_version == MVPP22 && !port->phy_node && port->link_irq)
> + if (port->link_irq)
> free_irq(port->link_irq, port);
>
> mvpp2_irqs_deinit(port);
> @@ -7029,20 +6893,26 @@ mvpp2_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
>
> static int mvpp2_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> {
> - int ret;
> + struct mvpp2_port *port = netdev_priv(dev);
>
> - if (!dev->phydev)
> + if (!port->phylink)
> return -ENOTSUPP;
>
> - ret = phy_mii_ioctl(dev->phydev, ifr, cmd);
> - if (!ret)
> - mvpp2_link_event(dev);
> -
> - return ret;
> + return phylink_mii_ioctl(port->phylink, ifr, cmd);
> }
>
> /* Ethtool methods */
>
> +static int mvpp2_ethtool_nway_reset(struct net_device *dev)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> +
> + if (!port->phylink)
> + return -ENOTSUPP;
> +
> + return phylink_ethtool_nway_reset(port->phylink);
> +}
> +
> /* Set interrupt coalescing for ethtools */
> static int mvpp2_ethtool_set_coalesce(struct net_device *dev,
> struct ethtool_coalesce *c)
> @@ -7170,6 +7040,50 @@ static int mvpp2_ethtool_set_ringparam(struct net_device *dev,
> return err;
> }
>
> +static void mvpp2_ethtool_get_pause_param(struct net_device *dev,
> + struct ethtool_pauseparam *pause)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> +
> + if (!port->phylink)
> + return;
> +
> + phylink_ethtool_get_pauseparam(port->phylink, pause);
> +}
> +
> +static int mvpp2_ethtool_set_pause_param(struct net_device *dev,
> + struct ethtool_pauseparam *pause)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> +
> + if (!port->phylink)
> + return -ENOTSUPP;
> +
> + return phylink_ethtool_set_pauseparam(port->phylink, pause);
> +}
> +
> +static int mvpp2_ethtool_get_link_ksettings(struct net_device *dev,
> + struct ethtool_link_ksettings *cmd)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> +
> + if (!port->phylink)
> + return -ENOTSUPP;
> +
> + return phylink_ethtool_ksettings_get(port->phylink, cmd);
> +}
> +
> +static int mvpp2_ethtool_set_link_ksettings(struct net_device *dev,
> + const struct ethtool_link_ksettings *cmd)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> +
> + if (!port->phylink)
> + return -ENOTSUPP;
> +
> + return phylink_ethtool_ksettings_set(port->phylink, cmd);
> +}
> +
> /* Device ops */
>
> static const struct net_device_ops mvpp2_netdev_ops = {
> @@ -7184,15 +7098,17 @@ static const struct net_device_ops mvpp2_netdev_ops = {
> };
>
> static const struct ethtool_ops mvpp2_eth_tool_ops = {
> - .nway_reset = phy_ethtool_nway_reset,
> + .nway_reset = mvpp2_ethtool_nway_reset,
> .get_link = ethtool_op_get_link,
> .set_coalesce = mvpp2_ethtool_set_coalesce,
> .get_coalesce = mvpp2_ethtool_get_coalesce,
> .get_drvinfo = mvpp2_ethtool_get_drvinfo,
> .get_ringparam = mvpp2_ethtool_get_ringparam,
> .set_ringparam = mvpp2_ethtool_set_ringparam,
> - .get_link_ksettings = phy_ethtool_get_link_ksettings,
> - .set_link_ksettings = phy_ethtool_set_link_ksettings,
> + .get_pauseparam = mvpp2_ethtool_get_pause_param,
> + .set_pauseparam = mvpp2_ethtool_set_pause_param,
> + .get_link_ksettings = mvpp2_ethtool_get_link_ksettings,
> + .set_link_ksettings = mvpp2_ethtool_set_link_ksettings,
> };
>
> /* Used for PPv2.1, or PPv2.2 with the old Device Tree binding that
> @@ -7492,17 +7408,185 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
> eth_hw_addr_random(dev);
> }
>
> +static void mvpp2_phylink_validate(struct net_device *dev,
> + unsigned long *supported,
> + struct phylink_link_state *state)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> + phylink_set_port_modes(mask);
> +
> + phylink_set(mask, Autoneg);
> + phylink_set(mask, Pause);
> + phylink_set(mask, Asym_Pause);
> +
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> + phylink_set(mask, 1000baseT_Half);
> + phylink_set(mask, 1000baseT_Full);
> + phylink_set(mask, 1000baseX_Full);
> + phylink_set(mask, 10000baseKR_Full);
> +
> + bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + bitmap_and(state->advertising, state->advertising, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> +}
> +
> +static int mvpp2_phylink_mac_link_state(struct net_device *dev,
> + struct phylink_link_state *state)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return 0;
> +
> + val = readl(port->base + MVPP2_GMAC_STATUS0);
> +
> + state->an_complete = !!(val & MVPP2_GMAC_STATUS0_AN_COMPLETE);
> + state->link = !!(val & MVPP2_GMAC_STATUS0_LINK_UP);
> + state->duplex = !!(val & MVPP2_GMAC_STATUS0_FULL_DUPLEX);
> +
> + if (val & MVPP2_GMAC_STATUS0_GMII_SPEED)
> + state->speed = SPEED_1000;
> + else
> + state->speed = (val & MVPP2_GMAC_STATUS0_MII_SPEED) ?
> + SPEED_100 : SPEED_10;
> +
> + state->pause = 0;
> + if (val & MVPP2_GMAC_STATUS0_RX_PAUSE)
> + state->pause |= MLO_PAUSE_RX;
> + if (val & MVPP2_GMAC_STATUS0_TX_PAUSE)
> + state->pause |= MLO_PAUSE_TX;
> +
> + return 1;
> +}
> +
> +static void mvpp2_mac_an_restart(struct net_device *dev)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return;
> +
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val |= MVPP2_GMAC_IN_BAND_RESTART_AN;
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +}
> +
> +static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
> + const struct phylink_link_state *state)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + /* disable current port for reconfiguration */
> + mvpp2_interrupts_disable(port);
> + netif_carrier_off(port->dev);
> + mvpp2_port_disable(port);
> + phy_power_off(port->comphy);
> +
> + /* comphy reconfiguration */
> + port->phy_interface = state->interface;
> + mvpp22_comphy_init(port);
> +
> + /* gop/mac reconfiguration */
> + mvpp22_gop_init(port);
> + mvpp2_port_mii_set(port);
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return;
> +
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
> + MVPP2_GMAC_CONFIG_GMII_SPEED |
> + MVPP2_GMAC_CONFIG_FULL_DUPLEX |
> + MVPP2_GMAC_AN_SPEED_EN |
> + MVPP2_GMAC_AN_DUPLEX_EN);
> +
> + if (state->duplex)
> + val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
> +
> + if (state->speed == SPEED_1000)
> + val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
> + else if (state->speed == SPEED_100)
> + val |= MVPP2_GMAC_CONFIG_MII_SPEED;
> +
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +
> + if (port->priv->hw_version == MVPP21 && port->flags & MVPP2_F_LOOPBACK)
> + mvpp2_port_loopback_set(port, state);
> +}
> +
> +static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + netif_tx_stop_all_queues(dev);
> + netif_carrier_off(dev);
> + mvpp2_ingress_disable(port);
> + mvpp2_egress_disable(port);
> +
> + mvpp2_port_disable(port);
> + mvpp2_interrupts_disable(port);
> +
> + if (!phylink_autoneg_inband(mode)) {
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val &= ~MVPP2_GMAC_FORCE_LINK_PASS;
> + val |= MVPP2_GMAC_FORCE_LINK_DOWN;
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + }
> +}
> +
> +static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
> + struct phy_device *phy)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + if (!phylink_autoneg_inband(mode)) {
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val &= ~MVPP2_GMAC_FORCE_LINK_DOWN;
> + val |= MVPP2_GMAC_FORCE_LINK_PASS;
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + }
> +
> + mvpp2_interrupts_enable(port);
> + mvpp2_port_enable(port);
> +
> + mvpp2_egress_enable(port);
> + mvpp2_ingress_enable(port);
> + netif_carrier_on(dev);
> + netif_tx_wake_all_queues(dev);
> +}
> +
> +static const struct phylink_mac_ops mvpp2_phylink_ops = {
> + .validate = mvpp2_phylink_validate,
> + .mac_link_state = mvpp2_phylink_mac_link_state,
> + .mac_an_restart = mvpp2_mac_an_restart,
> + .mac_config = mvpp2_mac_config,
> + .mac_link_down = mvpp2_mac_link_down,
> + .mac_link_up = mvpp2_mac_link_up,
> +};
> +
> /* Ports initialization */
> static int mvpp2_port_probe(struct platform_device *pdev,
> struct device_node *port_node,
> struct mvpp2 *priv)
> {
> - struct device_node *phy_node;
> struct phy *comphy;
> struct mvpp2_port *port;
> struct mvpp2_port_pcpu *port_pcpu;
> struct net_device *dev;
> struct resource *res;
> + struct phylink *phylink;
> char *mac_from = "";
> unsigned int ntxqs, nrxqs;
> bool has_tx_irqs;
> @@ -7526,7 +7610,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> if (!dev)
> return -ENOMEM;
>
> - phy_node = of_parse_phandle(port_node, "phy", 0);
> phy_mode = of_get_phy_mode(port_node);
> if (phy_mode < 0) {
> dev_err(&pdev->dev, "incorrect phy mode\n");
> @@ -7565,15 +7648,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> if (err)
> goto err_free_netdev;
>
> - port->link_irq = of_irq_get_byname(port_node, "link");
> - if (port->link_irq == -EPROBE_DEFER) {
> - err = -EPROBE_DEFER;
> - goto err_deinit_qvecs;
> - }
> - if (port->link_irq <= 0)
> - /* the link irq is optional */
> - port->link_irq = 0;
> -
> if (of_property_read_bool(port_node, "marvell,loopback"))
> port->flags |= MVPP2_F_LOOPBACK;
>
> @@ -7583,7 +7657,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> else
> port->first_rxq = port->id * priv->max_port_rxqs;
>
> - port->phy_node = phy_node;
> + port->of_node = port_node;
> port->phy_interface = phy_mode;
> port->comphy = comphy;
>
> @@ -7592,7 +7666,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> port->base = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(port->base)) {
> err = PTR_ERR(port->base);
> - goto err_free_irq;
> + goto err_deinit_qvecs;
> }
> } else {
> if (of_property_read_u32(port_node, "gop-port-id",
> @@ -7609,7 +7683,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> port->stats = netdev_alloc_pcpu_stats(struct mvpp2_pcpu_stats);
> if (!port->stats) {
> err = -ENOMEM;
> - goto err_free_irq;
> + goto err_deinit_qvecs;
> }
>
> mvpp2_port_copy_mac_addr(dev, priv, port_node, &mac_from);
> @@ -7662,16 +7736,47 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> /* 9676 == 9700 - 20 and rounding to 8 */
> dev->max_mtu = 9676;
>
> + /* The PHY node is optional. If not present the GoP link IRQ will be
> + * used to handle link updates. Otherwise use phylink.
> + */
> + if (of_find_property(port_node, "phy", NULL)) {
> + phylink = phylink_create(dev, port_node, phy_mode,
> + &mvpp2_phylink_ops);
> + if (IS_ERR(phylink)) {
> + err = PTR_ERR(phylink);
> + goto err_free_port_pcpu;
> + }
> + port->phylink = phylink;
> + port->link_irq = 0;
> + } else {
> + port->phylink = NULL;
> + if (priv->hw_version == MVPP22) {
> + port->link_irq = of_irq_get_byname(port_node, "link");
> + if (port->link_irq == -EPROBE_DEFER) {
> + err = -EPROBE_DEFER;
> + goto err_free_port_pcpu;
> + }
> + if (port->link_irq <= 0)
> + /* the link irq is optional */
> + port->link_irq = 0;
> + }
> + }
> +
> err = register_netdev(dev);
> if (err < 0) {
> dev_err(&pdev->dev, "failed to register netdev\n");
> - goto err_free_port_pcpu;
> + goto err_phylink_irq;
> }
> netdev_info(dev, "Using %s mac address %pM\n", mac_from, dev->dev_addr);
>
> priv->port_list[id] = port;
> return 0;
>
> +err_phylink_irq:
> + if (port->phylink)
> + phylink_destroy(port->phylink);
> + else if (port->link_irq)
> + irq_dispose_mapping(port->link_irq);
> err_free_port_pcpu:
> free_percpu(port->pcpu);
> err_free_txq_pcpu:
> @@ -7679,13 +7784,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> free_percpu(port->txqs[i]->pcpu);
> err_free_stats:
> free_percpu(port->stats);
> -err_free_irq:
> - if (port->link_irq)
> - irq_dispose_mapping(port->link_irq);
> err_deinit_qvecs:
> mvpp2_queue_vectors_deinit(port);
> err_free_netdev:
> - of_node_put(phy_node);
> free_netdev(dev);
> return err;
> }
> @@ -7696,7 +7797,8 @@ static void mvpp2_port_remove(struct mvpp2_port *port)
> int i;
>
> unregister_netdev(port->dev);
> - of_node_put(port->phy_node);
> + if (port->phylink)
> + phylink_destroy(port->phylink);
> free_percpu(port->pcpu);
> free_percpu(port->stats);
> for (i = 0; i < port->ntxqs; i++)
> --
> 2.13.5
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-21 13:45 [PATCH net-next] net: mvpp2: phylink support Antoine Tenart
2017-09-22 7:56 ` Marcin Wojtas
@ 2017-09-22 11:07 ` Russell King - ARM Linux
2017-09-25 9:55 ` Antoine Tenart
1 sibling, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2017-09-22 11:07 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, andrew, gregory.clement, thomas.petazzoni, miquel.raynal,
nadavh, linux-kernel, mw, stefanc, netdev
On Thu, Sep 21, 2017 at 03:45:22PM +0200, Antoine Tenart wrote:
> Convert the PPv2 driver to use phylink, which models the MAC to PHY
> link. The phylink support is made such a way the GoP link IRQ can still
> be used: the two modes are incompatible and the GoP link IRQ will be
> used if no PHY is described in the device tree. This is the same
> behaviour as before.
This makes no sense. The point of phylink is to be able to support SFP
cages, and SFP cages do not have a PHY described in DT. So, when you
want to use phylink because of SFP, you can't, because if you omit
the PHY the driver avoids using phylink.
> +static void mvpp2_phylink_validate(struct net_device *dev,
> + unsigned long *supported,
> + struct phylink_link_state *state)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> + phylink_set_port_modes(mask);
> +
> + phylink_set(mask, Autoneg);
> + phylink_set(mask, Pause);
> + phylink_set(mask, Asym_Pause);
> +
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> + phylink_set(mask, 1000baseT_Half);
> + phylink_set(mask, 1000baseT_Full);
> + phylink_set(mask, 1000baseX_Full);
> + phylink_set(mask, 10000baseKR_Full);
> +
> + bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + bitmap_and(state->advertising, state->advertising, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> +}
I don't think you've understood this despite the comments in the header
file. What you describe above basically means you don't support any
kind of copper connection at 10G speeds, or any fiber modes at 10G
speeds either.
You need to set 10000baseT_Full for copper, 10000baseSR_Full,
10000baseLR_Full, 10000baseLRM_Full etc for fiber. Had you looked at
my modifications for Marvell's mvpp2x driver you'd have spotted this...
> +
> +static int mvpp2_phylink_mac_link_state(struct net_device *dev,
> + struct phylink_link_state *state)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return 0;
You're blocking this for 1000base-X and 10G connections, which is not
correct. The expectation is that this function returns the current
MAC state irrespective of the interface mode.
> +
> + val = readl(port->base + MVPP2_GMAC_STATUS0);
> +
> + state->an_complete = !!(val & MVPP2_GMAC_STATUS0_AN_COMPLETE);
> + state->link = !!(val & MVPP2_GMAC_STATUS0_LINK_UP);
> + state->duplex = !!(val & MVPP2_GMAC_STATUS0_FULL_DUPLEX);
> +
> + if (val & MVPP2_GMAC_STATUS0_GMII_SPEED)
> + state->speed = SPEED_1000;
> + else
> + state->speed = (val & MVPP2_GMAC_STATUS0_MII_SPEED) ?
> + SPEED_100 : SPEED_10;
> +
> + state->pause = 0;
> + if (val & MVPP2_GMAC_STATUS0_RX_PAUSE)
> + state->pause |= MLO_PAUSE_RX;
> + if (val & MVPP2_GMAC_STATUS0_TX_PAUSE)
> + state->pause |= MLO_PAUSE_TX;
> +
> + return 1;
> +}
> +
> +static void mvpp2_mac_an_restart(struct net_device *dev)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return;
This prevents AN restart in 1000base-X mode, which is exactly the
mode that you need to do this. SGMII doesn't care, and RGMII doesn't
have inband AN.
> +
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val |= MVPP2_GMAC_IN_BAND_RESTART_AN;
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +}
> +
> +static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
> + const struct phylink_link_state *state)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + /* disable current port for reconfiguration */
> + mvpp2_interrupts_disable(port);
> + netif_carrier_off(port->dev);
> + mvpp2_port_disable(port);
> + phy_power_off(port->comphy);
> +
> + /* comphy reconfiguration */
> + port->phy_interface = state->interface;
> + mvpp22_comphy_init(port);
> +
> + /* gop/mac reconfiguration */
> + mvpp22_gop_init(port);
> + mvpp2_port_mii_set(port);
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return;
Again, 1000base-X is excluded, which will break it. You do need
to avoid touching the GMAC for 10G connections however.
> +
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
> + MVPP2_GMAC_CONFIG_GMII_SPEED |
> + MVPP2_GMAC_CONFIG_FULL_DUPLEX |
> + MVPP2_GMAC_AN_SPEED_EN |
> + MVPP2_GMAC_AN_DUPLEX_EN);
> +
> + if (state->duplex)
> + val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
> +
> + if (state->speed == SPEED_1000)
> + val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
> + else if (state->speed == SPEED_100)
> + val |= MVPP2_GMAC_CONFIG_MII_SPEED;
> +
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
You're assuming that this function only sets the current parameters for
the MAC. That's incorrect - it also needs to deal with autonegotiation
for inband AN, such as SGMII and 1000base-X.
> +
> + if (port->priv->hw_version == MVPP21 && port->flags & MVPP2_F_LOOPBACK)
> + mvpp2_port_loopback_set(port, state);
> +}
> +
> +static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + netif_tx_stop_all_queues(dev);
> + netif_carrier_off(dev);
> + mvpp2_ingress_disable(port);
> + mvpp2_egress_disable(port);
> +
> + mvpp2_port_disable(port);
> + mvpp2_interrupts_disable(port);
> +
> + if (!phylink_autoneg_inband(mode)) {
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val &= ~MVPP2_GMAC_FORCE_LINK_PASS;
> + val |= MVPP2_GMAC_FORCE_LINK_DOWN;
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + }
Please explain why you think its necessary to force the link down when
the link is already down - if there's no media connected, we only
need to stop the ingress and egress.
It's certainly wrong to disable interrupts - how do we end up with
link status changes reported from the MAC to phylink if interrupts
have been disabled?
phylink in the presence of a PHY checks that both the PHY _and_ MAC
are reporting link before it calls mac_link_up(), so if you've
disabled interrupts...
You guys know that I have working example code for both mvneta and the
Marvell PP2x driver. It probably would help if you looked at those
examples.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-22 11:07 ` Russell King - ARM Linux
@ 2017-09-25 9:55 ` Antoine Tenart
2017-09-25 10:45 ` Russell King - ARM Linux
2017-10-09 12:55 ` Antoine Tenart
0 siblings, 2 replies; 10+ messages in thread
From: Antoine Tenart @ 2017-09-25 9:55 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Antoine Tenart, davem, andrew, gregory.clement, thomas.petazzoni,
miquel.raynal, nadavh, linux-kernel, mw, stefanc, netdev
Hi Russell,
On Fri, Sep 22, 2017 at 12:07:31PM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 21, 2017 at 03:45:22PM +0200, Antoine Tenart wrote:
> > Convert the PPv2 driver to use phylink, which models the MAC to PHY
> > link. The phylink support is made such a way the GoP link IRQ can still
> > be used: the two modes are incompatible and the GoP link IRQ will be
> > used if no PHY is described in the device tree. This is the same
> > behaviour as before.
>
> This makes no sense. The point of phylink is to be able to support SFP
> cages, and SFP cages do not have a PHY described in DT. So, when you
> want to use phylink because of SFP, you can't, because if you omit
> the PHY the driver avoids using phylink.
Yes that's an issue. However we do need to support the GoP link IRQ
which is also needed in some cases where there is no PHY (and when
phylink cannot be used). What would you propose to differentiate those
two cases: no PHY using phylink, and no PHY using the GoP link IRQ?
> > +static void mvpp2_phylink_validate(struct net_device *dev,
> > + unsigned long *supported,
> > + struct phylink_link_state *state)
> > +{
> > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > +
> > + phylink_set_port_modes(mask);
> > +
> > + phylink_set(mask, Autoneg);
> > + phylink_set(mask, Pause);
> > + phylink_set(mask, Asym_Pause);
> > +
> > + phylink_set(mask, 10baseT_Half);
> > + phylink_set(mask, 10baseT_Full);
> > + phylink_set(mask, 100baseT_Half);
> > + phylink_set(mask, 100baseT_Full);
> > + phylink_set(mask, 1000baseT_Half);
> > + phylink_set(mask, 1000baseT_Full);
> > + phylink_set(mask, 1000baseX_Full);
> > + phylink_set(mask, 10000baseKR_Full);
> > +
> > + bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
> > + bitmap_and(state->advertising, state->advertising, mask,
> > + __ETHTOOL_LINK_MODE_MASK_NBITS);
> > +}
>
> I don't think you've understood this despite the comments in the header
> file. What you describe above basically means you don't support any
> kind of copper connection at 10G speeds, or any fiber modes at 10G
> speeds either.
>
> You need to set 10000baseT_Full for copper, 10000baseSR_Full,
> 10000baseLR_Full, 10000baseLRM_Full etc for fiber. Had you looked at
> my modifications for Marvell's mvpp2x driver you'd have spotted this...
Sure, I'll add these modes as they are supported as well.
> > +static int mvpp2_phylink_mac_link_state(struct net_device *dev,
> > + struct phylink_link_state *state)
> > +{
> > + struct mvpp2_port *port = netdev_priv(dev);
> > + u32 val;
> > +
> > + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> > + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > + return 0;
>
> You're blocking this for 1000base-X and 10G connections, which is not
> correct. The expectation is that this function returns the current
> MAC state irrespective of the interface mode.
I moved what was already supported in the PPv2 driver and did not
implemented the full set of what is supported. It's not perfect, but it
does move what was already supported.
Any reason not to first move what's already supported to phylink, and
then add more supported modes in separate patches?
> > +static void mvpp2_mac_an_restart(struct net_device *dev)
> > +{
> > + struct mvpp2_port *port = netdev_priv(dev);
> > + u32 val;
> > +
> > + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> > + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > + return;
>
> This prevents AN restart in 1000base-X mode, which is exactly the
> mode that you need to do this. SGMII doesn't care, and RGMII doesn't
> have inband AN.
I'll fix that.
> > + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > + val |= MVPP2_GMAC_IN_BAND_RESTART_AN;
> > + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > +}
> > +
> > +static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
> > + const struct phylink_link_state *state)
> > +{
> > + struct mvpp2_port *port = netdev_priv(dev);
> > + u32 val;
> > +
> > + /* disable current port for reconfiguration */
> > + mvpp2_interrupts_disable(port);
> > + netif_carrier_off(port->dev);
> > + mvpp2_port_disable(port);
> > + phy_power_off(port->comphy);
> > +
> > + /* comphy reconfiguration */
> > + port->phy_interface = state->interface;
> > + mvpp22_comphy_init(port);
> > +
> > + /* gop/mac reconfiguration */
> > + mvpp22_gop_init(port);
> > + mvpp2_port_mii_set(port);
> > +
> > + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> > + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > + return;
>
> Again, 1000base-X is excluded, which will break it. You do need
> to avoid touching the GMAC for 10G connections however.
Same comment as above.
> > + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > + val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
> > + MVPP2_GMAC_CONFIG_GMII_SPEED |
> > + MVPP2_GMAC_CONFIG_FULL_DUPLEX |
> > + MVPP2_GMAC_AN_SPEED_EN |
> > + MVPP2_GMAC_AN_DUPLEX_EN);
> > +
> > + if (state->duplex)
> > + val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
> > +
> > + if (state->speed == SPEED_1000)
> > + val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
> > + else if (state->speed == SPEED_100)
> > + val |= MVPP2_GMAC_CONFIG_MII_SPEED;
> > +
> > + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
>
> You're assuming that this function only sets the current parameters for
> the MAC. That's incorrect - it also needs to deal with autonegotiation
> for inband AN, such as SGMII and 1000base-X.
OK.
> > + if (port->priv->hw_version == MVPP21 && port->flags & MVPP2_F_LOOPBACK)
> > + mvpp2_port_loopback_set(port, state);
> > +}
> > +
> > +static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode)
> > +{
> > + struct mvpp2_port *port = netdev_priv(dev);
> > + u32 val;
> > +
> > + netif_tx_stop_all_queues(dev);
> > + netif_carrier_off(dev);
> > + mvpp2_ingress_disable(port);
> > + mvpp2_egress_disable(port);
> > +
> > + mvpp2_port_disable(port);
> > + mvpp2_interrupts_disable(port);
> > +
> > + if (!phylink_autoneg_inband(mode)) {
> > + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > + val &= ~MVPP2_GMAC_FORCE_LINK_PASS;
> > + val |= MVPP2_GMAC_FORCE_LINK_DOWN;
> > + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > + }
>
> Please explain why you think its necessary to force the link down when
> the link is already down - if there's no media connected, we only
> need to stop the ingress and egress.
Agreed, I'll remove this.
> It's certainly wrong to disable interrupts - how do we end up with
> link status changes reported from the MAC to phylink if interrupts
> have been disabled?
This only disables the vector IRQs used for tx/rx queues. That's already
the default when not using a port.
> You guys know that I have working example code for both mvneta and the
> Marvell PP2x driver. It probably would help if you looked at those
> examples.
I did used your mvneta phylink patch as an example. I'll have a look at
the other one, it'll be helpful.
Thank you for your thoughtful review.
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-25 9:55 ` Antoine Tenart
@ 2017-09-25 10:45 ` Russell King - ARM Linux
2017-09-25 11:53 ` Antoine Tenart
2017-10-09 12:55 ` Antoine Tenart
1 sibling, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2017-09-25 10:45 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, andrew, gregory.clement, thomas.petazzoni, miquel.raynal,
nadavh, linux-kernel, mw, stefanc, netdev
On Mon, Sep 25, 2017 at 11:55:14AM +0200, Antoine Tenart wrote:
> Hi Russell,
>
> On Fri, Sep 22, 2017 at 12:07:31PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 21, 2017 at 03:45:22PM +0200, Antoine Tenart wrote:
> > > Convert the PPv2 driver to use phylink, which models the MAC to PHY
> > > link. The phylink support is made such a way the GoP link IRQ can still
> > > be used: the two modes are incompatible and the GoP link IRQ will be
> > > used if no PHY is described in the device tree. This is the same
> > > behaviour as before.
> >
> > This makes no sense. The point of phylink is to be able to support SFP
> > cages, and SFP cages do not have a PHY described in DT. So, when you
> > want to use phylink because of SFP, you can't, because if you omit
> > the PHY the driver avoids using phylink.
>
> Yes that's an issue. However we do need to support the GoP link IRQ
> which is also needed in some cases where there is no PHY (and when
> phylink cannot be used). What would you propose to differentiate those
> two cases: no PHY using phylink, and no PHY using the GoP link IRQ?
Can you describe what the GoP link IRQ is doing please?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-25 10:45 ` Russell King - ARM Linux
@ 2017-09-25 11:53 ` Antoine Tenart
2017-09-25 12:13 ` Russell King - ARM Linux
0 siblings, 1 reply; 10+ messages in thread
From: Antoine Tenart @ 2017-09-25 11:53 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Antoine Tenart, davem, andrew, gregory.clement, thomas.petazzoni,
miquel.raynal, nadavh, linux-kernel, mw, stefanc, netdev
On Mon, Sep 25, 2017 at 11:45:32AM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 25, 2017 at 11:55:14AM +0200, Antoine Tenart wrote:
> > On Fri, Sep 22, 2017 at 12:07:31PM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Sep 21, 2017 at 03:45:22PM +0200, Antoine Tenart wrote:
> > > > Convert the PPv2 driver to use phylink, which models the MAC to PHY
> > > > link. The phylink support is made such a way the GoP link IRQ can still
> > > > be used: the two modes are incompatible and the GoP link IRQ will be
> > > > used if no PHY is described in the device tree. This is the same
> > > > behaviour as before.
> > >
> > > This makes no sense. The point of phylink is to be able to support SFP
> > > cages, and SFP cages do not have a PHY described in DT. So, when you
> > > want to use phylink because of SFP, you can't, because if you omit
> > > the PHY the driver avoids using phylink.
> >
> > Yes that's an issue. However we do need to support the GoP link IRQ
> > which is also needed in some cases where there is no PHY (and when
> > phylink cannot be used). What would you propose to differentiate those
> > two cases: no PHY using phylink, and no PHY using the GoP link IRQ?
>
> Can you describe what the GoP link IRQ is doing please?
In cases where there is no PHY connected to the MAC and no SFP cage is
used. One example is when a SOHO switch is connected directly to a
serdes lane. In such cases we still need to have a minimal link
management. The GoP link interrupt helps doing so as it raises when the
serdes is in sync and AN succeeded.
I also wonder if this is needed when using passive cables?
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-25 11:53 ` Antoine Tenart
@ 2017-09-25 12:13 ` Russell King - ARM Linux
2017-09-25 13:06 ` Antoine Tenart
0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2017-09-25 12:13 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, andrew, gregory.clement, thomas.petazzoni, miquel.raynal,
nadavh, linux-kernel, mw, stefanc, netdev
On Mon, Sep 25, 2017 at 01:53:03PM +0200, Antoine Tenart wrote:
> On Mon, Sep 25, 2017 at 11:45:32AM +0100, Russell King - ARM Linux wrote:
> > Can you describe what the GoP link IRQ is doing please?
>
> In cases where there is no PHY connected to the MAC and no SFP cage is
> used. One example is when a SOHO switch is connected directly to a
> serdes lane. In such cases we still need to have a minimal link
> management. The GoP link interrupt helps doing so as it raises when the
> serdes is in sync and AN succeeded.
Isn't this just like a fixed link scenario, or an in-band
autonegotiation scenario (both of which phylink supports natively)?
The situation on Clearfog with the 88E6176 switch is pretty similar -
a switch connected directly via serdes to the MAC. Currently, we
configure stuff there as a fixed link, but in actual fact the 88E6176
is configured to run the CPU facing port in 1000base-X mode, and with
appropriate tweaks, switching phylink to 1000base-X mode also works.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-25 12:13 ` Russell King - ARM Linux
@ 2017-09-25 13:06 ` Antoine Tenart
0 siblings, 0 replies; 10+ messages in thread
From: Antoine Tenart @ 2017-09-25 13:06 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Antoine Tenart, davem, andrew, gregory.clement, thomas.petazzoni,
miquel.raynal, nadavh, linux-kernel, mw, stefanc, netdev
On Mon, Sep 25, 2017 at 01:13:43PM +0100, Russell King - ARM Linux wrote:
> On Mon, Sep 25, 2017 at 01:53:03PM +0200, Antoine Tenart wrote:
> > On Mon, Sep 25, 2017 at 11:45:32AM +0100, Russell King - ARM Linux wrote:
> > > Can you describe what the GoP link IRQ is doing please?
> >
> > In cases where there is no PHY connected to the MAC and no SFP cage is
> > used. One example is when a SOHO switch is connected directly to a
> > serdes lane. In such cases we still need to have a minimal link
> > management. The GoP link interrupt helps doing so as it raises when the
> > serdes is in sync and AN succeeded.
>
> Isn't this just like a fixed link scenario, or an in-band
> autonegotiation scenario (both of which phylink supports natively)?
>
> The situation on Clearfog with the 88E6176 switch is pretty similar -
> a switch connected directly via serdes to the MAC. Currently, we
> configure stuff there as a fixed link, but in actual fact the 88E6176
> is configured to run the CPU facing port in 1000base-X mode, and with
> appropriate tweaks, switching phylink to 1000base-X mode also works.
Hmm, I think you're right, we should be able to represent the link
between the MAC and the switch as a fixed link. And when it's not fixed,
it could be done with in-band AN. I cannot test this myself but I've
asked someone who can to.
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-09-25 9:55 ` Antoine Tenart
2017-09-25 10:45 ` Russell King - ARM Linux
@ 2017-10-09 12:55 ` Antoine Tenart
2017-10-09 13:09 ` Russell King - ARM Linux
1 sibling, 1 reply; 10+ messages in thread
From: Antoine Tenart @ 2017-10-09 12:55 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Antoine Tenart, davem, andrew, gregory.clement, thomas.petazzoni,
miquel.raynal, nadavh, linux-kernel, mw, stefanc, netdev
Hi Russell,
On Mon, Sep 25, 2017 at 11:55:14AM +0200, Antoine Tenart wrote:
> On Fri, Sep 22, 2017 at 12:07:31PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 21, 2017 at 03:45:22PM +0200, Antoine Tenart wrote:
>
> > > +static int mvpp2_phylink_mac_link_state(struct net_device *dev,
> > > + struct phylink_link_state *state)
> > > +{
> > > + struct mvpp2_port *port = netdev_priv(dev);
> > > + u32 val;
> > > +
> > > + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> > > + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > + return 0;
> >
> > You're blocking this for 1000base-X and 10G connections, which is not
> > correct. The expectation is that this function returns the current
> > MAC state irrespective of the interface mode.
>
> I moved what was already supported in the PPv2 driver and did not
> implemented the full set of what is supported. It's not perfect, but it
> does move what was already supported.
>
> Any reason not to first move what's already supported to phylink, and
> then add more supported modes in separate patches?
Any thoughts on this?
> > > +static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
> > > + const struct phylink_link_state *state)
> > > +{
> > > + struct mvpp2_port *port = netdev_priv(dev);
> > > + u32 val;
> > > +
> > > + /* disable current port for reconfiguration */
> > > + mvpp2_interrupts_disable(port);
> > > + netif_carrier_off(port->dev);
> > > + mvpp2_port_disable(port);
> > > + phy_power_off(port->comphy);
> > > +
> > > + /* comphy reconfiguration */
> > > + port->phy_interface = state->interface;
> > > + mvpp22_comphy_init(port);
> > > +
> > > + /* gop/mac reconfiguration */
> > > + mvpp22_gop_init(port);
> > > + mvpp2_port_mii_set(port);
> > > +
> > > + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> > > + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > + return;
> >
> > Again, 1000base-X is excluded, which will break it. You do need
> > to avoid touching the GMAC for 10G connections however.
>
> Same comment as above.
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next] net: mvpp2: phylink support
2017-10-09 12:55 ` Antoine Tenart
@ 2017-10-09 13:09 ` Russell King - ARM Linux
0 siblings, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2017-10-09 13:09 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, andrew, gregory.clement, thomas.petazzoni, miquel.raynal,
nadavh, linux-kernel, mw, stefanc, netdev
On Mon, Oct 09, 2017 at 02:55:27PM +0200, Antoine Tenart wrote:
> Hi Russell,
>
> On Mon, Sep 25, 2017 at 11:55:14AM +0200, Antoine Tenart wrote:
> > On Fri, Sep 22, 2017 at 12:07:31PM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Sep 21, 2017 at 03:45:22PM +0200, Antoine Tenart wrote:
> >
> > > > +static int mvpp2_phylink_mac_link_state(struct net_device *dev,
> > > > + struct phylink_link_state *state)
> > > > +{
> > > > + struct mvpp2_port *port = netdev_priv(dev);
> > > > + u32 val;
> > > > +
> > > > + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> > > > + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> > > > + return 0;
> > >
> > > You're blocking this for 1000base-X and 10G connections, which is not
> > > correct. The expectation is that this function returns the current
> > > MAC state irrespective of the interface mode.
> >
> > I moved what was already supported in the PPv2 driver and did not
> > implemented the full set of what is supported. It's not perfect, but it
> > does move what was already supported.
> >
> > Any reason not to first move what's already supported to phylink, and
> > then add more supported modes in separate patches?
>
> Any thoughts on this?
You're asking me to comment about something I know little about as
I've not used mvpp2.c. I don't know the details of what your "already
supported" statement refers to. Maybe you could give some clues -
maybe produce a list of what mvpp2 currently supports?
Here's the link modes that phylink supports:
1. PHY based links
2. PHYless fixed links with details specified in DT, in the same way as
the existing "fixed-link" support works, but without needing to create
fake PHYs.
3. PHYless fixed links with GPIO link indication (again, same way as the
existing fixed-link support.)
4. Direct fibre connections via fixed-link or SFP.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-09 13:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21 13:45 [PATCH net-next] net: mvpp2: phylink support Antoine Tenart
2017-09-22 7:56 ` Marcin Wojtas
2017-09-22 11:07 ` Russell King - ARM Linux
2017-09-25 9:55 ` Antoine Tenart
2017-09-25 10:45 ` Russell King - ARM Linux
2017-09-25 11:53 ` Antoine Tenart
2017-09-25 12:13 ` Russell King - ARM Linux
2017-09-25 13:06 ` Antoine Tenart
2017-10-09 12:55 ` Antoine Tenart
2017-10-09 13:09 ` Russell King - ARM Linux
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.