[U-Boot] [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support

[Dirk Behme <dirk.behme@googlemail.com>]

Ben Warren wrote:
> dirk.behme at googlemail.com wrote:
> 
>> Subject: [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
>>
>> From: Dirk Behme <dirk.behme@gmail.com>
>>
>> Add I2C and network support
>>
>>   
> 
> I2C and network bits are kinda unrelated, don't you think?
> <snip>

Ben: Yes, you are right ;) The OMAP3 patch is ~300k and I tried to put 
this in as less patches as possible while having no patch > 40k. This 
resulted in some more or less unrelated code in one patch. Alternative 
would be to have more than the ~11 patches at the list we already have.

>> Index: u-boot_master/net/eth.c
>> ===================================================================
>> --- u-boot_master.orig/net/eth.c
>> +++ u-boot_master/net/eth.c
>> @@ -508,7 +508,7 @@ extern int emac4xx_miiphy_initialize(bd_
>>  extern int mcf52x2_miiphy_initialize(bd_t *bis);
>>  extern int ns7520_miiphy_initialize(bd_t *bis);
>>  extern int davinci_eth_miiphy_initialize(bd_t *bis);
>> -
>> +extern int eth_init(bd_t *bd);
>>  
>>  int eth_initialize(bd_t *bis)
>>  {
>> @@ -532,6 +532,9 @@ int eth_initialize(bd_t *bis)
>>  #if defined(CONFIG_DRIVER_TI_EMAC)
>>      davinci_eth_miiphy_initialize(bis);
>>  #endif
>> +#if defined(CONFIG_DRIVER_SMC911X)
>> +    eth_init(bis);
>>   
> 
> This isn't the right place to call eth_init(). I know the namespaces are 
> pretty convoluted, but the eth_initialize() family of functions are 
> intended to do things like register devices, initialize data structures 
> etc. without actually enabling the device. eth_init() enables a device. 
> The SMC911X driver doesn' t have such a thing, which is why none of the 
> other boards that use this chip have anything in this file.

Mani, Steve: Any comments on this?

Ben: Any hint where in existing code it is done right to take this as 
example?

Thanks

Dirk