netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB
       [not found] ` <20061025201341.GH21200@miggy.org>
@ 2006-10-25 22:17   ` Randy Dunlap
  2006-10-25 22:27     ` David Brownell
  0 siblings, 1 reply; 26+ messages in thread
From: Randy Dunlap @ 2006-10-25 22:17 UTC (permalink / raw)
  To: Athanasius, toralf.foerster, gregkh, akpm, netdev, lud
  Cc: Linus Torvalds, linux-kernel, David Brownell, Roman Zippel

From: Randy Dunlap <randy.dunlap@oracle.com>

USB network drivers that use mii_*() interfaces should
cause those interfaces to be built.  Or depend on them,
but this is what all drivers/net/ drivers do.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/usb/net/Kconfig |    3 +++
 1 file changed, 3 insertions(+)

--- linux-2619-rc3-pv.orig/drivers/usb/net/Kconfig
+++ linux-2619-rc3-pv/drivers/usb/net/Kconfig
@@ -84,6 +84,7 @@ config USB_PEGASUS
 config USB_RTL8150
 	tristate "USB RTL8150 based ethernet device support (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
+	select MII
 	help
 	  Say Y here if you have RTL8150 based usb-ethernet adapter.
 	  Send me <petkan@users.sourceforge.net> any comments you may have.
@@ -94,6 +95,7 @@ config USB_RTL8150
 
 config USB_USBNET
 	tristate "Multi-purpose USB Networking Framework"
+	select MII
 	---help---
 	  This driver supports several kinds of network links over USB,
 	  with "minidrivers" built around a common network driver core
@@ -210,6 +212,7 @@ config USB_NET_PLUSB
 config USB_NET_MCS7830
 	tristate "MosChip MCS7830 based Ethernet adapters"
 	depends on USB_USBNET
+	select MII
 	help
 	  Choose this option if you're using a 10/100 Ethernet USB2
 	  adapter based on the MosChip 7830 controller. This includes




---

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB
  2006-10-25 22:17   ` [PATCH] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB Randy Dunlap
@ 2006-10-25 22:27     ` David Brownell
  2006-10-25 23:58       ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Randy Dunlap
  2006-10-25 23:59       ` [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB Randy Dunlap
  0 siblings, 2 replies; 26+ messages in thread
From: David Brownell @ 2006-10-25 22:27 UTC (permalink / raw)
  To: toralf.foerster, randy.dunlap, netdev, linux-usb-devel, link,
	greg, akpm
  Cc: zippel, torvalds, linux-kernel, dbrownell

> @@ -94,6 +95,7 @@ config USB_RTL8150
>  
>  config USB_USBNET
>  	tristate "Multi-purpose USB Networking Framework"
> +	select MII
>  	---help---
>  	  This driver supports several kinds of network links over USB,
>  	  with "minidrivers" built around a common network driver core

The other parts are right, this isn't.

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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-25 22:27     ` David Brownell
@ 2006-10-25 23:58       ` Randy Dunlap
  2006-10-26  2:22         ` David Brownell
                           ` (2 more replies)
  2006-10-25 23:59       ` [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB Randy Dunlap
  1 sibling, 3 replies; 26+ messages in thread
From: Randy Dunlap @ 2006-10-25 23:58 UTC (permalink / raw)
  To: David Brownell
  Cc: toralf.foerster, netdev, linux-usb-devel, link, greg, akpm,
	zippel, torvalds, linux-kernel, dbrownell

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 <randy.dunlap@oracle.com>

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 <randy.dunlap@oracle.com>
---
 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);
 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB
  2006-10-25 22:27     ` David Brownell
  2006-10-25 23:58       ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Randy Dunlap
@ 2006-10-25 23:59       ` Randy Dunlap
  2006-10-26  2:24         ` David Brownell
  1 sibling, 1 reply; 26+ messages in thread
From: Randy Dunlap @ 2006-10-25 23:59 UTC (permalink / raw)
  To: David Brownell
  Cc: toralf.foerster, netdev, linux-usb-devel, link, greg, akpm,
	zippel, torvalds, linux-kernel, dbrownell

On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote:

> The other parts are right, this isn't.
> 
> 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.

Ugh.  OK.  How's this?  (2 patches)

(oh, OP mentioned CONFIG_PHYLIB but it's actually CONFIG_MII AFAIK)

---
From: Randy Dunlap <randy.dunlap@oracle.com>

pegasus and mcs7830 drivers use MII interfaces and should
select MII in the same way that drivers/net/ drivers do.

However, the MII config symbol should not be in the 10/100 Ethernet
menu, so that other drivers can use (enable) it or so that users
can enable it without needing to enable 10/100 Ethernet.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/net/Kconfig     |   15 +++++++--------
 drivers/usb/net/Kconfig |    2 ++
 2 files changed, 9 insertions(+), 8 deletions(-)

--- linux-2619-rc3-pv.orig/drivers/usb/net/Kconfig
+++ linux-2619-rc3-pv/drivers/usb/net/Kconfig
@@ -84,6 +84,7 @@ config USB_PEGASUS
 config USB_RTL8150
 	tristate "USB RTL8150 based ethernet device support (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
+	select MII
 	help
 	  Say Y here if you have RTL8150 based usb-ethernet adapter.
 	  Send me <petkan@users.sourceforge.net> any comments you may have.
@@ -210,6 +211,7 @@ config USB_NET_PLUSB
 config USB_NET_MCS7830
 	tristate "MosChip MCS7830 based Ethernet adapters"
 	depends on USB_USBNET
+	select MII
 	help
 	  Choose this option if you're using a 10/100 Ethernet USB2
 	  adapter based on the MosChip 7830 controller. This includes
--- linux-2619-rc3-pv.orig/drivers/net/Kconfig
+++ linux-2619-rc3-pv/drivers/net/Kconfig
@@ -145,6 +145,13 @@ config NET_SB1000
 
 source "drivers/net/arcnet/Kconfig"
 
+config MII
+	tristate "Generic Media Independent Interface device support"
+	help
+	  Most ethernet controllers have MII transceiver either as an external
+	  or internal device.  It is safe to say Y or M here even if your
+	  ethernet card lacks MII.
+
 source "drivers/net/phy/Kconfig"
 
 #
@@ -180,14 +187,6 @@ config NET_ETHERNET
 	  kernel: saying N will just cause the configurator to skip all
 	  the questions about Ethernet network cards. If unsure, say N.
 
-config MII
-	tristate "Generic Media Independent Interface device support"
-	depends on NET_ETHERNET
-	help
-	  Most ethernet controllers have MII transceiver either as an external
-	  or internal device.  It is safe to say Y or M here even if your
-	  ethernet card lack MII.
-
 source "drivers/net/arm/Kconfig"
 
 config MACE

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-25 23:58       ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Randy Dunlap
@ 2006-10-26  2:22         ` David Brownell
  2006-11-02  7:15           ` Greg KH
  2006-10-26 15:46         ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Adrian Bunk
  2006-10-28 11:21         ` Christoph Hellwig
  2 siblings, 1 reply; 26+ messages in thread
From: David Brownell @ 2006-10-26  2:22 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: toralf.foerster, netdev, linux-usb-devel, link, greg, akpm,
	zippel, torvalds, linux-kernel

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 <dbrownell@users.sourceforge.net>

Index: g26/drivers/usb/net/usbnet.c
===================================================================
--- g26.orig/drivers/usb/net/usbnet.c	2006-10-24 18:29:28.000000000 -0700
+++ g26/drivers/usb/net/usbnet.c	2006-10-25 19:07:16.000000000 -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,
 };

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but  CONFIG_USB_USBNET also needs CONFIG_PHYLIB
  2006-10-25 23:59       ` [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB Randy Dunlap
@ 2006-10-26  2:24         ` David Brownell
  2006-10-26  5:05           ` Randy.Dunlap
  0 siblings, 1 reply; 26+ messages in thread
From: David Brownell @ 2006-10-26  2:24 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: toralf.foerster, netdev, linux-usb-devel, link, greg, akpm,
	zippel, torvalds, linux-kernel, dbrownell

On Wednesday 25 October 2006 4:59 pm, Randy Dunlap wrote:
> On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote:
> 
> > The other parts are right, this isn't.
> > 
> > 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.
> 
> Ugh.  OK.  How's this?  (2 patches)

Looks about right, but ...


> However, the MII config symbol should not be in the 10/100 Ethernet
> menu, so that other drivers can use (enable) it or so that users
> can enable it without needing to enable 10/100 Ethernet.

... MII should still depend on ETHERNET, right?
Just not limited to 10/100 Ethernet.

> --- linux-2619-rc3-pv.orig/drivers/net/Kconfig
> +++ linux-2619-rc3-pv/drivers/net/Kconfig
> @@ -145,6 +145,13 @@ config NET_SB1000
>  
>  source "drivers/net/arcnet/Kconfig"
>  
> +config MII
> +	tristate "Generic Media Independent Interface device support"
> +	help
> +	  Most ethernet controllers have MII transceiver either as an external
> +	  or internal device.  It is safe to say Y or M here even if your
> +	  ethernet card lacks MII.
> +
>  source "drivers/net/phy/Kconfig"
>  
>  #
> @@ -180,14 +187,6 @@ config NET_ETHERNET
>  	  kernel: saying N will just cause the configurator to skip all
>  	  the questions about Ethernet network cards. If unsure, say N.
>  
> -config MII
> -	tristate "Generic Media Independent Interface device support"
> -	depends on NET_ETHERNET
> -	help
> -	  Most ethernet controllers have MII transceiver either as an external
> -	  or internal device.  It is safe to say Y or M here even if your
> -	  ethernet card lack MII.
> -
>  source "drivers/net/arm/Kconfig"
>  
>  config MACE
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but  CONFIG_USB_USBNET also needs CONFIG_PHYLIB
  2006-10-26  2:24         ` David Brownell
@ 2006-10-26  5:05           ` Randy.Dunlap
  2006-10-26  5:24             ` David Brownell
  0 siblings, 1 reply; 26+ messages in thread
From: Randy.Dunlap @ 2006-10-26  5:05 UTC (permalink / raw)
  To: David Brownell
  Cc: toralf.foerster, netdev, linux-usb-devel, link, greg, akpm,
	zippel, torvalds, linux-kernel, dbrownell

David Brownell wrote:
> On Wednesday 25 October 2006 4:59 pm, Randy Dunlap wrote:
>> On Wed, 25 Oct 2006 15:27:09 -0700 David Brownell wrote:
>>
>>> The other parts are right, this isn't.
>>>
>>> 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.
>> Ugh.  OK.  How's this?  (2 patches)
> 
> Looks about right, but ...
> 
> 
>> However, the MII config symbol should not be in the 10/100 Ethernet
>> menu, so that other drivers can use (enable) it or so that users
>> can enable it without needing to enable 10/100 Ethernet.
> 
> ... MII should still depend on ETHERNET, right?
> Just not limited to 10/100 Ethernet.

There is no such config symbol.  NET_ETHERNET means 10/100 ethernet.
Gigabit ethernet doesn't use the ETHERNET symbol (and doesn't use
this flavor of MII IIRC).
And all of this config space is surrounded by:

# All the following symbols are dependent on NETDEVICES - do not repeat
# that for each of the symbols.
if NETDEVICES

so it is actually
config MII
	depends on NETDEVICES

What do you suggest?

>> --- linux-2619-rc3-pv.orig/drivers/net/Kconfig
>> +++ linux-2619-rc3-pv/drivers/net/Kconfig
>> @@ -145,6 +145,13 @@ config NET_SB1000
>>  
>>  source "drivers/net/arcnet/Kconfig"
>>  
>> +config MII
>> +	tristate "Generic Media Independent Interface device support"
>> +	help
>> +	  Most ethernet controllers have MII transceiver either as an external
>> +	  or internal device.  It is safe to say Y or M here even if your
>> +	  ethernet card lacks MII.
>> +
>>  source "drivers/net/phy/Kconfig"
>>  
>>  #
>> @@ -180,14 +187,6 @@ config NET_ETHERNET
>>  	  kernel: saying N will just cause the configurator to skip all
>>  	  the questions about Ethernet network cards. If unsure, say N.
>>  
>> -config MII
>> -	tristate "Generic Media Independent Interface device support"
>> -	depends on NET_ETHERNET
>> -	help
>> -	  Most ethernet controllers have MII transceiver either as an external
>> -	  or internal device.  It is safe to say Y or M here even if your
>> -	  ethernet card lack MII.
>> -
>>  source "drivers/net/arm/Kconfig"
>>  
>>  config MACE
>>


-- 
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but  CONFIG_USB_USBNET also needs CONFIG_PHYLIB
  2006-10-26  5:05           ` Randy.Dunlap
@ 2006-10-26  5:24             ` David Brownell
  0 siblings, 0 replies; 26+ messages in thread
From: David Brownell @ 2006-10-26  5:24 UTC (permalink / raw)
  To: Randy.Dunlap
  Cc: toralf.foerster, netdev, linux-usb-devel, link, greg, akpm,
	zippel, torvalds, linux-kernel, dbrownell

On Wednesday 25 October 2006 10:05 pm, Randy.Dunlap wrote:

> > 
> > ... MII should still depend on ETHERNET, right?
> > Just not limited to 10/100 Ethernet.
> 
> There is no such config symbol.  NET_ETHERNET means 10/100 ethernet.
> Gigabit ethernet doesn't use the ETHERNET symbol (and doesn't use
> this flavor of MII IIRC).

Ah, you're right -- sorry.  Only Kconfig and net/ipv4/arp.c even
look for that config symbol.

- Dave


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-25 23:58       ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Randy Dunlap
  2006-10-26  2:22         ` David Brownell
@ 2006-10-26 15:46         ` Adrian Bunk
  2006-10-26 15:51           ` Randy.Dunlap
  2006-10-28 11:21         ` Christoph Hellwig
  2 siblings, 1 reply; 26+ messages in thread
From: Adrian Bunk @ 2006-10-26 15:46 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: David Brownell, toralf.foerster, netdev, linux-usb-devel, link,
	greg, akpm, zippel, torvalds, linux-kernel, dbrownell,
	Arnd Bergmann

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-26 15:46         ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Adrian Bunk
@ 2006-10-26 15:51           ` Randy.Dunlap
  0 siblings, 0 replies; 26+ messages in thread
From: Randy.Dunlap @ 2006-10-26 15:51 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: David Brownell, toralf.foerster, netdev, linux-usb-devel, link,
	greg, akpm, zippel, torvalds, linux-kernel, dbrownell,
	Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-25 23:58       ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Randy Dunlap
  2006-10-26  2:22         ` David Brownell
  2006-10-26 15:46         ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Adrian Bunk
@ 2006-10-28 11:21         ` Christoph Hellwig
  2006-10-28 21:10           ` David Brownell
  2 siblings, 1 reply; 26+ messages in thread
From: Christoph Hellwig @ 2006-10-28 11:21 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: David Brownell, toralf.foerster, netdev, linux-usb-devel, link,
	greg, akpm, zippel, torvalds, linux-kernel, dbrownell

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 <randy.dunlap@oracle.com>
> 
> 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 <randy.dunlap@oracle.com>
> ---
>  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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
---end quoted text---

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-28 11:21         ` Christoph Hellwig
@ 2006-10-28 21:10           ` David Brownell
  2006-10-28 21:13             ` Christoph Hellwig
  2006-10-28 21:39             ` Adrian Bunk
  0 siblings, 2 replies; 26+ messages in thread
From: David Brownell @ 2006-10-28 21:10 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Randy Dunlap, toralf.foerster, netdev, linux-usb-devel, link,
	greg, akpm, zippel, torvalds, linux-kernel

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 <dbrownell@users.sourceforge.net>

Index: g26/drivers/usb/net/usbnet.c
===================================================================
--- g26.orig/drivers/usb/net/usbnet.c	2006-10-24 18:29:28.000000000 -0700
+++ g26/drivers/usb/net/usbnet.c	2006-10-25 19:07:16.000000000 -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,
 };

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-28 21:10           ` David Brownell
@ 2006-10-28 21:13             ` Christoph Hellwig
  2006-10-28 22:30               ` David Brownell
  2006-10-28 21:39             ` Adrian Bunk
  1 sibling, 1 reply; 26+ messages in thread
From: Christoph Hellwig @ 2006-10-28 21:13 UTC (permalink / raw)
  To: David Brownell
  Cc: Christoph Hellwig, Randy Dunlap, toralf.foerster, netdev,
	linux-usb-devel, link, greg, akpm, zippel, torvalds, linux-kernel

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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-28 21:10           ` David Brownell
  2006-10-28 21:13             ` Christoph Hellwig
@ 2006-10-28 21:39             ` Adrian Bunk
  2006-10-31 17:40               ` [linux-usb-devel] " David Brownell
  1 sibling, 1 reply; 26+ messages in thread
From: Adrian Bunk @ 2006-10-28 21:39 UTC (permalink / raw)
  To: David Brownell
  Cc: Christoph Hellwig, Randy Dunlap, toralf.foerster, netdev,
	linux-usb-devel, link, greg, akpm, zippel, torvalds, linux-kernel

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.000000000 -0700
> +++ g26/drivers/usb/net/usbnet.c	2006-10-25 19:07:16.000000000 -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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-28 21:13             ` Christoph Hellwig
@ 2006-10-28 22:30               ` David Brownell
  0 siblings, 0 replies; 26+ messages in thread
From: David Brownell @ 2006-10-28 22:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: akpm, greg, link, linux-kernel, netdev, Randy Dunlap,
	toralf.foerster, torvalds, zippel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-28 21:39             ` Adrian Bunk
@ 2006-10-31 17:40               ` David Brownell
  2006-10-31 18:07                 ` Adrian Bunk
  0 siblings, 1 reply; 26+ messages in thread
From: David Brownell @ 2006-10-31 17:40 UTC (permalink / raw)
  To: linux-usb-devel
  Cc: Adrian Bunk, Randy Dunlap, akpm, zippel, netdev, linux-kernel,
	link, Christoph Hellwig, torvalds, greg, toralf.foerster


> > +#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.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-31 17:40               ` [linux-usb-devel] " David Brownell
@ 2006-10-31 18:07                 ` Adrian Bunk
  2006-10-31 19:36                   ` David Brownell
  0 siblings, 1 reply; 26+ messages in thread
From: Adrian Bunk @ 2006-10-31 18:07 UTC (permalink / raw)
  To: David Brownell
  Cc: linux-usb-devel, Randy Dunlap, akpm, zippel, netdev, linux-kernel,
	link, Christoph Hellwig, torvalds, greg, toralf.foerster

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-31 18:07                 ` Adrian Bunk
@ 2006-10-31 19:36                   ` David Brownell
  2006-11-01  1:23                     ` Adrian Bunk
  0 siblings, 1 reply; 26+ messages in thread
From: David Brownell @ 2006-10-31 19:36 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: linux-usb-devel, Randy Dunlap, akpm, zippel, netdev, linux-kernel,
	link, Christoph Hellwig, torvalds, greg, toralf.foerster


> > 		...
> > 		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.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-31 19:36                   ` David Brownell
@ 2006-11-01  1:23                     ` Adrian Bunk
  2006-11-02 20:19                       ` David Brownell
  0 siblings, 1 reply; 26+ messages in thread
From: Adrian Bunk @ 2006-11-01  1:23 UTC (permalink / raw)
  To: David Brownell
  Cc: linux-usb-devel, Randy Dunlap, akpm, zippel, netdev, linux-kernel,
	link, Christoph Hellwig, torvalds, greg, toralf.foerster

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-10-26  2:22         ` David Brownell
@ 2006-11-02  7:15           ` Greg KH
  2006-11-02 20:29             ` David Brownell
  0 siblings, 1 reply; 26+ messages in thread
From: Greg KH @ 2006-11-02  7:15 UTC (permalink / raw)
  To: David Brownell
  Cc: Randy Dunlap, toralf.foerster, netdev, linux-usb-devel, link,
	akpm, zippel, torvalds, linux-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [linux-usb-devel] [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-11-01  1:23                     ` Adrian Bunk
@ 2006-11-02 20:19                       ` David Brownell
  0 siblings, 0 replies; 26+ messages in thread
From: David Brownell @ 2006-11-02 20:19 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: linux-usb-devel, Randy Dunlap, akpm, zippel, netdev, linux-kernel,
	link, Christoph Hellwig, torvalds, greg, toralf.foerster

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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-11-02  7:15           ` Greg KH
@ 2006-11-02 20:29             ` David Brownell
  2006-11-03  2:27               ` Adrian Bunk
  0 siblings, 1 reply; 26+ messages in thread
From: David Brownell @ 2006-11-02 20:29 UTC (permalink / raw)
  To: Greg KH
  Cc: Randy Dunlap, toralf.foerster, netdev, linux-usb-devel, link,
	akpm, zippel, torvalds, linux-kernel

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 <dbrownell@users.sourceforge.net>

Index: at91/drivers/usb/net/Kconfig
===================================================================
--- at91.orig/drivers/usb/net/Kconfig	2006-11-02 10:58:49.000000000 -0800
+++ at91/drivers/usb/net/Kconfig	2006-11-02 12:10:13.000000000 -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.000000000 -0800
+++ at91/drivers/usb/net/usbnet.c	2006-11-02 11:09:29.000000000 -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 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,
 };

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-11-02 20:29             ` David Brownell
@ 2006-11-03  2:27               ` Adrian Bunk
  2006-11-03  2:47                 ` David Brownell
  0 siblings, 1 reply; 26+ messages in thread
From: Adrian Bunk @ 2006-11-03  2:27 UTC (permalink / raw)
  To: David Brownell
  Cc: Greg KH, Randy Dunlap, toralf.foerster, netdev, linux-usb-devel,
	link, akpm, zippel, torvalds, linux-kernel

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.000000000 -0800
> +++ at91/drivers/usb/net/Kconfig	2006-11-02 12:10:13.000000000 -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.000000000 -0800
> +++ at91/drivers/usb/net/usbnet.c	2006-11-02 11:09:29.000000000 -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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-11-03  2:27               ` Adrian Bunk
@ 2006-11-03  2:47                 ` David Brownell
  2006-11-03  2:58                   ` Randy.Dunlap
  2006-11-04  2:51                   ` [2.6 patch] USB_RTL8150 must select MII Adrian Bunk
  0 siblings, 2 replies; 26+ messages in thread
From: David Brownell @ 2006-11-03  2:47 UTC (permalink / raw)
  To: Adrian Bunk
  Cc: Greg KH, Randy Dunlap, toralf.foerster, netdev, linux-usb-devel,
	link, akpm, zippel, torvalds, linux-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled
  2006-11-03  2:47                 ` David Brownell
@ 2006-11-03  2:58                   ` Randy.Dunlap
  2006-11-04  2:51                   ` [2.6 patch] USB_RTL8150 must select MII Adrian Bunk
  1 sibling, 0 replies; 26+ messages in thread
From: Randy.Dunlap @ 2006-11-03  2:58 UTC (permalink / raw)
  To: David Brownell
  Cc: Adrian Bunk, Greg KH, Randy Dunlap, toralf.foerster, netdev,
	linux-usb-devel, link, akpm, zippel, torvalds, linux-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [2.6 patch] USB_RTL8150 must select MII
  2006-11-03  2:47                 ` David Brownell
  2006-11-03  2:58                   ` Randy.Dunlap
@ 2006-11-04  2:51                   ` Adrian Bunk
  1 sibling, 0 replies; 26+ messages in thread
From: Adrian Bunk @ 2006-11-04  2:51 UTC (permalink / raw)
  To: David Brownell
  Cc: Greg KH, Randy Dunlap, toralf.foerster, netdev, linux-usb-devel,
	link, akpm, zippel, torvalds, linux-kernel

On Thu, Nov 02, 2006 at 06:47:15PM -0800, 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.

OK, the patch for this driver is below.

> - Dave

cu
Adrian


<--  snip  -->


USB_RTL8150 must select MII to avoid link errors.

Stolen from a patch by Randy Dunlap.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2619-rc3-pv.orig/drivers/usb/net/Kconfig
+++ linux-2619-rc3-pv/drivers/usb/net/Kconfig
@@ -84,6 +84,7 @@ config USB_PEGASUS
 config USB_RTL8150
 	tristate "USB RTL8150 based ethernet device support (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
+	select MII
 	help
 	  Say Y here if you have RTL8150 based usb-ethernet adapter.
 	  Send me <petkan@users.sourceforge.net> any comments you may have.


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2006-11-04  2:51 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <Pine.LNX.4.64.0610231618510.3962@g5.osdl.org>
     [not found] ` <20061025201341.GH21200@miggy.org>
2006-10-25 22:17   ` [PATCH] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB Randy Dunlap
2006-10-25 22:27     ` David Brownell
2006-10-25 23:58       ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Randy Dunlap
2006-10-26  2:22         ` David Brownell
2006-11-02  7:15           ` Greg KH
2006-11-02 20:29             ` David Brownell
2006-11-03  2:27               ` Adrian Bunk
2006-11-03  2:47                 ` David Brownell
2006-11-03  2:58                   ` Randy.Dunlap
2006-11-04  2:51                   ` [2.6 patch] USB_RTL8150 must select MII Adrian Bunk
2006-10-26 15:46         ` [PATCH 2/2] usbnet: use MII hooks only if CONFIG_MII is enabled Adrian Bunk
2006-10-26 15:51           ` Randy.Dunlap
2006-10-28 11:21         ` Christoph Hellwig
2006-10-28 21:10           ` David Brownell
2006-10-28 21:13             ` Christoph Hellwig
2006-10-28 22:30               ` David Brownell
2006-10-28 21:39             ` Adrian Bunk
2006-10-31 17:40               ` [linux-usb-devel] " David Brownell
2006-10-31 18:07                 ` Adrian Bunk
2006-10-31 19:36                   ` David Brownell
2006-11-01  1:23                     ` Adrian Bunk
2006-11-02 20:19                       ` David Brownell
2006-10-25 23:59       ` [PATCH 1/2] !CONFIG_NET_ETHERNET unsets CONFIG_PHYLIB, but CONFIG_USB_USBNET also needs CONFIG_PHYLIB Randy Dunlap
2006-10-26  2:24         ` David Brownell
2006-10-26  5:05           ` Randy.Dunlap
2006-10-26  5:24             ` David Brownell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).