[PATCH] 3c59x and resume

[PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]


Here is a small patch tested with 2.4.18 and 2.4.19-pre4.
It was proposed by Andrew but not integrated in pre4.

The problem is when using the vortex driver and suspend/resuming the
machine. Without this patch the card id is each time greater. To resume
correctly this driver need the option enable_wol=1 but as-is it will
only be true for the first ID. You can enable it for the first 8 IDs
with enable_wol=1,1,1,1,1,1,1,1 but you can't do it for all IDs.
Said another way without this patch you can't suspend/resume more than
eight times your machine.

This is a fix for the most common use. The proper fix would be IMO to
keep a bitmap of used IDs but I don't know if it worsts it.
Also a fix would be to separate the suspend/resume functionality from
the wol functionality (wake up on lan).

Thanks,
Christophe

--- linux/drivers/net/3c59x.c	Sat Mar 23 10:24:56 2002
+++ linux/drivers/net/3c59x.c	Sat Mar 23 10:57:00 2002
@@ -2891,6 +2891,9 @@
 
 	vp = dev->priv;
 
+	if (vp->card_idx == vortex_cards_found - 1)
+         vortex_cards_found--;
+
 	/* AKPM: FIXME: we should have
 	 *	if (vp->cb_fn_base) iounmap(vp->cb_fn_base);
 	 * here

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

There's no sense in being precise when you don't even know what you're
talking about. -- John von Neumann

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Andrew Morton]

christophe barbé wrote:
> 
> Here is a small patch tested with 2.4.18 and 2.4.19-pre4.
> It was proposed by Andrew but not integrated in pre4.
> 
> The problem is when using the vortex driver and suspend/resuming the
> machine.

The patch looks fine to me.

This is a common bug in the Linux ethernet drivers.  Let's
review it:

- The module init function keeps a driver-wide `card_idx'
- Every time a new card is detected, card_idx is incremented
  and is associated with that card.
- The card instance's card_idx is used to index module options
  arrays.
- The global card_idx counter is never decremented.

So each time you hotplug a card, it gets the next index, and
eventually the index exceeds the size of the module options array
and the driver falls into its "oh, I've got more than eight
cards" mode.

The fix in this patch is to simply decrement card_idx in the
remove_one function.  Which somewhat assumes that cards are
removed and inserted in LIFO order, but that's true for
just one card :)

Same bug appears to be present in about eight divers - just
grep for card_idx.

I don't immediately see a simple and clean fix for this.
If we have, say,

	options 3c59x enable_wol=1,1,0,1,1,0,0,0

in modules.conf, and we really have eight NICS, and they're
being plugged and unplugged, how can we reliably associate
that option with the eight cards?  So the right option is
applied to each card eash time it's inserted?  Should the
option be associated with a card, or with a bus position?

Fun.

Anyway.  Let's apply the patch.

-

Re: [PATCH] 3c59x and resume

[Robert Love]

On Sat, 2002-03-23 at 13:39, Andrew Morton wrote:

> in modules.conf, and we really have eight NICS, and they're
> being plugged and unplugged, how can we reliably associate
> that option with the eight cards?  So the right option is
> applied to each card eash time it's inserted?  Should the
> option be associated with a card, or with a bus position?

Ugh, not pretty.

Associate it with the bus position I'd say?

If we want a statically allocated array, create one of size N such that
N is reasonably sane.  Then we can "hash" the bus position onto N ...
something that basically maps the slot number onto N, slot number % N
will do.  Dealing with collisions would be easy, but there really
shouldn't be any in a sane configuration.

Ideally we'd have a dynamically created array for the cards and hash
into that, but, ugh, this is getting gross especially since 99% of us
have one card and never remove it.

	Robert Love

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 2730 bytes --]

With the 'everything is module' and 'everything is hotplug' approach in
mind (which is a appealing way and IMHO this is the way we are going),
I see two part for this problem:

. Persistence after plug out/plug in 

. Persistence after suspend/resume

The first one is a userland problem. The card identification could be
based on the MAC address (for NICs at least, in the case of cardbus the
bus position has no real signification). This should then be the
responsibility of the userspace tool (hotplug) to indicate the correct
option for this card. The problem is when the module is already loaded,
the userspace tool has no way to indicate this option.

The second problem is a kernel problem. It seems not so difficult (as
soon as we have a way to identify each card, which is the case for
NICs) to keep in memory the options for further use.
We don't even need a hash here. We can keep in a fifo table the options
for each removed card and then when a card is inserted lookup with the
MAC address.

Clearly the vector of values for an option to control separately each
card is no more adapted to the today hardware.

As I see it what is missing is a way for hotplug to give some directives
(enable wol for this card) back to the kernel. Today this is possible
(done) for the first card for a given driver (via the module options).

Christophe


On Sat, Mar 23, 2002 at 03:06:49PM -0500, Robert Love wrote:
> On Sat, 2002-03-23 at 13:39, Andrew Morton wrote:
> 
> > in modules.conf, and we really have eight NICS, and they're
> > being plugged and unplugged, how can we reliably associate
> > that option with the eight cards?  So the right option is
> > applied to each card eash time it's inserted?  Should the
> > option be associated with a card, or with a bus position?
> 
> Ugh, not pretty.
> 
> Associate it with the bus position I'd say?
> 
> If we want a statically allocated array, create one of size N such that
> N is reasonably sane.  Then we can "hash" the bus position onto N ...
> something that basically maps the slot number onto N, slot number % N
> will do.  Dealing with collisions would be easy, but there really
> shouldn't be any in a sane configuration.
> 
> Ideally we'd have a dynamically created array for the cards and hash
> into that, but, ugh, this is getting gross especially since 99% of us
> have one card and never remove it.
> 
> 	Robert Love
> 

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

Cats are rather delicate creatures and they are subject to a good
many ailments, but I never heard of one who suffered from insomnia.
--Joseph Wood Krutch

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Greg KH]

On Sat, Mar 23, 2002 at 05:44:33PM -0500, christophe barbé wrote:
> With the 'everything is module' and 'everything is hotplug' approach in
> mind (which is a appealing way and IMHO this is the way we are going),
> I see two part for this problem:
> 
> . Persistence after plug out/plug in 
> 
> . Persistence after suspend/resume
> 
> The first one is a userland problem. The card identification could be
> based on the MAC address (for NICs at least, in the case of cardbus the
> bus position has no real signification). This should then be the
> responsibility of the userspace tool (hotplug) to indicate the correct
> option for this card. The problem is when the module is already loaded,
> the userspace tool has no way to indicate this option.

Untrue.  See
	http://www.kroah.com/linux/hotplug/ols_2001_hotplug_talk/html/mgp00014.html
for a 6 line version of /sbin/hotplug that always assigns the same
"ethX" value to the same MAC address.  I think the patch to nameif has
gone in to support this, but I'm not sure.

And why is there a limitation of only 8 devices?  Why not do what all
USB drivers do, and just create the structure that you need to use at
probe() time, and destroy it at remove() time?

Just my $0.02

thanks,

greg k-h

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]

On Sun, Mar 24, 2002 at 12:07:29AM -0800, Greg KH wrote:
> On Sat, Mar 23, 2002 at 05:44:33PM -0500, christophe barbé wrote:
> > With the 'everything is module' and 'everything is hotplug' approach in
> > mind (which is a appealing way and IMHO this is the way we are going),
> > I see two part for this problem:
> > 
> > . Persistence after plug out/plug in 
> > 
> > . Persistence after suspend/resume
> > 
> > The first one is a userland problem. The card identification could be
> > based on the MAC address (for NICs at least, in the case of cardbus the
> > bus position has no real signification). This should then be the
> > responsibility of the userspace tool (hotplug) to indicate the correct
> > option for this card. The problem is when the module is already loaded,
> > the userspace tool has no way to indicate this option.
> 
> Untrue.  See
> 	http://www.kroah.com/linux/hotplug/ols_2001_hotplug_talk/html/mgp00014.html
> for a 6 line version of /sbin/hotplug that always assigns the same
> "ethX" value to the same MAC address.  I think the patch to nameif has
> gone in to support this, but I'm not sure.

Untrue what ? The persistence after plug out/in ?
The problem here is not to give the same interface to a given NIC. The
problem is to give the same options to a given NIC. But a solution can
simply be to set the option from hotplug using the proc interface. The
3c59x doesn't support that for wol but that can be changed.

> And why is there a limitation of only 8 devices?  Why not do what all
> USB drivers do, and just create the structure that you need to use at
> probe() time, and destroy it at remove() time?

This is an implementation issue which is not really important. It comes
from Donald Becker. Your dynamic structure doesn't solve the problem
'which options for which cards', does it ? 

> Just my $0.02
> 
> thanks,
> 
> greg k-h

Thanks,
Christophe

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

There's no sense in being precise when you don't even know what you're
talking about. -- John von Neumann

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Greg KH]

On Sun, Mar 24, 2002 at 09:25:45AM -0500, christophe barbé wrote:
> On Sun, Mar 24, 2002 at 12:07:29AM -0800, Greg KH wrote:
> > On Sat, Mar 23, 2002 at 05:44:33PM -0500, christophe barbé wrote:
> > > With the 'everything is module' and 'everything is hotplug' approach in
> > > mind (which is a appealing way and IMHO this is the way we are going),
> > > I see two part for this problem:
> > > 
> > > . Persistence after plug out/plug in 
> > > 
> > > . Persistence after suspend/resume
> > > 
> > > The first one is a userland problem. The card identification could be
> > > based on the MAC address (for NICs at least, in the case of cardbus the
> > > bus position has no real signification). This should then be the
> > > responsibility of the userspace tool (hotplug) to indicate the correct
> > > option for this card. The problem is when the module is already loaded,
> > > the userspace tool has no way to indicate this option.
> > 
> > Untrue.  See
> > 	http://www.kroah.com/linux/hotplug/ols_2001_hotplug_talk/html/mgp00014.html
> > for a 6 line version of /sbin/hotplug that always assigns the same
> > "ethX" value to the same MAC address.  I think the patch to nameif has
> > gone in to support this, but I'm not sure.
> 
> Untrue what ? The persistence after plug out/in ?

No, the sentence, "The problem is when the module is already loaded..."
/sbin/hotplug gets called when the network device is started up, it
doesn't only get called before the module is loaded.

> The problem here is not to give the same interface to a given NIC. The
> problem is to give the same options to a given NIC. But a solution can
> simply be to set the option from hotplug using the proc interface. The
> 3c59x doesn't support that for wol but that can be changed.

Understood.

> > And why is there a limitation of only 8 devices?  Why not do what all
> > USB drivers do, and just create the structure that you need to use at
> > probe() time, and destroy it at remove() time?
> 
> This is an implementation issue which is not really important. It comes
> from Donald Becker. Your dynamic structure doesn't solve the problem
> 'which options for which cards', does it ? 

No, but it solves the problem, "only 8 devices max", and "what to do
when a card is removed and then plugged back in."  Both seems like good
things to fix in the driver :)

thanks,

greg k-h

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 3532 bytes --]

On Mon, Mar 25, 2002 at 10:01:33AM -0800, Greg KH wrote:
> On Sun, Mar 24, 2002 at 09:25:45AM -0500, christophe barbé wrote:
> > On Sun, Mar 24, 2002 at 12:07:29AM -0800, Greg KH wrote:
> > > On Sat, Mar 23, 2002 at 05:44:33PM -0500, christophe barbé wrote:
> > > > With the 'everything is module' and 'everything is hotplug' approach in
> > > > mind (which is a appealing way and IMHO this is the way we are going),
> > > > I see two part for this problem:
> > > > 
> > > > . Persistence after plug out/plug in 
> > > > 
> > > > . Persistence after suspend/resume
> > > > 
> > > > The first one is a userland problem. The card identification could be
> > > > based on the MAC address (for NICs at least, in the case of cardbus the
> > > > bus position has no real signification). This should then be the
> > > > responsibility of the userspace tool (hotplug) to indicate the correct
> > > > option for this card. The problem is when the module is already loaded,
> > > > the userspace tool has no way to indicate this option.
> > > 
> > > Untrue.  See
> > > 	http://www.kroah.com/linux/hotplug/ols_2001_hotplug_talk/html/mgp00014.html
> > > for a 6 line version of /sbin/hotplug that always assigns the same
> > > "ethX" value to the same MAC address.  I think the patch to nameif has
> > > gone in to support this, but I'm not sure.
> > 
> > Untrue what ? The persistence after plug out/in ?
> 
> No, the sentence, "The problem is when the module is already loaded..."
> /sbin/hotplug gets called when the network device is started up, it
> doesn't only get called before the module is loaded.

Ok I understand that but hotplug has no way to influence the way the
device is treated by the driver. The only way I can see is via the /proc
interface, but at least it is not possible with this driver.

> > The problem here is not to give the same interface to a given NIC. The
> > problem is to give the same options to a given NIC. But a solution can
> > simply be to set the option from hotplug using the proc interface. The
> > 3c59x doesn't support that for wol but that can be changed.
> 
> Understood.

So do you agree that something is missing here ?

> 
> > > And why is there a limitation of only 8 devices?  Why not do what all
> > > USB drivers do, and just create the structure that you need to use at
> > > probe() time, and destroy it at remove() time?
> > 
> > This is an implementation issue which is not really important. It comes
> > from Donald Becker. Your dynamic structure doesn't solve the problem
> > 'which options for which cards', does it ? 
> 
> No, but it solves the problem, "only 8 devices max", and "what to do
> when a card is removed and then plugged back in."  Both seems like good
> things to fix in the driver :)

I have not checked the module loading code but is it possible to define
for an option a vector with an undefined size ? Or do you consider that
all devices use the same option ? (the vortex driver is only limited to
8 cards for the options passed by modutils)

Could you point me to a specific usb driver ?

How is solved the "what to do when a card is removed and then plugged
back in." problem ? By keeping the entry for further use ? 

Christophe

> thanks,
> 
> greg k-h

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

There's no sense in being precise when you don't even know what you're
talking about. -- John von Neumann

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Greg KH]

On Mon, Mar 25, 2002 at 01:19:56PM -0500, christophe barbé wrote:
> 
> Ok I understand that but hotplug has no way to influence the way the
> device is treated by the driver. The only way I can see is via the /proc
> interface, but at least it is not possible with this driver.

Not true.  The link I pointed to changes the way the "ethX" names are
assigned to the device based on MAC addresses.  So that's just one way
to influence the way a device is treated by a driver.

> 
> > > The problem here is not to give the same interface to a given NIC. The
> > > problem is to give the same options to a given NIC. But a solution can
> > > simply be to set the option from hotplug using the proc interface. The
> > > 3c59x doesn't support that for wol but that can be changed.
> > 
> > Understood.
> 
> So do you agree that something is missing here ?

Yes I do.  See below for more.

> > > > And why is there a limitation of only 8 devices?  Why not do what all
> > > > USB drivers do, and just create the structure that you need to use at
> > > > probe() time, and destroy it at remove() time?
> > > 
> > > This is an implementation issue which is not really important. It comes
> > > from Donald Becker. Your dynamic structure doesn't solve the problem
> > > 'which options for which cards', does it ? 
> > 
> > No, but it solves the problem, "only 8 devices max", and "what to do
> > when a card is removed and then plugged back in."  Both seems like good
> > things to fix in the driver :)
> 
> I have not checked the module loading code but is it possible to define
> for an option a vector with an undefined size ? Or do you consider that
> all devices use the same option ? (the vortex driver is only limited to
> 8 cards for the options passed by modutils)

Ok, in looking more at the 3c59x driver's module parameters, I see your
main problem.  You are relying on module wide options to effect
different cards in a system.  And these different cards could need
different options, right?

If so, yes this is a problem as you have outlined.  Might I suggest a
driverfs entry for the device?  That way every point in the device tree
could have those options be unique for the different device?  This could
also be done like you said above, with a /proc entry (but we are moving
away from /proc entries, remember? :)

Then when /sbin/hotplug is run when your network device is brought up,
it could find the driverfs entry for your device and initialize it with
the proper options (full_duplex, hw_checksums, etc.)

> Could you point me to a specific usb driver ?

In the drivers/usb directory, the following are network drivers:
	CDCEther.c
	catc.c
	kaweth.c
	pegasus.c
	usbnet.c

> How is solved the "what to do when a card is removed and then plugged
> back in." problem ? By keeping the entry for further use ? 

No, all of the information is deleted when a device is removed.  When a
device is inserted again, a structure is created again.  They do not
remember information about the device across removals.

Hope this helps,

greg k-h

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 4499 bytes --]

On Mon, Mar 25, 2002 at 11:11:27AM -0800, Greg KH wrote:
> On Mon, Mar 25, 2002 at 01:19:56PM -0500, christophe barbé wrote:
> > 
> > Ok I understand that but hotplug has no way to influence the way the
> > device is treated by the driver. The only way I can see is via the /proc
> > interface, but at least it is not possible with this driver.
> 
> Not true.  The link I pointed to changes the way the "ethX" names are
> assigned to the device based on MAC addresses.  So that's just one way
> to influence the way a device is treated by a driver.
> 
> > 
> > > > The problem here is not to give the same interface to a given NIC. The
> > > > problem is to give the same options to a given NIC. But a solution can
> > > > simply be to set the option from hotplug using the proc interface. The
> > > > 3c59x doesn't support that for wol but that can be changed.
> > > 
> > > Understood.
> > 
> > So do you agree that something is missing here ?
> 
> Yes I do.  See below for more.
> 
> > > > > And why is there a limitation of only 8 devices?  Why not do what all
> > > > > USB drivers do, and just create the structure that you need to use at
> > > > > probe() time, and destroy it at remove() time?
> > > > 
> > > > This is an implementation issue which is not really important. It comes
> > > > from Donald Becker. Your dynamic structure doesn't solve the problem
> > > > 'which options for which cards', does it ? 
> > > 
> > > No, but it solves the problem, "only 8 devices max", and "what to do
> > > when a card is removed and then plugged back in."  Both seems like good
> > > things to fix in the driver :)
> > 
> > I have not checked the module loading code but is it possible to define
> > for an option a vector with an undefined size ? Or do you consider that
> > all devices use the same option ? (the vortex driver is only limited to
> > 8 cards for the options passed by modutils)
> 
> Ok, in looking more at the 3c59x driver's module parameters, I see your
> main problem.  You are relying on module wide options to effect
> different cards in a system.  And these different cards could need
> different options, right?

Yes. I don't have this problem. I am a mobile user of this driver
(hotplug and power-management) and the driver was done with other things
in mind. The good thing is that both views seems to converge today.
I am looking for a clean fix. As I see it, the vector of options is a
mistake. We should be able to give a default value for an option with
modutils but for more complicate configuration a userland tool should be
able to decide the correct options for a given device. This is a
generalisation to what hotplug does.

> If so, yes this is a problem as you have outlined.  Might I suggest a
> driverfs entry for the device?  That way every point in the device tree
> could have those options be unique for the different device?  This could
> also be done like you said above, with a /proc entry (but we are moving
> away from /proc entries, remember? :)

Yes I remember even if I am not yet convinced. 

> Then when /sbin/hotplug is run when your network device is brought up,
> it could find the driverfs entry for your device and initialize it with
> the proper options (full_duplex, hw_checksums, etc.)
> 
> > Could you point me to a specific usb driver ?
> 
> In the drivers/usb directory, the following are network drivers:
> 	CDCEther.c
> 	catc.c
> 	kaweth.c
> 	pegasus.c
> 	usbnet.c

I will have a look. Thanks.

> > How is solved the "what to do when a card is removed and then plugged
> > back in." problem ? By keeping the entry for further use ? 
> 
> No, all of the information is deleted when a device is removed.  When a
> device is inserted again, a structure is created again.  They do not
> remember information about the device across removals.

So this particular problem is not solved in the usb drivers ?

Christophe

> Hope this helps,
> 
> greg k-h
> -
> 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/

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

There's no sense in being precise when you don't even know what you're
talking about. -- John von Neumann

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]

On Mon, Mar 25, 2002 at 11:11:27AM -0800, Greg KH wrote:
> > Could you point me to a specific usb driver ?
> 
> In the drivers/usb directory, the following are network drivers:
> 	CDCEther.c
> 	catc.c
> 	kaweth.c
> 	pegasus.c
> 	usbnet.c

$ grep MODULE_PARM CDCEther.c catc.c kaweth.c pegasus.c usbnet.c
CDCEther.c:MODULE_PARM (multicast_filter_limit, "i");
CDCEther.c:MODULE_PARM_DESC (multicast_filter_limit, "CDCEther maximum number of filtered multicast addresses");
pegasus.c:MODULE_PARM(loopback, "i");
pegasus.c:MODULE_PARM(mii_mode, "i");
pegasus.c:MODULE_PARM_DESC(loopback, "Enable MAC loopback mode (bit 0)");
pegasus.c:MODULE_PARM_DESC(mii_mode, "Enable HomePNA mode (bit 0),default=MII mode = 0");

Note that this is exactly what I think.
Each option is defined with a unique value used for all devices.

/usr/src/linux/drivers/usb$ grep MODULE_PARM ../net/3c59x.c                     
MODULE_PARM(debug, "i");
...
MODULE_PARM(enable_wol, "1-" __MODULE_STRING(8) "i");
MODULE_PARM(rx_copybreak, "i");
...

In a sense the vortex is more flexible. Most options are defined by a
single value but for a few you can pass a vector. 
NOTE that the 8 limit is only in the MODULE_PARM lines.

But this flexibility is no more adapted. 

$ man nameif
...
DESCRIPTION
       nameif  renames network interfaces based on mac addresses.
       When no arguments are given /etc/mactab is read. Each line

nameif solved a problem but not during the device activation (this is
the difference between rename and name). Would not it be possible to add
to hotplug a way to give back some advices to the kernel.

kernel -> hotplug : I am going to insert this device.
hotplug -> kernel : ok but use this options optionA,optionB,...

You can then still use nameif during the register phase or eventually
pass a directive earlier to avoid possible races.

Christophe

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

L'experience, c'est une connerie par jour mais jamais la même.

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Joachim Breuer]

Robert Love <rml@tech9.net> writes:

> On Sat, 2002-03-23 at 13:39, Andrew Morton wrote:
>
>> in modules.conf, and we really have eight NICS, and they're
>> being plugged and unplugged, how can we reliably associate
>> that option with the eight cards?  So the right option is
>> applied to each card eash time it's inserted?  Should the
>> option be associated with a card, or with a bus position?
>
> Ugh, not pretty.
>
> Associate it with the bus position I'd say?

I don't know wherewith the <i> in eth<i> is associated (bus pos or
maybe linear ordering of the MAC addresses or somesuch), but I would
expect a selected combination of eth<i> userland configuration (IP
address, netmask) and driver level configuration (WOL, ...) to remain
stable.

Being able to redetect a pulled card put in a different slot as a
"known" one giving it the same eth<i> (and associated WOL etc. config)
as before would of course be nice, but I can't see how this can be
cleanly done over reboots.

With bus pos you get a lesser variant of the "SCSI disk association
problem", i.e. inserting an eth card in an empty slot between other
eth cards moves at least some of the others (I'm not sure, but I think
this would be the current behaviour. - Over reboots, not in the
hot-plug case, of course).

I wouldn't mind if the <i> in eth<i> was somehow derived from the
phy. bus pos; so I'd maybe have eth3 and eth7 and if I plugged another
one it could be eth4. That way I'd only have to worry about the cards
"wandering" around when changing/drastically reconfiguring (BIOS
update?) the motherboard.

To cut a lengthy dogfight short most of that useful functionality
could be had by indexing the eth configure scripts over, say, MAC
address instead of eth<i>; that way I'd have to touch the config when
exchanging the card against a different one; but no others would
decide to move around no matter what. OK, so there might be b0rked
cards with unusable MACs out there, but for the applications I have in
mind I wouldn't use those, anyway.

(All this comes to mind because in my PFY days I had to fight with a
firewall which, after card change (might even have been driver load
order, can't remember whether it was the same driver for all 3 cards)
shifted eth<i> in a most, ah, undesirable fashion.)


So long,
   Joe

-- 
"I use emacs, which might be thought of as a thermonuclear
 word processor."
-- Neal Stephenson, "In the beginning... was the command line"

Re: [PATCH] 3c59x and resume

[Xavier Bestel]

le lun 25-03-2002 à 12:34, Joachim Breuer a écrit :
> Being able to redetect a pulled card put in a different slot as a
> "known" one giving it the same eth<i> (and associated WOL etc. config)
> as before would of course be nice, but I can't see how this can be
> cleanly done over reboots.

Some may say that being able to give the same eth<i> to the same bus
position, even after swapping the card for a new one, is more important
- think of production machines which can't afford being off-service for
too long. You just shutdown, swap the cards, poweron and you go. No
reconfig, that's how it should run.

	Xav

Re: [PATCH] 3c59x and resume

[Joachim Breuer]

Xavier Bestel <xavier.bestel@free.fr> writes:

> le lun 25-03-2002 à 12:34, Joachim Breuer a écrit :
>> Being able to redetect a pulled card put in a different slot as a
>> "known" one giving it the same eth<i> (and associated WOL etc. config)
>> as before would of course be nice, but I can't see how this can be
>> cleanly done over reboots.
>
> Some may say that being able to give the same eth<i> to the same bus
> position, even after swapping the card for a new one, is more important
> - think of production machines which can't afford being off-service for
> too long. You just shutdown, swap the cards, poweron and you go. No
> reconfig, that's how it should run.

Reading it again I wasn't all too clear in that last posting - I meant
it to show two alternatives (eth<i> stays with bus vs. eth<i> stays
with card). Each with its own advantages and disadvantages; I don't
have a fixed preference, but a slight leaning towards fixed
bus-position based numbers (spanning different drivers, if at all
possible). That would allow Xavier's scenario even with a different
type of replacement card.

(Yes of course you'd have to reconfig to swap a PCI card for an ISA
one but let's not go there, OK?)


So long,
   Joe

-- 
"I use emacs, which might be thought of as a thermonuclear
 word processor."
-- Neal Stephenson, "In the beginning... was the command line"

Re: [PATCH] 3c59x and resume

[Bill Davidsen]

On 23 Mar 2002, Robert Love wrote:

> Ideally we'd have a dynamically created array for the cards and hash
> into that, but, ugh, this is getting gross especially since 99% of us
> have one card and never remove it.

  To address the problem of running out of id's, a bitmap of "id's in use"
could be used, and number recycled. This is done infrequently and overhead
is hardly a problem, although getting things released at suspect may be.

  Getting the right options on the right card and the right card on the
expected number is another problem. I fight that all the time on my
laptop, with one NIC in the laptop and one in the dock. In spite of clear
information in modules.conf giving which driver goes with each NIC (via
alias), I don't get eth1 with no eth0 as I want, the first one is always
eth0, loads the wrong driver when not docked, and then doesn't get
initialized right by the startup scripts.

  I also have another NIC I put in a pcmcia slot to become a router on
occasion, that also gets a random NIC number. Unfortunately it doesn't
look like a trivial job to use the info in modules.conf to fix the general
random numbering. The modules.conf interface seems to work in the wrong
direction, what I think we want is "when you load this driver use this
name", so eth2 could be the only NIC in the system under some conditions.

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 2344 bytes --]

On Mon, Mar 25, 2002 at 02:44:31PM -0500, Bill Davidsen wrote:
> On 23 Mar 2002, Robert Love wrote:
> 
> > Ideally we'd have a dynamically created array for the cards and hash
> > into that, but, ugh, this is getting gross especially since 99% of us
> > have one card and never remove it.
> 
>   To address the problem of running out of id's, a bitmap of "id's in use"
> could be used, and number recycled. This is done infrequently and overhead
> is hardly a problem, although getting things released at suspect may be.

I agree but I believe this is not the real issue.

>   Getting the right options on the right card and the right card on the
> expected number is another problem. I fight that all the time on my
> laptop, with one NIC in the laptop and one in the dock. In spite of clear
> information in modules.conf giving which driver goes with each NIC (via
> alias), I don't get eth1 with no eth0 as I want, the first one is always
> eth0, loads the wrong driver when not docked, and then doesn't get
> initialized right by the startup scripts.
> 
>   I also have another NIC I put in a pcmcia slot to become a router on
> occasion, that also gets a random NIC number. Unfortunately it doesn't
> look like a trivial job to use the info in modules.conf to fix the general
> random numbering. The modules.conf interface seems to work in the wrong
> direction, what I think we want is "when you load this driver use this
> name", so eth2 could be the only NIC in the system under some conditions.

This is a subset of the problem I try to explain. 
In this case Greg has posted a nice solution a few mails ago (using a
userland tool called ifname IIRC).

Christophe

> 
> -- 
> bill davidsen <davidsen@tmr.com>
>   CTO, TMR Associates, Inc
> Doing interesting things with little computers since 1979.
> 
> -
> 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/

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

Cats seem go on the principle that it never does any harm to ask for
what you want. --Joseph Wood Krutch

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Jeff Garzik]

christophe barbé wrote:

>Here is a small patch tested with 2.4.18 and 2.4.19-pre4.
>It was proposed by Andrew but not integrated in pre4.
>
>The problem is when using the vortex driver and suspend/resuming the
>machine. Without this patch the card id is each time greater. To resume
>correctly this driver need the option enable_wol=1 but as-is it will
>only be true for the first ID. You can enable it for the first 8 IDs
>with enable_wol=1,1,1,1,1,1,1,1 but you can't do it for all IDs.
>Said another way without this patch you can't suspend/resume more than
>eight times your machine.
>
>This is a fix for the most common use. The proper fix would be IMO to
>keep a bitmap of used IDs but I don't know if it worsts it.
>Also a fix would be to separate the suspend/resume functionality from
>the wol functionality (wake up on lan).
>
>Thanks,
>Christophe
>
>--- linux/drivers/net/3c59x.c	Sat Mar 23 10:24:56 2002
>+++ linux/drivers/net/3c59x.c	Sat Mar 23 10:57:00 2002
>@@ -2891,6 +2891,9 @@
> 
> 	vp = dev->priv;
> 
>+	if (vp->card_idx == vortex_cards_found - 1)
>+         vortex_cards_found--;
>+
> 	/* AKPM: FIXME: we should have
> 	 *	if (vp->cb_fn_base) iounmap(vp->cb_fn_base);
> 	 * here
>

This patch causes module defaults to be reused -- potentially incorrectly.

This is a personal solution, that might live on temporary as an 
outside-the-tree patch... but we cannot apply this to the stable kernel.

I agree the card idx is wrong on remove.  Insert and remove a 3c59x 
cardbus card several times, and you will lose your module options too. 
 However... take note that this problem cannot be solved "the easy way" 
-- because one solution people may desire will potentially result in 
module options getting re-used incorrectly.  The above is one such solution.

If you want WOL options to "stick" or vary per-interface, we already 
have an API for that -- ethtool.  Check out drivers/net/natsemi.c for an 
example implementation.  _Tested_ patches to 3c59x that add WOL ethtool 
support are welcome, pending Andrew's approval.  Do not remove 
enable_wol for now in a stable series, but we will deprecate its use 
once ethtool support appears.

    Jeff

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]

On Mon, Mar 25, 2002 at 07:57:19PM -0500, Jeff Garzik wrote:
> This patch causes module defaults to be reused -- potentially incorrectly.

Wrong. How can the fact that a suspend/resume cycle increment the id be
worst than the fact that the same cycle return idx to the previous
state?

The argument you have against this patch is WRONG.

You think about NICs in a PCI slot. 
That's changed the day the cardbus support was moved from pcmcia to the
today implementation.
You can't expect cardbus user to stop using the suspend mode because you
expect your id to be attributed one time (that doesn't even make sense).

I agree that this patch is not a full fix (I said it in my original
post) but I disagree that it does any bad things. I would be interested
to learn about a real case ?

But ethtool seems to be very interesting and it looks like what I was
looking for. I will have a closer look at it, thank you for pointing it
to me.

> This is a personal solution, that might live on temporary as an 
> outside-the-tree patch... but we cannot apply this to the stable kernel.
> 
> I agree the card idx is wrong on remove.  Insert and remove a 3c59x 
> cardbus card several times, and you will lose your module options too. 

NO -- If I can remove/insert suspend/remove my card as I want I ever get
the same ID. 
If you want to fail the patch you need to remove/insert 2 cards in FILO
order. Then you will get a ever bigger ID but this is what you get by
default without the patch.

> However... take note that this problem cannot be solved "the easy way" 
> -- because one solution people may desire will potentially result in 
> module options getting re-used incorrectly.  The above is one such solution.

I am waiting for a real case.

> If you want WOL options to "stick" or vary per-interface, we already 
> have an API for that -- ethtool.  Check out drivers/net/natsemi.c for an 
> example implementation.  _Tested_ patches to 3c59x that add WOL ethtool 
> support are welcome, pending Andrew's approval.  Do not remove 
> enable_wol for now in a stable series, but we will deprecate its use 
> once ethtool support appears.

Noted.

Christophe

>    Jeff

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

Dogs come when they're called;
cats take a message and get back to you later. --Mary Bly

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Jeff Garzik]

christophe barbé wrote:

>On Mon, Mar 25, 2002 at 07:57:19PM -0500, Jeff Garzik wrote:
>
>>This patch causes module defaults to be reused -- potentially incorrectly.
>>
>
>Wrong. How can the fact that a suspend/resume cycle increment the id be
>worst than the fact that the same cycle return idx to the previous
>state?
>
>The argument you have against this patch is WRONG.
>
>You think about NICs in a PCI slot. 
>That's changed the day the cardbus support was moved from pcmcia to the
>today implementation.
>You can't expect cardbus user to stop using the suspend mode because you
>expect your id to be attributed one time (that doesn't even make sense).
>
>I agree that this patch is not a full fix (I said it in my original
>post) but I disagree that it does any bad things. I would be interested
>to learn about a real case ?
>

Just exactly what I described.

The current system increments the card id on each ->probe, and does not 
decrement on ->remove, which makes sense if you are hotplugging one card 
and then another -- you don't want to reuse the same module options for 
a different card.  Your patch changes this logic to, "oh wait, let's 
stop doing this and have a special case once we reach MAX_UNITS"  Thus, 
you could potentially reuse the final slot when that was not the desired 
action.

Note that I am not defending this method of card_idx usage, because 
different use cases have different requirements (as indeed you do).  But 
your patch fixes one thing at the expense of another.

I just had another idea.  Create a new module option 'default_options', 
a single integer value.  Instead of assigning "-1" to options when we 
run out of MAX_UNITS ids, assign the default_options value.



>>This is a personal solution, that might live on temporary as an 
>>outside-the-tree patch... but we cannot apply this to the stable kernel.
>>
>>I agree the card idx is wrong on remove.  Insert and remove a 3c59x 
>>cardbus card several times, and you will lose your module options too. 
>>
>
>NO -- If I can remove/insert suspend/remove my card as I want I ever get
>the same ID. 
>
"same id" is vague.  Same PCI id?  Sure, but that doesn't mean it's the 
same card, from the driver's point of view. The driver really needs to 
keep track of whether or not a new ->probe indicates a card whose MAC 
address we have seen before.

To reiterate another issue, however, suspend/resume should _not_ be 
calling the code added in your patch.  That's a non-3c59x bug somewhere.

    Jeff

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 4127 bytes --]

On Mon, Mar 25, 2002 at 11:10:27PM -0500, Jeff Garzik wrote:
> christophe barbé wrote:
> 
> >On Mon, Mar 25, 2002 at 07:57:19PM -0500, Jeff Garzik wrote:
> >
> >>This patch causes module defaults to be reused -- potentially incorrectly.
> >>
> >
> >Wrong. How can the fact that a suspend/resume cycle increment the id be
> >worst than the fact that the same cycle return idx to the previous
> >state?
> >
> >The argument you have against this patch is WRONG.
> >
> >You think about NICs in a PCI slot. 
> >That's changed the day the cardbus support was moved from pcmcia to the
> >today implementation.
> >You can't expect cardbus user to stop using the suspend mode because you
> >expect your id to be attributed one time (that doesn't even make sense).
> >
> >I agree that this patch is not a full fix (I said it in my original
> >post) but I disagree that it does any bad things. I would be interested
> >to learn about a real case ?
> >
> 
> Just exactly what I described.
> 
> The current system increments the card id on each ->probe, and does not 
> decrement on ->remove, which makes sense if you are hotplugging one card 
> and then another -- you don't want to reuse the same module options for 
> a different card.  Your patch changes this logic to, "oh wait, let's 
> stop doing this and have a special case once we reach MAX_UNITS"  Thus, 
> you could potentially reuse the final slot when that was not the desired 
> action.

Ok I really appreciate your work but here I can't believe I read what I
read. Please reread the patch :

+       if (vp->card_idx == vortex_cards_found - 1)
+         vortex_cards_found--;

It is NOT a special case when we reach MAX_UNITS.
It is a special case when we remove the last inserted card.
Even in your case where the order is important it still works.

vortex_cards_found is used to attribute the next ID.

> Note that I am not defending this method of card_idx usage, because 
> different use cases have different requirements (as indeed you do).  But 
> your patch fixes one thing at the expense of another.

No and I hope you will agree with me now.
It fixes one thing at the expense of nothing.

> I just had another idea.  Create a new module option 'default_options', 
> a single integer value.  Instead of assigning "-1" to options when we 
> run out of MAX_UNITS ids, assign the default_options value.

This would be bloat code.
IMHO MAX_UNITS is a proof of unadequate design here. 

Options should be set on a per-device basis by a userland tool which has
the required information to set them in a persistent way.

Is it not the purpose of ethtool ?
By the way the only problem in this tool is its name. It solves a
general problem for a particular subset : network device. Why not the
same tool for all modules ?

I will try to ethtoolize the vortex driver.

> >>This is a personal solution, that might live on temporary as an 
> >>outside-the-tree patch... but we cannot apply this to the stable kernel.
> >>
> >>I agree the card idx is wrong on remove.  Insert and remove a 3c59x 
> >>cardbus card several times, and you will lose your module options too. 
> >>
> >
> >NO -- If I can remove/insert suspend/remove my card as I want I ever get
> >the same ID. 
> >
> "same id" is vague.  Same PCI id?  Sure, but that doesn't mean it's the 
> same card, from the driver's point of view. The driver really needs to 
> keep track of whether or not a new ->probe indicates a card whose MAC 
> address we have seen before.

Here by 'same' ID I mean the same id used in the driver to attribute the
option. But we agree (I hope) that it is not a good way to attribute
options to a given card.

> To reiterate another issue, however, suspend/resume should _not_ be 
> calling the code added in your patch.  That's a non-3c59x bug somewhere.

Good Point. 

Christophe

>    Jeff

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

A qui sait comprendre, peu de mots suffisent.
(Intelligenti pauca.) 

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Andrew Morton]

I've added three new module options:

	global_enable_wol=N
	global_options=N
	global_full_duplex=N

If you set one of these, they apply to all 3c59x NICs
in the machine.  If you also set an entry in the corresponding
array, that will override the global_* option.

How does that sound?

Oh look, it compiled :)  Wanna test it?


--- linux-2.4.19-pre4/drivers/net/3c59x.c	Fri Dec 21 11:19:13 2001
+++ linux-akpm/drivers/net/3c59x.c	Mon Mar 25 20:42:19 2002
@@ -166,7 +166,15 @@
     - Rename wait_for_completion() to issue_and_wait() to avoid completion.h
       clash.
 
-    - See http://www.uow.edu.au/~andrewm/linux/#3c59x-2.3 for more details.
+   LK1.1.17 18Dec01 akpm
+    - PCI ID 9805 is a Python-T, not a dual-port Cyclone.  Apparently.
+      And it has NWAY.
+    - Mask our advertised modes (vp->advertising) with our capabilities
+	  (MII reg5) when deciding which duplex mode to use.
+    - Add `global_options' as default for options[].  Ditto global_enable_wol,
+      global_full_duplex.
+
+    - See http://www.zip.com.au/~akpm/linux/#3c59x-2.3 for more details.
     - Also see Documentation/networking/vortex.txt
 */
 
@@ -181,8 +189,8 @@
 
 
 #define DRV_NAME	"3c59x"
-#define DRV_VERSION	"LK1.1.16"
-#define DRV_RELDATE	"19 July 2001"
+#define DRV_VERSION	"LK1.1.17"
+#define DRV_RELDATE	"18 Dec 2001"
 
 
 
@@ -270,10 +278,13 @@ MODULE_DESCRIPTION("3Com 3c59x/3c9xx eth
 MODULE_LICENSE("GPL");
 
 MODULE_PARM(debug, "i");
+MODULE_PARM(global_options, "i");
 MODULE_PARM(options, "1-" __MODULE_STRING(8) "i");
+MODULE_PARM(global_full_duplex, "i");
 MODULE_PARM(full_duplex, "1-" __MODULE_STRING(8) "i");
 MODULE_PARM(hw_checksums, "1-" __MODULE_STRING(8) "i");
 MODULE_PARM(flow_ctrl, "1-" __MODULE_STRING(8) "i");
+MODULE_PARM(global_enable_wol, "i");
 MODULE_PARM(enable_wol, "1-" __MODULE_STRING(8) "i");
 MODULE_PARM(rx_copybreak, "i");
 MODULE_PARM(max_interrupt_work, "i");
@@ -283,10 +294,13 @@ MODULE_PARM(compaq_device_id, "i");
 MODULE_PARM(watchdog, "i");
 MODULE_PARM_DESC(debug, "3c59x debug level (0-6)");
 MODULE_PARM_DESC(options, "3c59x: Bits 0-3: media type, bit 4: bus mastering, bit 9: full duplex");
+MODULE_PARM_DESC(global_options, "3c59x: same as options, but applies to all NICs if options is unset");
 MODULE_PARM_DESC(full_duplex, "3c59x full duplex setting(s) (1)");
+MODULE_PARM_DESC(global_full_duplex, "3c59x: same as full_duplex, but applies to all NICs if options is unset");
 MODULE_PARM_DESC(hw_checksums, "3c59x Hardware checksum checking by adapter(s) (0-1)");
 MODULE_PARM_DESC(flow_ctrl, "3c59x 802.3x flow control usage (PAUSE only) (0-1)");
 MODULE_PARM_DESC(enable_wol, "3c59x: Turn on Wake-on-LAN for adapter(s) (0-1)");
+MODULE_PARM_DESC(global_enable_wol, "3c59x: same as enable_wol, but applies to all NICs if options is unset");
 MODULE_PARM_DESC(rx_copybreak, "3c59x copy breakpoint for copy-only-tiny-frames");
 MODULE_PARM_DESC(max_interrupt_work, "3c59x maximum events handled per interrupt");
 MODULE_PARM_DESC(compaq_ioaddr, "3c59x PCI I/O base address (Compaq BIOS problem workaround)");
@@ -473,7 +487,7 @@ static struct vortex_chip_info {
 	{"3c900 Boomerang 10Mbps Combo",
 	 PCI_USES_IO|PCI_USES_MASTER, IS_BOOMERANG, 64, },
 	{"3c900 Cyclone 10Mbps TPO",						/* AKPM: from Don's 0.99M */
-	 PCI_USES_IO|PCI_USES_MASTER, IS_CYCLONE|HAS_NWAY|HAS_HWCKSM, 128, },
+	 PCI_USES_IO|PCI_USES_MASTER, IS_CYCLONE|HAS_HWCKSM, 128, },
 	{"3c900 Cyclone 10Mbps Combo",
 	 PCI_USES_IO|PCI_USES_MASTER, IS_CYCLONE|HAS_HWCKSM, 128, },
 
@@ -496,8 +510,8 @@ static struct vortex_chip_info {
 	 PCI_USES_IO|PCI_USES_MASTER, IS_TORNADO|HAS_NWAY|HAS_HWCKSM, 128, },
 	{"3c980 Cyclone",
 	 PCI_USES_IO|PCI_USES_MASTER, IS_CYCLONE|HAS_HWCKSM, 128, },
-	{"3c982 Dual Port Server Cyclone",
-	 PCI_USES_IO|PCI_USES_MASTER, IS_CYCLONE|HAS_HWCKSM, 128, },
+	{"3c980C Python-T",
+	 PCI_USES_IO|PCI_USES_MASTER, IS_CYCLONE|HAS_NWAY|HAS_HWCKSM, 128, },
 
 	{"3cSOHO100-TX Hurricane",
 	 PCI_USES_IO|PCI_USES_MASTER, IS_CYCLONE|HAS_NWAY|HAS_HWCKSM, 128, },
@@ -853,6 +867,9 @@ static int full_duplex[MAX_UNITS] = {-1,
 static int hw_checksums[MAX_UNITS] = {-1, -1, -1, -1, -1, -1, -1, -1};
 static int flow_ctrl[MAX_UNITS] = {-1, -1, -1, -1, -1, -1, -1, -1};
 static int enable_wol[MAX_UNITS] = {-1, -1, -1, -1, -1, -1, -1, -1};
+static int global_options = -1;
+static int global_full_duplex = -1;
+static int global_enable_wol = -1;
 
 /* #define dev_alloc_skb dev_alloc_skb_debug */
 
@@ -995,6 +1012,8 @@ static int __devinit vortex_probe1(struc
 	SET_MODULE_OWNER(dev);
 	vp = dev->priv;
 
+	option = global_options;
+
 	/* The lower four bits are the media type. */
 	if (dev->mem_start) {
 		/*
@@ -1003,10 +1022,10 @@ static int __devinit vortex_probe1(struc
 		 */
 		option = dev->mem_start;
 	}
-	else if (card_idx < MAX_UNITS)
-		option = options[card_idx];
-	else
-		option = -1;
+	else if (card_idx < MAX_UNITS) {
+		if (options[card_idx] >= 0)
+			option = options[card_idx];
+	}
 
 	if (option > 0) {
 		if (option & 0x8000)
@@ -1099,6 +1118,11 @@ static int __devinit vortex_probe1(struc
 		vp->bus_master = (option & 16) ? 1 : 0;
 	}
 
+	if (global_full_duplex > 0)
+		vp->full_duplex = 1;
+	if (global_enable_wol > 0)
+		vp->enable_wol = 1;
+
 	if (card_idx < MAX_UNITS) {
 		if (full_duplex[card_idx] > 0)
 			vp->full_duplex = 1;
@@ -1244,11 +1268,12 @@ static int __devinit vortex_probe1(struc
 	} else
 		dev->if_port = vp->default_media;
 
-	if (dev->if_port == XCVR_MII || dev->if_port == XCVR_NWAY) {
+	if ((vp->available_media & 0x4b) || (vci->drv_flags & HAS_NWAY) ||
+		dev->if_port == XCVR_MII || dev->if_port == XCVR_NWAY) {
 		int phy, phy_idx = 0;
 		EL3WINDOW(4);
 		mii_preamble_required++;
-		mii_preamble_required++;
+		mdio_sync(ioaddr, 32);
 		mdio_read(dev, 24, 1);
 		for (phy = 0; phy < 32 && phy_idx < 1; phy++) {
 			int mii_status, phyx;
@@ -1264,6 +1289,8 @@ static int __devinit vortex_probe1(struc
 			else
 				phyx = phy;
 			mii_status = mdio_read(dev, phyx, 1);
+		printk("phy=%d, phyx=%d, mii_status=0x%04x\n",
+			phy, phyx, mii_status);
 			if (mii_status  &&  mii_status != 0xffff) {
 				vp->phys[phy_idx++] = phyx;
 				if (print_info) {
@@ -1444,11 +1471,14 @@ vortex_up(struct net_device *dev)
 		/* Read BMSR (reg1) only to clear old status. */
 		mii_reg1 = mdio_read(dev, vp->phys[0], 1);
 		mii_reg5 = mdio_read(dev, vp->phys[0], 5);
-		if (mii_reg5 == 0xffff  ||  mii_reg5 == 0x0000)
+		if (mii_reg5 == 0xffff  ||  mii_reg5 == 0x0000) {
 			;					/* No MII device or no link partner report */
-		else if ((mii_reg5 & 0x0100) != 0	/* 100baseTx-FD */
+		} else {
+			mii_reg5 &= vp->advertising;
+			if ((mii_reg5 & 0x0100) != 0	/* 100baseTx-FD */
 				 || (mii_reg5 & 0x00C0) == 0x0040) /* 10T-FD, but not 100-HD */
 			vp->full_duplex = 1;
+		}
 		vp->partner_flow_ctrl = ((mii_reg5 & 0x0400) != 0);
 		if (vortex_debug > 1)
 			printk(KERN_INFO "%s: MII #%d status %4.4x, link partner capability %4.4x,"
@@ -1669,8 +1699,10 @@ vortex_timer(unsigned long data)
 			if (mii_status & 0x0004) {
 				int mii_reg5 = mdio_read(dev, vp->phys[0], 5);
 				if (! vp->force_fd  &&  mii_reg5 != 0xffff) {
-					int duplex = (mii_reg5&0x0100) ||
-						(mii_reg5 & 0x01C0) == 0x0040;
+					int duplex;
+
+					mii_reg5 &= vp->advertising;
+					duplex = (mii_reg5&0x0100) || (mii_reg5 & 0x01C0) == 0x0040;
 					if (vp->full_duplex != duplex) {
 						vp->full_duplex = duplex;
 						printk(KERN_INFO "%s: Setting %s-duplex based on MII "
@@ -1753,9 +1785,11 @@ static void vortex_tx_timeout(struct net
 		   dev->name, inb(ioaddr + TxStatus),
 		   inw(ioaddr + EL3_STATUS));
 	EL3WINDOW(4);
-	printk(KERN_ERR "  diagnostics: net %04x media %04x dma %8.8x.\n",
-		   inw(ioaddr + Wn4_NetDiag), inw(ioaddr + Wn4_Media),
-		   inl(ioaddr + PktStatus));
+	printk(KERN_ERR "  diagnostics: net %04x media %04x dma %08x fifo %04x\n",
+			inw(ioaddr + Wn4_NetDiag),
+			inw(ioaddr + Wn4_Media),
+			inl(ioaddr + PktStatus),
+			inw(ioaddr + Wn4_FIFODiag));
 	/* Slight code bloat to be user friendly. */
 	if ((inb(ioaddr + TxStatus) & 0x88) == 0x88)
 		printk(KERN_ERR "%s: Transmitter encountered 16 collisions --"
@@ -2538,7 +2572,6 @@ vortex_close(struct net_device *dev)
 			((vp->drv_flags & HAS_HWCKSM) == 0) &&
 			(hw_checksums[vp->card_idx] == -1)) {
 		printk(KERN_WARNING "%s supports hardware checksums, and we're not using them!\n", dev->name);
-		printk(KERN_WARNING "Please see http://www.uow.edu.au/~andrewm/zerocopy.html\n");
 	}
 #endif
 		
--- linux-2.4.19-pre4/Documentation/networking/vortex.txt	Sun Aug 12 12:27:36 2001
+++ linux-akpm/Documentation/networking/vortex.txt	Mon Mar 25 20:45:35 2002
@@ -117,6 +117,12 @@ options=N1,N2,N3,...
   will force full-duplex 100base-TX, rather than allowing the usual
   autonegotiation.
 
+global_options=N
+
+  Sets the `options' parameter for all 3c59x NICs in the machine. 
+  Entries in the `options' array above will override any setting of
+  this.
+
 full_duplex=N1,N2,N3...
 
   Similar to bit 9 of 'options'.  Forces the corresponding card into
@@ -126,6 +132,11 @@ full_duplex=N1,N2,N3...
   In fact, please don't use this at all! You're better off getting
   autonegotiation working properly.
 
+global_full_duplex=N1
+
+  Sets full duplex mode for all 3c59x NICs in the machine.  Entries
+  in the `full_duplex' array above will override any setting of this.
+
 flow_ctrl=N1,N2,N3...
 
   Use 802.3x MAC-layer flow control.  The 3com cards only support the
@@ -211,6 +222,12 @@ enable_wol=N1,N2,N3,...
   Becker's `ether-wake' application may be used to wake suspended
   machines.
 
+  Also enables the NIC's power management support.
+
+global_enable_wol=N
+
+  Sets enable_wol mode for all 3c59x NICs in the machine.  Entries in
+  the `enable_wol' array above will override any setting of this.
 
 Media selection
 ---------------


-

Re: [PATCH] 3c59x and resume

[christophe barbé]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Mon, Mar 25, 2002 at 08:50:38PM -0800, Andrew Morton wrote:
> I've added three new module options:
> 
> 	global_enable_wol=N
> 	global_options=N
> 	global_full_duplex=N
> 
> If you set one of these, they apply to all 3c59x NICs
> in the machine.  If you also set an entry in the corresponding
> array, that will override the global_* option.
> 
> How does that sound?

A different fix. Which in my case is enough (as was the previous one).

> Oh look, it compiled :)  Wanna test it?

Applied, Compiled, Tested.
<marcelo> Hope to see it in 2.4.19. </marcelo>

Andrew would you be interested in a patch to ethtoolize this driver ?
Has ethtool a futur in the next kernel serie ?

Thank you,
Christophe

-- 
Christophe Barbé <christophe.barbe@ufies.org>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E

Cats are rather delicate creatures and they are subject to a good
many ailments, but I never heard of one who suffered from insomnia.
--Joseph Wood Krutch

[-- Attachment #2: Type: application/pgp-signature, Size: 241 bytes --]

Re: [PATCH] 3c59x and resume

[Jeff Garzik]

christophe barbé wrote:

>On Mon, Mar 25, 2002 at 08:50:38PM -0800, Andrew Morton wrote:
>
>>Oh look, it compiled :)  Wanna test it?
>>
>
>Applied, Compiled, Tested.
><marcelo> Hope to see it in 2.4.19. </marcelo>
>

Great..


>Andrew would you be interested in a patch to ethtoolize this driver ?
>Has ethtool a futur in the next kernel serie ?
>

Yep, we want ethtool support for all net drivers, in fact...

    Jeff