[PATCH] drivers: net: fec_imx: Fix system halt after FEC reinit

[PATCH] drivers: net: fec_imx: Fix system halt after FEC reinit

[Anže Lešnik]

Fix problem with system hang when trying to re-init ethernet interface
on i.MX devices. The problem is that RX and TX buffer descriptor address
registers are only being set in fec_probe(). When fec_halt() is called
when bringing down the interface, this resets the FEC and values are
lost. As per documentation, these registers should be reconfigured
before re-enabling the FEC. Additionally, move fec_init() call from
probe to open, since the FEC needs to re-initialized after every reset.

Signed-off-by: Anže Lešnik <anze.lesnik@norik.com>
Link: https://lore.barebox.org/barebox/c51b2c94-61b7-df44-5d4c-025d18c4b24f@norik.com
---
 drivers/net/fec_imx.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
index 0c2d600d1..673555a48 100644
--- a/drivers/net/fec_imx.c
+++ b/drivers/net/fec_imx.c
@@ -320,6 +320,10 @@ static int fec_init(struct eth_device *dev)
 	/* size of each buffer */
 	writel(FEC_MAX_PKT_SIZE, fec->regs + FEC_EMRBR);
 
+	/* set rx and tx buffer descriptor base address */
+	writel(virt_to_phys(fec->tbd_base), fec->regs + FEC_ETDSR);
+	writel(virt_to_phys(fec->rbd_base), fec->regs + FEC_ERDSR);
+
 	return 0;
 }
 
@@ -359,6 +363,8 @@ static int fec_open(struct eth_device *edev)
 	if (fec->phy_init)
 		fec->phy_init(edev->phydev);
 
+	fec_init(edev);
+
 	/*
 	 * Initialize RxBD/TxBD rings
 	 */
@@ -839,9 +845,6 @@ static int fec_probe(struct device_d *dev)
 	base += FEC_RBD_NUM * sizeof(struct buffer_descriptor);
 	fec->tbd_base = base;
 
-	writel(virt_to_phys(fec->tbd_base), fec->regs + FEC_ETDSR);
-	writel(virt_to_phys(fec->rbd_base), fec->regs + FEC_ERDSR);
-
 	ret = fec_alloc_receive_packets(fec, FEC_RBD_NUM, FEC_MAX_PKT_SIZE);
 	if (ret < 0)
 		goto free_xbd;
@@ -861,8 +864,6 @@ static int fec_probe(struct device_d *dev)
 	if (ret)
 		goto free_receive_packets;
 
-	fec_init(edev);
-
 	fec->miibus.read = fec_miibus_read;
 	fec->miibus.write = fec_miibus_write;
 
-- 
2.37.0

Re: [PATCH] drivers: net: fec_imx: Fix system halt after FEC reinit

[Sascha Hauer]

On Tue, Jul 05, 2022 at 10:55:34AM +0200, Anže Lešnik wrote:
> Fix problem with system hang when trying to re-init ethernet interface
> on i.MX devices. The problem is that RX and TX buffer descriptor address
> registers are only being set in fec_probe(). When fec_halt() is called
> when bringing down the interface, this resets the FEC and values are
> lost. As per documentation, these registers should be reconfigured
> before re-enabling the FEC. Additionally, move fec_init() call from
> probe to open, since the FEC needs to re-initialized after every reset.
> 
> Signed-off-by: Anže Lešnik <anze.lesnik@norik.com>
> Link: https://lore.barebox.org/barebox/c51b2c94-61b7-df44-5d4c-025d18c4b24f@norik.com
> ---
>  drivers/net/fec_imx.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

This patch fixes the hang problem and is otherwise correct, so:

Applied, thanks

It is not the whole story though. The driver will work with a 100M link,
but not with other link speeds. This is because fec_update_linkspeed()
is not called again when fec_open() is called for the second time.

A possible hotfix is to call fec_update_linkspeed() manually from the
driver. Note it has to be called after FEC_ECNTRL is initialized with a
fixed value in fec_open().

Other drivers will have similar problems and this should be fixed at
framework level, so I hesitate to include the hotfix in barebox. Linux
has phy_start() and phy_stop() to be called by the drivers on
open/close. We should probably do the same.

Sascha

-----------------------8<----------------------------

diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c
index 673555a48a..f2ee211992 100644
--- a/drivers/net/fec_imx.c
+++ b/drivers/net/fec_imx.c
@@ -385,6 +385,9 @@ static int fec_open(struct eth_device *edev)
 		ecr |= 0x100;
 
 	writel(ecr, fec->regs + FEC_ECNTRL);
+
+	fec_update_linkspeed(edev);
+
 	/*
 	 * Enable SmartDMA receive task
 	 */

-- 
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 |