[U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com --- net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df192..26f7e0b8cd 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -531,7 +531,8 @@ static int eth_pre_remove(struct udevice *dev) { struct eth_pdata *pdata = dev->platdata;
- eth_get_ops(dev)->stop(dev); + if (eth_is_active(dev)) + eth_get_ops(dev)->stop(dev);
/* clear the MAC address */ memset(pdata->enetaddr, 0, ARP_HLEN);
On Wed, 20 Feb 2019 at 05:32, Keerthy j-keerthy@ti.com wrote:
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df192..26f7e0b8cd 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -531,7 +531,8 @@ static int eth_pre_remove(struct udevice *dev) { struct eth_pdata *pdata = dev->platdata;
eth_get_ops(dev)->stop(dev);
if (eth_is_active(dev))eth_get_ops(dev)->stop(dev); /* clear the MAC address */ memset(pdata->enetaddr, 0, ARP_HLEN);-- 2.17.1
Reviewed-by: Simon Glass sjg@chromium.org
On Wed, Feb 20, 2019 at 8:33 PM Keerthy j-keerthy@ti.com wrote:
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Could you please provide a test case?
Regards, Bin
On Wed, Feb 20, 2019 at 9:01 PM Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Feb 20, 2019 at 8:33 PM Keerthy j-keerthy@ti.com wrote:
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Could you please provide a test case?
Presumably that test should live in test/dm/eth.c with some potential support needed by drivers/net/sandbox.c
Thanks, -Joe
Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Wed, Feb 20, 2019 at 6:33 AM Keerthy j-keerthy@ti.com wrote:
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df192..26f7e0b8cd 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -531,7 +531,8 @@ static int eth_pre_remove(struct udevice *dev) { struct eth_pdata *pdata = dev->platdata;
eth_get_ops(dev)->stop(dev);
if (eth_is_active(dev))eth_get_ops(dev)->stop(dev);
This seems reasonable... What was the case that provoked an issue? Which driver was having trouble?
/* clear the MAC address */ memset(pdata->enetaddr, 0, ARP_HLEN);-- 2.17.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 22/02/19 12:08 AM, Joe Hershberger wrote:
On Wed, Feb 20, 2019 at 6:33 AM Keerthy j-keerthy@ti.com wrote:
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df192..26f7e0b8cd 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -531,7 +531,8 @@ static int eth_pre_remove(struct udevice *dev) { struct eth_pdata *pdata = dev->platdata;
eth_get_ops(dev)->stop(dev);
if (eth_is_active(dev))eth_get_ops(dev)->stop(dev);This seems reasonable... What was the case that provoked an issue? Which driver was having trouble?
I am trying to implement pru based ethernet driver(multiple instances) with cpsw driver on top on am654-evm. I observed that even though i had not started a particular instance of my pru based device stop was getting called and it was crashing right at the end.
/* clear the MAC address */ memset(pdata->enetaddr, 0, ARP_HLEN);-- 2.17.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Wed, Feb 27, 2019 at 1:47 PM Keerthy a0393675@ti.com wrote:
On 22/02/19 12:08 AM, Joe Hershberger wrote:
On Wed, Feb 20, 2019 at 6:33 AM Keerthy j-keerthy@ti.com wrote:
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df192..26f7e0b8cd 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -531,7 +531,8 @@ static int eth_pre_remove(struct udevice *dev) { struct eth_pdata *pdata = dev->platdata;
eth_get_ops(dev)->stop(dev);
if (eth_is_active(dev))eth_get_ops(dev)->stop(dev);This seems reasonable... What was the case that provoked an issue? Which driver was having trouble?
I am trying to implement pru based ethernet driver(multiple instances) with cpsw driver on top on am654-evm. I observed that even though i had not started a particular instance of my pru based device stop was getting called and it was crashing right at the end.
Please add a proper test case in test/dm/eth.c to cover such scenarios. Thanks!
Regards, Bin
On 27/02/19 11:17 AM, Keerthy wrote:
On 22/02/19 12:08 AM, Joe Hershberger wrote:
On Wed, Feb 20, 2019 at 6:33 AM Keerthy j-keerthy@ti.com wrote:
Currently stop is being called unconditionally without even checking if start is called. In case of multiple instances eth being present many devices might just be initialized without a start call in such cases stop might lead unpredictable behaviors including aborts and crashes. Hence add a check before calling stop.
Signed-off-by: Keerthy j-keerthy@ti.com
net/eth-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 2ef20df192..26f7e0b8cd 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -531,7 +531,8 @@ static int eth_pre_remove(struct udevice *dev) { struct eth_pdata *pdata = dev->platdata;
- eth_get_ops(dev)->stop(dev); + if (eth_is_active(dev)) + eth_get_ops(dev)->stop(dev);
This seems reasonable... What was the case that provoked an issue? Which driver was having trouble?
I am trying to implement pru based ethernet driver(multiple instances) with cpsw driver on top on am654-evm. I observed that even though i had not started a particular instance of my pru based device stop was getting called and it was crashing right at the end.
Joe,
I am yet to implement a test case for this. Can this patch be applied independently?
Regards, Keerthy
/* clear the MAC address */ memset(pdata->enetaddr, 0, ARP_HLEN); -- 2.17.1
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (5)
-
Bin Meng -
Joe Hershberger -
Keerthy -
Keerthy -
Simon Glass