[U-Boot] [PATCH 1/4] net/miiphy/serial: drop duplicate "NAMESIZE" define

[U-Boot] [PATCH 1/4] net/miiphy/serial: drop duplicate "NAMESIZE" define

[Mike Frysinger]

A few subsystems are using the same define "NAMESIZE".  This has been
working so far because they define it to the same number.  However, I
want to change the size of eth_device's NAMESIZE, so rather than tweak
the define names, simply drop references to it.  Almost no one does,
and the handful that do can easily be changed to a sizeof().

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 board/Marvell/db64360/mv_eth.c |    2 +-
 board/Marvell/db64460/mv_eth.c |    2 +-
 board/esd/cpci750/mv_eth.c     |    2 +-
 board/evb64260/eth.c           |    2 +-
 board/prodrive/p3mx/mv_eth.c   |    2 +-
 drivers/net/armada100_fec.c    |    2 +-
 drivers/net/mvgbe.c            |    2 +-
 drivers/qe/uec_phy.c           |    2 +-
 include/miiphy.h               |    2 +-
 include/net.h                  |    4 +---
 include/serial.h               |    5 ++---
 net/eth.c                      |    2 +-
 12 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/board/Marvell/db64360/mv_eth.c b/board/Marvell/db64360/mv_eth.c
index 30304b0..82e08a1 100644
--- a/board/Marvell/db64360/mv_eth.c
+++ b/board/Marvell/db64360/mv_eth.c
@@ -221,7 +221,7 @@ void mv6436x_eth_initialize (bd_t * bis)
 			return;
 		}
 
-		/* must be less than NAMESIZE (16) */
+		/* must be less than sizeof(dev->name) */
 		sprintf (dev->name, "mv_enet%d", devnum);
 
 #ifdef DEBUG
diff --git a/board/Marvell/db64460/mv_eth.c b/board/Marvell/db64460/mv_eth.c
index cd9d5a4..e20ca20 100644
--- a/board/Marvell/db64460/mv_eth.c
+++ b/board/Marvell/db64460/mv_eth.c
@@ -221,7 +221,7 @@ void mv6446x_eth_initialize (bd_t * bis)
 			return;
 		}
 
-		/* must be less than NAMESIZE (16) */
+		/* must be less than sizeof(dev->name) */
 		sprintf (dev->name, "mv_enet%d", devnum);
 
 #ifdef DEBUG
diff --git a/board/esd/cpci750/mv_eth.c b/board/esd/cpci750/mv_eth.c
index 781ad23..06ae6ff 100644
--- a/board/esd/cpci750/mv_eth.c
+++ b/board/esd/cpci750/mv_eth.c
@@ -221,7 +221,7 @@ void mv6436x_eth_initialize (bd_t * bis)
 			return;
 		}
 
-		/* must be less than NAMESIZE (16) */
+		/* must be less than sizeof(dev->name) */
 		sprintf (dev->name, "mv_enet%d", devnum);
 
 #ifdef DEBUG
diff --git a/board/evb64260/eth.c b/board/evb64260/eth.c
index 1492ffc..e96b0f4 100644
--- a/board/evb64260/eth.c
+++ b/board/evb64260/eth.c
@@ -685,7 +685,7 @@ gt6426x_eth_initialize(bd_t *bis)
 			return;
 		}
 
-		/* must be less than NAMESIZE (16) */
+		/* must be less than sizeof(dev->name) */
 		sprintf(dev->name, "gal_enet%d", devnum);
 
 #ifdef DEBUG
diff --git a/board/prodrive/p3mx/mv_eth.c b/board/prodrive/p3mx/mv_eth.c
index 15b3bfc..9e3213b 100644
--- a/board/prodrive/p3mx/mv_eth.c
+++ b/board/prodrive/p3mx/mv_eth.c
@@ -274,7 +274,7 @@ void mv6446x_eth_initialize (bd_t * bis)
 			return;
 		}
 
-		/* must be less than NAMESIZE (16) */
+		/* must be less than sizeof(dev->name) */
 		sprintf (dev->name, "mv_enet%d", devnum);
 
 #ifdef DEBUG
diff --git a/drivers/net/armada100_fec.c b/drivers/net/armada100_fec.c
index fbf9763..0a45f17 100644
--- a/drivers/net/armada100_fec.c
+++ b/drivers/net/armada100_fec.c
@@ -709,7 +709,7 @@ int armada100_fec_register(unsigned long base_addr)
 	/* Assign ARMADA100 Fast Ethernet Controller Base Address */
 	darmdfec->regs = (void *)base_addr;
 
-	/* must be less than NAMESIZE (16) */
+	/* must be less than sizeof(dev->name) */
 	strcpy(dev->name, "armd-fec0");
 
 	dev->init = armdfec_init;
diff --git a/drivers/net/mvgbe.c b/drivers/net/mvgbe.c
index fd13428..9ce5f6b 100644
--- a/drivers/net/mvgbe.c
+++ b/drivers/net/mvgbe.c
@@ -700,7 +700,7 @@ error1:
 
 		dev = &dmvgbe->dev;
 
-		/* must be less than NAMESIZE (16) */
+		/* must be less than sizeof(dev->name) */
 		sprintf(dev->name, "egiga%d", devnum);
 
 		/* Extract the MAC address from the environment */
diff --git a/drivers/qe/uec_phy.c b/drivers/qe/uec_phy.c
index e26218b..ac580a0 100644
--- a/drivers/qe/uec_phy.c
+++ b/drivers/qe/uec_phy.c
@@ -85,7 +85,7 @@
 #endif
 
 struct fixed_phy_port {
-	char name[NAMESIZE];	/* ethernet port name */
+	char name[16];	/* ethernet port name */
 	unsigned int speed;	/* specified speed 10,100 or 1000 */
 	unsigned int duplex;	/* specified duplex FULL or HALF */
 };
diff --git a/include/miiphy.h b/include/miiphy.h
index 7e70cf8..ca5040e 100644
--- a/include/miiphy.h
+++ b/include/miiphy.h
@@ -86,7 +86,7 @@ void mdio_list_devices(void);
 #define BB_MII_DEVNAME	"bb_miiphy"
 
 struct bb_miiphy_bus {
-	char name[NAMESIZE];
+	char name[16];
 	int (*init)(struct bb_miiphy_bus *bus);
 	int (*mdio_active)(struct bb_miiphy_bus *bus);
 	int (*mdio_tristate)(struct bb_miiphy_bus *bus);
diff --git a/include/net.h b/include/net.h
index ad9afbf..b4acd8f 100644
--- a/include/net.h
+++ b/include/net.h
@@ -66,8 +66,6 @@ typedef void rxhand_icmp_f(unsigned type, unsigned code, unsigned dport,
  */
 typedef void	thand_f(void);
 
-#define NAMESIZE 16
-
 enum eth_state_t {
 	ETH_STATE_INIT,
 	ETH_STATE_PASSIVE,
@@ -75,7 +73,7 @@ enum eth_state_t {
 };
 
 struct eth_device {
-	char name[NAMESIZE];
+	char name[16];
 	unsigned char enetaddr[6];
 	int iobase;
 	int state;
diff --git a/include/serial.h b/include/serial.h
index 5926244..2b9e647 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -3,10 +3,9 @@
 
 #include <post.h>
 
-#define NAMESIZE 16
-
 struct serial_device {
-	char name[NAMESIZE];
+	/* enough bytes to match alignment of following func pointer */
+	char name[16];
 
 	int  (*init) (void);
 	int  (*uninit) (void);
diff --git a/net/eth.c b/net/eth.c
index 4280d6d..cdc3234 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -220,7 +220,7 @@ int eth_register(struct eth_device *dev)
 {
 	struct eth_device *d;
 
-	assert(strlen(dev->name) < NAMESIZE);
+	assert(strlen(dev->name) < sizeof(dev->name));
 
 	if (!eth_devices) {
 		eth_current = eth_devices = dev;
-- 
1.7.6.1

[U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

[Mike Frysinger]

The current eth_device leaves a 2 byte hole after "enetaddr" and before
"iobase".  Since the enetaddr member has to be 6 bytes, we might as well
fill that 2 byte hole with something useful.

Further, most device drivers want to load enetaddr from memory into the
hardware as 1 32bit value and 1 16bit value.

So re-arrange the structure slightly, and add an anonymous union to make
eth_device even better:
 - expand the name field to fill the 2 byte hole
 - make sure enetaddr is aligned, and provides 32bit/16bit members

Now device driver code can simply use "dev->enetaddr32" and
"dev->enetaddr16[2]" to access the values without having to manually
shift the bytes out of dev->enetaddr.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 include/net.h |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/net.h b/include/net.h
index b4acd8f..e8c12d9 100644
--- a/include/net.h
+++ b/include/net.h
@@ -73,8 +73,17 @@ enum eth_state_t {
 };
 
 struct eth_device {
-	char name[16];
-	unsigned char enetaddr[6];
+	/* Keep enetaddr at start so it is guaranteed aligned */
+	union {
+		u32 enetaddr32;
+		u16 enetaddr16[3];
+		unsigned char enetaddr[6];
+	};
+	/*
+	 * Note: name size is picked to fill the holes in memory after
+	 * enetaddr, and to match up to alignment for following "int".
+	 */
+	char name[18];
 	int iobase;
 	int state;
 
-- 
1.7.6.1

[U-Boot] [PATCH 3/4] Blackfin: bfin_mac: use new eth_device enetaddr members

[Mike Frysinger]

Simplifies the code nicely.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/net/bfin_mac.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index dcc781a..ab2fe71 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -281,16 +281,8 @@ static int bfin_miiphy_init(struct eth_device *dev, int *opmode)
 
 static int bfin_EMAC_setup_addr(struct eth_device *dev)
 {
-	bfin_write_EMAC_ADDRLO(
-		dev->enetaddr[0] |
-		dev->enetaddr[1] << 8 |
-		dev->enetaddr[2] << 16 |
-		dev->enetaddr[3] << 24
-	);
-	bfin_write_EMAC_ADDRHI(
-		dev->enetaddr[4] |
-		dev->enetaddr[5] << 8
-	);
+	bfin_write_EMAC_ADDRLO(dev->enetaddr32);
+	bfin_write_EMAC_ADDRHI(dev->enetaddr16[2]);
 	return 0;
 }
 
-- 
1.7.6.1

[U-Boot] [PATCH 4/4] usb: net: smsc95xx: attempt to fix enetaddr loading

[Mike Frysinger]

The previous commit (79ad54400932d6484178a GCC4.6: Squash warnings in
smsc95xx.c) broke loading of enetaddr into the hardware.  It changed
the semantics from reading 4 bytes to 1 byte.

Use the new enetaddr members instead to load the values out of memory.
I don't have any hardware to test with, but it *looks* correct, and we
know the current code is def broken ...

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/usb/eth/smsc95xx.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c
index 7ee4f87..f43b4b5 100644
--- a/drivers/usb/eth/smsc95xx.c
+++ b/drivers/usb/eth/smsc95xx.c
@@ -372,26 +372,20 @@ static int smsc95xx_init_mac_address(struct eth_device *eth,
 static int smsc95xx_write_hwaddr(struct eth_device *eth)
 {
 	struct ueth_data *dev = (struct ueth_data *)eth->priv;
-	u32 addr_lo, addr_hi;
 	int ret;
 
 	/* set hardware address */
 	debug("** %s()\n", __func__);
-	addr_lo = cpu_to_le32(*eth->enetaddr);
-	addr_hi = cpu_to_le16(*((u16 *)(eth->enetaddr + 4)));
-	ret = smsc95xx_write_reg(dev, ADDRL, addr_lo);
+	ret = smsc95xx_write_reg(dev, ADDRL, eth->enetaddr32);
 	if (ret < 0) {
 		debug("Failed to write ADDRL: %d\n", ret);
 		return ret;
 	}
 
-	ret = smsc95xx_write_reg(dev, ADDRH, addr_hi);
+	ret = smsc95xx_write_reg(dev, ADDRH, eth->enetaddr16[2]);
 	if (ret < 0)
 		return ret;
-	debug("MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
-		eth->enetaddr[0], eth->enetaddr[1],
-		eth->enetaddr[2], eth->enetaddr[3],
-		eth->enetaddr[4], eth->enetaddr[5]);
+	debug("MAC %pM\n", eth->enetaddr);
 	dev->have_hwaddr = 1;
 	return 0;
 }
-- 
1.7.6.1

[U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

[thomas.langer at lantiq.com]

Hello Mike,

> +	union {
> +		u32 enetaddr32;
> +		u16 enetaddr16[3];
> +		unsigned char enetaddr[6];
> +	};

This will work only as long the endianess is matching.

Picking single chars from enetaddr[] and combine them to a u32 register
will be more independent from endianess.

If this goes in, I would like to see at least a comment about the problem.

Best Regards,
Thomas

[U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1320970267-22297-2-git-send-email-vapier@gentoo.org> you wrote:
> The current eth_device leaves a 2 byte hole after "enetaddr" and before
> "iobase".  Since the enetaddr member has to be 6 bytes, we might as well
> fill that 2 byte hole with something useful.
> 
> Further, most device drivers want to load enetaddr from memory into the
> hardware as 1 32bit value and 1 16bit value.
> 
> So re-arrange the structure slightly, and add an anonymous union to make
> eth_device even better:
>  - expand the name field to fill the 2 byte hole
>  - make sure enetaddr is aligned, and provides 32bit/16bit members

I'm OK with expanding the name[] field, but as Thomas pointed out,
providing "convenient" u32 / u16 variables for the MAC address is
actually a faux ami that misleads people into using these variables
without thinking about endianess and such.

Please omit this part.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
People seldom know what they want until you give them what  they  ask
for.

[U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

[Mike Frysinger]

On Friday 11 November 2011 06:55:45 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > The current eth_device leaves a 2 byte hole after "enetaddr" and before
> > "iobase".  Since the enetaddr member has to be 6 bytes, we might as well
> > fill that 2 byte hole with something useful.
> > 
> > Further, most device drivers want to load enetaddr from memory into the
> > hardware as 1 32bit value and 1 16bit value.
> > 
> > So re-arrange the structure slightly, and add an anonymous union to make
> > 
> > eth_device even better:
> >  - expand the name field to fill the 2 byte hole
> >  - make sure enetaddr is aligned, and provides 32bit/16bit members
> 
> I'm OK with expanding the name[] field, but as Thomas pointed out,
> providing "convenient" u32 / u16 variables for the MAC address is
> actually a faux ami that misleads people into using these variables
> without thinking about endianess and such.
> 
> Please omit this part.

there's always the endian issue.  i'd wager that the vast majority of the 
time, the endian of the target hardware is the same as the core.  so omitting 
this for odd devices or device driver writers who aren't careful is being too 
pessimistic imo.  i can add qualifiers to the name like "enetaddr_cpu32" if you 
want.  looking at the generated code shows really nice improvements: a single 
cpu load instead of 4 loads interspersed with bitshifts.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111111/0821e42c/attachment.pgp 

[U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

[Andy Fleming]

On Fri, Nov 11, 2011 at 9:03 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Friday 11 November 2011 06:55:45 Wolfgang Denk wrote:
>> Mike Frysinger wrote:
>> > The current eth_device leaves a 2 byte hole after "enetaddr" and before
>> > "iobase". ?Since the enetaddr member has to be 6 bytes, we might as well
>> > fill that 2 byte hole with something useful.
>> >
>> > Further, most device drivers want to load enetaddr from memory into the
>> > hardware as 1 32bit value and 1 16bit value.
>> >
>> > So re-arrange the structure slightly, and add an anonymous union to make
>> >
>> > eth_device even better:
>> > ?- expand the name field to fill the 2 byte hole
>> > ?- make sure enetaddr is aligned, and provides 32bit/16bit members
>>
>> I'm OK with expanding the name[] field, but as Thomas pointed out,
>> providing "convenient" u32 / u16 variables for the MAC address is
>> actually a faux ami that misleads people into using these variables
>> without thinking about endianess and such.
>>
>> Please omit this part.
>
> there's always the endian issue. ?i'd wager that the vast majority of the
> time, the endian of the target hardware is the same as the core. ?so omitting
> this for odd devices or device driver writers who aren't careful is being too
> pessimistic imo. ?i can add qualifiers to the name like "enetaddr_cpu32" if you
> want. ?looking at the generated code shows really nice improvements: a single
> cpu load instead of 4 loads interspersed with bitshifts.

All of the Freescale ethernet devices have their MAC address register
in "little-endian" mode. I have no idea why. But this means that the
change would still require shifts, but not it would also require
masking.

Plus, I have to say, accessing a variable via the second array entry
(enetaddr16[2]) is a bit convoluted. If you want your driver to pull
in the address in two loads, and you want C code which indicates that
level of explicit awareness of the layout of a structure, then you
might as well:

addrhi = *((u32 *)(dev->enetaddr));
addrlo = *((u16 *)(&dev->enetaddr[4]));

But I'm pretty sure the TSEC, UCC, and FM drivers will have to
continue doing byte-by-byte stuff.

Andy

[U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

[Mike Frysinger]

On Friday 11 November 2011 10:44:45 Andy Fleming wrote:
> On Fri, Nov 11, 2011 at 9:03 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Friday 11 November 2011 06:55:45 Wolfgang Denk wrote:
> >> Mike Frysinger wrote:
> >> > The current eth_device leaves a 2 byte hole after "enetaddr" and
> >> > before "iobase".  Since the enetaddr member has to be 6 bytes, we
> >> > might as well fill that 2 byte hole with something useful.
> >> > 
> >> > Further, most device drivers want to load enetaddr from memory into
> >> > the hardware as 1 32bit value and 1 16bit value.
> >> > 
> >> > So re-arrange the structure slightly, and add an anonymous union to
> >> > make
> >> > 
> >> > eth_device even better:
> >> >  - expand the name field to fill the 2 byte hole
> >> >  - make sure enetaddr is aligned, and provides 32bit/16bit members
> >> 
> >> I'm OK with expanding the name[] field, but as Thomas pointed out,
> >> providing "convenient" u32 / u16 variables for the MAC address is
> >> actually a faux ami that misleads people into using these variables
> >> without thinking about endianess and such.
> >> 
> >> Please omit this part.
> > 
> > there's always the endian issue.  i'd wager that the vast majority of the
> > time, the endian of the target hardware is the same as the core.  so
> > omitting this for odd devices or device driver writers who aren't
> > careful is being too pessimistic imo.  i can add qualifiers to the name
> > like "enetaddr_cpu32" if you want.  looking at the generated code shows
> > really nice improvements: a single cpu load instead of 4 loads
> > interspersed with bitshifts.
> 
> All of the Freescale ethernet devices have their MAC address register
> in "little-endian" mode. I have no idea why. But this means that the
> change would still require shifts, but not it would also require
> masking.

so you'd use:
	addrhi = cpu_to_le32(dev->enetaddr32);
sounds simple to me

> Plus, I have to say, accessing a variable via the second array entry
> (enetaddr16[2]) is a bit convoluted.

it is.  i couldn't quite figure out how to do an anonymous struct though so the 
eneaddr16 represented just the last set of 16bits.

> If you want your driver to pull
> in the address in two loads, and you want C code which indicates that
> level of explicit awareness of the layout of a structure, then you
> might as well:
> 
> addrhi = *((u32 *)(dev->enetaddr));
> addrlo = *((u16 *)(&dev->enetaddr[4]));

casting like this is ugly, and enetaddr is uchar*, so there is no alignment 
guarantee in the type.  which is why i'm suggesting people do:
	addrhi = __get_unaligned_le32(&dev->enetaddr[0]);
	addrlo = __get_unaligned_le32(&dev->enetaddr[4]);

so this improves things for some arches while keeping the code safe for all.  
but if we have enetaddr32, it'd be even better.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111111/65aa47a4/attachment.pgp 

[U-Boot] [PATCH 1/4] net/miiphy/serial: drop duplicate "NAMESIZE" define

[Andy Fleming]

On Thu, Nov 10, 2011 at 6:11 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> diff --git a/include/net.h b/include/net.h
> index ad9afbf..b4acd8f 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -66,8 +66,6 @@ typedef void rxhand_icmp_f(unsigned type, unsigned code, unsigned dport,
> ?*/
> ?typedef void ? thand_f(void);
>
> -#define NAMESIZE 16
> -
> ?enum eth_state_t {
> ? ? ? ?ETH_STATE_INIT,
> ? ? ? ?ETH_STATE_PASSIVE,
> @@ -75,7 +73,7 @@ enum eth_state_t {
> ?};
>
> ?struct eth_device {
> - ? ? ? char name[NAMESIZE];
> + ? ? ? char name[16];


I like all of the earlier NAMESIZE->sizeof changes, but I'm not as
comfortable with changing the various name declarations. Seems like we
might want named constants for them, just so any dependencies can be
clearly-documented. For instance, in Linux the MDIO bus specifiers are
designed to be bigger than the MDIO device name, and the size
declarations reflect this.

Andy

[U-Boot] [PATCH 1/4] net/miiphy/serial: drop duplicate "NAMESIZE" define

[Mike Frysinger]

On Friday 11 November 2011 17:07:46 Andy Fleming wrote:
> On Thu, Nov 10, 2011 at 6:11 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > diff --git a/include/net.h b/include/net.h
> > index ad9afbf..b4acd8f 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -66,8 +66,6 @@ typedef void rxhand_icmp_f(unsigned type, unsigned
> > code, unsigned dport, */
> >  typedef void   thand_f(void);
> > 
> > -#define NAMESIZE 16
> > -
> >  enum eth_state_t {
> >        ETH_STATE_INIT,
> >        ETH_STATE_PASSIVE,
> > @@ -75,7 +73,7 @@ enum eth_state_t {
> >  };
> > 
> >  struct eth_device {
> > -       char name[NAMESIZE];
> > +       char name[16];
> 
> I like all of the earlier NAMESIZE->sizeof changes, but I'm not as
> comfortable with changing the various name declarations. Seems like we
> might want named constants for them, just so any dependencies can be
> clearly-documented. For instance, in Linux the MDIO bus specifiers are
> designed to be bigger than the MDIO device name, and the size
> declarations reflect this.

sounds like something that can easily be expressed with:
	BUILD_BUG_ON(sizeof(dev->name) < sizeof(mii->name));
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111111/5b759da0/attachment.pgp 

[U-Boot] [PATCH 2/4] net: tweak eth_device layout to simplify enetaddr use

[Wolfgang Denk]

Dear Mike Frysinger,

In message <201111111003.15436.vapier@gentoo.org> you wrote:
>
> > I'm OK with expanding the name[] field, but as Thomas pointed out,
> > providing "convenient" u32 / u16 variables for the MAC address is
> > actually a faux ami that misleads people into using these variables
> > without thinking about endianess and such.
> > 
> > Please omit this part.
> 
> there's always the endian issue.  i'd wager that the vast majority of the 
> time, the endian of the target hardware is the same as the core.  so omitting 
> this for odd devices or device driver writers who aren't careful is being too 
> pessimistic imo.  i can add qualifiers to the name like "enetaddr_cpu32" if you 

No. Please drop this.

> want.  looking at the generated code shows really nice improvements: a single 
> cpu load instead of 4 loads interspersed with bitshifts.

This is neither memory nor performance critical, but it is easy to
misuse and thus dangerous.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There's no honorable way to kill, no gentle way to destroy.  There is
nothing good in war.  Except its ending.
	-- Abraham Lincoln, "The Savage Curtain", stardate 5906.5

[U-Boot] [PATCH 1/4] net/miiphy/serial: drop duplicate "NAMESIZE" define

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1320970267-22297-1-git-send-email-vapier@gentoo.org> you wrote:
> A few subsystems are using the same define "NAMESIZE".  This has been
> working so far because they define it to the same number.  However, I
> want to change the size of eth_device's NAMESIZE, so rather than tweak
> the define names, simply drop references to it.  Almost no one does,
> and the handful that do can easily be changed to a sizeof().
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  board/Marvell/db64360/mv_eth.c |    2 +-
>  board/Marvell/db64460/mv_eth.c |    2 +-
>  board/esd/cpci750/mv_eth.c     |    2 +-
>  board/evb64260/eth.c           |    2 +-
>  board/prodrive/p3mx/mv_eth.c   |    2 +-
>  drivers/net/armada100_fec.c    |    2 +-
>  drivers/net/mvgbe.c            |    2 +-
>  drivers/qe/uec_phy.c           |    2 +-
>  include/miiphy.h               |    2 +-
>  include/net.h                  |    4 +---
>  include/serial.h               |    5 ++---
>  net/eth.c                      |    2 +-
>  12 files changed, 13 insertions(+), 16 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

There were meetings. There were always meetings. And they were  dull,
which is part of the reason they were meetings. Dull likes company.
                                    - Terry Pratchett, _Making_Money_