[U-Boot] Commit ecee9... (Program net device MAC addresses after initializing) breakage
Hi All,
I finally got a few spare hours to do some U-Boot hacking and to my dismay found the build for my board (eNET) had broken. It builds OK, but crashed during Ethernet initialisation. I tried winding back to a last known good commit without much luck.
I realised I had upgraded to Ubuntu 10.10 lately, and thought maybe a tool-chain change was the culprit (was running gcc-4.4.1 and binutils 2.19 from source, now running Ubuntu gcc 4.4.5 and Binutils 2.20)
But, I think that commit ecee9324d73555e744593f3e0d387bec4c566f55 may be 'not quite right'. My board uses the rtl8139 driver, and the following patch gets my board booting again:
diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c index db8a727..3646148 100644 --- a/drivers/net/rtl8139.c +++ b/drivers/net/rtl8139.c @@ -229,6 +229,7 @@ int rtl8139_initialize(bd_t *bis) dev->halt = rtl_disable; dev->send = rtl_transmit; dev->recv = rtl_poll; + dev->write_hwaddr = NULL; #ifdef CONFIG_MCAST_TFTP dev->mcast = rtl_bcast_addr; #endif
Just before this code we see that dev is malloc'd:
dev = (struct eth_device *)malloc(sizeof *dev);
So there is no guarantee that dev is NULL'd.
Has anyone else run into similar problems?
I saw discussion a little while ago regarding implementing a version of malloc that returns cleared memory - did this gain any traction?
Regards,
Graeme
On Nov 6, 2010, at 5:13 PM, Graeme Russ wrote:
Hi All,
I finally got a few spare hours to do some U-Boot hacking and to my dismay found the build for my board (eNET) had broken. It builds OK, but crashed during Ethernet initialisation. I tried winding back to a last known good commit without much luck.
I realised I had upgraded to Ubuntu 10.10 lately, and thought maybe a tool-chain change was the culprit (was running gcc-4.4.1 and binutils 2.19 from source, now running Ubuntu gcc 4.4.5 and Binutils 2.20)
But, I think that commit ecee9324d73555e744593f3e0d387bec4c566f55 may be 'not quite right'. My board uses the rtl8139 driver, and the following patch gets my board booting again:
diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c index db8a727..3646148 100644 --- a/drivers/net/rtl8139.c +++ b/drivers/net/rtl8139.c @@ -229,6 +229,7 @@ int rtl8139_initialize(bd_t *bis) dev->halt = rtl_disable; dev->send = rtl_transmit; dev->recv = rtl_poll;
dev->write_hwaddr = NULL;#ifdef CONFIG_MCAST_TFTP dev->mcast = rtl_bcast_addr; #endif
Just before this code we see that dev is malloc'd:
dev = (struct eth_device *)malloc(sizeof *dev);
So there is no guarantee that dev is NULL'd.
Has anyone else run into similar problems?
I saw discussion a little while ago regarding implementing a version of malloc that returns cleared memory - did this gain any traction?
I believe we are seeing similar behavior w/e1000 driver.
- k
On Saturday, November 06, 2010 18:13:08 Graeme Russ wrote:
I finally got a few spare hours to do some U-Boot hacking and to my dismay found the build for my board (eNET) had broken. It builds OK, but crashed during Ethernet initialisation. I tried winding back to a last known good commit without much luck.
I realised I had upgraded to Ubuntu 10.10 lately, and thought maybe a tool-chain change was the culprit (was running gcc-4.4.1 and binutils 2.19 from source, now running Ubuntu gcc 4.4.5 and Binutils 2.20)
But, I think that commit ecee9324d73555e744593f3e0d387bec4c566f55 may be 'not quite right'. My board uses the rtl8139 driver, and the following patch gets my board booting again:
Just before this code we see that dev is malloc'd:
dev = (struct eth_device *)malloc(sizeof *dev);
So there is no guarantee that dev is NULL'd.
rtl8139 is broken. it should be clearing its memory.
someone posted a whole bunch of patches to memset() net drivers ...
I saw discussion a little while ago regarding implementing a version of malloc that returns cleared memory - did this gain any traction?
i dont think anyone posted a patch. it would make sense though to generalize the zalloc() code since some places are already doing it. -mike
On 10/11/10 14:36, Mike Frysinger wrote:
On Saturday, November 06, 2010 18:13:08 Graeme Russ wrote:
I saw discussion a little while ago regarding implementing a version of malloc that returns cleared memory - did this gain any traction?
i dont think anyone posted a patch. it would make sense though to generalize the zalloc() code since some places are already doing it.
Actually, I think we should be using calloc() more than we are
Regards,
Graeme
On Friday, November 12, 2010 05:55:55 Graeme Russ wrote:
On 10/11/10 14:36, Mike Frysinger wrote:
On Saturday, November 06, 2010 18:13:08 Graeme Russ wrote:
I saw discussion a little while ago regarding implementing a version of malloc that returns cleared memory - did this gain any traction?
i dont think anyone posted a patch. it would make sense though to generalize the zalloc() code since some places are already doing it.
Actually, I think we should be using calloc() more than we are
if we want to define zalloc() to calloc(), then sure. but forcing people to use calloc() with a size=1 is silly. the API is not nearly as obvious to passing observers as zalloc(). -mike
On 13/11/10 12:11, Mike Frysinger wrote:
On Friday, November 12, 2010 05:55:55 Graeme Russ wrote:
On 10/11/10 14:36, Mike Frysinger wrote:
On Saturday, November 06, 2010 18:13:08 Graeme Russ wrote:
I saw discussion a little while ago regarding implementing a version of malloc that returns cleared memory - did this gain any traction?
i dont think anyone posted a patch. it would make sense though to generalize the zalloc() code since some places are already doing it.
Actually, I think we should be using calloc() more than we are
if we want to define zalloc() to calloc(), then sure. but forcing people to use calloc() with a size=1 is silly. the API is not nearly as obvious to passing observers as zalloc().
Hmm, U-Boot has a zalloc in lib/gunzip.c which does not zero memory
void *zalloc(void *x, unsigned items, unsigned size) { void *p;
size *= items; size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1);
p = malloc (size);
return (p); }
and mtd has kzalloc defined thus:
#define kzalloc(size, flags) calloc(size, 1)
So there is some confusion surrounding 'zalloc' anyway ;)
Regards,
Graeme
On Friday, November 12, 2010 20:51:12 Graeme Russ wrote:
On 13/11/10 12:11, Mike Frysinger wrote:
On Friday, November 12, 2010 05:55:55 Graeme Russ wrote:
On 10/11/10 14:36, Mike Frysinger wrote:
On Saturday, November 06, 2010 18:13:08 Graeme Russ wrote:
I saw discussion a little while ago regarding implementing a version of malloc that returns cleared memory - did this gain any traction?
i dont think anyone posted a patch. it would make sense though to generalize the zalloc() code since some places are already doing it.
Actually, I think we should be using calloc() more than we are
if we want to define zalloc() to calloc(), then sure. but forcing people to use calloc() with a size=1 is silly. the API is not nearly as obvious to passing observers as zalloc().
Hmm, U-Boot has a zalloc in lib/gunzip.c which does not zero memory
yes, that's why i said we should generalize things -mike
participants (3)
-
Graeme Russ -
Kumar Gala -
Mike Frysinger