| Message ID | 20170724134848.19330-11-antoine.tenart@free-electrons.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello, On Mon, 24 Jul 2017 15:48:40 +0200, Antoine Tenart wrote: > + > + port->link_irq = of_irq_get_byname(port_node, "link"); > + if (port->link_irq == -EPROBE_DEFER) { > + err = -EPROBE_DEFER; > + goto err_free_irq; > + } > + if (port->link_irq <= 0) > + /* the link irq is optional */ > + port->link_irq = 0; You need to add the irq_dispose_mapping() call corresponding to this of_irq_get_by_name() in the error path and in the remove path. Best regards, Thomas
Hi Thomas, On Tue, Jul 25, 2017 at 03:17:48PM +0200, Thomas Petazzoni wrote: > On Mon, 24 Jul 2017 15:48:40 +0200, Antoine Tenart wrote: > > + > > + port->link_irq = of_irq_get_byname(port_node, "link"); > > + if (port->link_irq == -EPROBE_DEFER) { > > + err = -EPROBE_DEFER; > > + goto err_free_irq; > > + } > > + if (port->link_irq <= 0) > > + /* the link irq is optional */ > > + port->link_irq = 0; > > You need to add the irq_dispose_mapping() call corresponding to this > of_irq_get_by_name() in the error path and in the remove path. That's right. I'll fix that in v2. Thanks! Antoine
On Mon, Jul 24, 2017 at 03:48:40PM +0200, Antoine Tenart wrote: > This patch adds the GoP link interrupt support for when a port isn't > connected to a PHY. Because of this the phylib callback is never called > and the link status management isn't done. This patch use the GoP link > interrupt in such cases to still have a minimal link management. Without > this patch ports not connected to a PHY cannot work. Hi Antoine When is a GoP link interrupt signalled? When is a port without a PHY actually up/down? With SFF/SFP ports, you generally need a gpio line the fibre module can use to indicate if it has link. Fixed-phy has such support, and your link_change function will get called when the link changes. And this is another bit of code you probably need to change in a while with phylink lands. Andrew
On Wed, Jul 26, 2017 at 06:26:48PM +0200, Andrew Lunn wrote: > And this is another bit of code you probably need to change in a while > with phylink lands. The way the MAC driver handles link up/down and configuration events changes significantly when a MAC driver switches to phylink, since a directly connected SFP cage needs to have the MAC reconfigured between SGMII and 1000base-X modes. If you add SFP+ into that, also 10Gbase-KR as well. Note also that the "link up" condition for SFP (and probably SFF) is more complex than just "is the module reporting that it's receiving a signal" - especially with 1000base-X, there's negotiation to be performed, so you also need to know (if the module is connected directly to the MAC) whether the Serdes is in sync and has finished negotiation (and itself says it has link with the remote end.) With the Marvell 88x3310 PHY, the MAC driver already needs to switch between 10Gbase-KR and SGMII modes, as the 88x3310 automatically makes that switch on its MAC facing interface without software intervention.
Hi Andrew, Russell, On Wed, Jul 26, 2017 at 06:26:48PM +0200, Andrew Lunn wrote: > On Mon, Jul 24, 2017 at 03:48:40PM +0200, Antoine Tenart wrote: > > This patch adds the GoP link interrupt support for when a port isn't > > connected to a PHY. Because of this the phylib callback is never called > > and the link status management isn't done. This patch use the GoP link > > interrupt in such cases to still have a minimal link management. Without > > this patch ports not connected to a PHY cannot work. > > When is a GoP link interrupt signalled? When is a port without a PHY > actually up/down? When the cable is connected (there is signal) and the serdes is in sync and AN succeeded. > With SFF/SFP ports, you generally need a gpio line the fibre module > can use to indicate if it has link. Fixed-phy has such support, and > your link_change function will get called when the link changes. So that would work when using SFP modules but I wonder if the GoP irq isn't needed when using passive cable, in which case this patch would still be needed (and of course we should support the new Russell phylib capabilities). What's your thoughts on this? Thanks! Antoine
> When the cable is connected (there is signal) and the serdes is in sync and AN > succeeded. > > > With SFF/SFP ports, you generally need a gpio line the fibre module > > can use to indicate if it has link. Fixed-phy has such support, and > > your link_change function will get called when the link changes. > > So that would work when using SFP modules but I wonder if the GoP irq isn't > needed when using passive cable, in which case this patch would still be needed > (and of course we should support the new Russell phylib capabilities). > > What's your thoughts on this? > > Thanks! > Antoine > Even if new phylib driver supports passive direct cables connection, GoP IRQ required for SOHO/Peridot external switch support. SOHO/Peridot external switch could be connected directly to Serdes line. Regards, Stefan
On Wed, Aug 23, 2017 at 03:24:55PM +0000, Stefan Chulski wrote: > > When the cable is connected (there is signal) and the serdes is in sync and AN > > succeeded. > > > > > With SFF/SFP ports, you generally need a gpio line the fibre module > > > can use to indicate if it has link. Fixed-phy has such support, and > > > your link_change function will get called when the link changes. > > > > So that would work when using SFP modules but I wonder if the GoP irq isn't > > needed when using passive cable, in which case this patch would still be needed > > (and of course we should support the new Russell phylib capabilities). > > Even if new phylib driver supports passive direct cables connection, > GoP IRQ required for SOHO/Peridot external switch support. > SOHO/Peridot external switch could be connected directly to Serdes line. So I guess the GoP link irq patches are needed. Should I resend them then? We'll have to discuss how to handle fixed-phy vs GoP IRQ, but I guess we can do this when adding the fixed-phy support later. Thanks, Antoine
Hi Antoine, 2017-08-23 18:04 GMT+02:00 Antoine Tenart <antoine.tenart@free-electrons.com>: > On Wed, Aug 23, 2017 at 03:24:55PM +0000, Stefan Chulski wrote: >> > When the cable is connected (there is signal) and the serdes is in sync and AN >> > succeeded. >> > >> > > With SFF/SFP ports, you generally need a gpio line the fibre module >> > > can use to indicate if it has link. Fixed-phy has such support, and >> > > your link_change function will get called when the link changes. >> > >> > So that would work when using SFP modules but I wonder if the GoP irq isn't >> > needed when using passive cable, in which case this patch would still be needed >> > (and of course we should support the new Russell phylib capabilities). >> >> Even if new phylib driver supports passive direct cables connection, >> GoP IRQ required for SOHO/Peridot external switch support. >> SOHO/Peridot external switch could be connected directly to Serdes line. > > So I guess the GoP link irq patches are needed. Should I resend them > then? > > We'll have to discuss how to handle fixed-phy vs GoP IRQ, but I guess we > can do this when adding the fixed-phy support later. > Please check mvneta.c - you can find there coexistence of normal libphy support, fixed phy and link irq for the inband management mode. IMO it's exactly, what you need here. Best regards, Marcin
Hi Marcin, On Wed, Aug 23, 2017 at 11:05:33PM +0200, Marcin Wojtas wrote: > 2017-08-23 18:04 GMT+02:00 Antoine Tenart <antoine.tenart@free-electrons.com>: > > On Wed, Aug 23, 2017 at 03:24:55PM +0000, Stefan Chulski wrote: > >> > When the cable is connected (there is signal) and the serdes is in sync and AN > >> > succeeded. > >> > > >> > > With SFF/SFP ports, you generally need a gpio line the fibre module > >> > > can use to indicate if it has link. Fixed-phy has such support, and > >> > > your link_change function will get called when the link changes. > >> > > >> > So that would work when using SFP modules but I wonder if the GoP irq isn't > >> > needed when using passive cable, in which case this patch would still be needed > >> > (and of course we should support the new Russell phylib capabilities). > >> > >> Even if new phylib driver supports passive direct cables connection, > >> GoP IRQ required for SOHO/Peridot external switch support. > >> SOHO/Peridot external switch could be connected directly to Serdes line. > > > > So I guess the GoP link irq patches are needed. Should I resend them > > then? > > > > We'll have to discuss how to handle fixed-phy vs GoP IRQ, but I guess we > > can do this when adding the fixed-phy support later. > > > > Please check mvneta.c - you can find there coexistence of normal > libphy support, fixed phy and link irq for the inband management mode. > IMO it's exactly, what you need here. From what I see it should be pretty easy to support phy, fixed-phy and the GoP irq without breaking anything: if no phy is found in the dt, call of_phy_register_fixed_link() and if the fixed-phy property isn't found either use the GoP link IRQ. This way all the possibilities would be supported. So we should be able to support fixed-phy in a future series. Antoine
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c index 77eef2cc40a1..33a7eb834855 100644 --- a/drivers/net/ethernet/marvell/mvpp2.c +++ b/drivers/net/ethernet/marvell/mvpp2.c @@ -339,16 +339,24 @@ #define MVPP2_GMAC_FLOW_CTRL_AUTONEG BIT(11) #define MVPP2_GMAC_CONFIG_FULL_DUPLEX BIT(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_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 #define MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(v) (((v) << 6) & \ MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK) +#define MVPP22_GMAC_INT_STAT 0x20 +#define MVPP22_GMAC_INT_STAT_LINK BIT(1) +#define MVPP22_GMAC_INT_MASK 0x24 +#define MVPP22_GMAC_INT_MASK_LINK_STAT BIT(1) #define MVPP22_GMAC_CTRL_4_REG 0x90 #define MVPP22_CTRL4_EXT_PIN_GMII_SEL BIT(0) #define MVPP22_CTRL4_DP_CLK_SEL BIT(5) #define MVPP22_CTRL4_SYNC_BYPASS_DIS BIT(6) #define MVPP22_CTRL4_QSGMII_BYPASS_ACTIVE BIT(7) +#define MVPP22_GMAC_INT_SUM_MASK 0xa4 +#define MVPP22_GMAC_INT_SUM_MASK_LINK_STAT BIT(1) /* Per-port XGMAC registers. PPv2.2 only, only for GOP port 0, * relative to port->base. @@ -358,12 +366,19 @@ #define MVPP22_XLG_CTRL0_MAC_RESET_DIS BIT(1) #define MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN BIT(7) #define MVPP22_XLG_CTRL0_MIB_CNT_DIS BIT(14) - +#define MVPP22_XLG_STATUS 0x10c +#define MVPP22_XLG_STATUS_LINK_UP BIT(0) +#define MVPP22_XLG_INT_STAT 0x114 +#define MVPP22_XLG_INT_STAT_LINK BIT(1) +#define MVPP22_XLG_INT_MASK 0x118 +#define MVPP22_XLG_INT_MASK_LINK BIT(1) #define MVPP22_XLG_CTRL3_REG 0x11c #define MVPP22_XLG_CTRL3_MACMODESELECT_MASK (7 << 13) #define MVPP22_XLG_CTRL3_MACMODESELECT_GMAC (0 << 13) #define MVPP22_XLG_CTRL3_MACMODESELECT_10G (1 << 13) - +#define MVPP22_XLG_EXT_INT_MASK 0x15c +#define MVPP22_XLG_EXT_INT_MASK_XLG BIT(1) +#define MVPP22_XLG_EXT_INT_MASK_GIG BIT(2) #define MVPP22_XLG_CTRL4_REG 0x184 #define MVPP22_XLG_CTRL4_FWD_FC BIT(5) #define MVPP22_XLG_CTRL4_FWD_PFC BIT(6) @@ -814,6 +829,7 @@ struct mvpp2_port { int gop_id; int irq; + int link_irq; struct mvpp2 *priv; @@ -4330,6 +4346,63 @@ static int mvpp22_gop_init(struct mvpp2_port *port) return -EINVAL; } +static void mvpp22_gop_unmask_irq(struct mvpp2_port *port) +{ + u32 val; + + if (port->phy_interface == PHY_INTERFACE_MODE_RGMII || + port->phy_interface == PHY_INTERFACE_MODE_SGMII) { + /* Enable the GMAC link status irq for this port */ + val = readl(port->base + MVPP22_GMAC_INT_SUM_MASK); + val |= MVPP22_GMAC_INT_SUM_MASK_LINK_STAT; + writel(val, port->base + MVPP22_GMAC_INT_SUM_MASK); + } + + /* Enable the XLG/GIG irqs for this port */ + val = readl(port->base + MVPP22_XLG_EXT_INT_MASK); + if (port->gop_id == 0 && + port->phy_interface == PHY_INTERFACE_MODE_10GKR) + val |= MVPP22_XLG_EXT_INT_MASK_XLG; + else + val |= MVPP22_XLG_EXT_INT_MASK_GIG; + writel(val, port->base + MVPP22_XLG_EXT_INT_MASK); +} + +static void mvpp22_gop_mask_irq(struct mvpp2_port *port) +{ + u32 val; + + val = readl(port->base + MVPP22_XLG_EXT_INT_MASK); + val &= ~(MVPP22_XLG_EXT_INT_MASK_XLG | + MVPP22_XLG_EXT_INT_MASK_GIG); + writel(val, port->base + MVPP22_XLG_EXT_INT_MASK); + + if (port->phy_interface == PHY_INTERFACE_MODE_RGMII || + port->phy_interface == PHY_INTERFACE_MODE_SGMII) { + val = readl(port->base + MVPP22_GMAC_INT_SUM_MASK); + val &= ~MVPP22_GMAC_INT_SUM_MASK_LINK_STAT; + writel(val, port->base + MVPP22_GMAC_INT_SUM_MASK); + } +} + +static void mvpp22_gop_setup_irq(struct mvpp2_port *port) +{ + u32 val; + + if (port->phy_interface == PHY_INTERFACE_MODE_RGMII || + port->phy_interface == PHY_INTERFACE_MODE_SGMII) { + val = readl(port->base + MVPP22_GMAC_INT_MASK); + val |= MVPP22_GMAC_INT_MASK_LINK_STAT; + writel(val, port->base + MVPP22_GMAC_INT_MASK); + } + + val = readl(port->base + MVPP22_XLG_INT_MASK); + val |= MVPP22_XLG_INT_MASK_LINK; + writel(val, port->base + MVPP22_XLG_INT_MASK); + + mvpp22_gop_unmask_irq(port); +} + static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port) { u32 val; @@ -5529,6 +5602,60 @@ static irqreturn_t mvpp2_isr(int irq, void *dev_id) return IRQ_HANDLED; } +/* Per-port interrupt for link status changes */ +static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id) +{ + struct mvpp2_port *port = (struct mvpp2_port *)dev_id; + struct net_device *dev = port->dev; + bool event = false, link = false; + u32 val; + + mvpp22_gop_mask_irq(port); + + if (port->gop_id == 0 && + port->phy_interface == PHY_INTERFACE_MODE_10GKR) { + val = readl(port->base + MVPP22_XLG_INT_STAT); + if (val & MVPP22_XLG_INT_STAT_LINK) { + event = true; + val = readl(port->base + MVPP22_XLG_STATUS); + if (val & MVPP22_XLG_STATUS_LINK_UP) + link = true; + } + } else if (port->phy_interface == PHY_INTERFACE_MODE_RGMII || + port->phy_interface == PHY_INTERFACE_MODE_SGMII) { + val = readl(port->base + MVPP22_GMAC_INT_STAT); + if (val & MVPP22_GMAC_INT_STAT_LINK) { + event = true; + val = readl(port->base + MVPP2_GMAC_STATUS0); + if (val & MVPP2_GMAC_STATUS0_LINK_UP) + link = true; + } + } + + if (!netif_running(dev) || !event) + goto handled; + + if (link) { + mvpp2_interrupts_enable(port); + + mvpp2_egress_enable(port); + mvpp2_ingress_enable(port); + netif_carrier_on(dev); + netif_tx_wake_all_queues(dev); + } else { + netif_tx_stop_all_queues(dev); + netif_carrier_off(dev); + mvpp2_ingress_disable(port); + mvpp2_egress_disable(port); + + mvpp2_interrupts_disable(port); + } + +handled: + mvpp22_gop_unmask_irq(port); + return IRQ_HANDLED; +} + /* Adjust link */ static void mvpp2_link_event(struct net_device *dev) { @@ -6221,6 +6348,7 @@ static void mvpp2_phy_disconnect(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; @@ -6266,12 +6394,24 @@ static int mvpp2_open(struct net_device *dev) goto err_cleanup_txqs; } + if (priv->hw_version == MVPP22 && !port->phy_node && port->link_irq) { + err = request_irq(port->link_irq, mvpp2_link_status_isr, 0, + dev->name, port); + if (err) { + netdev_err(port->dev, "cannot request link IRQ %d\n", + port->link_irq); + goto err_free_irq; + } + + mvpp22_gop_setup_irq(port); + } + /* In default link is down */ netif_carrier_off(port->dev); err = mvpp2_phy_connect(port); if (err < 0) - goto err_free_irq; + goto err_free_link_irq; /* Unmask interrupts on all CPUs */ on_each_cpu(mvpp2_interrupts_unmask, port, 1); @@ -6280,6 +6420,8 @@ static int mvpp2_open(struct net_device *dev) return 0; +err_free_link_irq: + free_irq(port->link_irq, port); err_free_irq: free_irq(port->irq, port); err_cleanup_txqs: @@ -6796,6 +6938,15 @@ static int mvpp2_port_probe(struct platform_device *pdev, -EPROBE_DEFER : -EINVAL; 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_free_irq; + } + if (port->link_irq <= 0) + /* the link irq is optional */ + port->link_irq = 0; } else { /* kept for dt compatibility */ port->irq = irq_of_parse_and_map(port_node, 0);
This patch adds the GoP link interrupt support for when a port isn't connected to a PHY. Because of this the phylib callback is never called and the link status management isn't done. This patch use the GoP link interrupt in such cases to still have a minimal link management. Without this patch ports not connected to a PHY cannot work. Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> --- drivers/net/ethernet/marvell/mvpp2.c | 157 ++++++++++++++++++++++++++++++++++- 1 file changed, 154 insertions(+), 3 deletions(-)