[RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open

STALE3338d

2 messages, 2 authors, 2017-07-28 · open the first message on its own page

[RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open

From: Måns Rullgård <hidden>
Date: 2017-07-28 16:17:42

Marc Gonzalez [off-list ref] writes:
ndo_stop breaks RX in a way that ndo_open is unable to undo.
Please elaborate.  Why can't it be fixed in a less heavy-handed way?
Work around the issue by resetting the HW in ndo_open.
This will provide the basis for suspend/resume support.

Signed-off-by: Marc Gonzalez <redacted>
---
 drivers/net/ethernet/aurora/nb8800.c | 40 +++++++++++++++++-------------------
 drivers/net/ethernet/aurora/nb8800.h |  1 +
 2 files changed, 20 insertions(+), 21 deletions(-)
I'm pretty sure this doesn't preserve everything it should.
quoted hunk
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index e94159507847..954a28542c3b 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);
@@ -1350,6 +1353,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 +1406,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 +1425,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,16 +1466,6 @@ 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;
@@ -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
-- 
Måns Rullgård

[RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open

From: Marc Gonzalez <hidden>
Date: 2017-07-28 16:43:42

On 28/07/2017 18:17, Måns Rullgård wrote:
Marc Gonzalez wrote:
quoted
ndo_stop breaks RX in a way that ndo_open is unable to undo.
Please elaborate.  Why can't it be fixed in a less heavy-handed way?
I'm not sure what "elaborate" means. After we've been through
ndo_stop once, the board can send packets, but it doesn't see
any replies from remote systems. RX is wedged.

I think ndo_stop is rare enough an event that doing a full
reset is not an issue, in terms of performance. Also I will
need this infrastructure anyway for suspend/resume support.

It might also make sense to put the HW in reset at close
(to save power). I will try measuring the power savings,
if any.

quoted
Work around the issue by resetting the HW in ndo_open.
This will provide the basis for suspend/resume support.

Signed-off-by: Marc Gonzalez <redacted>
---
 drivers/net/ethernet/aurora/nb8800.c | 40 +++++++++++++++++-------------------
 drivers/net/ethernet/aurora/nb8800.h |  1 +
 2 files changed, 20 insertions(+), 21 deletions(-)
I'm pretty sure this doesn't preserve everything it should.
Hmmm, we're supposed to start fresh ("full reset").
What could there be to preserve?
You mentioned flow control and multicast elsewhere.
I will take a closer look. Thanks for the heads up.

Regards.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help