From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Warren Date: Sun, 06 Jul 2008 00:01:42 -0700 Subject: [U-Boot-Users] [PATCH v2] net: Conditional COBJS inclusion of network drivers In-Reply-To: <20080705223203.17B71248DE@gemini.denx.de> References: <20080705223203.17B71248DE@gemini.denx.de> Message-ID: <48706DD6.3040500@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.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 >> --- >> >> 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