From 8486830549189b109bfd40e8d38c6dd8b8f476db Mon Sep 17 00:00:00 2001 From: Claudiu Manoil Date: Fri, 31 Jul 2015 18:38:31 +0300 Subject: [PATCH 1/3] gianfar: Fix warning when CONFIG_PM off CC drivers/net/ethernet/freescale/gianfar.o drivers/net/ethernet/freescale/gianfar.c:568:13: warning: 'lock_tx_qs' defined but not used [-Wunused-function] static void lock_tx_qs(struct gfar_private *priv) ^ drivers/net/ethernet/freescale/gianfar.c:576:13: warning: 'unlock_tx_qs' defined but not used [-Wunused-function] static void unlock_tx_qs(struct gfar_private *priv) ^ Reported-by: Scott Wood Signed-off-by: Claudiu Manoil Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/gianfar.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index ff875028fdff5..caad327e3f224 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -565,6 +565,7 @@ static void gfar_ints_enable(struct gfar_private *priv) } } +#ifdef CONFIG_PM static void lock_tx_qs(struct gfar_private *priv) { int i; @@ -580,6 +581,7 @@ static void unlock_tx_qs(struct gfar_private *priv) for (i = 0; i < priv->num_tx_queues; i++) spin_unlock(&priv->tx_queue[i]->txlock); } +#endif static int gfar_alloc_tx_queues(struct gfar_private *priv) { From 614b42426cc3483e8d5bc68a158c2dd47dc831d0 Mon Sep 17 00:00:00 2001 From: Claudiu Manoil Date: Fri, 31 Jul 2015 18:38:32 +0300 Subject: [PATCH 2/3] gianfar: Fix suspend/resume for wol magic packet If we disable NAPI in the first place we can mask the device's interrupts (and halt it) without fearing that imask may be concurrently accessed from interrupt context, so there's no need to do local_irq_save() around gfar_halt_nodisable(). lock_rx_qs()/unlock_tx_qs() are just obsoleted and potentially buggy routines. The txlock is currently used in the driver only to manage TX congestion, it has nothing to do with halting the device. With these changes, the TX processing is stopped before gfar_halt(). Compact gfar_halt() is used instead of gfar_halt_nodisable(), as it disables Rx/TX DMA h/w blocks and the Rx/TX h/w queues. gfar_start() re-enables all these blocks on resume. Enabling the magic-packet mode remains the same, note that the RX block is re-enabled just before entering sleep mode. Add IRQF_NO_SUSPEND flag for the error interrupt line, to signal that the interrupt line must remain active during sleep in order to wake the system by magic packet (MAG) reception interrupt. (On some systems the MAG interrupt did trigger w/o this flag as well, but on others it didn't.) Without these fixes, when suspended during fair Tx traffic the interface occasionally failed to be woken up by magic packet. Signed-off-by: Claudiu Manoil Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/gianfar.c | 98 ++++++++---------------- 1 file changed, 30 insertions(+), 68 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index caad327e3f224..084c25d1467f2 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -565,24 +565,6 @@ static void gfar_ints_enable(struct gfar_private *priv) } } -#ifdef CONFIG_PM -static void lock_tx_qs(struct gfar_private *priv) -{ - int i; - - for (i = 0; i < priv->num_tx_queues; i++) - spin_lock(&priv->tx_queue[i]->txlock); -} - -static void unlock_tx_qs(struct gfar_private *priv) -{ - int i; - - for (i = 0; i < priv->num_tx_queues; i++) - spin_unlock(&priv->tx_queue[i]->txlock); -} -#endif - static int gfar_alloc_tx_queues(struct gfar_private *priv) { int i; @@ -1542,48 +1524,37 @@ static int gfar_suspend(struct device *dev) struct gfar_private *priv = dev_get_drvdata(dev); struct net_device *ndev = priv->ndev; struct gfar __iomem *regs = priv->gfargrp[0].regs; - unsigned long flags; u32 tempval; - int magic_packet = priv->wol_en && (priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET); + if (!netif_running(ndev)) + return 0; + + disable_napi(priv); + netif_tx_lock(ndev); netif_device_detach(ndev); + netif_tx_unlock(ndev); - if (netif_running(ndev)) { + gfar_halt(priv); - local_irq_save(flags); - lock_tx_qs(priv); + if (magic_packet) { + /* Enable interrupt on Magic Packet */ + gfar_write(®s->imask, IMASK_MAG); - gfar_halt_nodisable(priv); + /* Enable Magic Packet mode */ + tempval = gfar_read(®s->maccfg2); + tempval |= MACCFG2_MPEN; + gfar_write(®s->maccfg2, tempval); - /* Disable Tx, and Rx if wake-on-LAN is disabled. */ + /* re-enable the Rx block */ tempval = gfar_read(®s->maccfg1); - - tempval &= ~MACCFG1_TX_EN; - - if (!magic_packet) - tempval &= ~MACCFG1_RX_EN; - + tempval |= MACCFG1_RX_EN; gfar_write(®s->maccfg1, tempval); - unlock_tx_qs(priv); - local_irq_restore(flags); - - disable_napi(priv); - - if (magic_packet) { - /* Enable interrupt on Magic Packet */ - gfar_write(®s->imask, IMASK_MAG); - - /* Enable Magic Packet mode */ - tempval = gfar_read(®s->maccfg2); - tempval |= MACCFG2_MPEN; - gfar_write(®s->maccfg2, tempval); - } else { - phy_stop(priv->phydev); - } + } else { + phy_stop(priv->phydev); } return 0; @@ -1594,37 +1565,26 @@ static int gfar_resume(struct device *dev) struct gfar_private *priv = dev_get_drvdata(dev); struct net_device *ndev = priv->ndev; struct gfar __iomem *regs = priv->gfargrp[0].regs; - unsigned long flags; u32 tempval; int magic_packet = priv->wol_en && (priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET); - if (!netif_running(ndev)) { - netif_device_attach(ndev); + if (!netif_running(ndev)) return 0; - } - if (!magic_packet && priv->phydev) + if (magic_packet) { + /* Disable Magic Packet mode */ + tempval = gfar_read(®s->maccfg2); + tempval &= ~MACCFG2_MPEN; + gfar_write(®s->maccfg2, tempval); + } else { phy_start(priv->phydev); - - /* Disable Magic Packet mode, in case something - * else woke us up. - */ - local_irq_save(flags); - lock_tx_qs(priv); - - tempval = gfar_read(®s->maccfg2); - tempval &= ~MACCFG2_MPEN; - gfar_write(®s->maccfg2, tempval); + } gfar_start(priv); - unlock_tx_qs(priv); - local_irq_restore(flags); - netif_device_attach(ndev); - enable_napi(priv); return 0; @@ -2047,7 +2007,8 @@ static int register_grp_irqs(struct gfar_priv_grp *grp) /* Install our interrupt handlers for Error, * Transmit, and Receive */ - err = request_irq(gfar_irq(grp, ER)->irq, gfar_error, 0, + err = request_irq(gfar_irq(grp, ER)->irq, gfar_error, + IRQF_NO_SUSPEND, gfar_irq(grp, ER)->name, grp); if (err < 0) { netif_err(priv, intr, dev, "Can't get IRQ %d\n", @@ -2070,7 +2031,8 @@ static int register_grp_irqs(struct gfar_priv_grp *grp) goto rx_irq_fail; } } else { - err = request_irq(gfar_irq(grp, TX)->irq, gfar_interrupt, 0, + err = request_irq(gfar_irq(grp, TX)->irq, gfar_interrupt, + IRQF_NO_SUSPEND, gfar_irq(grp, TX)->name, grp); if (err < 0) { netif_err(priv, intr, dev, "Can't get IRQ %d\n", From b0734b6dc895258b74c6e7a441cb47b6b0ba3465 Mon Sep 17 00:00:00 2001 From: Claudiu Manoil Date: Fri, 31 Jul 2015 18:38:33 +0300 Subject: [PATCH 3/3] gianfar: Enable device wakeup when appropriate The wol_en flag is 0 by default anyway, and we have the following inconsistency: a MAGIC packet wol capable eth interface is registered as a wake-up source but unable to wake-up the system as wol_en is 0 (wake-on flag set to 'd'). Calling set_wakeup_enable() at netdev open is just redundant because wol_en is 0 by default. Let only ethtool call set_wakeup_enable() for now. The bflock is obviously obsoleted, its utility has been corroded over time. The bitfield flags used today in gianfar are accessed only on the init/ config path, with no real possibility of concurrency - nothing that would justify smth. like bflock. Signed-off-by: Claudiu Manoil Signed-off-by: David S. Miller --- drivers/net/ethernet/freescale/gianfar.c | 8 ++------ drivers/net/ethernet/freescale/gianfar.h | 3 --- drivers/net/ethernet/freescale/gianfar_ethtool.c | 5 +---- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index 084c25d1467f2..2b7610f341b09 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -1360,7 +1360,6 @@ static int gfar_probe(struct platform_device *ofdev) priv->dev = &ofdev->dev; SET_NETDEV_DEV(dev, &ofdev->dev); - spin_lock_init(&priv->bflock); INIT_WORK(&priv->reset_task, gfar_reset_task); platform_set_drvdata(ofdev, priv); @@ -1454,9 +1453,8 @@ static int gfar_probe(struct platform_device *ofdev) goto register_fail; } - device_init_wakeup(&dev->dev, - priv->device_flags & - FSL_GIANFAR_DEV_HAS_MAGIC_PACKET); + device_set_wakeup_capable(&dev->dev, priv->device_flags & + FSL_GIANFAR_DEV_HAS_MAGIC_PACKET); /* fill out IRQ number and name fields */ for (i = 0; i < priv->num_grps; i++) { @@ -2133,8 +2131,6 @@ static int gfar_enet_open(struct net_device *dev) if (err) return err; - device_set_wakeup_enable(&dev->dev, priv->wol_en); - return err; } diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h index daa1d37de6427..5545e41033686 100644 --- a/drivers/net/ethernet/freescale/gianfar.h +++ b/drivers/net/ethernet/freescale/gianfar.h @@ -1145,9 +1145,6 @@ struct gfar_private { int oldduplex; int oldlink; - /* Bitfield update lock */ - spinlock_t bflock; - uint32_t msg_enable; struct work_struct reset_task; diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c index fda12fb32ec77..3c0a8f825b630 100644 --- a/drivers/net/ethernet/freescale/gianfar_ethtool.c +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c @@ -653,7 +653,6 @@ static void gfar_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol) static int gfar_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) { struct gfar_private *priv = netdev_priv(dev); - unsigned long flags; if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET) && wol->wolopts != 0) @@ -664,9 +663,7 @@ static int gfar_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) device_set_wakeup_enable(&dev->dev, wol->wolopts & WAKE_MAGIC); - spin_lock_irqsave(&priv->bflock, flags); - priv->wol_en = !!device_may_wakeup(&dev->dev); - spin_unlock_irqrestore(&priv->bflock, flags); + priv->wol_en = !!device_may_wakeup(&dev->dev); return 0; }