* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
@ 2019-02-20 12:32 Keerthy
2019-02-21 2:48 ` Simon Glass
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Keerthy @ 2019-02-20 12:32 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
2019-02-20 12:32 [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices Keerthy
@ 2019-02-21 2:48 ` Simon Glass
2019-02-21 3:00 ` Bin Meng
2019-02-21 18:38 ` Joe Hershberger
2 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2019-02-21 2:48 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
2019-02-20 12:32 [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices Keerthy
2019-02-21 2:48 ` Simon Glass
@ 2019-02-21 3:00 ` Bin Meng
2019-02-21 18:40 ` Joe Hershberger
2019-02-21 18:38 ` Joe Hershberger
2 siblings, 1 reply; 8+ messages in thread
From: Bin Meng @ 2019-02-21 3:00 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
2019-02-20 12:32 [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices Keerthy
2019-02-21 2:48 ` Simon Glass
2019-02-21 3:00 ` Bin Meng
@ 2019-02-21 18:38 ` Joe Hershberger
2019-02-27 5:47 ` Keerthy
2 siblings, 1 reply; 8+ messages in thread
From: Joe Hershberger @ 2019-02-21 18:38 UTC (permalink / raw)
To: 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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
2019-02-21 3:00 ` Bin Meng
@ 2019-02-21 18:40 ` Joe Hershberger
0 siblings, 0 replies; 8+ messages in thread
From: Joe Hershberger @ 2019-02-21 18:40 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
2019-02-21 18:38 ` Joe Hershberger
@ 2019-02-27 5:47 ` Keerthy
2019-02-27 6:05 ` Bin Meng
2019-03-29 9:52 ` Keerthy
0 siblings, 2 replies; 8+ messages in thread
From: Keerthy @ 2019-02-27 5:47 UTC (permalink / raw)
To: 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 at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
2019-02-27 5:47 ` Keerthy
@ 2019-02-27 6:05 ` Bin Meng
2019-03-29 9:52 ` Keerthy
1 sibling, 0 replies; 8+ messages in thread
From: Bin Meng @ 2019-02-27 6:05 UTC (permalink / raw)
To: 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices
2019-02-27 5:47 ` Keerthy
2019-02-27 6:05 ` Bin Meng
@ 2019-03-29 9:52 ` Keerthy
1 sibling, 0 replies; 8+ messages in thread
From: Keerthy @ 2019-03-29 9:52 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
>>> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-29 9:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-20 12:32 [U-Boot] [PATCH] net: eth-uclass: call stop only for active devices Keerthy
2019-02-21 2:48 ` Simon Glass
2019-02-21 3:00 ` Bin Meng
2019-02-21 18:40 ` Joe Hershberger
2019-02-21 18:38 ` Joe Hershberger
2019-02-27 5:47 ` Keerthy
2019-02-27 6:05 ` Bin Meng
2019-03-29 9:52 ` Keerthy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.