login
Search
--Boundary-01=_GgiKHm+HxLJWJlY Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Hello, the build with the attached .config failed, make ends with: =2E.. CC arch/x86/lib/usercopy_32.o AR arch/x86/lib/lib.a GEN .version CHK include/linux/compile.h UPD include/linux/compile.h CC init/version.o LD init/built-in.o LD .tmp_vmlinux1 drivers/built-in.o: In function `usbnet_set_settings': (.text+0xf1876): undefined reference to `mii_ethtool_sset' drivers/built-in.o: In function `usbnet_get_settings': (.text+0xf1836): undefined reference to `mii_ethtool_gset' drivers/built-in.o: In function `usbnet_get_link': (.text+0xf18d6): undefined reference to `mii_link_ok' drivers/built-in.o: In function `usbnet_nway_reset': (.text+0xf18f6): undefined reference to `mii_nway_restart' make: *** [.tmp_vmlinux1] Error 1 The build was made with : $> make mrproper && make rndconfig && <tweak config file> && make oldconfig= && make Here's the config: # # Automatically generated make config: don't edit # Linux kernel version: 2.6.24-rc1 # Thu Nov 1 19:33:29 2007 # CONFIG_X86_32=3Dy CONFIG_GENERIC_TIME=3Dy CONFIG_GENERIC_CMOS_UPDATE=3Dy CONFIG_CLOCKSOURCE_WATCHDOG=3Dy CONFIG_GENERIC_CLOCKEVENTS=3Dy CONFIG_LOCKDEP_SUPPORT=3Dy CONFIG_STACKTRACE_SUPPORT=3Dy CONFIG_SEMAPHORE_SLEEPERS=3Dy CONFIG_X86=3Dy CONFIG_MMU=3Dy CONFIG_ZONE_DMA=3Dy CONFIG_QUICKLIST=3Dy CONFIG_GENERIC_ISA_DMA=3Dy CONFIG_GENERIC_IOMAP=3Dy CONFIG_GENERIC_BUG=3Dy CONFIG_GENERIC_HWEIGHT=3Dy CONFIG_ARCH_MAY_HAVE_PC_FDC=3Dy CONFIG_DMI=3Dy CONFIG_DEFCONFIG_LIST=3D"/lib/modules/$UNAME_RELEASE/.config" # # General setup # # CONFIG_EXPERIMENTAL is not set CONFIG_BROKEN_ON_SMP=3Dy CONFIG_LOCK_KERNEL=3Dy CONFIG_INIT_ENV_ARG_LIMIT=3D32 CONFIG_LOCALVERSION=3D"" # CONFIG_LOCALVERSION_AUTO is not ...
On Thu, 1 Nov 2007 20:24:54 +0100 Toralf Förster wrote:
The MII functions aren't available unless NET_ETHERNET=y.
Howver, the MII functions aren't always needed...
David, any ideas on this one?
config USB_USBNET
tristate "Multi-purpose USB Networking Framework"
+ depends on NET_ETHERNET if USB_USBNET_MII != n
select MII if USB_USBNET_MII != n
would be handy. But invalid.
Hm, wait. Haven't we seen this before and decided that MII should
---
~Randy
-
It's been several years since I looked at this. It used to behave just fine. Something must have changed in the not-too-distant Some of us keep wanting to see "select" work properly, not omitting dependencies... Re interdependencies MII and NET_ETHERNET, I'll leave that up to the netedev folk. - Dave -
It seems to be an old bug.
The following combination of options is simply an unusual one:
CONFIG_MII=m
CONFIG_USB_USBNET=y
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
I though that had been fixed for ages ... This should do a better job of it. - Dave ========== CUT HERE Simplify handling of the MII-dependent usbnet based adapters: stick to forward dependencies, and explicitly handle the core dependency. Signed-off-by: David Brownell <dbrownell@users.sourceforge.net> --- drivers/net/usb/Kconfig | 23 ++++++++++++----------- drivers/net/usb/usbnet.c | 9 ++++++++- 2 files changed, 20 insertions(+), 12 deletions(-) --- a.orig/drivers/net/usb/Kconfig 2007-10-21 10:35:16.000000000 -0700 +++ a/drivers/net/usb/Kconfig 2007-11-02 11:32:15.000000000 -0700 @@ -93,13 +93,8 @@ config USB_RTL8150 To compile this driver as a module, choose M here: the module will be called rtl8150. -config USB_USBNET_MII - tristate - default n - config USB_USBNET tristate "Multi-purpose USB Networking Framework" - select MII if USB_USBNET_MII != n ---help--- This driver supports several kinds of network links over USB, with "minidrivers" built around a common network driver core @@ -131,11 +126,19 @@ config USB_USBNET To compile this driver as a module, choose M here: the module will be called usbnet. +# usbnet core will support MII when MII is static, or both are modules +config USB_NET_MII + tristate + depends on USB_USBNET && NET_ETHERNET && (MII = y || MII = USB_USBNET) + default MII + +comment "MII support is needed for most Ethernet adapters" + depends on USB_USBNET && USB_NET_MII=n + config USB_NET_AX8817X tristate "ASIX AX88xxx Based USB 2.0 Ethernet Adapters" - depends on USB_USBNET && NET_ETHERNET + depends on USB_USBNET && USB_NET_MII select CRC32 - select USB_USBNET_MII default y help This option adds support for ASIX AX88xxx based USB 2.0 @@ -188,9 +191,8 @@ config USB_NET_CDCETHER config USB_NET_DM9601 tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices" - depends on USB_USBNET + depends on USB_USBNET && USB_NET_MII select CRC32 - select ...
This approach has two disadvantages:
- it's complicated
- the MII stuff is an implementation detail, and we shouldn't bother
the user with it (especially since we can do better)
If you want to keep the #ifdef's, what's the problem with the second
patch I proposed to fix this bug?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
That's a Kconfig policy that's not always followed. In this case, I was getting fed up with "select". It so rarely does what it needs to do, and I've started to think it'd be better For one thing, I didn't see it until after I posted this one... other than that, the basic approach could well be fine; I didn't go through it in detail. But on the other hand, it seems that only the ASIX code will work right; the DM9601 and MCS7830 Kconfig is different/wrong. - Dave -
Sure it's not yet always followed. But kernel developers have to become more aware that the vast majority of kconfig users are not kernel hackers and act accordingly. I'm not talking about the infamous Aunt Tillie, but being able to I'm not seeing the problem. cu Adrian [1] http://www.lpi.org/en/lpi/english/certification/the_lpic_program/exam_102_detailed_obj... -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -
Notice how only the ASIX kconfig depended on NET_ETHERNET... since MII depends on NET_ETHERNET, and (last I knew) the reverse dependencies didn't capture the complete dependency tree, selecting only MII would leave out some stuff. -
Except for one s390 net driver (I'll check why it's doing this) the
NET_ETHERNET option does not influence what code is being generated -
it's just a Kconfig-internal option allowing to disable a huge bunch
of drivers at once.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
Drivers like ... AX88xxx, DM9601, and MCS7830!! Except as it turns out, only the first one behaves as intended. You can tell it's a problem by the way it's inconsistent, regardless of the details of the problem. :) - Dave -
I'm all for cleanups that make things consistent. :)
As long as we can agree that there's a difference between a problem like
a compile or runtime error and an opportunity for making things
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
Damn, I shouldn't have only grep'ed under drivers/.
@davem:
Please look at net/ipv4/arp.c:arp_process()
Am I right that CONFIG_NET_ETHERNET=n and CONFIG_NETDEV_1000=y or
CONFIG_NETDEV_10000=y will not be handled correctly there?
And the best solution is to nuke all #ifdef's in this function and make
the code unconditionally available?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
From: Adrian Bunk <bunk@kernel.org> I think removing those specific ifdefs in arp_process() is the best option, yes. -
Patch below. cu Adrian <-- snip --> The #ifdef's in arp_process() were not only a mess, they were also wrong in the CONFIG_NET_ETHERNET=n and (CONFIG_NETDEV_1000=y or CONFIG_NETDEV_10000=y) cases. Since they are not required this patch removes them. Also removed are some #ifdef's around #include's that caused compile errors after this change. Signed-off-by: Adrian Bunk <bunk@kernel.org> --- net/ipv4/arp.c | 19 ------------------- 1 file changed, 19 deletions(-) 759b820456b1400b2a6b061eca9667bf7a6f053d diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index 36d6798..0c5d549 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -111,12 +111,8 @@ #include <net/tcp.h> #include <net/sock.h> #include <net/arp.h> -#if defined(CONFIG_AX25) || defined(CONFIG_AX25_MODULE) #include <net/ax25.h> -#if defined(CONFIG_NETROM) || defined(CONFIG_NETROM_MODULE) #include <net/netrom.h> -#endif -#endif #if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE) #include <net/atmclip.h> struct neigh_table *clip_tbl_hook; @@ -731,20 +727,10 @@ static int arp_process(struct sk_buff *skb) htons(dev_type) != arp->ar_hrd) goto out; break; -#ifdef CONFIG_NET_ETHERNET case ARPHRD_ETHER: -#endif -#ifdef CONFIG_TR case ARPHRD_IEEE802_TR: -#endif -#ifdef CONFIG_FDDI case ARPHRD_FDDI: -#endif -#ifdef CONFIG_NET_FC case ARPHRD_IEEE802: -#endif -#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_TR) || \ - defined(CONFIG_FDDI) || defined(CONFIG_NET_FC) /* * ETHERNET, Token Ring and Fibre Channel (which are IEEE 802 * devices, according to RFC 2625) devices will accept ARP @@ -759,21 +745,16 @@ static int arp_process(struct sk_buff *skb) arp->ar_pro != htons(ETH_P_IP)) goto out; break; -#endif -#if defined(CONFIG_AX25) || defined(CONFIG_AX25_MODULE) case ARPHRD_AX25: if (arp->ar_pro != htons(AX25_P_IP) || arp->ar_hrd != htons(ARPHRD_AX25)) goto out; break; -#if ...
Thanks Adrian. Patch applied to net-2.6. Do we need this for stable too? Chers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -
Unless I'm misunderstanding the code we currently wrongly ignore
some ARP packages based on the setting of an unrelated option, so
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
Despite all what you do in Kconfig and what you wrongly blame on
"select" the bug is in usbnet.c and this fix to usbnet.c _alone_
would be enough to fix the bug.
But since you said you care about not including bloat you should better
take my second patch that results in smaller code in some configurations.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
David, I hate to say this and point you out like this, but you are a real cancer for bug fixes to USB things in the kernel, and I'm very tired of seeing things stuck in the mud (and engineering resources wasted) because of how you handle things. It's very bad for Linux, and the USB code in particular. If I had a nickel for every patch from someone else you grinded into the ground and stalled I'd truly be a millionare. You absolutely stifle development progress. I thought my OHCI deadlock patch was an isolated case (and nothing is still applied, which is just awesome, my original patch was posted more than a month ago), but you're doing the same exact thing to Adrian here too. You want to see things fixed your way. But you can get away with the if, and only if, you can spend every day working on your own version of fixes when you don't like the submitters version. But unlike me you don't have that luxury so you have to give patch submitters a larger level of freedom and, plainly, just "let go". -
You didn't hate it enough to find a way to deal with your issue that doesn't involve namecalling or other flamage, I'll note. I know, I know ... you wouldn't want anyone to get the mistaken impression that LKML is one of those easy-to-get-along-on mailing Hyperbole helps too... Twenty million patches, surely from some large fraction of a million kernel patch submitters ... what a crazy dream! (An enviable one, maybe; but way below the last statistics on the subject which I read.) As for "grind into the ground and stall" ... clearly that's I'm not sure why the patch I signed off on didn't merge either; that was specificially related to the OHCI part of that problem. Hmm, digging through my mail I see several other patches doing the same thing were also floating around. None of them had any improvement over what I had signed off on. Maybe there was just too much confusion for Greg to want to merge the one which I'd already signed off on... It hasn't been removed from my merge queue, since it's not yet shown up from kernel.org ... which means that it'd be one of the patches to be resubmitted before we get much deeper into this particular RC series. In fact, I just resent it (with a few minor tweaks, essentially comment updates). Of course, *YOU* are the roadblock on the more generic side of that problem. I don't recall hearing back from you about the minor/obvious update to the HUB+EHCI patch adding a msleep() to bypass the hardware race you reported. That is, other than your refusal to even try letting the three or more clock domains finish synchronizing, before releasing that lock and then starting something that relied on that synch Hmm, and there I was prepared to sign off on Adrian's last patch. True, I hadn't yet done so. But there were several workable fixes in hand, plus a trivial workaround ... I just didn't make time to try that one out over the weekend. I'm glad you grabbed the patch I would have signed off on, if I'd You know, ...
All this USB_USBNET_MII trickery is simply not worth it considering how few code it saves. As a side effect, this also fixes the following compile error reported by Toralf Förster: <-- snip --> ... LD .tmp_vmlinux1 drivers/built-in.o: In function `usbnet_set_settings': (.text+0xf1876): undefined reference to `mii_ethtool_sset' drivers/built-in.o: In function `usbnet_get_settings': (.text+0xf1836): undefined reference to `mii_ethtool_gset' drivers/built-in.o: In function `usbnet_get_link': (.text+0xf18d6): undefined reference to `mii_link_ok' drivers/built-in.o: In function `usbnet_nway_reset': (.text+0xf18f6): undefined reference to `mii_nway_restart' make: *** [.tmp_vmlinux1] Error 1 <-- snip --> Signed-off-by: Adrian Bunk <bunk@kernel.org> --- drivers/net/usb/Kconfig | 9 +-------- drivers/net/usb/usbnet.c | 7 ------- 2 files changed, 1 insertion(+), 15 deletions(-) 3b7f6290c639b9042fead1698fdbe1c84132c953 diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig index 5a96d74..a12c9c4 100644 --- a/drivers/net/usb/Kconfig +++ b/drivers/net/usb/Kconfig @@ -93,13 +93,9 @@ config USB_RTL8150 To compile this driver as a module, choose M here: the module will be called rtl8150. -config USB_USBNET_MII - tristate - default n - config USB_USBNET tristate "Multi-purpose USB Networking Framework" - select MII if USB_USBNET_MII != n + select MII ---help--- This driver supports several kinds of network links over USB, with "minidrivers" built around a common network driver core @@ -135,7 +131,6 @@ config USB_NET_AX8817X tristate "ASIX AX88xxx Based USB 2.0 Ethernet Adapters" depends on USB_USBNET && NET_ETHERNET select CRC32 - select USB_USBNET_MII default y help This option adds support for ASIX AX88xxx based USB 2.0 @@ -190,7 +185,6 @@ config USB_NET_DM9601 tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices" depends on USB_USBNET select CRC32 - select ...
Depends on what systems you're talking about. Forcing unused code into the kernel is not free, especially if that's made into Why not just fix the thing which changed and broke the build? Or if reverse dependencies can't be made to work sanely, then have those Ethernet-adapter minidrivers depend on NET_ETHERNET and then select MII. (To make the relationships be simple enough that current Kconfig can handle them.) I have a fair number of usbnet devices. Not one of them needs MII or NET_ETHERNET. - Dave -
If it was turned into a design policy... My impression is that in some parts of the kernel every byte gets counted, while in other parts noone would notice a few kilobytes more or Today it's exactly one year since your commit entered the tree. It's not Kconfig's fault that you test for the wrong variable in I don't understand why you think this bug was in any way related to NET_ETHERNET - set NET_ETHERNET=y and the bug is still present. If you insist on the #ifdef's take the patch below - it even saves a few additional bytes if you have non-usbnet net drivers requiring MII enabled statically or as modules in your .config but no usbnet drivers cu Adrian <-- snip --> This patch fixes the following compile error with CONFIG_MII=m, CONFIG_USB_USBNET=y, CONFIG_USB_USBNET_MII=n: <-- snip --> ... LD .tmp_vmlinux1 drivers/built-in.o: In function `usbnet_set_settings': (.text+0xf1876): undefined reference to `mii_ethtool_sset' drivers/built-in.o: In function `usbnet_get_settings': (.text+0xf1836): undefined reference to `mii_ethtool_gset' drivers/built-in.o: In function `usbnet_get_link': (.text+0xf18d6): undefined reference to `mii_link_ok' drivers/built-in.o: In function `usbnet_nway_reset': (.text+0xf18f6): undefined reference to `mii_nway_restart' make: *** [.tmp_vmlinux1] Error 1 <-- snip --> This bug was introduced by commit 18ee91fa9815fa3bb4e51cdcb8229bd0a0f11a70 and reported by Toralf Förster. Signed-off-by: Adrian Bunk <bunk@kernel.org> --- BTW: The Kconfig part of this patch is not really required, but testing for #if defined(CONFIG_USB_USBNET_MII) || defined(CONFIG_USB_USBNET_MII_MODULE) would look needlessly ugly. drivers/net/usb/Kconfig | 5 ++--- drivers/net/usb/usbnet.c | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) a421e4910eb30b140a315e274632e87c7a218df6 diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig index 5a96d74..9261371 100644 --- ...
This is the patch I liked better, if there was going to be one going upstream without additional test from me ... sorry, the patch you appear to have accepted was the worst of the -
From: Adrian Bunk <bunk@kernel.org> Applied, thanks Adrian. -
