[PATCH] devexit fixes in i82092.c

[PATCH] devexit fixes in i82092.c

[Anders Gustafsson]

Hi,

this patch fixes "undefined reference to `local symbols in discarded
section .text.exit'" linking error.

-- 

//anders/g

--- linux-2.5.7-pre1/drivers/pcmcia/i82092.c	Fri Nov  9 22:45:35 2001
+++ linux-2.5.7-pre1-reiser/drivers/pcmcia/i82092.c	Sat Mar 16 01:39:42 2002
@@ -42,7 +42,7 @@
 	name:           "i82092aa",
 	id_table:       i82092aa_pci_ids,
 	probe:          i82092aa_pci_probe,
-	remove:         i82092aa_pci_remove,
+	remove:         __devexit_p(i82092aa_pci_remove),
 	suspend:        NULL,
 	resume:         NULL 
 };
@@ -88,7 +88,7 @@
 static int socket_count;  /* shortcut */                                  	                                	
 
 
-static int __init i82092aa_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
+static int __devinit i82092aa_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	unsigned char configbyte;
 	int i;
@@ -160,7 +160,7 @@
 	return 0;
 }
 
-static void __exit i82092aa_pci_remove(struct pci_dev *dev)
+static void __devexit i82092aa_pci_remove(struct pci_dev *dev)
 {
 	enter("i82092aa_pci_remove");
 	

Re: [PATCH] devexit fixes in i82092.c

[Linus Torvalds]

On Sat, 16 Mar 2002, Anders Gustafsson wrote:
>
> this patch fixes "undefined reference to `local symbols in discarded
> section .text.exit'" linking error.

Looking more at this, I actually think that the _real_ fix is to call all
drivers exit functions at kernel shutdown, and not discard the exit
section when linking into the tree.

That, together with the device tree, automatically gives us the
_correct_ shutdown sequence, soemthing we don't have right now.

Anybody willing to look into this, and get rid of that __devexit_p()
thing?

		Linus

Re: [PATCH] devexit fixes in i82092.c

[Jeff Garzik]

Linus Torvalds wrote:

>
>On Sat, 16 Mar 2002, Anders Gustafsson wrote:
>
>>this patch fixes "undefined reference to `local symbols in discarded
>>section .text.exit'" linking error.
>>
>
>Looking more at this, I actually think that the _real_ fix is to call all
>drivers exit functions at kernel shutdown, and not discard the exit
>section when linking into the tree.
>
>That, together with the device tree, automatically gives us the
>_correct_ shutdown sequence, soemthing we don't have right now.
>
>Anybody willing to look into this, and get rid of that __devexit_p()
>thing?
>

(thinking vaguely long-term)

I wonder if mochel already code for this, or has thought about this... 
 Just like suspend, IMO we ideally should use the device tree to 
shutdown the system, agreed?

Further, I wonder if the reboot/shutdown notifiers can be replaced with 
device tree control over those events...

    Jeff

Re: [PATCH] devexit fixes in i82092.c

[Linus Torvalds]

On Fri, 15 Mar 2002, Jeff Garzik wrote:
> 
> I wonder if mochel already code for this, or has thought about this... 
>  Just like suspend, IMO we ideally should use the device tree to 
> shutdown the system, agreed?

Ideally we should, yes. Although if we really turn off power, it doesn't 
much matter.

> Further, I wonder if the reboot/shutdown notifiers can be replaced with 
> device tree control over those events...

This is what I want. Those reboot/shutdown notifiers are completely and 
utterly buggy, and cannot sanely handle any kind of device hierarchy.

		Linus

Re: [PATCH] devexit fixes in i82092.c

[Keith Owens]

On Fri, 15 Mar 2002 23:40:30 -0800 (PST), 
Linus Torvalds <torvalds@transmeta.com> wrote:
>On Fri, 15 Mar 2002, Jeff Garzik wrote:
>> Further, I wonder if the reboot/shutdown notifiers can be replaced with 
>> device tree control over those events...
>
>This is what I want. Those reboot/shutdown notifiers are completely and 
>utterly buggy, and cannot sanely handle any kind of device hierarchy.

Does that mean that we also get rid of the initcall methods?  If
shutdown follows a device tree then startup should also use that tree.
OTOH module load and unload require well defined startup and shutdown
functions, modules cannot rely on device trees.

Re: [PATCH] devexit fixes in i82092.c

[Linus Torvalds]

On Sat, 16 Mar 2002, Keith Owens wrote:
>
> Does that mean that we also get rid of the initcall methods?  If
> shutdown follows a device tree then startup should also use that tree.

You cannot _build_ the tree without the initcall methods - it's populating
the tree that the initcalls mostly do, after all.

We could make one of the methods be "startup", of course, and move the
actual device initialization there (and leave just the "find driver" in
the initcall logic), but that would not get rid of the initcalls, it would
just split them into two parts.

		Linus

Re: [PATCH] devexit fixes in i82092.c

[Jeff Garzik]

Linus Torvalds wrote:

>On Fri, 15 Mar 2002, Jeff Garzik wrote:
>
>>I wonder if mochel already code for this, or has thought about this... 
>> Just like suspend, IMO we ideally should use the device tree to 
>>shutdown the system, agreed?
>>
>
>Ideally we should, yes. Although if we really turn off power, it doesn't 
>much matter.
>
It matters to a software engineering wonk like me :)   I know it 
-really- doesn't matter, but from a theoretical perspective, if we are 
trying to achieve the "everything is hotpluggable" model, poweroff via 
device tree will naturally fall out from that.

If it makes it easier for some, I consider poweroff not as an act unto 
itself, but as a transition to state D3cold. :)  And since we will 
eventually be able to handle transition to similar low-power states, we 
might as well follow similar/the same code paths.

>>Further, I wonder if the reboot/shutdown notifiers can be replaced with 
>>device tree control over those events...
>>
>
>This is what I want. Those reboot/shutdown notifiers are completely and 
>utterly buggy, and cannot sanely handle any kind of device hierarchy.
>
yep

    Jeff

Re: [PATCH] devexit fixes in i82092.c

[Jeff Garzik]

Linus Torvalds wrote:

>We could make one of the methods be "startup", of course, and move the
>actual device initialization there (and leave just the "find driver" in
>the initcall logic), but that would not get rid of the initcalls, it would
>just split them into two parts.
>
doing this has been mentioned independently a couple times, in fact...

And this may be a tangent, or maybe not:  like I was trying to explain 
to Gerard about regarding SCSI devices, it is often valuable to separate 
the two steps.  Gerard complained about the new PCI API not being able 
to register devices (_register_, not probe) in the order he wished.  And 
my response was... sure you can.  Just break it up into two steps, find 
devices, and register devices.  The ordering of the register calls in 
the second step are entirely up the driver and/or subsystem.  Similarly, 
the IDE subsystem could handle the mapping of the BIOS-ordered 
/dev/hd[a-d] completely independently of probing and registering hosts 
and devices.  [which, in turn, is useful in moving to a more dynamic 
/dev among other things]  Right now the IDE probe order is a bit 
delicate, and full of hueristics that could be cleaned up with such a 
separation.

    Jeff, in a rambling mood

Re: [PATCH] devexit fixes in i82092.c

[Arjan van de Ven]

On Fri, Mar 15, 2002 at 11:40:30PM -0800, Linus Torvalds wrote:
> 
> On Fri, 15 Mar 2002, Jeff Garzik wrote:
> > 
> > I wonder if mochel already code for this, or has thought about this... 
> >  Just like suspend, IMO we ideally should use the device tree to 
> > shutdown the system, agreed?
> 
> Ideally we should, yes. Although if we really turn off power, it doesn't 
> much matter.

It kind of does for warm reboots. I'm getting more and more reports that
on warm reboot, the bios then can't boot again because we left some
hardware (usually the scsi or ide controller) in a state the bios didn't expect.
While I consider it a bios duty to reset the hw, using the device-tree for
clean shutdown of hardware at least would allow us to make it work.

> This is what I want. Those reboot/shutdown notifiers are completely and 
> utterly buggy, and cannot sanely handle any kind of device hierarchy.

Device owned notifiers could indeed go; the question is if
non-device owned ones (the only purpose of those would
probably cluster filesystems) should make a "fake" device or
keep using the current mechanism.

Greetings,
  Arjan van de Ven

Re: [PATCH] devexit fixes in i82092.c

[Keith Owens]

On Sat, 16 Mar 2002 00:13:21 -0800 (PST), 
Linus Torvalds <torvalds@transmeta.com> wrote:
>On Sat, 16 Mar 2002, Keith Owens wrote:
>>
>> Does that mean that we also get rid of the initcall methods?  If
>> shutdown follows a device tree then startup should also use that tree.
>
>You cannot _build_ the tree without the initcall methods - it's populating
>the tree that the initcalls mostly do, after all.

Confusion of names.  I read 'device tree' and thought you were talking
about an initialization tree that was built from PCI and USB data,
independent of kernel link order.  If your 'device tree' is just the
existing initcall list derived from $(obj-y) then I agree that we
should bot have separate shutdown functions.  module_exit should be
retained for objects that are built into the kernel and the shutdown
code should run the module_exit functions in reverse order to
initialization.

The question of separating register and probe is independent of the
above.  It is definitely a good idea to separate register and probe, if
only to avoid all the existing races on module unload (using
__this_module and mod_inc_count everywhere in mainline code is not a
perfect solution).

Re: [PATCH] devexit fixes in i82092.c

[David Woodhouse]

torvalds@transmeta.com said:
>  Ideally we should, yes. Although if we really turn off power, it
> doesn't  much matter. 

But we might just be rebooting, not turning off power. And in that case we 
may want to ensure devices are returned to a sane state. Like ensuring that 
the CPU startup vector is pointing at a flash chip which is in _read_ mode, 
not returning status words.

Yes, it happens. I was pondering a reboot notifier but didn't like that much
so was ignoring the problem for now.

--
dwmw2

Re: [PATCH] devexit fixes in i82092.c

[Jeff Garzik]

Alan Cox wrote:

>>If it makes it easier for some, I consider poweroff not as an act unto 
>>itself, but as a transition to state D3cold. :)  And since we will 
>>
>
>That isnt neccessarily a good idea. Not every BIOS is terribly keen when
>faced with a soft boot and someone having powered off all the PCI bridges.
>

s/D3cold/D2bios/ if that's easier to think about :)  I think we can 
apply reboot and poweroff scenarios to the device tree without a 
problem, was the basic point.  It's just another device powerdown state. 
 Instead of powering off the PCI bridge, (random example) we restore the 
PCI bridge settings to exactly the state it was in when the BIOS booted 
the kernel.

    Jeff

Re: [PATCH] devexit fixes in i82092.c

[Alan Cox]

> If it makes it easier for some, I consider poweroff not as an act unto 
> itself, but as a transition to state D3cold. :)  And since we will 

That isnt neccessarily a good idea. Not every BIOS is terribly keen when
faced with a soft boot and someone having powered off all the PCI bridges.

> >This is what I want. Those reboot/shutdown notifiers are completely and 
> >utterly buggy, and cannot sanely handle any kind of device hierarchy.
> >
> yep

They were never designed to. They should go - the new PM code can handle
everything they do, including refusals and more. It will also fix the ordering 
problems some people hit with them.

Alan

Re: [PATCH] devexit fixes in i82092.c

[Eric W. Biederman]

Jeff Garzik <jgarzik@mandrakesoft.com> writes:

> 
> (thinking vaguely long-term)
> 
> I wonder if mochel already code for this, or has thought about this... Just like
> 
> suspend, IMO we ideally should use the device tree to shutdown the system,
> agreed?
> 
> Further, I wonder if the reboot/shutdown notifiers can be replaced with device
> tree control over those events...

Please for the Linux booting Linux scenario it is mandatory we get this right
for reboot.  I know for a fact that currently we leave active receive buffers on
network cards when we reboot. (If you haven't downed the interface).  So it
is possible for a network packet to come in and hose a machine that is rebooting.

Occasionally I test my linux booting linux code by loading memtest86 to make certain
I don't have something like that going on.  And the first time I tried it I hadn't
downed the interfaces and I actually saw a network packet come in and corrupt
memory.  I also have similar reports from other about disk subsystems.

Eric

Re: [PATCH] devexit fixes in i82092.c

[Alan Cox]

> Please for the Linux booting Linux scenario it is mandatory we get this right
> for reboot.  I know for a fact that currently we leave active receive buffers on
> network cards when we reboot. (If you haven't downed the interface).  So it
> is possible for a network packet to come in and hose a machine that is rebooting.

Thats a bios bug. Its pretty much the whole reason for having bus master
enable bits in the PCI configuration. The BIOS should have killed the bus
masters.

Alan

Re: [PATCH] devexit fixes in i82092.c

[Eric W. Biederman]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> > Please for the Linux booting Linux scenario it is mandatory we get this right
> > for reboot.  I know for a fact that currently we leave active receive buffers
> on
> 
> > network cards when we reboot. (If you haven't downed the interface).  So it
> > is possible for a network packet to come in and hose a machine that is
> rebooting.
> 
> 
> Thats a bios bug. Its pretty much the whole reason for having bus master
> enable bits in the PCI configuration. The BIOS should have killed the bus
> masters.

In the general reboot case yes it is a BIOS bug.  In the general Linux
booting Linux case there is no BIOS involved.

Eric

Re: [PATCH] devexit fixes in i82092.c

[Eric W. Biederman]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> > In the general reboot case yes it is a BIOS bug.  In the general Linux
> > booting Linux case there is no BIOS involved.
> 
> In that case yes I can see why you want to turn the bus masters off when you
> boot the new kernel

And in the general case I'd like to what rmmod does so that
the device is in a sane state so the Linux driver can handle it.

Very rarely does rmmod leave a device in a state where you cannot
run imsmod.

Eric

Re: [PATCH] devexit fixes in i82092.c

[Linus Torvalds]

On Sat, 16 Mar 2002, Alan Cox wrote:
> > In the general reboot case yes it is a BIOS bug.  In the general Linux
> > booting Linux case there is no BIOS involved.
>
> In that case yes I can see why you want to turn the bus masters off when you
> boot the new kernel

Truning bus masters off is easy enough, and could just be done by some
generic PCI reboot code. But for a linux-linux boot I think you also want
to turn off interrupt generation to make sure some device isn't screaming
on some irq, and that definitely requires explicit help by the driver
itself.

One question that hasn't come up: do we actually want to use the "remove"
function for this, or have a separate shutdown function? Are there reasons
to not use "remove" for shutdown?

		Linus

Re: [PATCH] devexit fixes in i82092.c

[Alan Cox]

> In the general reboot case yes it is a BIOS bug.  In the general Linux
> booting Linux case there is no BIOS involved.

In that case yes I can see why you want to turn the bus masters off when you
boot the new kernel

Re: [PATCH] devexit fixes in i82092.c

[Eric W. Biederman]

Linus Torvalds <torvalds@transmeta.com> writes:

> On Sat, 16 Mar 2002, Alan Cox wrote:
> > > In the general reboot case yes it is a BIOS bug.  In the general Linux
> > > booting Linux case there is no BIOS involved.
> >
> > In that case yes I can see why you want to turn the bus masters off when you
> > boot the new kernel
> 
> Truning bus masters off is easy enough, and could just be done by some
> generic PCI reboot code. But for a linux-linux boot I think you also want
> to turn off interrupt generation to make sure some device isn't screaming
> on some irq, and that definitely requires explicit help by the driver
> itself.
> 
> One question that hasn't come up: do we actually want to use the "remove"
> function for this, or have a separate shutdown function? Are there reasons
> to not use "remove" for shutdown?

I had initially thought of using module_exit().  Which has the problem
of being hard to get the order right.  

remove is good (and the driver testing scenario of rmmod old_driver;
insmod new_driver) makes certain all of the code is there.

The only current downside with remove is it doesn't currently handle
non-pci devices.  Which just means that we need to generalize a little
bit.  That was already on the todo list right?

A primary example is things like the apics need cleanup code run on
them as well.  Because on x86 you go back to using the legacy PIC
and be in virtual wire mode or you really confuse the kernel.

I suspect we might want to run both the removes and the module_exit
code.  If we are going to get rid of the reboot notifier.

Eric

Re: [PATCH] devexit fixes in i82092.c

[Dave Jones]

On Sat, Mar 16, 2002 at 02:26:23PM -0800, Linus Torvalds wrote:

 > One question that hasn't come up: do we actually want to use the "remove"
 > function for this, or have a separate shutdown function? Are there reasons
 > to not use "remove" for shutdown?

 Something that came to mind on reading this thread that I want
 clarification on.. Take for example, the case of an IDE controller.
 If on shutdown we walk the driverfs tree shutting things down,
 it's going to power off its hard disks, then do whatever magic is
 needed to the ide host bridge.

 This makes sense for a shutdown, and suspend-to-disk, but not for
 a reboot imo (senseless spinning down/up of drives).
 So some means is probably going to be needed for drivers to
 distinguish between a reboot & shutdown/suspend.

 There may be other such devices too, but this was the more
 obvious one that came to mind. Or am I way off base here?
 
-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs

Re: [PATCH] devexit fixes in i82092.c

[Pavel Machek]

Hi!

>  This makes sense for a shutdown, and suspend-to-disk, but not for
>  a reboot imo (senseless spinning down/up of drives).
>  So some means is probably going to be needed for drivers to
>  distinguish between a reboot & shutdown/suspend.

I'd guess 'suspend' callback would handle this, and suspend already has
"state we want to enter" as a parameter.
								Pavel
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: [PATCH] devexit fixes in i82092.c

[Pavel Machek]

Hi!

> > Ideally we should, yes. Although if we really turn off power, it doesn't 
> > much matter.
> 
> It kind of does for warm reboots. I'm getting more and more reports that
> on warm reboot, the bios then can't boot again because we left some
> hardware (usually the scsi or ide controller) in a state the bios didn't expect.

Actually, on omnibook xe3, linux will not come up after warm boot, because it
leaves soundcard in too bad state :-(.
									Pavel
-- 
Philips Velo 1: 1"x4"x8", 300gram, 60, 12MB, 40bogomips, linux, mutt,
details at http://atrey.karlin.mff.cuni.cz/~pavel/velo/index.html.

Re: [PATCH] devexit fixes in i82092.c

[Alexander Stohr]

I thought Linux is an OS of choices.
Calling folks for removing an opition
is not really what i do think is good.

There might be still situations where
it makes sense stripping of the exit
codings of the kernel.

Namely i would classify diskless embedded
systems (like consumer devices), that are 
allowed to shut down instantly by just
powering them off, to be such cases.

It would make life much easier for people
that do program for such targets if the
already existing optional macros would
stay in the source as they are now.

> On Sat, 16 Mar 2002, Anders Gustafsson wrote:
> >
> > this patch fixes "undefined reference to `local symbols in discarded
> > section .text.exit'" linking error.
> 
> Looking more at this, I actually think that the _real_ fix is to call all
> drivers exit functions at kernel shutdown, and not discard the exit
> section when linking into the tree.
> 
> That, together with the device tree, automatically gives us the
> _correct_ shutdown sequence, soemthing we don't have right now.
> 
> Anybody willing to look into this, and get rid of that __devexit_p()
> thing?
> 
>                 Linus

Re: [PATCH] devexit fixes in i82092.c

[Patrick Mochel]

(Sorry, been on vacation...)

On Fri, 15 Mar 2002, Jeff Garzik wrote:

> Linus Torvalds wrote:
> 
> >
> >On Sat, 16 Mar 2002, Anders Gustafsson wrote:
> >
> >>this patch fixes "undefined reference to `local symbols in discarded
> >>section .text.exit'" linking error.
> >>
> >
> >Looking more at this, I actually think that the _real_ fix is to call all
> >drivers exit functions at kernel shutdown, and not discard the exit
> >section when linking into the tree.
> >
> >That, together with the device tree, automatically gives us the
> >_correct_ shutdown sequence, soemthing we don't have right now.
> >
> >Anybody willing to look into this, and get rid of that __devexit_p()
> >thing?
> >
> 
> (thinking vaguely long-term)
> 
> I wonder if mochel already code for this, or has thought about this... 
>  Just like suspend, IMO we ideally should use the device tree to 
> shutdown the system, agreed?

Yes, I have thought about this, and we should be doing it. Not necessarily 
for the case of shutdown, but for the case of reboot, and things like 
linux-linux booting. 

The code is simply a depth-first walk of the device tree. I had code to do 
it once upon a time, for the purpose of suspending devices; it's 
straightforward. 

> Further, I wonder if the reboot/shutdown notifiers can be replaced with 
> device tree control over those events...

Yes, I think so. And the old PM code can also be replaced with the generic 
interface, with similar walks of the tree. I've been procrastinating, but 
now is a good time to start making the change.

What I'm thinking for the reboot case is that we just use the remove() 
callback. This would queisce the device and put it in a low power state. 
This would be different that module_exit, though. Perhaps it would only 
decrement the module usage count. Maybe it already does that; I haven't 
looked.

	-pat

Re: [PATCH] devexit fixes in i82092.c

[Patrick Mochel]

On Sat, 16 Mar 2002, Linus Torvalds wrote:

> 
> 
> On Sat, 16 Mar 2002, Keith Owens wrote:
> >
> > Does that mean that we also get rid of the initcall methods?  If
> > shutdown follows a device tree then startup should also use that tree.
> 
> You cannot _build_ the tree without the initcall methods - it's populating
> the tree that the initcalls mostly do, after all.

Actually, you could theoretically get rid of the device initcalls. 

The device tree is built by the bus drivers, before the driver initcalls. 
The driver initcalls simply check for existence of devices they support. 

Once the device tree is built, you could do one walk of the tree, take the 
device id of each device, and query the device drivers to see if they 
support it or not. 

This requires the device ID tables to be in a special section; or to use 
the probe() function and pass it a device id. Or, pass the device id to 
/sbin/hotplug so it can load the right module.

> We could make one of the methods be "startup", of course, and move the
> actual device initialization there (and leave just the "find driver" in
> the initcall logic), but that would not get rid of the initcalls, it would
> just split them into two parts.

Yes. jgarzik has been saying this for some time, and I fully agree. You 
really only want to check if you have a device you support and continue 
on. (By delaying initialization, you can also keep the device in a low 
power state until you're really ready to use it as well.)

	-pat

Re: [PATCH] devexit fixes in i82092.c

[Patrick Mochel]

On Sat, 16 Mar 2002, Alan Cox wrote:

> > If it makes it easier for some, I consider poweroff not as an act unto 
> > itself, but as a transition to state D3cold. :)  And since we will 
> 
> That isnt neccessarily a good idea. Not every BIOS is terribly keen when
> faced with a soft boot and someone having powered off all the PCI bridges.

You can get around that by creating a PCI bridge driver. There really 
isn't much that you want to do to a bridge anyway, even on power 
transitions. Remove would be simply something like (I think) Jeff 
mentioned later - simply restore config space to it's boot-time 
configuration. 

	-pat