Subject: Re: [PATCH] Check mac-address first in fsl_soc.c

From: Sergei Shtylyov <[email-masked]>
Date: Tue, 13 Feb 2007 20:33:13 +0300

Hello.

Timur Tabi wrote:

>>    I'm mainly concerned about 85xx. I guess the needed change should 
>> be very alike...

> I've submitted U-Boot patches for 83xx, 85xx, 86xx, and 5xxx to the 
> maintainers.  You'll have to ask them when the patches will be applied.

    In fact, I/'m regretting not fixing this myself -- Andy said it'll be 
fixed RSN and it took about 2 months (actually, more since the issue was first 
noticed by me in October)...

>>> I wanted to keep my patch simple.

>>    That's clearly an over-simplification to repeat the same code thrice.

> Actually, the 2nd and third instances are conditionally compiled, so in 

    I think that's a bad excuse. You could have coded inline function at least.

> most cases only one instance of the code will actually be compiled.  
> gfar_of_init() should probably also be conditionally compiled, so if I 

    But it's not?

> make it a separate function, I'd have to prefix it with something like 
> this:

> #if defined(CONFIG_GIANFAR) || defined(CONFIG_CPM2) || defined(CONFIG_8xx)

> and then I would need to pass a bunch of parameters.  I don't really 
> think that's an improvement.

> Paul, if there are no serious objections, please apply this patch.

    I object -- such practice produces the code that is harder to change and 
which is more likely to get out of sync between the 3 instances later. :-/

WBR, Sergei