| Message ID | 1377170269-8988-1-git-send-email-B38611@freescale.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Am Donnerstag, den 22.08.2013, 19:17 +0800 schrieb Fugang Duan: > For imx6slx evk platform, the fec driver cannot work since there > have no valid mac address set in physical mac registers. > > For imx serial platform, fec/enet IPs both need the physical MAC > address initial, otherwise it cannot work. > > After acquiring the valid MAC address from devices tree/pfuse/ > kernel command line/random mac address, and then set it to the > physical MAC address registers. > Yeah, we have also seen that we need to set this explicitly. The strange thing is that I recall this used to work without calling fec_set_mac_address() explicitly, so either the netdev core was calling it at some point, or our userspace was. I didn't got around to investigate this further. So anyway: Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > Signed-off-by: Fugang Duan <B38611@freescale.com> > --- > drivers/net/ethernet/freescale/fec_main.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c > index 4349a9e..9b5e08c 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device *ndev, void *p) > struct fec_enet_private *fep = netdev_priv(ndev); > struct sockaddr *addr = p; > > - if (!is_valid_ether_addr(addr->sa_data)) > - return -EADDRNOTAVAIL; > + if (p) { > + if (!is_valid_ether_addr(addr->sa_data)) > + return -EADDRNOTAVAIL; > > - memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > + memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > + } > > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) | > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), > @@ -1969,6 +1971,7 @@ static int fec_enet_init(struct net_device *ndev) > > /* Get the Ethernet address */ > fec_get_mac(ndev); > + fec_set_mac_address(ndev, NULL); > > /* Set receive and transmit descriptor base. */ > fep->rx_bd_base = cbd_base;
From: Lucas Stach [mailto:l.stach@pengutronix.de] Data: Tuesday, August 27, 2013 10:19 PM > To: Duan Fugang-B38611 > Cc: Li Frank-B20596; Zhou Luwei-B45643; davem@davemloft.net; > netdev@vger.kernel.org; shawn.guo@linaro.org; bhutchings@solarflare.com; > Estevam Fabio-R49496; stephen@networkplumber.org > Subject: Re: [PATCH] net: fec: add the initial to set the physical mac > address > > Am Donnerstag, den 22.08.2013, 19:17 +0800 schrieb Fugang Duan: > > For imx6slx evk platform, the fec driver cannot work since there have > > no valid mac address set in physical mac registers. > > > > For imx serial platform, fec/enet IPs both need the physical MAC > > address initial, otherwise it cannot work. > > > > After acquiring the valid MAC address from devices tree/pfuse/ kernel > > command line/random mac address, and then set it to the physical MAC > > address registers. > > > Yeah, we have also seen that we need to set this explicitly. The strange > thing is that I recall this used to work without calling > fec_set_mac_address() explicitly, so either the netdev core was calling it > at some point, or our userspace was. I didn't got around to investigate > this further. Netdev core don't call the function willingly, if only user config MAC address such as call ioctl(sockfd, SIOCSIFHWADDR, ifreq). > So anyway: > Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > > > Signed-off-by: Fugang Duan <B38611@freescale.com> > > --- > > drivers/net/ethernet/freescale/fec_main.c | 9 ++++++--- > > 1 files changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > > b/drivers/net/ethernet/freescale/fec_main.c > > index 4349a9e..9b5e08c 100644 > > --- a/drivers/net/ethernet/freescale/fec_main.c > > +++ b/drivers/net/ethernet/freescale/fec_main.c > > @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device *ndev, > void *p) > > struct fec_enet_private *fep = netdev_priv(ndev); > > struct sockaddr *addr = p; > > > > - if (!is_valid_ether_addr(addr->sa_data)) > > - return -EADDRNOTAVAIL; > > + if (p) { > > + if (!is_valid_ether_addr(addr->sa_data)) > > + return -EADDRNOTAVAIL; > > > > - memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > > + memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > > + } > > > > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) | > > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), @@ - > 1969,6 > > +1971,7 @@ static int fec_enet_init(struct net_device *ndev) > > > > /* Get the Ethernet address */ > > fec_get_mac(ndev); > > + fec_set_mac_address(ndev, NULL); > > > > /* Set receive and transmit descriptor base. */ > > fep->rx_bd_base = cbd_base; > > -- > Pengutronix e.K. | Lucas Stach | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
Hi, David, Please don't ignore the patch. Thanks, Andy > -----Original Message----- > From: Duan Fugang-B38611 > Sent: Friday, August 30, 2013 10:01 AM > To: Lucas Stach; davem@davemloft.net > Cc: davem@davemloft.net; netdev@vger.kernel.org; bhutchings@solarflare.com; > Estevam Fabio-R49496; stephen@networkplumber.org > Subject: RE: [PATCH] net: fec: add the initial to set the physical mac > address > > Ping... > > > -----Original Message----- > > From: netdev-owner@vger.kernel.org > > [mailto:netdev-owner@vger.kernel.org] > > On Behalf Of Duan Fugang-B38611 > > Sent: Wednesday, August 28, 2013 10:11 AM > > To: Lucas Stach > > Cc: Li Frank-B20596; Zhou Luwei-B45643; davem@davemloft.net; > > netdev@vger.kernel.org; shawn.guo@linaro.org; > > bhutchings@solarflare.com; Estevam Fabio-R49496; > > stephen@networkplumber.org > > Subject: RE: [PATCH] net: fec: add the initial to set the physical mac > > address > > > > From: Lucas Stach [mailto:l.stach@pengutronix.de] > > Data: Tuesday, August 27, 2013 10:19 PM > > > > > To: Duan Fugang-B38611 > > > Cc: Li Frank-B20596; Zhou Luwei-B45643; davem@davemloft.net; > > > netdev@vger.kernel.org; shawn.guo@linaro.org; > > > bhutchings@solarflare.com; Estevam Fabio-R49496; > > > stephen@networkplumber.org > > > Subject: Re: [PATCH] net: fec: add the initial to set the physical > > > mac address > > > > > > Am Donnerstag, den 22.08.2013, 19:17 +0800 schrieb Fugang Duan: > > > > For imx6slx evk platform, the fec driver cannot work since there > > > > have no valid mac address set in physical mac registers. > > > > > > > > For imx serial platform, fec/enet IPs both need the physical MAC > > > > address initial, otherwise it cannot work. > > > > > > > > After acquiring the valid MAC address from devices tree/pfuse/ > > > > kernel command line/random mac address, and then set it to the > > > > physical MAC address registers. > > > > > > > Yeah, we have also seen that we need to set this explicitly. The > > > strange thing is that I recall this used to work without calling > > > fec_set_mac_address() explicitly, so either the netdev core was > > > calling it at some point, or our userspace was. I didn't got around > > > to investigate this further. > > Netdev core don't call the function willingly, if only user config MAC > > address such as call ioctl(sockfd, SIOCSIFHWADDR, ifreq). > > > > > So anyway: > > > Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > > > > > > > Signed-off-by: Fugang Duan <B38611@freescale.com> > > > > --- > > > > drivers/net/ethernet/freescale/fec_main.c | 9 ++++++--- > > > > 1 files changed, 6 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > > > > b/drivers/net/ethernet/freescale/fec_main.c > > > > index 4349a9e..9b5e08c 100644 > > > > --- a/drivers/net/ethernet/freescale/fec_main.c > > > > +++ b/drivers/net/ethernet/freescale/fec_main.c > > > > @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device > > > > *ndev, > > > void *p) > > > > struct fec_enet_private *fep = netdev_priv(ndev); > > > > struct sockaddr *addr = p; > > > > > > > > - if (!is_valid_ether_addr(addr->sa_data)) > > > > - return -EADDRNOTAVAIL; > > > > + if (p) { > > > > + if (!is_valid_ether_addr(addr->sa_data)) > > > > + return -EADDRNOTAVAIL; > > > > > > > > - memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > > > > + memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > > > > + } > > > > > > > > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) | > > > > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), > @@ - > > > 1969,6 > > > > +1971,7 @@ static int fec_enet_init(struct net_device *ndev) > > > > > > > > /* Get the Ethernet address */ > > > > fec_get_mac(ndev); > > > > + fec_set_mac_address(ndev, NULL); > > > > > > > > /* Set receive and transmit descriptor base. */ > > > > fep->rx_bd_base = cbd_base; > > > > > > -- > > > Pengutronix e.K. | Lucas Stach > > | > > > Industrial Linux Solutions | > http://www.pengutronix.de/ > > | > > > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: > > > +49-5121-206917-5076 > > | > > > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- > 5555 > > | > > > > > >
From: Duan Fugang-B38611 <B38611@freescale.com> Date: Tue, 10 Sep 2013 01:45:52 +0000 > Please don't ignore the patch. Please don't just pray that it's in my queue and I will apply it. If it's not in my patchwork queue, you need to simply resubmit it. If it is in my patchwork queue, you need to be patient. Thank you. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Miller [mailto:davem@davemloft.net] Data: Tuesday, September 10, 2013 1:08 PM > To: Duan Fugang-B38611 > Cc: l.stach@pengutronix.de; netdev@vger.kernel.org; > bhutchings@solarflare.com; Estevam Fabio-R49496; > stephen@networkplumber.org; s.hauer@pengutronix.de > Subject: Re: [PATCH] net: fec: add the initial to set the physical mac > address > > From: Duan Fugang-B38611 <B38611@freescale.com> > Date: Tue, 10 Sep 2013 01:45:52 +0000 > > > Please don't ignore the patch. > > Please don't just pray that it's in my queue and I will apply it. > > If it's not in my patchwork queue, you need to simply resubmit it. > > If it is in my patchwork queue, you need to be patient. > > Thank you. > Ok, I know, just a notify to avoid forget it because I did ping to you. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/22/13, Fugang Duan <B38611@freescale.com> wrote: > For imx6slx evk platform, the fec driver cannot work since there > have no valid mac address set in physical mac registers. > > For imx serial platform, fec/enet IPs both need the physical MAC > address initial, otherwise it cannot work. > > After acquiring the valid MAC address from devices tree/pfuse/ > kernel command line/random mac address, and then set it to the > physical MAC address registers. > > Signed-off-by: Fugang Duan <B38611@freescale.com> > --- > drivers/net/ethernet/freescale/fec_main.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index 4349a9e..9b5e08c 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device *ndev, void > *p) > struct fec_enet_private *fep = netdev_priv(ndev); > struct sockaddr *addr = p; > > - if (!is_valid_ether_addr(addr->sa_data)) > - return -EADDRNOTAVAIL; > + if (p) { I don't see how the p pointer can be NULL... > + if (!is_valid_ether_addr(addr->sa_data)) > + return -EADDRNOTAVAIL; > > - memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > + memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > + } > > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) | > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), > @@ -1969,6 +1971,7 @@ static int fec_enet_init(struct net_device *ndev) > > /* Get the Ethernet address */ > fec_get_mac(ndev); > + fec_set_mac_address(ndev, NULL); > > /* Set receive and transmit descriptor base. */ > fep->rx_bd_base = cbd_base; > -- > 1.7.1 > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Denis Kirjanov [mailto:kda@linux-powerpc.org] Data: Tuesday, September 10, 2013 10:19 PM > To: Duan Fugang-B38611 > Cc: Li Frank-B20596; Zhou Luwei-B45643; davem@davemloft.net; > netdev@vger.kernel.org; shawn.guo@linaro.org; bhutchings@solarflare.com; > Estevam Fabio-R49496; stephen@networkplumber.org > Subject: Re: [PATCH] net: fec: add the initial to set the physical mac > address > > On 8/22/13, Fugang Duan <B38611@freescale.com> wrote: > > For imx6slx evk platform, the fec driver cannot work since there have > > no valid mac address set in physical mac registers. > > > > For imx serial platform, fec/enet IPs both need the physical MAC > > address initial, otherwise it cannot work. > > > > After acquiring the valid MAC address from devices tree/pfuse/ kernel > > command line/random mac address, and then set it to the physical MAC > > address registers. > > > > Signed-off-by: Fugang Duan <B38611@freescale.com> > > --- > > drivers/net/ethernet/freescale/fec_main.c | 9 ++++++--- > > 1 files changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > > b/drivers/net/ethernet/freescale/fec_main.c > > index 4349a9e..9b5e08c 100644 > > --- a/drivers/net/ethernet/freescale/fec_main.c > > +++ b/drivers/net/ethernet/freescale/fec_main.c > > @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device *ndev, > > void > > *p) > > struct fec_enet_private *fep = netdev_priv(ndev); > > struct sockaddr *addr = p; > > > > - if (!is_valid_ether_addr(addr->sa_data)) > > - return -EADDRNOTAVAIL; > > + if (p) { > > I don't see how the p pointer can be NULL... As below, fec_enet_init() function calls fec_set_mac_address(ndev, NULL). > > > + if (!is_valid_ether_addr(addr->sa_data)) > > + return -EADDRNOTAVAIL; > > > > - memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > > + memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); > > + } > > > > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) | > > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), @@ - > 1969,6 > > +1971,7 @@ static int fec_enet_init(struct net_device *ndev) > > > > /* Get the Ethernet address */ > > fec_get_mac(ndev); > > + fec_set_mac_address(ndev, NULL); > > > > /* Set receive and transmit descriptor base. */ > > fep->rx_bd_base = cbd_base; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 4349a9e..9b5e08c 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device *ndev, void *p) struct fec_enet_private *fep = netdev_priv(ndev); struct sockaddr *addr = p; - if (!is_valid_ether_addr(addr->sa_data)) - return -EADDRNOTAVAIL; + if (p) { + if (!is_valid_ether_addr(addr->sa_data)) + return -EADDRNOTAVAIL; - memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); + memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len); + } writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) | (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), @@ -1969,6 +1971,7 @@ static int fec_enet_init(struct net_device *ndev) /* Get the Ethernet address */ fec_get_mac(ndev); + fec_set_mac_address(ndev, NULL); /* Set receive and transmit descriptor base. */ fep->rx_bd_base = cbd_base;
For imx6slx evk platform, the fec driver cannot work since there have no valid mac address set in physical mac registers. For imx serial platform, fec/enet IPs both need the physical MAC address initial, otherwise it cannot work. After acquiring the valid MAC address from devices tree/pfuse/ kernel command line/random mac address, and then set it to the physical MAC address registers. Signed-off-by: Fugang Duan <B38611@freescale.com> --- drivers/net/ethernet/freescale/fec_main.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)