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
next prev 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.