From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-ey0-f177.google.com ([209.85.215.177]) by canuck.infradead.org with esmtps (Exim 4.72 #1 (Red Hat Linux)) id 1Q9But-0003cM-QO for barebox@lists.infradead.org; Mon, 11 Apr 2011 07:52:13 +0000 Received: by eyh6 with SMTP id 6so1755605eyh.36 for ; Mon, 11 Apr 2011 00:52:10 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20110411074750.GF7285@pengutronix.de> References: <1302358087-9756-1-git-send-email-franck.jullien@gmail.com> <1302358087-9756-2-git-send-email-franck.jullien@gmail.com> <20110410040649.GA8392@game.jcrosoft.org> <20110410105159.GA18343@game.jcrosoft.org> <20110411023702.GA18925@game.jcrosoft.org> <20110411074750.GF7285@pengutronix.de> Date: Mon, 11 Apr 2011 09:52:09 +0200 Message-ID: From: Franck JULLIEN List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: multipart/mixed; boundary="===============0435953739==" Sender: barebox-bounces@lists.infradead.org Errors-To: barebox-bounces+u.kleine-koenig=pengutronix.de@lists.infradead.org Subject: Re: [PATCH] [v2] Nios2: Add Altera TSE MAC driver To: Sascha Hauer Cc: barebox --===============0435953739== Content-Type: multipart/alternative; boundary=0015174c45a24ce78304a09fda5b --0015174c45a24ce78304a09fda5b Content-Type: text/plain; charset=ISO-8859-1 2011/4/11 Sascha Hauer > On Mon, Apr 11, 2011 at 09:23:56AM +0200, Franck JULLIEN wrote: > > 2011/4/11 Jean-Christophe PLAGNIOL-VILLARD > > > > > On 20:21 Sun 10 Apr , Franck JULLIEN wrote: > > > > 2011/4/10 Jean-Christophe PLAGNIOL-VILLARD > > > > > > > > > > > index 0000000..2687377 > > > > > > --- /dev/null > > > > > > + > > > > > > +static int tse_get_ethaddr(struct eth_device *edev, > > > unsigned > > > > char *m) > > > > > > +{ > > > > > > + /* There is no eeprom */ > > > > > so return the content of the register no? > > > > > > > > > > Well, the register is reseted to 0 when the MAC starts so > there > > > is > > > > no > > > > > Ethernet address > > > > > to get. > > > > > > > > > except this function is supposed to return the mac address of > the > > > device > > > > at > > > > any time so after a set of it it will not be true any more > > > > > > > > If I implement the function I get a "eth@eth0: got MAC address > from > > > > EEPROM: 00:00:00:00:00:00" at startup. > > > > That why I returned -1 as what I could find int at91_ether.c...... > > > > Or, I could find something to return -1 as long as the MAC address > > > hasn't > > > > been set. > > > > > > > I known this issue I re-write recently the at91_ether and the same on > macb > > > will post the patch soon > > > > > > It's fine the uperlayer will see that it's not a valid mac so this will > > > generate a random one > > > cf net/net.c IIRC > > > > > > > > For me, it's a bit annoying to get this message at startup. > > > > Don't you really think I could have a flag in the private structure to > check > > if an address has been set and then return -1 or the address in the > > tse_get_ethaddr function ? > > How about the following: > > > diff --git a/net/eth.c b/net/eth.c > index 0251e59..c5b346c 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -167,8 +167,10 @@ int eth_register(struct eth_device *edev) > > if (edev->get_ethaddr(edev, ethaddr) == 0) { > ethaddr_to_string(ethaddr, ethaddr_str); > - dev_info(dev, "got MAC address from EEPROM: %s\n", > ethaddr_str); > - dev_set_param(dev, "ethaddr", ethaddr_str); > + if (is_valid_ether_addr(ethaddr)) { > + dev_info(dev, "got MAC address from EEPROM: %s\n", > ethaddr_str); > + dev_set_param(dev, "ethaddr", ethaddr_str); > + } > } > > if (!eth_current) { > > Sascha > > Sounds good :) > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > --0015174c45a24ce78304a09fda5b Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

2011/4/11 Sascha Hauer <s.hauer@pengutronix.de&g= t;
On Mon, Apr 11, 2011 at 09:23:56AM +0200,= Franck JULLIEN wrote:
> 2011/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>
> > On 20:21 Sun 10 Apr =A0 =A0 , Franck JULLIEN wrote:
> > > =A0 =A02011/4/10 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > >
> > > =A0 =A0 =A0> =A0 =A0 =A0> index 0000000..2687377
> > > =A0 =A0 =A0> =A0 =A0 =A0> --- /dev/null
> > > =A0 =A0 =A0> =A0 =A0 =A0> +
> > > =A0 =A0 =A0> =A0 =A0 =A0> +static int tse_get_ethaddr(= struct eth_device *edev,
> > unsigned
> > > =A0 =A0 =A0char *m)
> > > =A0 =A0 =A0> =A0 =A0 =A0> +{
> > > =A0 =A0 =A0> =A0 =A0 =A0> + =A0 =A0 /* There is no eep= rom */
> > > =A0 =A0 =A0> =A0 =A0 =A0so return the content of the regi= ster no?
> > > =A0 =A0 =A0>
> > > =A0 =A0 =A0> =A0 =A0Well, the register is reseted to 0 wh= en the MAC starts so there
> > is
> > > =A0 =A0 =A0no
> > > =A0 =A0 =A0> =A0 =A0Ethernet address
> > > =A0 =A0 =A0> =A0 =A0to get.
> > > =A0 =A0 =A0>
> > > =A0 =A0 =A0except this function is supposed to return the ma= c address of the
> > device
> > > =A0 =A0 =A0at
> > > =A0 =A0 =A0any time so after a set of it it will not be true= any more
> > >
> > > =A0 =A0If I implement the function I get a "eth@eth0: g= ot MAC address from
> > > =A0 =A0EEPROM: 00:00:00:00:00:00" at startup.
> > > =A0 =A0That why I returned -1 as what I could find int at91_= ether.c......
> > > =A0 =A0Or, I could find something to return -1 as long as th= e MAC address
> > hasn't
> > > =A0 =A0been set.
> > >
> > I known this issue I re-write recently the at91_ether and the sam= e on macb
> > will post the patch soon
> >
> > It's fine the uperlayer will see that it's not a valid ma= c so this will
> > generate a random one
> > cf net/net.c IIRC
> >
> >
> For me, it's a bit annoying =A0to get this message at startup.
>
> Don't you really think I could have a flag in the private structur= e to check
> if an address has been set and then return -1 or the address in the > tse_get_ethaddr function ?

How about the following:


diff --git a/net/eth.c b/net/eth.c
index 0251e59..c5b346c 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -167,8 +167,10 @@ int eth_register(struct eth_device *edev)

=A0 =A0 =A0 =A0if (edev->get_ethaddr(edev, ethaddr) =3D=3D 0) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ethaddr_to_string(ethaddr, ethaddr_str); - =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(dev, "got MAC address from EEPR= OM: %s\n", ethaddr_str);
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_set_param(dev, "ethaddr", ethad= dr_str);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (is_valid_ether_addr(ethaddr)) {
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(dev, "got MAC a= ddress from EEPROM: %s\n", ethaddr_str);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_set_param(dev, "etha= ddr", ethaddr_str);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0}

=A0 =A0 =A0 =A0if (!eth_current) {

Sascha


Sounds = good :)
=A0
--
Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 |
Industrial Linux Solutions =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | http://www.pengutronix.de/ = =A0|
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 =A0 = =A0|
Amtsgericht Hildesheim, HRA 2686 =A0 =A0 =A0 =A0 =A0 | Fax: =A0 +49-5121-20= 6917-5555 |

--0015174c45a24ce78304a09fda5b-- --===============0435953739== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox --===============0435953739==--