Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wednesday 01 November 2006 11:15 pm, Greg KH wrote:
Argh, there were just too many different versions of these patches
floating around. Can you resend the final versions please?
This should replace BOTH of Randy's patches. It addresses all the
issues I've heard raised, and resolves the regresssion introduced
when adding the mcs7830 minidriver.
- Dave
CUT HERE
Fix mcs7830 patch
The recent mcs7830 update to make the MII support sharable goofed various
pre-existing configurations in two ways:
- it made the usbnet infrastructure reference MII symbols even
when they're not needed in the kernel being built
- it didn't enable MII along with the mcs7830 minidriver
This patch fixes these two problems.
However, there does seem to be a Kconfig reverse dependency bug in that MII
gets wrongly enabled in some cases (like USBNET=y and USBNET_MII=n); I think
I've noticed that same problem in other situations too. So the result can
mean kernels being bloated by stuff that's needlessly enabled ... better
than wrongly being disabled, but contributing to bloat.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Index: at91/drivers/usb/net/Kconfig
===
--- at91.orig/drivers/usb/net/Kconfig 2006-11-02 10:58:49.0 -0800
+++ at91/drivers/usb/net/Kconfig2006-11-02 12:10:13.0 -0800
@@ -92,8 +92,13 @@ 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 USBNET_MII != n
---help---
This driver supports several kinds of network links over USB,
with minidrivers built around a common network driver core
@@ -129,7 +134,7 @@ config USB_NET_AX8817X
tristate ASIX AX88xxx Based USB 2.0 Ethernet Adapters
depends on USB_USBNET NET_ETHERNET
select CRC32
- select MII
+ select USB_USBNET_MII
default y
help
This option adds support for ASIX AX88xxx based USB 2.0
@@ -210,6 +215,7 @@ config USB_NET_PLUSB
config USB_NET_MCS7830
tristate MosChip MCS7830 based Ethernet adapters
depends on USB_USBNET
+ select USB_USBNET_MII
help
Choose this option if you're using a 10/100 Ethernet USB2
adapter based on the MosChip 7830 controller. This includes
Index: at91/drivers/usb/net/usbnet.c
===
--- at91.orig/drivers/usb/net/usbnet.c 2006-11-02 10:58:49.0 -0800
+++ at91/drivers/usb/net/usbnet.c 2006-11-02 11:09:29.0 -0800
@@ -669,6 +669,9 @@ done:
* they'll probably want to use this base set.
*/
+#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#define HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
@@ -699,20 +702,6 @@ int usbnet_set_settings (struct net_devi
}
EXPORT_SYMBOL_GPL(usbnet_set_settings);
-
-void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct usbnet *dev = netdev_priv(net);
-
- /* REVISIT don't always return usbnet */
- strncpy (info-driver, driver_name, sizeof info-driver);
- strncpy (info-version, DRIVER_VERSION, sizeof info-version);
- strncpy (info-fw_version, dev-driver_info-description,
- sizeof info-fw_version);
- usb_make_path (dev-udev, info-bus_info, sizeof info-bus_info);
-}
-EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-
u32 usbnet_get_link (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
@@ -730,40 +719,57 @@ u32 usbnet_get_link (struct net_device *
}
EXPORT_SYMBOL_GPL(usbnet_get_link);
-u32 usbnet_get_msglevel (struct net_device *net)
+int usbnet_nway_reset(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- return dev-msg_enable;
+ if (!dev-mii.mdio_write)
+ return -EOPNOTSUPP;
+
+ return mii_nway_restart(dev-mii);
}
-EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_nway_reset);
-void usbnet_set_msglevel (struct net_device *net, u32 level)
+#endif /* HAVE_MII */
+
+void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
{
struct usbnet *dev = netdev_priv(net);
- dev-msg_enable = level;
+ /* REVISIT don't always return usbnet */
+ strncpy (info-driver, driver_name, sizeof info-driver);
+ strncpy (info-version, DRIVER_VERSION, sizeof info-version);
+ strncpy (info-fw_version, dev-driver_info-description,
+ sizeof info-fw_version);
+ usb_make_path (dev-udev, info-bus_info, sizeof info-bus_info);
}
-EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-int
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Tuesday 31 October 2006 5:23 pm, Adrian Bunk wrote: select MII if USB_NET_AX8817X!=n || USB_NET_MCS7830!=n Thing is, I'm seeing that get morphed inside Kconfig to select MII in some cases ... the if x != n gets ignored, MII can't be deselected. That looks to me like a Kconfig dependency engine bug, so I'm just noting it here rather than fixing it. I guess it's not quite enough of a Prolog engine ... ;) - Dave - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Thu, Nov 02, 2006 at 12:29:12PM -0800, David Brownell wrote: On Wednesday 01 November 2006 11:15 pm, Greg KH wrote: Argh, there were just too many different versions of these patches floating around. Can you resend the final versions please? This should replace BOTH of Randy's patches. It addresses all the issues I've heard raised, and resolves the regresssion introduced when adding the mcs7830 minidriver. It seems to lack the select MII at the USB_RTL8150 option that was in Randy's first patch? - Dave ... --- at91.orig/drivers/usb/net/Kconfig 2006-11-02 10:58:49.0 -0800 +++ at91/drivers/usb/net/Kconfig 2006-11-02 12:10:13.0 -0800 @@ -92,8 +92,13 @@ 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 USBNET_MII != n ---help--- This driver supports several kinds of network links over USB, with minidrivers built around a common network driver core @@ -129,7 +134,7 @@ config USB_NET_AX8817X tristate ASIX AX88xxx Based USB 2.0 Ethernet Adapters depends on USB_USBNET NET_ETHERNET select CRC32 - select MII + select USB_USBNET_MII default y help This option adds support for ASIX AX88xxx based USB 2.0 @@ -210,6 +215,7 @@ config USB_NET_PLUSB config USB_NET_MCS7830 tristate MosChip MCS7830 based Ethernet adapters depends on USB_USBNET + select USB_USBNET_MII help Choose this option if you're using a 10/100 Ethernet USB2 adapter based on the MosChip 7830 controller. This includes Index: at91/drivers/usb/net/usbnet.c === --- at91.orig/drivers/usb/net/usbnet.c2006-11-02 10:58:49.0 -0800 +++ at91/drivers/usb/net/usbnet.c 2006-11-02 11:09:29.0 -0800 ... 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 - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Thursday 02 November 2006 6:27 pm, Adrian Bunk wrote: It seems to lack the select MII at the USB_RTL8150 option that was in Randy's first patch? I was just addressing the usbnet issues ... that driver doesn't use the usbnet framework. - Dave - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Thu, 2 Nov 2006, David Brownell wrote: On Thursday 02 November 2006 6:27 pm, Adrian Bunk wrote: It seems to lack the select MII at the USB_RTL8150 option that was in Randy's first patch? I was just addressing the usbnet issues ... that driver doesn't use the usbnet framework. and Randy is away for a few days with very limited net access. -- ~Randy - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wed, Oct 25, 2006 at 07:22:08PM -0700, David Brownell wrote: On Wednesday 25 October 2006 4:58 pm, Randy Dunlap wrote: On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote: Instead, usbnet.c should #ifdef the relevant ethtool hooks according to CONFIG_MII ... since it's completely legit to use usbnet with peripherals that don't need MII. I had in mind something simpler -- #ifdeffing the entire functions, as in this patch. It looks more complicated than it is, because diff gets confused by moving two functions earlier in the file. (Thanks for starting this, Randy ... these two patches should be merged before RC4 ships.) Argh, there were just too many different versions of these patches floating around. Can you resend the final versions please? thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Tue, Oct 31, 2006 at 10:40:15AM -0700, David Brownell wrote: +#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) +#define HAVE_MII ... This seems to cause a CONFIG_USB_USBNET=y, CONFIG_MII=m breakage (as already described earlier in this thread)? Well, alluded to not described. Fixable by the equivalent of config USB_USBNET ... depends on MII if MII != n except that Kconfig doesn't comprehend conditionals like that. You can express this in Kconfig: depends MII || MII=n But my suggestion was: #if defined(CONFIG_MII) || (defined(CONFIG_MII_MODULE) defined(MODULE)) Or simply select MII ... 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 - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
+#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) +#define HAVE_MII ... This seems to cause a CONFIG_USB_USBNET=y, CONFIG_MII=m breakage (as already described earlier in this thread)? Well, alluded to not described. Fixable by the equivalent of config USB_USBNET ... depends on MII if MII != n except that Kconfig doesn't comprehend conditionals like that. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
... depends on MII if MII != n except that Kconfig doesn't comprehend conditionals like that. You can express this in Kconfig: depends MII || MII=n Except that: Warning! Found recursive dependency: USB_USBNET USB_NET_AX8817X MII USB_USBNET I think this is another case where Kconfig gets in the way and forces introduction of a pseudovariable. I'll give that a try. But my suggestion was: #if defined(CONFIG_MII) || (defined(CONFIG_MII_MODULE) defined(MODULE)) Or simply select MII ... Nope; those both prevent completely legit configurations. MII is not required, except for those two adapter options. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Tue, Oct 31, 2006 at 11:36:52AM -0800, David Brownell wrote: ... depends on MII if MII != n except that Kconfig doesn't comprehend conditionals like that. You can express this in Kconfig: depends MII || MII=n Except that: Warning! Found recursive dependency: USB_USBNET USB_NET_AX8817X MII USB_USBNET I think this is another case where Kconfig gets in the way and forces introduction of a pseudovariable. I'll give that a try. But my suggestion was: #if defined(CONFIG_MII) || (defined(CONFIG_MII_MODULE) defined(MODULE)) Or simply select MII ... Nope; those both prevent completely legit configurations. MII is not required, except for those two adapter options. What should work (with the USB_NET_MCS7830 part from Randy's patch removed) together with your patch and the #if defined(CONFIG_MII) || (defined(CONFIG_MII_MODULE) defined(MODULE)) is: config USB_USBNET tristate Multi-purpose USB Networking Framework select MII if USB_NET_AX8817X!=n || USB_NET_MCS7830!=n ---help--- ... 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 - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wed, Oct 25, 2006 at 04:58:58PM -0700, Randy Dunlap wrote: On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote: Instead, usbnet.c should #ifdef the relevant ethtool hooks according to CONFIG_MII ... since it's completely legit to use usbnet with peripherals that don't need MII. --- From: Randy Dunlap [EMAIL PROTECTED] usbnet driver should use mii_*() interfaces if they are available in the kernel (config enabled) but usbnet does not require or depend on these interfaces. Build tested with CONFIG_MII=y, m, n. This is really awkward and against what we do in any other driver. Lots of PCI ethernet drivers use the MII code but have non-MII variants, and I'd expect usb code to do the same. If you really need to squeeze the last bytes out of usbnet for some embedded thing add a CONFIG_USB_NET_MII opention and explain in the help text which devices require it. Otherwise a normal user has no way to find out why his mii-requiring usb device randomly stopped working. Signed-off-by: Randy Dunlap [EMAIL PROTECTED] --- drivers/usb/net/usbnet.c | 18 ++ 1 file changed, 18 insertions(+) --- linux-2619-rc3-pv.orig/drivers/usb/net/usbnet.c +++ linux-2619-rc3-pv/drivers/usb/net/usbnet.c @@ -47,6 +47,12 @@ #define DRIVER_VERSION 22-Aug-2005 +#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) +#define HAVE_MII 1 +#else +#define HAVE_MII 0 +#endif + /*-*/ @@ -676,7 +682,10 @@ int usbnet_get_settings (struct net_devi if (!dev-mii.mdio_read) return -EOPNOTSUPP; +#if HAVE_MII return mii_ethtool_gset(dev-mii, cmd); +#endif + return -EOPNOTSUPP; } EXPORT_SYMBOL_GPL(usbnet_get_settings); @@ -688,7 +697,11 @@ int usbnet_set_settings (struct net_devi if (!dev-mii.mdio_write) return -EOPNOTSUPP; +#if HAVE_MII retval = mii_ethtool_sset(dev-mii, cmd); +#else + retval = -EOPNOTSUPP; +#endif /* link speed/duplex might have changed */ if (dev-driver_info-link_reset) @@ -721,9 +734,11 @@ u32 usbnet_get_link (struct net_device * if (dev-driver_info-check_connect) return dev-driver_info-check_connect (dev) == 0; +#if HAVE_MII /* if the device has mii operations, use those */ if (dev-mii.mdio_read) return mii_link_ok(dev-mii); +#endif /* Otherwise, say we're up (to avoid breaking scripts) */ return 1; @@ -753,7 +768,10 @@ int usbnet_nway_reset(struct net_device if (!dev-mii.mdio_write) return -EOPNOTSUPP; +#if HAVE_MII return mii_nway_restart(dev-mii); +#endif + return -EOPNOTSUPP; } EXPORT_SYMBOL_GPL(usbnet_nway_reset); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ---end quoted text--- - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Saturday 28 October 2006 2:13 pm, Christoph Hellwig wrote: I still don't quite like the approach. What about simply putting the mii using functions into usbnet-mii.c and let makefile doing all the work? This would require a second set of ethtool ops, but I'd actually consider that a cleanup, as it makes clear which one we're using and allows to kill all the checks for non-mii hardware in the methods. Feel free to do such a patch yourself, so long as the MII doesn't go into a separate module. You'll have to also modify the two Ethernet adapters currently using that usbnet core code (and MII). That'd still feel like needless complexity to me, though. So unless you're keen on doing that quickly, I'd say to just merge the two existing patches and be done with it. - Dave - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Saturday 28 October 2006 4:21 am, Christoph Hellwig wrote:
This is really awkward and against what we do in any other driver.
Awkward, yes -- which is why I posted the non-awkward version,
which is repeated below. (No thanks to diff for making the
patch ugly though; the resulting code is clean and non-awkward,
moving that function helped.)
Against what other drivers do? Since usbnet.c is infrastructure
code, not a driver, your comment can't apply. Infrastructure uses
conditional compilation routinely in such cases.
But remember that the actual drivers follow the standard convention
(select MII) given Randy's patch #1 of 2.
- Dave
-
The usbnet infrastructure must not reference MII symbols unless they're
provided in the kernel being built. This extends also to the ethtool
hooks that reference those symbols.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Index: g26/drivers/usb/net/usbnet.c
===
--- g26.orig/drivers/usb/net/usbnet.c 2006-10-24 18:29:28.0 -0700
+++ g26/drivers/usb/net/usbnet.c2006-10-25 19:07:16.0 -0700
@@ -669,6 +669,9 @@ done:
* they'll probably want to use this base set.
*/
+#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#define HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
@@ -699,20 +702,6 @@ int usbnet_set_settings (struct net_devi
}
EXPORT_SYMBOL_GPL(usbnet_set_settings);
-
-void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct usbnet *dev = netdev_priv(net);
-
- /* REVISIT don't always return usbnet */
- strncpy (info-driver, driver_name, sizeof info-driver);
- strncpy (info-version, DRIVER_VERSION, sizeof info-version);
- strncpy (info-fw_version, dev-driver_info-description,
- sizeof info-fw_version);
- usb_make_path (dev-udev, info-bus_info, sizeof info-bus_info);
-}
-EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-
u32 usbnet_get_link (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
@@ -730,40 +719,57 @@ u32 usbnet_get_link (struct net_device *
}
EXPORT_SYMBOL_GPL(usbnet_get_link);
-u32 usbnet_get_msglevel (struct net_device *net)
+int usbnet_nway_reset(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- return dev-msg_enable;
+ if (!dev-mii.mdio_write)
+ return -EOPNOTSUPP;
+
+ return mii_nway_restart(dev-mii);
}
-EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_nway_reset);
-void usbnet_set_msglevel (struct net_device *net, u32 level)
+#endif /* HAVE_MII */
+
+void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
{
struct usbnet *dev = netdev_priv(net);
- dev-msg_enable = level;
+ /* REVISIT don't always return usbnet */
+ strncpy (info-driver, driver_name, sizeof info-driver);
+ strncpy (info-version, DRIVER_VERSION, sizeof info-version);
+ strncpy (info-fw_version, dev-driver_info-description,
+ sizeof info-fw_version);
+ usb_make_path (dev-udev, info-bus_info, sizeof info-bus_info);
}
-EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-int usbnet_nway_reset(struct net_device *net)
+u32 usbnet_get_msglevel (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- if (!dev-mii.mdio_write)
- return -EOPNOTSUPP;
+ return dev-msg_enable;
+}
+EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
- return mii_nway_restart(dev-mii);
+void usbnet_set_msglevel (struct net_device *net, u32 level)
+{
+ struct usbnet *dev = netdev_priv(net);
+
+ dev-msg_enable = level;
}
-EXPORT_SYMBOL_GPL(usbnet_nway_reset);
+EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
/* drivers may override default ethtool_ops in their bind() routine */
static struct ethtool_ops usbnet_ethtool_ops = {
+#ifdef HAVE_MII
.get_settings = usbnet_get_settings,
.set_settings = usbnet_set_settings,
- .get_drvinfo= usbnet_get_drvinfo,
.get_link = usbnet_get_link,
.nway_reset = usbnet_nway_reset,
+#endif
+ .get_drvinfo= usbnet_get_drvinfo,
.get_msglevel = usbnet_get_msglevel,
.set_msglevel = usbnet_set_msglevel,
};
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Sat, Oct 28, 2006 at 02:10:09PM -0700, David Brownell wrote: On Saturday 28 October 2006 4:21 am, Christoph Hellwig wrote: This is really awkward and against what we do in any other driver. Awkward, yes -- which is why I posted the non-awkward version, which is repeated below. (No thanks to diff for making the patch ugly though; the resulting code is clean and non-awkward, moving that function helped.) Against what other drivers do? Since usbnet.c is infrastructure code, not a driver, your comment can't apply. Infrastructure uses conditional compilation routinely in such cases. But remember that the actual drivers follow the standard convention (select MII) given Randy's patch #1 of 2. Ah sorry - I missed that. I still don't quite like the approach. What about simply putting the mii using functions into usbnet-mii.c and let makefile doing all the work? This would require a second set of ethtool ops, but I'd actually consider that a cleanup, as it makes clear which one we're using and allows to kill all the checks for non-mii hardware in the methods. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Sat, Oct 28, 2006 at 02:10:09PM -0700, David Brownell wrote: On Saturday 28 October 2006 4:21 am, Christoph Hellwig wrote: This is really awkward and against what we do in any other driver. Awkward, yes -- which is why I posted the non-awkward version, which is repeated below. (No thanks to diff for making the patch ugly though; the resulting code is clean and non-awkward, moving that function helped.) ... --- g26.orig/drivers/usb/net/usbnet.c 2006-10-24 18:29:28.0 -0700 +++ g26/drivers/usb/net/usbnet.c 2006-10-25 19:07:16.0 -0700 @@ -669,6 +669,9 @@ done: * they'll probably want to use this base set. */ +#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) +#define HAVE_MII ... This seems to cause a CONFIG_USB_USBNET=y, CONFIG_MII=m breakage (as already described earlier in this thread)? 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 - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wed, Oct 25, 2006 at 04:58:58PM -0700, Randy Dunlap wrote: ... Build tested with CONFIG_MII=y, m, n. ... --- linux-2619-rc3-pv.orig/drivers/usb/net/usbnet.c +++ linux-2619-rc3-pv/drivers/usb/net/usbnet.c @@ -47,6 +47,12 @@ #define DRIVER_VERSION 22-Aug-2005 +#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) +#define HAVE_MII 1 +#else +#define HAVE_MII 0 +#endif ... I'm too lame to test it, but I bet this will break with CONFIG_USB_USBNET=y, CONFIG_MII=m, and you'll actually need #if defined(CONFIG_MII) || (defined(CONFIG_MII_MODULE) defined(MODULE)) And then there's the question whether this amount of #ifdef's is actually worth avoiding the select MII... 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 - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
Adrian Bunk wrote: On Wed, Oct 25, 2006 at 04:58:58PM -0700, Randy Dunlap wrote: ... Build tested with CONFIG_MII=y, m, n. ... --- linux-2619-rc3-pv.orig/drivers/usb/net/usbnet.c +++ linux-2619-rc3-pv/drivers/usb/net/usbnet.c @@ -47,6 +47,12 @@ #define DRIVER_VERSION 22-Aug-2005 +#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) +#define HAVE_MII 1 +#else +#define HAVE_MII 0 +#endif ... I'm too lame to test it, but I bet this will break with CONFIG_USB_USBNET=y, CONFIG_MII=m, and you'll actually need #if defined(CONFIG_MII) || (defined(CONFIG_MII_MODULE) defined(MODULE)) And then there's the question whether this amount of #ifdef's is actually worth avoiding the select MII... Thanks, but that's OK, David posted a different patch for it. -- ~Randy - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote: Instead, usbnet.c should #ifdef the relevant ethtool hooks according to CONFIG_MII ... since it's completely legit to use usbnet with peripherals that don't need MII. --- From: Randy Dunlap [EMAIL PROTECTED] usbnet driver should use mii_*() interfaces if they are available in the kernel (config enabled) but usbnet does not require or depend on these interfaces. Build tested with CONFIG_MII=y, m, n. Signed-off-by: Randy Dunlap [EMAIL PROTECTED] --- drivers/usb/net/usbnet.c | 18 ++ 1 file changed, 18 insertions(+) --- linux-2619-rc3-pv.orig/drivers/usb/net/usbnet.c +++ linux-2619-rc3-pv/drivers/usb/net/usbnet.c @@ -47,6 +47,12 @@ #define DRIVER_VERSION 22-Aug-2005 +#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE) +#define HAVE_MII 1 +#else +#define HAVE_MII 0 +#endif + /*-*/ @@ -676,7 +682,10 @@ int usbnet_get_settings (struct net_devi if (!dev-mii.mdio_read) return -EOPNOTSUPP; +#if HAVE_MII return mii_ethtool_gset(dev-mii, cmd); +#endif + return -EOPNOTSUPP; } EXPORT_SYMBOL_GPL(usbnet_get_settings); @@ -688,7 +697,11 @@ int usbnet_set_settings (struct net_devi if (!dev-mii.mdio_write) return -EOPNOTSUPP; +#if HAVE_MII retval = mii_ethtool_sset(dev-mii, cmd); +#else + retval = -EOPNOTSUPP; +#endif /* link speed/duplex might have changed */ if (dev-driver_info-link_reset) @@ -721,9 +734,11 @@ u32 usbnet_get_link (struct net_device * if (dev-driver_info-check_connect) return dev-driver_info-check_connect (dev) == 0; +#if HAVE_MII /* if the device has mii operations, use those */ if (dev-mii.mdio_read) return mii_link_ok(dev-mii); +#endif /* Otherwise, say we're up (to avoid breaking scripts) */ return 1; @@ -753,7 +768,10 @@ int usbnet_nway_reset(struct net_device if (!dev-mii.mdio_write) return -EOPNOTSUPP; +#if HAVE_MII return mii_nway_restart(dev-mii); +#endif + return -EOPNOTSUPP; } EXPORT_SYMBOL_GPL(usbnet_nway_reset); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
On Wednesday 25 October 2006 4:58 pm, Randy Dunlap wrote:
On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote:
Instead, usbnet.c should #ifdef the relevant ethtool hooks
according to CONFIG_MII ... since it's completely legit to
use usbnet with peripherals that don't need MII.
I had in mind something simpler -- #ifdeffing the entire functions,
as in this patch. It looks more complicated than it is, because
diff gets confused by moving two functions earlier in the file.
(Thanks for starting this, Randy ... these two patches should be merged
before RC4 ships.)
- Dave
The usbnet infrastructure must not reference MII symbols unless they're
provided in the kernel being built. This extends also to the ethtool
hooks that reference those symbols.
Signed-off-by: David Brownell [EMAIL PROTECTED]
Index: g26/drivers/usb/net/usbnet.c
===
--- g26.orig/drivers/usb/net/usbnet.c 2006-10-24 18:29:28.0 -0700
+++ g26/drivers/usb/net/usbnet.c2006-10-25 19:07:16.0 -0700
@@ -669,6 +669,9 @@ done:
* they'll probably want to use this base set.
*/
+#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#define HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
@@ -699,20 +702,6 @@ int usbnet_set_settings (struct net_devi
}
EXPORT_SYMBOL_GPL(usbnet_set_settings);
-
-void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct usbnet *dev = netdev_priv(net);
-
- /* REVISIT don't always return usbnet */
- strncpy (info-driver, driver_name, sizeof info-driver);
- strncpy (info-version, DRIVER_VERSION, sizeof info-version);
- strncpy (info-fw_version, dev-driver_info-description,
- sizeof info-fw_version);
- usb_make_path (dev-udev, info-bus_info, sizeof info-bus_info);
-}
-EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-
u32 usbnet_get_link (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
@@ -730,40 +719,57 @@ u32 usbnet_get_link (struct net_device *
}
EXPORT_SYMBOL_GPL(usbnet_get_link);
-u32 usbnet_get_msglevel (struct net_device *net)
+int usbnet_nway_reset(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- return dev-msg_enable;
+ if (!dev-mii.mdio_write)
+ return -EOPNOTSUPP;
+
+ return mii_nway_restart(dev-mii);
}
-EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_nway_reset);
-void usbnet_set_msglevel (struct net_device *net, u32 level)
+#endif /* HAVE_MII */
+
+void usbnet_get_drvinfo (struct net_device *net, struct ethtool_drvinfo *info)
{
struct usbnet *dev = netdev_priv(net);
- dev-msg_enable = level;
+ /* REVISIT don't always return usbnet */
+ strncpy (info-driver, driver_name, sizeof info-driver);
+ strncpy (info-version, DRIVER_VERSION, sizeof info-version);
+ strncpy (info-fw_version, dev-driver_info-description,
+ sizeof info-fw_version);
+ usb_make_path (dev-udev, info-bus_info, sizeof info-bus_info);
}
-EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
+EXPORT_SYMBOL_GPL(usbnet_get_drvinfo);
-int usbnet_nway_reset(struct net_device *net)
+u32 usbnet_get_msglevel (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- if (!dev-mii.mdio_write)
- return -EOPNOTSUPP;
+ return dev-msg_enable;
+}
+EXPORT_SYMBOL_GPL(usbnet_get_msglevel);
- return mii_nway_restart(dev-mii);
+void usbnet_set_msglevel (struct net_device *net, u32 level)
+{
+ struct usbnet *dev = netdev_priv(net);
+
+ dev-msg_enable = level;
}
-EXPORT_SYMBOL_GPL(usbnet_nway_reset);
+EXPORT_SYMBOL_GPL(usbnet_set_msglevel);
/* drivers may override default ethtool_ops in their bind() routine */
static struct ethtool_ops usbnet_ethtool_ops = {
+#ifdef HAVE_MII
.get_settings = usbnet_get_settings,
.set_settings = usbnet_set_settings,
- .get_drvinfo= usbnet_get_drvinfo,
.get_link = usbnet_get_link,
.nway_reset = usbnet_nway_reset,
+#endif
+ .get_drvinfo= usbnet_get_drvinfo,
.get_msglevel = usbnet_get_msglevel,
.set_msglevel = usbnet_set_msglevel,
};
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
