public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Ben Warren <biggerbadderben@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] [PATCH v2] net: Conditional COBJS inclusion of network drivers
Date: Sun, 06 Jul 2008 00:01:42 -0700	[thread overview]
Message-ID: <48706DD6.3040500@gmail.com> (raw)
In-Reply-To: <20080705223203.17B71248DE@gemini.denx.de>

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

  parent reply	other threads:[~2008-07-06  7:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-07 16:01 [U-Boot-Users] drivers/net/Makefile: Conditional COBJS inclusion cleanups Shinya Kuribayashi
2008-06-07 16:03 ` [U-Boot-Users] [PATCH 1/10] net: Conditional COBJS inclusion of bcm570x and tigon3 modules Shinya Kuribayashi
2008-06-07 16:04   ` [U-Boot-Users] [PATCH 2/10] net: Conditional COBJS inclusion of Realtek modules Shinya Kuribayashi
2008-06-07 16:06     ` [U-Boot-Users] [PATCH 3/10] net: Conditional COBJS inclusion of Intel modules Shinya Kuribayashi
2008-06-07 16:07     ` [U-Boot-Users] [PATCH 4/10] net: Conditional COBJS inclusion of INCA-IP switch Shinya Kuribayashi
2008-06-07 16:08     ` [U-Boot-Users] [PATCH 5/10] net: Conditional COBJS inclusion of National Semiconductor modules Shinya Kuribayashi
2008-06-07 16:10       ` [U-Boot-Users] [PATCH 6/10] net: Conditional COBJS inclusion of NET+ARM modules Shinya Kuribayashi
2008-06-07 16:11         ` [U-Boot-Users] [PATCH 7/10] net: Conditional COBJS inclusion of TSEC and Vitesse modules Shinya Kuribayashi
2008-06-07 16:12           ` [U-Boot-Users] [PATCH 8/10] net: Conditional COBJS inclusion of SMC modules Shinya Kuribayashi
2008-06-07 16:14             ` [U-Boot-Users] [PATCH 9/10] net: Conditional COBJS inclusion of Freescale FEC modules Shinya Kuribayashi
2008-06-07 16:16               ` [U-Boot-Users] [PATCH 10/10] net: Conditional COBJS inclusino of remainings Shinya Kuribayashi
2008-06-09 13:19                 ` Ben Warren
2008-06-09 13:43                   ` Shinya Kuribayashi
2008-06-09 14:37 ` [U-Boot-Users] [PATCH v2] net: Conditional COBJS inclusion of network drivers Shinya Kuribayashi
2008-07-05 22:32   ` Wolfgang Denk
2008-07-06  4:42     ` Shinya Kuribayashi
2008-07-06 10:56       ` Jean-Christophe PLAGNIOL-VILLARD
2008-07-07  1:20         ` Shinya Kuribayashi
2008-07-06  7:01     ` Ben Warren [this message]
2008-07-06  7:52       ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48706DD6.3040500@gmail.com \
    --to=biggerbadderben@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox