[PATCHlet] 2.5.23 usb, ide

[PATCHlet] 2.5.23 usb, ide

[Andries.Brouwer]

Earlier, I reported that 2.5.22 and 2.5.23 do not boot.
With Marcin's IDE-93 this is corrected, and the system boots.

Earlier, I reported an oops at shutdown. I just looked at
what causes the oops and find that the call
	hcd->driver->stop()
is executed while hcd->driver->stop is NULL.

So, applying

diff -r -u linux-2.5.23/linux/drivers/usb/core/hcd-pci.c linux-2.5.23a/linux/drivers/usb/core/hcd-pci.c
--- linux-2.5.23/linux/drivers/usb/core/hcd-pci.c	Mon Jun 17 19:35:40 2002
+++ linux-2.5.23a/linux/drivers/usb/core/hcd-pci.c	Thu Jun 20 17:48:09 2002
@@ -209,13 +209,16 @@
 
 	if (in_interrupt ()) BUG ();
 
+	if (!hcd->driver) BUG ();
+
 	hub = hcd->self.root_hub;
 	hcd->state = USB_STATE_QUIESCING;
 
 	dbg ("%s: roothub graceful disconnect", hcd->self.bus_name);
 	usb_disconnect (&hub);
 
-	hcd->driver->stop (hcd);
+	if (hcd->driver->stop)
+		hcd->driver->stop (hcd);
 	hcd->state = USB_STATE_HALT;
 
 	free_irq (hcd->irq, hcd);
@@ -232,7 +235,8 @@
 	if (atomic_read (&hcd->self.refcnt) != 1)
 		err ("usb_hcd_pci_remove %s, count != 1", hcd->self.bus_name);
 
-	hcd->driver->hcd_free (hcd);
+	if (hcd->driver->hcd_free)
+		hcd->driver->hcd_free (hcd);
 }
 EXPORT_SYMBOL (usb_hcd_pci_remove);
 

stops the oops.
USB people may worry whether hcd->driver->stop should
have been non-NULL.

Andries
with a somewhat working 2.5

Re: [linux-usb-devel] [PATCHlet] 2.5.23 usb, ide

[David Brownell]

> Earlier, I reported an oops at shutdown. I just looked at
> what causes the oops and find that the call
> 	hcd->driver->stop()
> is executed while hcd->driver->stop is NULL.
> 
> ...
> USB people may worry whether hcd->driver->stop should
> have been non-NULL.

Not supposed to be possible.  All those hc_driver structures
are declared "static const", with non-null stop().  Looks like
something was zeroing some driver's readonly data segment while
it was still in use.  (And who knows that else!)

What driver was getting that treatment?

- Dave

Re: [linux-usb-devel] [PATCHlet] 2.5.23 usb, ide

[Andries.Brouwer]

    From: David Brownell <david-b@pacbell.net>

    > Earlier, I reported an oops at shutdown. I just looked at
    > what causes the oops and find that the call
    >     hcd->driver->stop()
    > is executed while hcd->driver->stop is NULL.
    > 
    > ...
    > USB people may worry whether hcd->driver->stop should
    > have been non-NULL.

    Not supposed to be possible.  All those hc_driver structures
    are declared "static const", with non-null stop().  Looks like
    something was zeroing some driver's readonly data segment while
    it was still in use.  (And who knows that else!)

Now wild pointers happen, and it took me some effort a few
weeks ago to catch one.  But at first one should look at
simpler explanations.

Now that you tell me that these things are set at initialization
and never changed, the initialization must be wrong. And indeed,
it says "__devexit_p(uhci_stop)" and this yields NULL.

So, my previous stopoops patch can be replaced by

diff -r linux-2.5.23/linux/drivers/usb/host/uhci-hcd.c linux-2.5.23a/linux/drivers/usb/host/uhci-hcd.c
2431c2431
< static void __devexit uhci_stop(struct usb_hcd *hcd)
---
> static void uhci_stop(struct usb_hcd *hcd)
2512c2512
<       stop:                   __devexit_p(uhci_stop),
---
>       stop:                   uhci_stop,

(pasted from another window).

Andries

Re: [linux-usb-devel] [PATCHlet] 2.5.23 usb, ide

[Petr Vandrovec]

On 20 Jun 02 at 20:49, Andries.Brouwer@cwi.nl wrote:
> 
> Now that you tell me that these things are set at initialization
> and never changed, the initialization must be wrong. And indeed,
> it says "__devexit_p(uhci_stop)" and this yields NULL.

Now when kernel tries to shutdown devices on poweroff, should
not we change __devexit_p() meaning? Like always build kernel with
hotplug enabled?
                                        Thanks,
                                            Petr Vandrovec
                                            vandrove@vc.cvut.cz
                                            

Re: [linux-usb-devel] [PATCHlet] 2.5.23 usb, ide

[Greg KH]

On Thu, Jun 20, 2002 at 09:40:27PM +0200, Petr Vandrovec wrote:
> On 20 Jun 02 at 20:49, Andries.Brouwer@cwi.nl wrote:
> > 
> > Now that you tell me that these things are set at initialization
> > and never changed, the initialization must be wrong. And indeed,
> > it says "__devexit_p(uhci_stop)" and this yields NULL.
> 
> Now when kernel tries to shutdown devices on poweroff, should
> not we change __devexit_p() meaning? Like always build kernel with
> hotplug enabled?

You should always enable CONFIG_HOTPLUG anyway :)

Seriously, you are correct, we will probably have to go back and audit
the __devexit functions when we put poweroff support in.

thanks,

greg k-h

Re: [linux-usb-devel] [PATCHlet] 2.5.23 usb, ide

[Greg KH]

On Thu, Jun 20, 2002 at 08:49:36PM +0200, Andries.Brouwer@cwi.nl wrote:
> Now that you tell me that these things are set at initialization
> and never changed, the initialization must be wrong. And indeed,
> it says "__devexit_p(uhci_stop)" and this yields NULL.
> 
> So, my previous stopoops patch can be replaced by

<snip>

Yes, that's a correct patch.  I've added it to my tree, thanks.

greg k-h