[PATCH] net: fec: set mac address unconditionally

[PATCH] net: fec: set mac address unconditionally

[Gavin Schenk]

Fixes: 9638d19e4816 ("net: fec: add netif status check before set mac address")

If the mac address origin is not dt, you can only safe assign a
mac address after "link up" of the device. If the link is down the
clocks are disabled and because of issues assigning registers when
clocks are down the new mac address is discarded on some soc's. This fix
sets the mac address unconditionally in fec_restart(...) and ensures
consistens between fec registers and the network layer.

Signed-off-by: Gavin Schenk <g.schenk@eckelmann.de>
---
 drivers/net/ethernet/freescale/fec_main.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2a03857cca18..bdabea6cd981 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -903,13 +903,11 @@ fec_restart(struct net_device *ndev)
 	 * enet-mac reset will reset mac address registers too,
 	 * so need to reconfigure it.
 	 */
-	if (fep->quirks & FEC_QUIRK_ENET_MAC) {
-		memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
-		writel((__force u32)cpu_to_be32(temp_mac[0]),
-		       fep->hwp + FEC_ADDR_LOW);
-		writel((__force u32)cpu_to_be32(temp_mac[1]),
-		       fep->hwp + FEC_ADDR_HIGH);
-	}
+	memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
+	writel((__force u32)cpu_to_be32(temp_mac[0]),
+	       fep->hwp + FEC_ADDR_LOW);
+	writel((__force u32)cpu_to_be32(temp_mac[1]),
+	       fep->hwp + FEC_ADDR_HIGH);
 
 	/* Clear any outstanding interrupt. */
 	writel(0xffffffff, fep->hwp + FEC_IEVENT);
-- 
1.9.1


Eckelmann AG
Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp Eckelmann
Dr.-Ing. Frank-Thomas Mellert Dr.-Ing. Marco Münchhof Dr.-Ing. Frank Uhlemann
Vorsitzender des Aufsichtsrats: Hubertus G. Krossa
Sitz der Gesellschaft: Berliner Str. 161, 65205 Wiesbaden, Amtsgericht Wiesbaden HRB 12636
http://www.eckelmann.de 

RE: [PATCH] net: fec: set mac address unconditionally

[Andy Duan]

From: Gavin Schenk <g.schenk@eckelmann.de> Sent: Wednesday, September 21, 2016 9:31 PM
> To: Andy Duan <fugang.duan@nxp.com>
> Cc: netdev@vger.kernel.org; kernel@pengutronix.de; Gavin Schenk
> <g.schenk@eckelmann.de>
> Subject: [PATCH] net: fec: set mac address unconditionally
> 
> Fixes: 9638d19e4816 ("net: fec: add netif status check before set mac
> address")
> 
> If the mac address origin is not dt, you can only safe assign a mac address
> after "link up" of the device. If the link is down the clocks are disabled and
> because of issues assigning registers when clocks are down the new mac
> address is discarded on some soc's. This fix sets the mac address
> unconditionally in fec_restart(...) and ensures consistens between fec
> registers and the network layer.
> 
> Signed-off-by: Gavin Schenk <g.schenk@eckelmann.de>
> ---

It make sense, thanks.

Acked-by: Fugang Duan <fugang.duan@nxp.com>

>  drivers/net/ethernet/freescale/fec_main.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 2a03857cca18..bdabea6cd981 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -903,13 +903,11 @@ fec_restart(struct net_device *ndev)
>  	 * enet-mac reset will reset mac address registers too,
>  	 * so need to reconfigure it.
>  	 */
> -	if (fep->quirks & FEC_QUIRK_ENET_MAC) {
> -		memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
> -		writel((__force u32)cpu_to_be32(temp_mac[0]),
> -		       fep->hwp + FEC_ADDR_LOW);
> -		writel((__force u32)cpu_to_be32(temp_mac[1]),
> -		       fep->hwp + FEC_ADDR_HIGH);
> -	}
> +	memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
> +	writel((__force u32)cpu_to_be32(temp_mac[0]),
> +	       fep->hwp + FEC_ADDR_LOW);
> +	writel((__force u32)cpu_to_be32(temp_mac[1]),
> +	       fep->hwp + FEC_ADDR_HIGH);
> 
>  	/* Clear any outstanding interrupt. */
>  	writel(0xffffffff, fep->hwp + FEC_IEVENT);
> --
> 1.9.1
> 
> 
> Eckelmann AG
> Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp
> Eckelmann
> Dr.-Ing. Frank-Thomas Mellert Dr.-Ing. Marco Münchhof Dr.-Ing. Frank
> Uhlemann
> Vorsitzender des Aufsichtsrats: Hubertus G. Krossa
> Sitz der Gesellschaft: Berliner Str. 161, 65205 Wiesbaden, Amtsgericht
> Wiesbaden HRB 12636
> http://www.eckelmann.de

Re: [PATCH] net: fec: set mac address unconditionally

[Uwe Kleine-König]

Hello,

just a few nitpicks:

On Wed, Sep 21, 2016 at 03:30:55PM +0200, Gavin Schenk wrote:
> Fixes: 9638d19e4816 ("net: fec: add netif status check before set mac address")

This line belongs to in the S-o-B area below.

> If the mac address origin is not dt, you can only safe assign a

s/safe/safely/

> mac address after "link up" of the device. If the link is down the
> clocks are disabled and because of issues assigning registers when
> clocks are down the new mac address is discarded on some soc's. This fix

s/down/off/; s/is discarded/cannot be written in .ndo_set_mac_address()/

> sets the mac address unconditionally in fec_restart(...) and ensures
> consistens between fec registers and the network layer.

s/consistens/consistency/

Other than that:

Acked-by: Uwe Kleine-König <u.kleine-koenig@eckelmann.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] net: fec: set mac address unconditionally

[Uwe Kleine-König]

On Thu, Sep 22, 2016 at 08:08:36AM +0200, Uwe Kleine-König wrote:
> Acked-by: Uwe Kleine-König <u.kleine-koenig@eckelmann.de>
huch, cut and paste borkage: This should be

	Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

of course.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |