| Message ID | 1422401245-8452-2-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | RFC |
| Delegated to: | Joe Hershberger |
| Headers | show |
Hi Joe, On 27 January 2015 at 16:27, Joe Hershberger <joe.hershberger@ni.com> wrote: > The current implementation exposes the eth_device struct to code that > needs to access the MAC address. Add a wrapper function for this to > abstract away the pointer for this operation. > > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > --- > > arch/mips/cpu/mips32/au1x00/au1x00_eth.c | 2 +- > arch/powerpc/cpu/mpc8260/ether_fcc.c | 2 +- > arch/powerpc/cpu/mpc85xx/ether_fcc.c | 2 +- > arch/powerpc/cpu/mpc8xx/scc.c | 2 +- > include/net.h | 8 ++++++++ > net/net.c | 2 +- > 6 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c > index 4770f56..535d713 100644 > --- a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c > +++ b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c > @@ -238,7 +238,7 @@ static int au1x00_init(struct eth_device* dev, bd_t * bd){ > } > > /* Put mac addr in little endian */ > -#define ea eth_get_dev()->enetaddr > +#define ea eth_get_ethaddr() > *mac_addr_high = (ea[5] << 8) | (ea[4] ) ; > *mac_addr_low = (ea[3] << 24) | (ea[2] << 16) | I know this is existing code, but (perhaps separately) it might be nice to remove the #define and assign it it to a local variable, i.e.: unsigned char *ea = eth_get_ethaddr(); > (ea[1] << 8) | (ea[0] ) ; > diff --git a/arch/powerpc/cpu/mpc8260/ether_fcc.c b/arch/powerpc/cpu/mpc8260/ether_fcc.c > index f9f15b5..f777ba1 100644 > --- a/arch/powerpc/cpu/mpc8260/ether_fcc.c > +++ b/arch/powerpc/cpu/mpc8260/ether_fcc.c > @@ -299,7 +299,7 @@ static int fec_init(struct eth_device* dev, bd_t *bis) > * it unique by setting a few bits in the upper byte of the > * non-static part of the address. > */ > -#define ea eth_get_dev()->enetaddr > +#define ea eth_get_ethaddr() > pram_ptr->fen_paddrh = (ea[5] << 8) + ea[4]; > pram_ptr->fen_paddrm = (ea[3] << 8) + ea[2]; > pram_ptr->fen_paddrl = (ea[1] << 8) + ea[0]; > diff --git a/arch/powerpc/cpu/mpc85xx/ether_fcc.c b/arch/powerpc/cpu/mpc85xx/ether_fcc.c > index 166dc9e..58d4bfb 100644 > --- a/arch/powerpc/cpu/mpc85xx/ether_fcc.c > +++ b/arch/powerpc/cpu/mpc85xx/ether_fcc.c > @@ -338,7 +338,7 @@ static int fec_init(struct eth_device* dev, bd_t *bis) > * it unique by setting a few bits in the upper byte of the > * non-static part of the address. > */ > -#define ea eth_get_dev()->enetaddr > +#define ea eth_get_ethaddr() > pram_ptr->fen_paddrh = (ea[5] << 8) + ea[4]; > pram_ptr->fen_paddrm = (ea[3] << 8) + ea[2]; > pram_ptr->fen_paddrl = (ea[1] << 8) + ea[0]; > diff --git a/arch/powerpc/cpu/mpc8xx/scc.c b/arch/powerpc/cpu/mpc8xx/scc.c > index 251966b..66e4014 100644 > --- a/arch/powerpc/cpu/mpc8xx/scc.c > +++ b/arch/powerpc/cpu/mpc8xx/scc.c > @@ -339,7 +339,7 @@ static int scc_init (struct eth_device *dev, bd_t * bis) > pram_ptr->sen_gaddr3 = 0x0; /* Group Address Filter 3 (unused) */ > pram_ptr->sen_gaddr4 = 0x0; /* Group Address Filter 4 (unused) */ > > -#define ea eth_get_dev()->enetaddr > +#define ea eth_get_ethaddr() > pram_ptr->sen_paddrh = (ea[5] << 8) + ea[4]; > pram_ptr->sen_paddrm = (ea[3] << 8) + ea[2]; > pram_ptr->sen_paddrl = (ea[1] << 8) + ea[0]; > diff --git a/include/net.h b/include/net.h > index 73ea88b..a9579ee 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -111,6 +111,14 @@ struct eth_device *eth_get_dev(void) > { > return eth_current; > } > + > +static inline unsigned char *eth_get_ethaddr(void) > +{ > + if (eth_current) > + return eth_current->enetaddr; > + return NULL; > +} > + > extern struct eth_device *eth_get_dev_by_name(const char *devname); > extern struct eth_device *eth_get_dev_by_index(int index); /* get dev @ index */ > extern int eth_get_dev_index(void); /* get the device index */ > diff --git a/net/net.c b/net/net.c > index 2bea07b..ddd630c 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -275,7 +275,7 @@ static void NetInitLoop(void) > env_changed_id = env_id; > } > if (eth_get_dev()) > - memcpy(NetOurEther, eth_get_dev()->enetaddr, 6); > + memcpy(NetOurEther, eth_get_ethaddr(), 6); > > return; > } > -- > 1.7.11.5 > Regards, Simon
On Tue, Jan 27, 2015 at 8:33 PM, Simon Glass <sjg@chromium.org> wrote: > > Hi Joe, > > On 27 January 2015 at 16:27, Joe Hershberger <joe.hershberger@ni.com> wrote: > > The current implementation exposes the eth_device struct to code that > > needs to access the MAC address. Add a wrapper function for this to > > abstract away the pointer for this operation. > > > > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > > --- > > > > arch/mips/cpu/mips32/au1x00/au1x00_eth.c | 2 +- > > arch/powerpc/cpu/mpc8260/ether_fcc.c | 2 +- > > arch/powerpc/cpu/mpc85xx/ether_fcc.c | 2 +- > > arch/powerpc/cpu/mpc8xx/scc.c | 2 +- > > include/net.h | 8 ++++++++ > > net/net.c | 2 +- > > 6 files changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c > > index 4770f56..535d713 100644 > > --- a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c > > +++ b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c > > @@ -238,7 +238,7 @@ static int au1x00_init(struct eth_device* dev, bd_t * bd){ > > } > > > > /* Put mac addr in little endian */ > > -#define ea eth_get_dev()->enetaddr > > +#define ea eth_get_ethaddr() > > *mac_addr_high = (ea[5] << 8) | (ea[4] ) ; > > *mac_addr_low = (ea[3] << 24) | (ea[2] << 16) | > > I know this is existing code, but (perhaps separately) it might be > nice to remove the #define and assign it it to a local variable, i.e.: > > unsigned char *ea = eth_get_ethaddr(); I'm sure that if this code is not deleted before it is reworked that this sort of improvement will be made. For this series I just wanted to change as little as possible. I agree that the #define is ugly. > > (ea[1] << 8) | (ea[0] ) ; > > diff --git a/arch/powerpc/cpu/mpc8260/ether_fcc.c b/arch/powerpc/cpu/mpc8260/ether_fcc.c > > index f9f15b5..f777ba1 100644 > > --- a/arch/powerpc/cpu/mpc8260/ether_fcc.c > > +++ b/arch/powerpc/cpu/mpc8260/ether_fcc.c > > @@ -299,7 +299,7 @@ static int fec_init(struct eth_device* dev, bd_t *bis) > > * it unique by setting a few bits in the upper byte of the > > * non-static part of the address. > > */ > > -#define ea eth_get_dev()->enetaddr > > +#define ea eth_get_ethaddr() > > pram_ptr->fen_paddrh = (ea[5] << 8) + ea[4]; > > pram_ptr->fen_paddrm = (ea[3] << 8) + ea[2]; > > pram_ptr->fen_paddrl = (ea[1] << 8) + ea[0]; > > diff --git a/arch/powerpc/cpu/mpc85xx/ether_fcc.c b/arch/powerpc/cpu/mpc85xx/ether_fcc.c > > index 166dc9e..58d4bfb 100644 > > --- a/arch/powerpc/cpu/mpc85xx/ether_fcc.c > > +++ b/arch/powerpc/cpu/mpc85xx/ether_fcc.c > > @@ -338,7 +338,7 @@ static int fec_init(struct eth_device* dev, bd_t *bis) > > * it unique by setting a few bits in the upper byte of the > > * non-static part of the address. > > */ > > -#define ea eth_get_dev()->enetaddr > > +#define ea eth_get_ethaddr() > > pram_ptr->fen_paddrh = (ea[5] << 8) + ea[4]; > > pram_ptr->fen_paddrm = (ea[3] << 8) + ea[2]; > > pram_ptr->fen_paddrl = (ea[1] << 8) + ea[0]; > > diff --git a/arch/powerpc/cpu/mpc8xx/scc.c b/arch/powerpc/cpu/mpc8xx/scc.c > > index 251966b..66e4014 100644 > > --- a/arch/powerpc/cpu/mpc8xx/scc.c > > +++ b/arch/powerpc/cpu/mpc8xx/scc.c > > @@ -339,7 +339,7 @@ static int scc_init (struct eth_device *dev, bd_t * bis) > > pram_ptr->sen_gaddr3 = 0x0; /* Group Address Filter 3 (unused) */ > > pram_ptr->sen_gaddr4 = 0x0; /* Group Address Filter 4 (unused) */ > > > > -#define ea eth_get_dev()->enetaddr > > +#define ea eth_get_ethaddr() > > pram_ptr->sen_paddrh = (ea[5] << 8) + ea[4]; > > pram_ptr->sen_paddrm = (ea[3] << 8) + ea[2]; > > pram_ptr->sen_paddrl = (ea[1] << 8) + ea[0]; > > diff --git a/include/net.h b/include/net.h > > index 73ea88b..a9579ee 100644 > > --- a/include/net.h > > +++ b/include/net.h > > @@ -111,6 +111,14 @@ struct eth_device *eth_get_dev(void) > > { > > return eth_current; > > } > > + > > +static inline unsigned char *eth_get_ethaddr(void) > > +{ > > + if (eth_current) > > + return eth_current->enetaddr; > > + return NULL; > > +} > > + > > extern struct eth_device *eth_get_dev_by_name(const char *devname); > > extern struct eth_device *eth_get_dev_by_index(int index); /* get dev @ index */ > > extern int eth_get_dev_index(void); /* get the device index */ > > diff --git a/net/net.c b/net/net.c > > index 2bea07b..ddd630c 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -275,7 +275,7 @@ static void NetInitLoop(void) > > env_changed_id = env_id; > > } > > if (eth_get_dev()) > > - memcpy(NetOurEther, eth_get_dev()->enetaddr, 6); > > + memcpy(NetOurEther, eth_get_ethaddr(), 6); > > > > return; > > } > > -- > > 1.7.11.5 > > > > Regards, > Simon > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe, On 28 January 2015 at 02:45, Joe Hershberger <joe.hershberger@gmail.com> wrote: > On Tue, Jan 27, 2015 at 8:33 PM, Simon Glass <sjg@chromium.org> wrote: >> >> Hi Joe, >> >> On 27 January 2015 at 16:27, Joe Hershberger <joe.hershberger@ni.com> >> wrote: >> > The current implementation exposes the eth_device struct to code that >> > needs to access the MAC address. Add a wrapper function for this to >> > abstract away the pointer for this operation. >> > >> > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >> > --- >> > >> > arch/mips/cpu/mips32/au1x00/au1x00_eth.c | 2 +- >> > arch/powerpc/cpu/mpc8260/ether_fcc.c | 2 +- >> > arch/powerpc/cpu/mpc85xx/ether_fcc.c | 2 +- >> > arch/powerpc/cpu/mpc8xx/scc.c | 2 +- >> > include/net.h | 8 ++++++++ >> > net/net.c | 2 +- >> > 6 files changed, 13 insertions(+), 5 deletions(-) >> > >> > diff --git a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c >> > b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c >> > index 4770f56..535d713 100644 >> > --- a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c >> > +++ b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c >> > @@ -238,7 +238,7 @@ static int au1x00_init(struct eth_device* dev, bd_t >> > * bd){ >> > } >> > >> > /* Put mac addr in little endian */ >> > -#define ea eth_get_dev()->enetaddr >> > +#define ea eth_get_ethaddr() >> > *mac_addr_high = (ea[5] << 8) | (ea[4] ) ; >> > *mac_addr_low = (ea[3] << 24) | (ea[2] << 16) | >> >> I know this is existing code, but (perhaps separately) it might be >> nice to remove the #define and assign it it to a local variable, i.e.: >> >> unsigned char *ea = eth_get_ethaddr(); > > I'm sure that if this code is not deleted before it is reworked that this > sort of improvement will be made. For this series I just wanted to change > as little as possible. I agree that the #define is ugly. OK sounds good. Regards, Simon
diff --git a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c index 4770f56..535d713 100644 --- a/arch/mips/cpu/mips32/au1x00/au1x00_eth.c +++ b/arch/mips/cpu/mips32/au1x00/au1x00_eth.c @@ -238,7 +238,7 @@ static int au1x00_init(struct eth_device* dev, bd_t * bd){ } /* Put mac addr in little endian */ -#define ea eth_get_dev()->enetaddr +#define ea eth_get_ethaddr() *mac_addr_high = (ea[5] << 8) | (ea[4] ) ; *mac_addr_low = (ea[3] << 24) | (ea[2] << 16) | (ea[1] << 8) | (ea[0] ) ; diff --git a/arch/powerpc/cpu/mpc8260/ether_fcc.c b/arch/powerpc/cpu/mpc8260/ether_fcc.c index f9f15b5..f777ba1 100644 --- a/arch/powerpc/cpu/mpc8260/ether_fcc.c +++ b/arch/powerpc/cpu/mpc8260/ether_fcc.c @@ -299,7 +299,7 @@ static int fec_init(struct eth_device* dev, bd_t *bis) * it unique by setting a few bits in the upper byte of the * non-static part of the address. */ -#define ea eth_get_dev()->enetaddr +#define ea eth_get_ethaddr() pram_ptr->fen_paddrh = (ea[5] << 8) + ea[4]; pram_ptr->fen_paddrm = (ea[3] << 8) + ea[2]; pram_ptr->fen_paddrl = (ea[1] << 8) + ea[0]; diff --git a/arch/powerpc/cpu/mpc85xx/ether_fcc.c b/arch/powerpc/cpu/mpc85xx/ether_fcc.c index 166dc9e..58d4bfb 100644 --- a/arch/powerpc/cpu/mpc85xx/ether_fcc.c +++ b/arch/powerpc/cpu/mpc85xx/ether_fcc.c @@ -338,7 +338,7 @@ static int fec_init(struct eth_device* dev, bd_t *bis) * it unique by setting a few bits in the upper byte of the * non-static part of the address. */ -#define ea eth_get_dev()->enetaddr +#define ea eth_get_ethaddr() pram_ptr->fen_paddrh = (ea[5] << 8) + ea[4]; pram_ptr->fen_paddrm = (ea[3] << 8) + ea[2]; pram_ptr->fen_paddrl = (ea[1] << 8) + ea[0]; diff --git a/arch/powerpc/cpu/mpc8xx/scc.c b/arch/powerpc/cpu/mpc8xx/scc.c index 251966b..66e4014 100644 --- a/arch/powerpc/cpu/mpc8xx/scc.c +++ b/arch/powerpc/cpu/mpc8xx/scc.c @@ -339,7 +339,7 @@ static int scc_init (struct eth_device *dev, bd_t * bis) pram_ptr->sen_gaddr3 = 0x0; /* Group Address Filter 3 (unused) */ pram_ptr->sen_gaddr4 = 0x0; /* Group Address Filter 4 (unused) */ -#define ea eth_get_dev()->enetaddr +#define ea eth_get_ethaddr() pram_ptr->sen_paddrh = (ea[5] << 8) + ea[4]; pram_ptr->sen_paddrm = (ea[3] << 8) + ea[2]; pram_ptr->sen_paddrl = (ea[1] << 8) + ea[0]; diff --git a/include/net.h b/include/net.h index 73ea88b..a9579ee 100644 --- a/include/net.h +++ b/include/net.h @@ -111,6 +111,14 @@ struct eth_device *eth_get_dev(void) { return eth_current; } + +static inline unsigned char *eth_get_ethaddr(void) +{ + if (eth_current) + return eth_current->enetaddr; + return NULL; +} + extern struct eth_device *eth_get_dev_by_name(const char *devname); extern struct eth_device *eth_get_dev_by_index(int index); /* get dev @ index */ extern int eth_get_dev_index(void); /* get the device index */ diff --git a/net/net.c b/net/net.c index 2bea07b..ddd630c 100644 --- a/net/net.c +++ b/net/net.c @@ -275,7 +275,7 @@ static void NetInitLoop(void) env_changed_id = env_id; } if (eth_get_dev()) - memcpy(NetOurEther, eth_get_dev()->enetaddr, 6); + memcpy(NetOurEther, eth_get_ethaddr(), 6); return; }
The current implementation exposes the eth_device struct to code that needs to access the MAC address. Add a wrapper function for this to abstract away the pointer for this operation. Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> --- arch/mips/cpu/mips32/au1x00/au1x00_eth.c | 2 +- arch/powerpc/cpu/mpc8260/ether_fcc.c | 2 +- arch/powerpc/cpu/mpc85xx/ether_fcc.c | 2 +- arch/powerpc/cpu/mpc8xx/scc.c | 2 +- include/net.h | 8 ++++++++ net/net.c | 2 +- 6 files changed, 13 insertions(+), 5 deletions(-)