[PATCH 0/1] net: designware: eqos: stop DMA on halt

[PATCH 0/1] net: designware: eqos: stop DMA on halt

[Ahmad Fatoum]

Hello Sascha,

Would be great if you could apply this to master.

Thanks,
Ahmad

Ahmad Fatoum (1):
  net: designware: eqos: stop DMA on halt

 drivers/net/designware_stm32.c    | 2 ++
 drivers/net/designware_tegra186.c | 2 ++
 2 files changed, 4 insertions(+)

-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/1] net: designware: eqos: stop DMA on halt

[Ahmad Fatoum]

designware_eqos.c contains an eqos_stop implementation to stop the NIC
when halting the interface. Unfortunately it wasn't used leading to
memory corruption on boot, possibly due to DMA. Fix this.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/net/designware_stm32.c    | 2 ++
 drivers/net/designware_tegra186.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/designware_stm32.c b/drivers/net/designware_stm32.c
index 5b087ad5a371..ed54ff2b2615 100644
--- a/drivers/net/designware_stm32.c
+++ b/drivers/net/designware_stm32.c
@@ -199,6 +199,8 @@ static void eqos_stop_stm32(struct eth_device *edev)
 {
 	struct eqos_stm32 *priv = to_stm32(edev->priv);
 
+	eqos_stop(edev);
+
 	clk_bulk_disable(priv->num_clks, priv->clks);
 }
 
diff --git a/drivers/net/designware_tegra186.c b/drivers/net/designware_tegra186.c
index 58484d4095dc..618ae113971d 100644
--- a/drivers/net/designware_tegra186.c
+++ b/drivers/net/designware_tegra186.c
@@ -282,6 +282,8 @@ static void eqos_stop_tegra186(struct eth_device *edev)
 {
 	struct eqos_tegra186 *priv = to_tegra186(edev->priv);
 
+	eqos_stop(edev);
+
 	eqos_reset_tegra186(priv, true);
 
 	clk_bulk_disable(priv->num_clks, priv->clks);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/1] net: designware: eqos: stop DMA on halt

[Sascha Hauer]

On Mon, Nov 11, 2019 at 10:56:07AM +0100, Ahmad Fatoum wrote:
> designware_eqos.c contains an eqos_stop implementation to stop the NIC
> when halting the interface. Unfortunately it wasn't used leading to
> memory corruption on boot, possibly due to DMA. Fix this.
> 
> Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
> ---

Applied to master, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 0/3] net: designware: eqos: stop DMA on halt

[Ahmad Fatoum]

I recently learnt that barebox Ethernet drivers need to call ->halt 
themselves, either directly or indirectly.

The first patch does this. Sascha, do you do fixups into master? If so,
you could fix the first commit up into my previous patch.

After applying the patch and doing a round of boot bnet, the first MDIO
access during probe times out:

    ERROR: <NULL>: MDIO not idle at entry

This is fixed by the other two commits.

The driver isn't in a release yet, so I hope they could join the driver
in master.

Cheers
Ahmad

Ahmad Fatoum (3):
  fixup! net: designware: eqos: stop DMA on halt
  net: designware: eqos: enable clocks before mdio_register
  net: designware: eqos: fix NULL pointer use in dev_printf

 drivers/net/designware_eqos.c     | 18 ++++++------
 drivers/net/designware_eqos.h     |  4 ---
 drivers/net/designware_stm32.c    | 33 +---------------------
 drivers/net/designware_tegra186.c | 47 ++++++-------------------------
 4 files changed, 20 insertions(+), 82 deletions(-)

-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] net: designware: eqos: properly stop DMA on halt

[Ahmad Fatoum]

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

Specifying ->halt only means that it's called along with eth_unregister.
If we want to halt the DMA, we will have to call it ourselves in the
remove callback. Do this.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/net/designware_eqos.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/designware_eqos.c b/drivers/net/designware_eqos.c
index a49239e0573e..52a5ec272e7f 100644
--- a/drivers/net/designware_eqos.c
+++ b/drivers/net/designware_eqos.c
@@ -869,6 +869,8 @@ void eqos_remove(struct device_d *dev)
 {
 	struct eqos *eqos = dev->priv;
 
+	eth_unregister(&eqos->netdev);
+
 	mdiobus_unregister(&eqos->miibus);
 
 	dma_free(phys_to_virt(eqos->rx_descs[0].des0));
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] net: designware: eqos: enable clocks before mdio_register

[Ahmad Fatoum]

We can't be using the MAC including the MDIO controller while the clocks
are off, but this is exactly the case when mdio_register is called and
the interface is not yet up. To allow reading the PHY id to succeed
before the interface is up, turn on the clocks as part of the
initialization in the probe.

This fixes following error at probe time:

    ERROR: <NULL>: MDIO not idle at entry

The NULL is fixed in a follow-up commit.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/net/designware_eqos.c     |  8 +++---
 drivers/net/designware_eqos.h     |  4 ---
 drivers/net/designware_stm32.c    | 33 +---------------------
 drivers/net/designware_tegra186.c | 47 ++++++-------------------------
 4 files changed, 14 insertions(+), 78 deletions(-)

diff --git a/drivers/net/designware_eqos.c b/drivers/net/designware_eqos.c
index 52a5ec272e7f..84dcd04a3fc9 100644
--- a/drivers/net/designware_eqos.c
+++ b/drivers/net/designware_eqos.c
@@ -365,7 +365,7 @@ static int phy_resume(struct phy_device *phydev)
 	return 0;
 }
 
-int eqos_start(struct eth_device *edev)
+static int eqos_start(struct eth_device *edev)
 {
 	struct eqos *eqos = edev->priv;
 	u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl;
@@ -611,7 +611,7 @@ int eqos_start(struct eth_device *edev)
 	return 0;
 }
 
-void eqos_stop(struct eth_device *edev)
+static void eqos_stop(struct eth_device *edev)
 {
 	struct eqos *eqos = edev->priv;
 	int i;
@@ -841,10 +841,10 @@ int eqos_probe(struct device_d *dev, const struct eqos_ops *ops, void *priv)
 	dev->priv = edev->priv = eqos;
 
 	edev->parent = dev;
-	edev->open = ops->start;
+	edev->open = eqos_start;
 	edev->send = eqos_send;
 	edev->recv = eqos_recv;
-	edev->halt = ops->stop;
+	edev->halt = eqos_stop;
 	edev->get_ethaddr = ops->get_ethaddr;
 	edev->set_ethaddr = ops->set_ethaddr;
 
diff --git a/drivers/net/designware_eqos.h b/drivers/net/designware_eqos.h
index 969a524c0a95..f794195db4a1 100644
--- a/drivers/net/designware_eqos.h
+++ b/drivers/net/designware_eqos.h
@@ -11,8 +11,6 @@ struct eth_device;
 
 struct eqos_ops {
 	int (*init)(struct device_d *dev, struct eqos *priv);
-	int (*start)(struct eth_device *edev);
-	void (*stop)(struct eth_device *edev);
 	int (*get_ethaddr)(struct eth_device *dev, unsigned char *mac);
 	int (*set_ethaddr)(struct eth_device *edev, const unsigned char *mac);
 	void (*adjust_link)(struct eth_device *edev);
@@ -73,8 +71,6 @@ int eqos_reset(struct eqos *priv);
 
 int eqos_get_ethaddr(struct eth_device *edev, unsigned char *mac);
 int eqos_set_ethaddr(struct eth_device *edev, const unsigned char *mac);
-int eqos_start(struct eth_device *edev);
-void eqos_stop(struct eth_device *edev);
 void eqos_adjust_link(struct eth_device *edev);
 
 #define eqos_dbg(eqos, ...) dev_dbg(&eqos->netdev.dev, __VA_ARGS__)
diff --git a/drivers/net/designware_stm32.c b/drivers/net/designware_stm32.c
index bb6781fb3450..4c682a5daca3 100644
--- a/drivers/net/designware_stm32.c
+++ b/drivers/net/designware_stm32.c
@@ -164,16 +164,6 @@ static int eqos_init_stm32(struct device_d *dev, struct eqos *eqos)
 		dev_dbg(dev, "No phy clock provided. Continuing without.\n");
 	}
 
-	return 0;
-
-}
-
-static int eqos_start_stm32(struct eth_device *edev)
-{
-	struct eqos *eqos = edev->priv;
-	struct eqos_stm32 *priv = to_stm32(eqos);
-	int ret;
-
 	ret = clk_bulk_enable(priv->num_clks, priv->clks);
 	if (ret < 0) {
 		eqos_err(eqos, "clk_bulk_enable() failed: %s\n",
@@ -181,35 +171,13 @@ static int eqos_start_stm32(struct eth_device *edev)
 		return ret;
 	}
 
-	udelay(10);
-
-	ret = eqos_start(edev);
-	if (ret)
-		goto err_stop_clks;
-
 	return 0;
-
-err_stop_clks:
-	clk_bulk_disable(priv->num_clks, priv->clks);
-
-	return ret;
-}
-
-static void eqos_stop_stm32(struct eth_device *edev)
-{
-	struct eqos_stm32 *priv = to_stm32(edev->priv);
-
-	eqos_stop(edev);
-
-	clk_bulk_disable(priv->num_clks, priv->clks);
 }
 
 static struct eqos_ops stm32_ops = {
 	.init = eqos_init_stm32,
 	.get_ethaddr = eqos_get_ethaddr,
 	.set_ethaddr = eqos_set_ethaddr,
-	.start = eqos_start_stm32,
-	.stop = eqos_stop_stm32,
 	.adjust_link = eqos_adjust_link,
 	.get_csr_clk_rate = eqos_get_csr_clk_rate_stm32,
 
@@ -229,6 +197,7 @@ static void eqos_remove_stm32(struct device_d *dev)
 
 	eqos_remove(dev);
 
+	clk_bulk_disable(priv->num_clks, priv->clks);
 	clk_bulk_put(priv->num_clks, priv->clks);
 }
 
diff --git a/drivers/net/designware_tegra186.c b/drivers/net/designware_tegra186.c
index 618ae113971d..20521db1c711 100644
--- a/drivers/net/designware_tegra186.c
+++ b/drivers/net/designware_tegra186.c
@@ -230,29 +230,16 @@ static int eqos_init_tegra186(struct device_d *dev, struct eqos *eqos)
 	priv->clks = xmemdup(tegra186_clks, sizeof(tegra186_clks));
 	priv->num_clks = ARRAY_SIZE(tegra186_clks);
 
-	return 0;
-
-release_res:
-	reset_control_put(priv->rst);
-	return ret;
-}
-
-static int eqos_start_tegra186(struct eth_device *edev)
-{
-	struct eqos *eqos = edev->priv;
-	struct eqos_tegra186 *priv = to_tegra186(eqos);
-	int ret;
-
 	ret = clk_bulk_enable(priv->num_clks, priv->clks);
 	if (ret < 0) {
 		eqos_err(eqos, "clk_bulk_enable() failed: %s\n", strerror(-ret));
-		return ret;
+		goto release_res;
 	}
 
 	ret = eqos_clks_set_rate_tegra186(priv);
 	if (ret < 0) {
 		eqos_err(eqos, "clks_set_rate() failed: %s\n", strerror(-ret));
-		goto err;
+		goto err_stop_clks;
 	}
 
 	eqos_reset_tegra186(priv, false);
@@ -261,32 +248,14 @@ static int eqos_start_tegra186(struct eth_device *edev)
 		goto err_stop_clks;
 	}
 
-	udelay(10);
-
-	ret = eqos_start(edev);
-	if (ret)
-		goto err_stop_resets;
-
 	return 0;
 
-err_stop_resets:
-	eqos_reset_tegra186(priv, true);
 err_stop_clks:
 	clk_bulk_disable(priv->num_clks, priv->clks);
-err:
-	return ret;
-}
-
-
-static void eqos_stop_tegra186(struct eth_device *edev)
-{
-	struct eqos_tegra186 *priv = to_tegra186(edev->priv);
-
-	eqos_stop(edev);
-
-	eqos_reset_tegra186(priv, true);
+release_res:
+	reset_control_put(priv->rst);
 
-	clk_bulk_disable(priv->num_clks, priv->clks);
+	return ret;
 }
 
 static void eqos_adjust_link_tegra186(struct eth_device *edev)
@@ -308,8 +277,6 @@ static const struct eqos_ops tegra186_ops = {
 	.init = eqos_init_tegra186,
 	.get_ethaddr = eqos_get_ethaddr,
 	.set_ethaddr = eqos_set_ethaddr_tegra186,
-	.start = eqos_start_tegra186,
-	.stop = eqos_stop_tegra186,
 	.adjust_link = eqos_adjust_link_tegra186,
 	.get_csr_clk_rate = eqos_get_csr_clk_rate_tegra186,
 
@@ -329,6 +296,10 @@ static void eqos_remove_tegra186(struct device_d *dev)
 
 	eqos_remove(dev);
 
+	eqos_reset_tegra186(priv, true);
+
+	clk_bulk_disable(priv->num_clks, priv->clks);
+
 	clk_bulk_put(priv->num_clks, priv->clks);
 
 	gpio_free(priv->phy_reset_gpio);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] net: designware: eqos: fix NULL pointer use in dev_printf

[Ahmad Fatoum]

Inside mdio_register, a read of the PHY's id register is attempted.
If it fails, we print an error message with eqos_err, which uses the
ethernet device's unique name, but at this time there has been none set,
because eth_register was not yet called. Fix this by using the MDIO bus
device instead.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/net/designware_eqos.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/designware_eqos.c b/drivers/net/designware_eqos.c
index 84dcd04a3fc9..da67adf9a0f0 100644
--- a/drivers/net/designware_eqos.c
+++ b/drivers/net/designware_eqos.c
@@ -204,7 +204,7 @@ static int eqos_mdio_read(struct mii_bus *bus, int addr, int reg)
 
 	ret = eqos_mdio_wait_idle(eqos);
 	if (ret) {
-		eqos_err(eqos, "MDIO not idle at entry\n");
+		dev_err(&bus->dev, "MDIO not idle at entry\n");
 		return ret;
 	}
 
@@ -222,7 +222,7 @@ static int eqos_mdio_read(struct mii_bus *bus, int addr, int reg)
 
 	ret = eqos_mdio_wait_idle(eqos);
 	if (ret) {
-		eqos_err(eqos, "MDIO read didn't complete\n");
+		dev_err(&bus->dev, "MDIO read didn't complete\n");
 		return ret;
 	}
 
@@ -237,7 +237,7 @@ static int eqos_mdio_write(struct mii_bus *bus, int addr, int reg, u16 val)
 
 	ret = eqos_mdio_wait_idle(eqos);
 	if (ret) {
-		eqos_err(eqos, "MDIO not idle at entry\n");
+		dev_err(&bus->dev, "MDIO not idle at entry\n");
 		return ret;
 	}
 
@@ -256,7 +256,7 @@ static int eqos_mdio_write(struct mii_bus *bus, int addr, int reg, u16 val)
 
 	ret = eqos_mdio_wait_idle(eqos);
 	if (ret) {
-		eqos_err(eqos, "MDIO read didn't complete\n");
+		dev_err(&bus->dev, "MDIO read didn't complete\n");
 		return ret;
 	}
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/3] net: designware: eqos: stop DMA on halt

[Sascha Hauer]

On Fri, Nov 15, 2019 at 08:32:36AM +0100, Ahmad Fatoum wrote:
> I recently learnt that barebox Ethernet drivers need to call ->halt 
> themselves, either directly or indirectly.
> 
> The first patch does this. Sascha, do you do fixups into master? If so,
> you could fix the first commit up into my previous patch.
> 
> After applying the patch and doing a round of boot bnet, the first MDIO
> access during probe times out:
> 
>     ERROR: <NULL>: MDIO not idle at entry
> 
> This is fixed by the other two commits.
> 
> The driver isn't in a release yet, so I hope they could join the driver
> in master.

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox