[U-Boot-Users] [PATCH v2] net: Conditional COBJS inclusion of network drivers

Ben Warren
2008-07-06T07:01:42Z

Hi Wolfgang,

welcome back...

Wolfgang Denk wrote:
> In message <484D4038.5000707@ruby.dti.ne.jp> you wrote:
>
>> Replace COBJS-y with appropriate driver config names.
>>
>> Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
>> ---
>>
>> Changes v2:
>>
>> * Kill more CONFIG_CMD_NET and CONFIG_NET_MULTI defines from
>> - fsl_mcdmafec.c
>> - mcffec.c
>> - netarm_eth.c
>>
>> * Revised DM9000 part against the net repo.
>>
>
> Hm... Are you absolutely sure your changes, especially the
> CONFIG_CMD_NET and even more the CONFIG_NET_MULTI related ones, do not
> cause any trouble on any systems?
>
> Let's for example check the E1000 network driver. with your changes,
> it will be built (and enabled), if...
>
> ...
>
>> +COBJS-$(CONFIG_E1000) += e1000.o
>>
> ...
>
> ... if CONFIG_E1000 is set.
>
> However, the old code:
>
> ...
>
>> -#if defined(CONFIG_CMD_NET) \
>> - && defined(CONFIG_NET_MULTI) && defined(CONFIG_E1000)
>> -
>>
>
> ...*also* required that CONFIG_CMD_NET *AND* CONFIG_NET_MULTI were
> set, too.
>
> [For the E1000 driver this is easy to verify, as only few boards
> enable this option, but you are changing this for many drivers, so it
> affects many boards...]
>
>
> It seems not obvious to me that your change is really harmless, or
> tested.
>
> Could you please comment?
>
> [Note that I like your patch and would like to apply it (or ask Ben
> to do that), but it seems kind of risly to me...]
>
>
I decided to accept this and push it upstream because it's definitely a
step in the right direction. Addressing your concerns, for any driver:

if CONFIG_CMD_NET isn't defined, networking isn't enabled, and at worst
your image is bigger than necessary because of libnet.a

if CONFIG_NET_MULTI isn't defined, but CONFIG_CMD_NET is, networking
will use the 'old-school' API and if the driver doesn't export
eth_init() etc., you'll get a compile error.

So, IMHO the worst case scenario is a bit of code bloat or compile
error, which aren't disastrous. Run-time errors bad, compile-time, not
so much.

Personally, I want to merge the two networking APIs so this MULTI
business goes away. Baby steps are necessary here.
> Best regards,
>
> Wolfgang Denk
>
>
regards,
Ben