diff mbox series

net: eth-uclass: avoid running start() twice without stop()

Message ID 20201104134514.28413-1-matthias.schiffer@ew.tq-group.com
State Accepted
Delegated to: Tom Rini
Headers show
Series net: eth-uclass: avoid running start() twice without stop() | expand

Commit Message

Matthias Schiffer Nov. 4, 2020, 1:45 p.m. UTC
Running the start() handler twice without a stop() inbetween completely
breaks communication for some ethernet drivers like fec_mxc.

eth_halt() is called before each eth_init(). Due to the switch to
eth_is_active() in commit 68acb51f442f ("net: Only call halt on a driver
that has been init'ed"), this is not sufficient anymore when netconsole
is active: eth_init_state_only()/eth_halt_state_only() manipulate the
state check that is performed by eth_is_active() without actually
calling into the driver.

The issue can be triggered by starting a network operation (e.g. ping or
tftp) while netconsole is active.

Add an additional "running" flag that reflects the actual state of the
driver and use it to ensure that eth_halt() actually stops the device as
it is supposed to.

Fixes: 68acb51f442f ("net: Only call halt on a driver that has been init'ed")
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 net/eth-uclass.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Tom Rini Jan. 19, 2021, 8 p.m. UTC | #1
On Wed, Nov 04, 2020 at 02:45:14PM +0100, Matthias Schiffer wrote:

> Running the start() handler twice without a stop() inbetween completely
> breaks communication for some ethernet drivers like fec_mxc.
> 
> eth_halt() is called before each eth_init(). Due to the switch to
> eth_is_active() in commit 68acb51f442f ("net: Only call halt on a driver
> that has been init'ed"), this is not sufficient anymore when netconsole
> is active: eth_init_state_only()/eth_halt_state_only() manipulate the
> state check that is performed by eth_is_active() without actually
> calling into the driver.
> 
> The issue can be triggered by starting a network operation (e.g. ping or
> tftp) while netconsole is active.
> 
> Add an additional "running" flag that reflects the actual state of the
> driver and use it to ensure that eth_halt() actually stops the device as
> it is supposed to.
> 
> Fixes: 68acb51f442f ("net: Only call halt on a driver that has been init'ed")
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index e14695c0f1..7c9278f3a9 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -26,6 +26,7 @@  DECLARE_GLOBAL_DATA_PTR;
  */
 struct eth_device_priv {
 	enum eth_state_t state;
+	bool running;
 };
 
 /**
@@ -293,6 +294,7 @@  int eth_init(void)
 						current->uclass_priv;
 
 					priv->state = ETH_STATE_ACTIVE;
+					priv->running = true;
 					return 0;
 				}
 			} else {
@@ -322,13 +324,16 @@  void eth_halt(void)
 	struct eth_device_priv *priv;
 
 	current = eth_get_dev();
-	if (!current || !eth_is_active(current))
+	if (!current)
 		return;
 
-	eth_get_ops(current)->stop(current);
 	priv = current->uclass_priv;
-	if (priv)
-		priv->state = ETH_STATE_PASSIVE;
+	if (!priv || !priv->running)
+		return;
+
+	eth_get_ops(current)->stop(current);
+	priv->state = ETH_STATE_PASSIVE;
+	priv->running = false;
 }
 
 int eth_is_active(struct udevice *dev)
@@ -537,6 +542,7 @@  static int eth_post_probe(struct udevice *dev)
 #endif
 
 	priv->state = ETH_STATE_INIT;
+	priv->running = false;
 
 	/* Check if the device has a valid MAC address in device tree */
 	if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||