* [PATCH net-next 1/1] net: fec: add netif status check before set mac address
@ 2015-09-09 2:42 Fugang Duan
2015-09-09 3:37 ` Florian Fainelli
2015-09-09 9:14 ` Lucas Stach
0 siblings, 2 replies; 6+ messages in thread
From: Fugang Duan @ 2015-09-09 2:42 UTC (permalink / raw)
To: davem; +Cc: netdev, bhutchings, b38611
There exist one issue by below case that case system hang:
ifconfig eth0 down
ifconfig eth0 hw ether 00:10:19:19:81:19
After eth0 down, all fec clocks are gated off. In the .fec_set_mac_address()
function, it will set new MAC address to registers, which causes system hang.
So it needs to add netif status check to avoid registers access when clocks are
gated off. Until eth0 up the new MAC address are wrote into related registers.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 91925e3..cd09dbb 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3029,6 +3029,9 @@ fec_set_mac_address(struct net_device *ndev, void *p)
memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
}
+ if (!netif_running(ndev))
+ return 0;
+
writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
(ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
fep->hwp + FEC_ADDR_LOW);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/1] net: fec: add netif status check before set mac address
2015-09-09 2:42 [PATCH net-next 1/1] net: fec: add netif status check before set mac address Fugang Duan
@ 2015-09-09 3:37 ` Florian Fainelli
2015-09-09 3:45 ` Duan Andy
2015-09-09 9:14 ` Lucas Stach
1 sibling, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2015-09-09 3:37 UTC (permalink / raw)
To: Fugang Duan, davem; +Cc: netdev, bhutchings
Le 09/08/15 19:42, Fugang Duan a écrit :
> There exist one issue by below case that case system hang:
> ifconfig eth0 down
> ifconfig eth0 hw ether 00:10:19:19:81:19
>
> After eth0 down, all fec clocks are gated off. In the .fec_set_mac_address()
> function, it will set new MAC address to registers, which causes system hang.
>
> So it needs to add netif status check to avoid registers access when clocks are
> gated off. Until eth0 up the new MAC address are wrote into related registers.
Since this is a bug fix, do not you intend to target the "net" tree
instead of "net-next"?
>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 91925e3..cd09dbb 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3029,6 +3029,9 @@ fec_set_mac_address(struct net_device *ndev, void *p)
> memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> }
>
> + if (!netif_running(ndev))
> + return 0;
> +
> writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
> fep->hwp + FEC_ADDR_LOW);
>
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next 1/1] net: fec: add netif status check before set mac address
2015-09-09 3:37 ` Florian Fainelli
@ 2015-09-09 3:45 ` Duan Andy
2015-09-09 21:13 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Duan Andy @ 2015-09-09 3:45 UTC (permalink / raw)
To: Florian Fainelli, davem@davemloft.net
Cc: netdev@vger.kernel.org, bhutchings@solarflare.com
From: Florian Fainelli <f.fainelli@gmail.com> Sent: Wednesday, September 09, 2015 11:38 AM
> To: Duan Fugang-B38611; davem@davemloft.net
> Cc: netdev@vger.kernel.org; bhutchings@solarflare.com
> Subject: Re: [PATCH net-next 1/1] net: fec: add netif status check before
> set mac address
>
> Le 09/08/15 19:42, Fugang Duan a écrit :
> > There exist one issue by below case that case system hang:
> > ifconfig eth0 down
> > ifconfig eth0 hw ether 00:10:19:19:81:19
> >
> > After eth0 down, all fec clocks are gated off. In the
> > .fec_set_mac_address() function, it will set new MAC address to
> registers, which causes system hang.
> >
> > So it needs to add netif status check to avoid registers access when
> > clocks are gated off. Until eth0 up the new MAC address are wrote into
> related registers.
>
> Since this is a bug fix, do not you intend to target the "net" tree
> instead of "net-next"?
>
Thanks for your reminding, it is better to enter net tree.
David, if you apply it, pls put it into net tree. Thanks.
> >
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > ---
> > drivers/net/ethernet/freescale/fec_main.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index 91925e3..cd09dbb 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -3029,6 +3029,9 @@ fec_set_mac_address(struct net_device *ndev, void
> *p)
> > memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> > }
> >
> > + if (!netif_running(ndev))
> > + return 0;
> > +
> > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
> > fep->hwp + FEC_ADDR_LOW);
> >
>
>
> --
> Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/1] net: fec: add netif status check before set mac address
2015-09-09 3:45 ` Duan Andy
@ 2015-09-09 21:13 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-09-09 21:13 UTC (permalink / raw)
To: fugang.duan; +Cc: f.fainelli, netdev, bhutchings
From: Duan Andy <fugang.duan@freescale.com>
Date: Wed, 9 Sep 2015 03:45:48 +0000
> From: Florian Fainelli <f.fainelli@gmail.com> Sent: Wednesday, September 09, 2015 11:38 AM
>> To: Duan Fugang-B38611; davem@davemloft.net
>> Cc: netdev@vger.kernel.org; bhutchings@solarflare.com
>> Subject: Re: [PATCH net-next 1/1] net: fec: add netif status check before
>> set mac address
>>
>> Le 09/08/15 19:42, Fugang Duan a écrit :
>> > There exist one issue by below case that case system hang:
>> > ifconfig eth0 down
>> > ifconfig eth0 hw ether 00:10:19:19:81:19
>> >
>> > After eth0 down, all fec clocks are gated off. In the
>> > .fec_set_mac_address() function, it will set new MAC address to
>> registers, which causes system hang.
>> >
>> > So it needs to add netif status check to avoid registers access when
>> > clocks are gated off. Until eth0 up the new MAC address are wrote into
>> related registers.
>>
>> Since this is a bug fix, do not you intend to target the "net" tree
>> instead of "net-next"?
>>
> Thanks for your reminding, it is better to enter net tree.
>
> David, if you apply it, pls put it into net tree. Thanks.
You need to address the feedback given by Lucas first.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/1] net: fec: add netif status check before set mac address
2015-09-09 2:42 [PATCH net-next 1/1] net: fec: add netif status check before set mac address Fugang Duan
2015-09-09 3:37 ` Florian Fainelli
@ 2015-09-09 9:14 ` Lucas Stach
2015-09-10 1:05 ` Duan Andy
1 sibling, 1 reply; 6+ messages in thread
From: Lucas Stach @ 2015-09-09 9:14 UTC (permalink / raw)
To: Fugang Duan; +Cc: davem, netdev, bhutchings
Am Mittwoch, den 09.09.2015, 10:42 +0800 schrieb Fugang Duan:
> There exist one issue by below case that case system hang:
> ifconfig eth0 down
> ifconfig eth0 hw ether 00:10:19:19:81:19
>
> After eth0 down, all fec clocks are gated off. In the .fec_set_mac_address()
> function, it will set new MAC address to registers, which causes system hang.
>
> So it needs to add netif status check to avoid registers access when clocks are
> gated off. Until eth0 up the new MAC address are wrote into related registers.
>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 91925e3..cd09dbb 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3029,6 +3029,9 @@ fec_set_mac_address(struct net_device *ndev, void *p)
> memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> }
>
> + if (!netif_running(ndev))
> + return 0;
This deserves a comment in the code as to why it is needed and how it
still works.
Regards,
Lucas
> +
> writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
> fep->hwp + FEC_ADDR_LOW);
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next 1/1] net: fec: add netif status check before set mac address
2015-09-09 9:14 ` Lucas Stach
@ 2015-09-10 1:05 ` Duan Andy
0 siblings, 0 replies; 6+ messages in thread
From: Duan Andy @ 2015-09-10 1:05 UTC (permalink / raw)
To: Lucas Stach
Cc: davem@davemloft.net, netdev@vger.kernel.org,
bhutchings@solarflare.com
From: Lucas Stach <l.stach@pengutronix.de> Sent: Wednesday, September 09, 2015 5:14 PM
> To: Duan Fugang-B38611
> Cc: davem@davemloft.net; netdev@vger.kernel.org;
> bhutchings@solarflare.com
> Subject: Re: [PATCH net-next 1/1] net: fec: add netif status check before
> set mac address
>
> Am Mittwoch, den 09.09.2015, 10:42 +0800 schrieb Fugang Duan:
> > There exist one issue by below case that case system hang:
> > ifconfig eth0 down
> > ifconfig eth0 hw ether 00:10:19:19:81:19
> >
> > After eth0 down, all fec clocks are gated off. In the
> > .fec_set_mac_address() function, it will set new MAC address to
> registers, which causes system hang.
> >
> > So it needs to add netif status check to avoid registers access when
> > clocks are gated off. Until eth0 up the new MAC address are wrote into
> related registers.
> >
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > ---
> > drivers/net/ethernet/freescale/fec_main.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index 91925e3..cd09dbb 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -3029,6 +3029,9 @@ fec_set_mac_address(struct net_device *ndev, void
> *p)
> > memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> > }
> >
> > + if (!netif_running(ndev))
> > + return 0;
> This deserves a comment in the code as to why it is needed and how it
> still works.
>
> Regards,
> Lucas
> > +
Ok, I will add comment on here in next version.
Thanks.
> > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
> > fep->hwp + FEC_ADDR_LOW);
>
> --
> Pengutronix e.K. | Lucas Stach |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-10 1:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-09 2:42 [PATCH net-next 1/1] net: fec: add netif status check before set mac address Fugang Duan
2015-09-09 3:37 ` Florian Fainelli
2015-09-09 3:45 ` Duan Andy
2015-09-09 21:13 ` David Miller
2015-09-09 9:14 ` Lucas Stach
2015-09-10 1:05 ` Duan Andy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).