2000-11-15 18:16:11

by Adam J. Richter

[permalink] [raw]
Subject: CONFIG_USB_HOTPLUG (was Patch(?): linux-2.4.0-test11-pre4/drivers/sound/yss225.c compile failure)

>From [email protected] Wed Nov 15 09:04:36 2000
>> From: Jeff Garzik [mailto:[email protected]]
>>
>> Greg KH wrote:
>> > On Wed, Nov 15, 2000 at 12:29:15AM -0500, Jeff Garzik wrote:
>> > > If we are going to create CONFIG_USB_HOTPLUG, we must -eliminate-
>> > > CONFIG_HOTPLUG, and create CONFIG_PCI_HOTPLUG, and
>> > > CONFIG_ANOTHERBUS_HOTPLUG and so on, for each hotplug bus.
>> >
>> > Argh!
>> > I thought the whole point of this was to make there be only
>> one hotplug
>> > strategy, due to the fact that this is a real need.
>> >
>> > Please let's not go down this path. It was all starting to look so
>> > nice...
>>
>> I -want- there to be only one hotplug strategy, but Adam seemed to be
>> talking about the opposite, with his CONFIG_USB_HOTPLUG suggestion.

>I told Adam that I didn't want that patch, but it's not
>up to me now.

You said you wanted to "hold of on CONFIG_USB_HOTPLUG for now",
which I take to mean up to 2.4.0.

I have not asked that CONFIG_USB_HOTPLUG be put in 2.4.0, although
I would not mind. I am just agreeing with you (Randy) when you
identified the problem and wrote in linux-usb-devel "[Why] is it safe to
use __devinitdata on the usb_device_id table? It's used during any new
device connect, after driver init, right ...?" You were right: the
__devinitdata being used in the USB drivers will probably crash the
kernel if CONFIG_HOTPLUG is not defined and the USB code attempts to
recover from an error by faking disconnect/reconnect.

The statements about how we might address this issue more
fully have been about in the context of after 2.4.0. To refresh your
memory, in my first message on this thread I wrote:

| After 2.4.0, and after the fake disconnect/reconnect code in
^^^^^^^^^^^
| drivers/usb/{devio,storage/scsiglue}.c is designed out, then we may
| want to explore adding __usbdevinit{,data} defines in include/linux/init.h
| that would be controlled by a new CONFIG_USB_HOTPLUG option, as in
| the patches that I posted for this to linux-usb-devel.

Until there is __usbdev{init,exit}{,data}, the incorrect
__devinitdata qualifiers should be removed from the USB device
drivers (but not from the host controller drivers, which are PCI drivers).

Would you like to propose a different solution, Randy?

Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."


2000-11-15 21:11:11

by Jeff Garzik

[permalink] [raw]
Subject: Re: CONFIG_USB_HOTPLUG (was Patch(?): linux-2.4.0-test11-pre4/drivers/sound/yss225.c compile failure)

"Adam J. Richter" wrote:
> You were right: the
> __devinitdata being used in the USB drivers will probably crash the
> kernel if CONFIG_HOTPLUG is not defined and the USB code attempts to
> recover from an error by faking disconnect/reconnect.
[...]
> Until there is __usbdev{init,exit}{,data}, the incorrect
> __devinitdata qualifiers should be removed from the USB device
> drivers (but not from the host controller drivers, which are PCI drivers).

If a user hotplugs a device into a kernel which does not support
CONFIG_HOTPLUG, they are shooting themselves in the foot.

I don't see that __devinitdata should be removed.

*plonk*

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso

2000-11-15 22:35:21

by Adam J. Richter

[permalink] [raw]
Subject: Re: CONFIG_USB_HOTPLUG (was Patch(?): linux-2.4.0-test11-pre4/drivers/sound/yss225.c compile failure)

Jeff Garzik wrote:
>"Adam J. Richter" wrote:
>> You were right: the
>> __devinitdata being used in the USB drivers will probably crash the
>> kernel if CONFIG_HOTPLUG is not defined and the USB code attempts to
>> recover from an error by faking disconnect/reconnect.
>[...]
>> Until there is __usbdev{init,exit}{,data}, the incorrect
>> __devinitdata qualifiers should be removed from the USB device
>> drivers (but not from the host controller drivers, which are PCI drivers).

>If a user hotplugs a device into a kernel which does not support
>CONFIG_HOTPLUG, they are shooting themselves in the foot.

I have always agreed with that (ultimately, I would have the
non-hot-plug kernel simply ignore hot insert events, which would
make it a bit smaller anyhow). That was not the scenario I was
warning about. Please reread this section of the message you
were resonding to:

| __devinitdata being used in the USB drivers will probably crash the
| kernel if CONFIG_HOTPLUG is not defined and the USB code attempts to
| recover from an error by faking disconnect/reconnect.

It has nothing to do with the user physically trying
to do hot plugging.


>I don't see that __devinitdata should be removed.

The reason that __devinitdata should be removed from
the USB drivers (or replaced with __usbdevinitdata) is that
under the status quo, USB storage error recovery code and the
usbdevfs reset code will crash on any non-CONFIG_HOTPLUG kernel
without the user doing anything wrong.


>*plonk*

I expect an apology for that.


Adam J. Richter __ ______________ 4880 Stevens Creek Blvd, Suite 104
[email protected] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."