Revision v2 of 9 in this series.
5 messages, 1 author, 2017-08-02 · open the first message on its own page
From: Mason <hidden>
Date: 2017-08-01 16:32:21
Hello, I need suspend/resume support in the nb8800 driver. On tango platforms, suspend loses all context (MMIO registers). To make the task easy, we just close the device on suspend, and open it again on resume. This requires properly resetting the HW on resume. Patch 1 moves all the HW init to nb8800_init() Patch 2 adds suspend/resume support Regards.
From: Mason <hidden>
Date: 2017-08-01 16:37:15
Move all HW initializations to nb8800_init. This provides the basis for suspend/resume support. --- drivers/net/ethernet/aurora/nb8800.c | 50 +++++++++++++++++------------------- drivers/net/ethernet/aurora/nb8800.h | 1 + 2 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index e94159507847..aa18ea25d91f 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c@@ -39,6 +39,7 @@ #include "nb8800.h" +static void nb8800_init(struct net_device *dev); static void nb8800_tx_done(struct net_device *dev); static int nb8800_dma_stop(struct net_device *dev);
@@ -957,6 +958,8 @@ static int nb8800_open(struct net_device *dev) struct phy_device *phydev; int err; + nb8800_init(dev); + /* clear any pending interrupts */ nb8800_writel(priv, NB8800_RXC_SR, 0xf); nb8800_writel(priv, NB8800_TXC_SR, 0xf);
@@ -1246,11 +1249,6 @@ static int nb8800_hw_init(struct net_device *dev) nb8800_writeb(priv, NB8800_PQ1, val >> 8); nb8800_writeb(priv, NB8800_PQ2, val & 0xff); - /* Auto-negotiate by default */ - priv->pause_aneg = true; - priv->pause_rx = true; - priv->pause_tx = true; - nb8800_mc_init(dev, 0); return 0;
@@ -1350,6 +1348,20 @@ static const struct of_device_id nb8800_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, nb8800_dt_ids); +static void nb8800_init(struct net_device *dev) +{ + struct nb8800_priv *priv = netdev_priv(dev); + const struct nb8800_ops *ops = priv->ops; + + if (ops && ops->reset) + ops->reset(dev); + nb8800_hw_init(dev); + if (ops && ops->init) + ops->init(dev); + nb8800_update_mac_addr(dev); + priv->speed = 0; +} + static int nb8800_probe(struct platform_device *pdev) { const struct of_device_id *match;
@@ -1389,6 +1401,7 @@ static int nb8800_probe(struct platform_device *pdev) priv = netdev_priv(dev); priv->base = base; + priv->ops = ops; priv->phy_mode = of_get_phy_mode(pdev->dev.of_node); if (priv->phy_mode < 0)
@@ -1407,12 +1420,6 @@ static int nb8800_probe(struct platform_device *pdev) spin_lock_init(&priv->tx_lock); - if (ops && ops->reset) { - ret = ops->reset(dev); - if (ret) - goto err_disable_clk; - } - bus = devm_mdiobus_alloc(&pdev->dev); if (!bus) { ret = -ENOMEM;
@@ -1454,21 +1461,16 @@ static int nb8800_probe(struct platform_device *pdev) priv->mii_bus = bus; - ret = nb8800_hw_init(dev); - if (ret) - goto err_deregister_fixed_link; - - if (ops && ops->init) { - ret = ops->init(dev); - if (ret) - goto err_deregister_fixed_link; - } - dev->netdev_ops = &nb8800_netdev_ops; dev->ethtool_ops = &nb8800_ethtool_ops; dev->flags |= IFF_MULTICAST; dev->irq = irq; + /* Auto-negotiate by default */ + priv->pause_aneg = true; + priv->pause_rx = true; + priv->pause_tx = true; + mac = of_get_mac_address(pdev->dev.of_node); if (mac) ether_addr_copy(dev->dev_addr, mac);
@@ -1476,14 +1478,12 @@ static int nb8800_probe(struct platform_device *pdev) if (!is_valid_ether_addr(dev->dev_addr)) eth_hw_addr_random(dev); - nb8800_update_mac_addr(dev); - netif_carrier_off(dev); ret = register_netdev(dev); if (ret) { netdev_err(dev, "failed to register netdev\n"); - goto err_free_dma; + goto err_deregister_fixed_link; } netif_napi_add(dev, &priv->napi, nb8800_poll, NAPI_POLL_WEIGHT);
@@ -1492,8 +1492,6 @@ static int nb8800_probe(struct platform_device *pdev) return 0; -err_free_dma: - nb8800_dma_free(dev); err_deregister_fixed_link: if (of_phy_is_fixed_link(pdev->dev.of_node)) of_phy_deregister_fixed_link(pdev->dev.of_node);
diff --git a/drivers/net/ethernet/aurora/nb8800.h b/drivers/net/ethernet/aurora/nb8800.h
index 6ec4a956e1e5..d5f4481a2c7b 100644
--- a/drivers/net/ethernet/aurora/nb8800.h
+++ b/drivers/net/ethernet/aurora/nb8800.h@@ -305,6 +305,7 @@ struct nb8800_priv { dma_addr_t tx_desc_dma; struct clk *clk; + const struct nb8800_ops *ops; }; struct nb8800_ops {
--
2.11.0
From: Mason <hidden>
Date: 2017-08-01 16:43:56
Wrappers around nb8800_stop and nb8800_open. --- drivers/net/ethernet/aurora/nb8800.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index aa18ea25d91f..607064a6d7a1 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c@@ -1012,7 +1012,6 @@ static int nb8800_stop(struct net_device *dev) netif_stop_queue(dev); napi_disable(&priv->napi); - nb8800_dma_stop(dev); nb8800_mac_rx(dev, false); nb8800_mac_tx(dev, false);
@@ -1526,6 +1525,26 @@ static int nb8800_remove(struct platform_device *pdev) return 0; } +static int nb8800_suspend(struct platform_device *pdev, pm_message_t state) +{ + struct net_device *dev = platform_get_drvdata(pdev); + + if (netif_running(dev)) + nb8800_stop(dev); + + return 0; +} + +static int nb8800_resume(struct platform_device *pdev) +{ + struct net_device *dev = platform_get_drvdata(pdev); + + if (netif_running(dev)) + nb8800_open(dev); + + return 0; +} + static struct platform_driver nb8800_driver = { .driver = { .name = "nb8800",
@@ -1533,6 +1552,8 @@ static struct platform_driver nb8800_driver = { }, .probe = nb8800_probe, .remove = nb8800_remove, + .suspend = nb8800_suspend, + .resume = nb8800_resume, }; module_platform_driver(nb8800_driver);
--
2.11.0
From: Mason <hidden>
Date: 2017-08-02 14:41:07
On 01/08/2017 18:32, Mason wrote:
I need suspend/resume support in the nb8800 driver. On tango platforms, suspend loses all context (MMIO registers). To make the task easy, we just close the device on suspend, and open it again on resume. This requires properly resetting the HW on resume. Patch 1 moves all the HW init to nb8800_init() Patch 2 adds suspend/resume support
I have now confirmed that the "flow control" issue I reported in another thread has nothing to do with flow control per se. The problem is that nb8800_pause_config() calls nb8800_dma_stop() and when it does, RX is borked. On a GigE switch: [ 21.444268] ENTER nb8800_pause_config [ 21.448604] rxcr=06100a8f pause_rx=1 pause_tx=0 pause=1 asym_pause=1 [ 21.455020] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx In this case, pause_tx and RCR_FL match, so we skip the silly dance. On a FastE switch: [ 11.611613] ENTER nb8800_pause_config [ 11.615942] rxcr=06100a8f pause_rx=1 pause_tx=1 pause=1 asym_pause=0 [ 11.825535] nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx In this case pause_tx and RCR_FL don't match, so we hit nb8800_dma_stop() and the HW block becomes deaf. In conclusion, two issues I've been discussing: A) RX wedged after ndo_close B) flow control breaks on FastE switches are in fact two symptoms of the same root issue. Regards.
From: Mason <hidden>
Date: 2017-08-02 15:26:21
On 02/08/2017 16:41, Mason wrote:
On 01/08/2017 18:32, Mason wrote:quoted
I need suspend/resume support in the nb8800 driver. On tango platforms, suspend loses all context (MMIO registers). To make the task easy, we just close the device on suspend, and open it again on resume. This requires properly resetting the HW on resume. Patch 1 moves all the HW init to nb8800_init() Patch 2 adds suspend/resume supportI have now confirmed that the "flow control" issue I reported in another thread has nothing to do with flow control per se. The problem is that nb8800_pause_config() calls nb8800_dma_stop() and when it does, RX is borked. On a GigE switch: [ 21.444268] ENTER nb8800_pause_config [ 21.448604] rxcr=06100a8f pause_rx=1 pause_tx=0 pause=1 asym_pause=1 [ 21.455020] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx In this case, pause_tx and RCR_FL match, so we skip the silly dance.
The documentation states: Receive Channel Control Register Description: The Receive Channel Control Register holds channel enable, mode, endian, DMA control, and interrupt control information. This register can only be written when the Receive DMA Channel is idle - the Enable bit in it is "0". Register Number: 0x200 Access: read/write Reset Value: le = AMBA_LE; all other bits = 0x0 Fields: fl: Flow control enable. "1" indicates flow control is enabled. When flow control is enabled and a Receive FIFO overrun occurs, the Ethernet 10/100/1000 Subsystem will send PAUSE frames if in full duplex mode. This continues until the Receive FIFO is emptied. en: Receive DMA Channel enable. "1" indicates that the Receive DMA Channel is being configured from a descriptor, or that the DMA operation is in progress. The Receive DMA Channel is idle when this enable bit is "0". Software sets this bit to "1" to start the configuration from a descriptor and DMA operation. When the DMA operation is finished, this bit is automatically reset to "0".
Receive DMA Channel Disabling When the entire receive frame has been read from the Receive FIFO and sent over the AMBA bus, the DMA operation ends, and the Receive DMA Channel is automatically disabled. To do this, hardware resets the Enable bit in the Receive Channel Control Register to "0" after the last data has been read from the Receive FIFO and sent over the AMBA bus. When operating in descriptor mode, upon completion of a receive frame DMA operation, if the descriptor chain has not ended when a receive frame DMA operation completes, the next receive frame DMA operation begins. The last descriptor in a descriptor chain is indicated by having its End Of Chain- EOC, flag set to "1". If this EOC flag is "0", to begin the next receive frame DMA operation, the next descriptor is automatically retrieved and used to configure the Receive DMA Channel. The Receive DMA Channel is then automatically re-enabled and the next receive frame DMA operation begins. In descriptor mode, an AMBA bus error can occur when reading receive descriptor data. If this happens, receive descriptor processing ends and the Receive DMA Channel is turned off. The Descriptor Error bit in the Receive Status Register is set to "1".
Hmmm, I guess this is what Maxime/Mans did... Looking at the tango-specific integration, I note this nugget: 1.5.4 Stopping & Starting the DMA This feature has been added to allow the software to stop and start the DMA without any issues. Procedure: 1- STOP: 2- Stop RX core; 3- Set OWN bit of all descriptor of the chain to 1; 4- Stop DMA by writing dma_stop bit to 1 in RX_DMA_Stop register 5- Wait around 100 clock cycles. The pending packets are held until the system will re-start. RE-START: 1- Clear dma_stop bit (note that if at the time of stopping the DMA, the next packet in the FIFO was an UDP packet, when clearing dma_stop, this packet will directly start being written in the DRAM since UDP packets are not controlled by the descriptor mechanism); 2- Program a new chain of descriptor; 3- Re-enable DMA (rx_ctrl register) rx_dma_stop: Software control to stop the Rx DMA. A write to this bit with ?1? will gracefully stop the Rx DMA by after transferring the current packet. If more packets are pending they will be held until the software clears this bit. Hmmm, what do you think? This looks promising... Regards.