Hi all,
This line triggers an oops in kvm boot test:
usb_match_id():
==> 748 for (; id->idVendor || id->idProduct || id->bDeviceClass ||
749 id->bInterfaceClass || id->driver_info; id++) {
750 if (usb_match_one_id(interface, id))
751 return id;
752 }
It's an old bug and happens also in linux 3.0. It's very reproducible
for the attached config. I can send the initrd (yocto-minimal-i386.cgz)
on your request in private email.
[ 4.365871] gadget: notify speed 425984000
/bin/sh: /proc/self/fd/9: No such file or directory
[ 4.365871] BUG: unable to handle kernel paging request at c1f91ca2
[ 4.365871] IP: [<c14be7c3>] usb_match_id+0x5b/0xcd
[ 4.365871] *pde = 023dc067 *pte = 01f91162
[ 4.365871] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 4.365871] Pid: 17, comm: khubd Not tainted 3.6.0-rc1-00011-gf8cdda8 #1
[ 4.365871] EIP: 0060:[<c14be7c3>] EFLAGS: 00010246 CPU: 1
[ 4.365871] EIP is at usb_match_id+0x5b/0xcd
[ 4.365871] EAX: c6a0c800 EBX: c1f91ca0 ECX: c14be835 EDX: c1f91ca0
[ 4.365871] ESI: c6a0c800 EDI: 00000000 EBP: cd4bfd6c ESP: cd4bfd64
[ 4.365871] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[ 4.365871] CR0: 8005003b CR2: c1f91ca2 CR3: 069f1000 CR4: 00000690
[ 4.365871] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 4.365871] DR6: ffff0ff0 DR7: 00000400
[ 4.365871] Process khubd (pid: 17, ti=cd4be000 task=cd4c8000 task.ti=cd4be000)
[ 4.365871] Stack:
[ 4.365871] c1d42710 c6a0c800 cd4bfd7c c14be8b7 c6a0c81c c6a0c81c cd4bfd88 c13bc374
[ 4.365871] c1d42710 cd4bfd98 c13bd4b9 cd4bfda0 c6a0c81c cd4bfdb4 c13baa17 cd48976c
[ 4.365871] c695e1b8 c6a0c81c c1d36234 c6a0c81c cd4bfdc8 c13bcb4a c13bd49d c6a0c81c
[ 4.365871] Call Trace:
[ 4.365871] [<c14be8b7>] usb_device_match+0x82/0xd7
[ 4.365871] [<c13bc374>] driver_match_device+0x32/0x4a
[ 4.365871] [<c13bd4b9>] __device_attach+0x1c/0x5b
[ 4.365871] [<c13baa17>] bus_for_each_drv+0x82/0x11e
[ 4.365871] [<c13bcb4a>] device_attach+0xb4/0xf8
[ 4.365871] [<c13bd49d>] ? __driver_attach+0xed/0xed
[ 4.365871] [<c13bad5d>] bus_probe_device+0x50/0x116
[ 4.365871] [<c13b858e>] device_add+0x849/0xaf9
[ 4.365871] [<c19e343f>] ? mutex_lock+0x39/0x6e
[ 4.365871] [<c14bd8f9>] usb_set_configuration+0xa07/0xa91
[ 4.365871] [<c14c9b41>] generic_probe+0xaa/0x109
[ 4.365871] [<c14bdc54>] usb_probe_device+0x1a/0x2a
[ 4.365871] [<c13bd16f>] driver_probe_device+0x180/0x3c1
[ 4.365871] [<c13bd4e2>] __device_attach+0x45/0x5b
[ 4.365871] [<c13baa17>] bus_for_each_drv+0x82/0x11e
[ 4.365871] [<c13bcb4a>] device_attach+0xb4/0xf8
[ 4.365871] [<c13bd49d>] ? __driver_attach+0xed/0xed
[ 4.365871] [<c13bad5d>] bus_probe_device+0x50/0x116
[ 4.365871] [<c13b858e>] device_add+0x849/0xaf9
[ 4.365871] [<c13917a3>] ? add_device_randomness+0xa1/0xb5
[ 4.365871] [<c14b0c21>] usb_new_device+0x329/0x488
[ 4.365871] [<c14b43cd>] hub_thread+0x1239/0x196e
[ 4.365871] [<c19e6546>] ? _raw_spin_unlock_irqrestore+0x66/0xab
[ 4.365871] [<c10c8d53>] ? abort_exclusive_wait+0xd6/0xd6
[ 4.365871] [<c10c7d81>] kthread+0xa2/0xb5
[ 4.365871] [<c14b3194>] ? usb_reset_device+0x217/0x217
[ 4.365871] [<c10c7cdf>] ? list_del_init+0x2f/0x2f
[ 4.365871] [<c19e7af6>] kernel_thread_helper+0x6/0xd
[ 4.365871] Code: da 89 f0 e8 78 fe ff ff 83 05 18 14 1f c2 01 83 15 1c 14 1f c2 00 85 c0 75 7d 83 c3 18 83 05 20 14 1f c2 01 83 15 24 14 1f c2 00 <66> 83 7b 02 00 75 cd 83 05 28 14 1f c2 01 83 15 2c 14 1f c2 00
[ 4.365871] EIP: [<c14be7c3>] usb_match_id+0x5b/0xcd SS:ESP 0068:cd4bfd64
[ 4.365871] CR2: 00000000c1f91ca2
Thanks,
Fengguang
On Sun, Aug 05, 2012 at 10:59:38AM +0800, Fengguang Wu wrote:
> Hi all,
>
> This line triggers an oops in kvm boot test:
>
> usb_match_id():
> ==> 748 for (; id->idVendor || id->idProduct || id->bDeviceClass ||
> 749 id->bInterfaceClass || id->driver_info; id++) {
> 750 if (usb_match_one_id(interface, id))
> 751 return id;
> 752 }
>
> It's an old bug and happens also in linux 3.0. It's very reproducible
> for the attached config. I can send the initrd (yocto-minimal-i386.cgz)
> on your request in private email.
Odds are a driver without a terminating NULL for the device id list is
causing this to fail.
What devices are in the system and what drivers are trying to be bound?
greg k-h
On Sun, Aug 05, 2012 at 09:58:26AM -0700, Greg KH wrote:
> On Sun, Aug 05, 2012 at 10:59:38AM +0800, Fengguang Wu wrote:
> > Hi all,
> >
> > This line triggers an oops in kvm boot test:
> >
> > usb_match_id():
> > ==> 748 for (; id->idVendor || id->idProduct || id->bDeviceClass ||
> > 749 id->bInterfaceClass || id->driver_info; id++) {
> > 750 if (usb_match_one_id(interface, id))
> > 751 return id;
> > 752 }
> >
> > It's an old bug and happens also in linux 3.0. It's very reproducible
> > for the attached config. I can send the initrd (yocto-minimal-i386.cgz)
> > on your request in private email.
>
> Odds are a driver without a terminating NULL for the device id list is
> causing this to fail.
>
> What devices are in the system and what drivers are trying to be bound?
The last match is for: drivers/usb/misc/emi62.c
Located down by Tianyu's debug patch:
[ 2.206708] usb_device_match: device 1-1:1.0, driver cytherm
[ 2.207627] usb_device_match: device 1-1:1.0, driver emi62 - firmware loader
[ 2.208769] BUG: unable to handle kernel paging request at c1f7478e
[ 2.209726] IP: [<c14ac1c0>] usb_match_id+0x5b/0xcd
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -778,7 +778,8 @@ static int usb_device_match(struct device *dev, struct device_driver *drv)
>
> intf = to_usb_interface(dev);
> usb_drv = to_usb_driver(drv);
> -
> +
> + pr_info("%s: device %s, driver %s \n", dev_name(dev), drv->name);
> id = usb_match_id(intf, usb_drv->id_table);
> if (id)
> return 1;
Thanks,
Fengguang
On Fri, Aug 17, 2012 at 10:00:46AM +0800, Fengguang Wu wrote:
> On Sun, Aug 05, 2012 at 09:58:26AM -0700, Greg KH wrote:
> > On Sun, Aug 05, 2012 at 10:59:38AM +0800, Fengguang Wu wrote:
> > > Hi all,
> > >
> > > This line triggers an oops in kvm boot test:
> > >
> > > usb_match_id():
> > > ==> 748 for (; id->idVendor || id->idProduct || id->bDeviceClass ||
> > > 749 id->bInterfaceClass || id->driver_info; id++) {
> > > 750 if (usb_match_one_id(interface, id))
> > > 751 return id;
> > > 752 }
> > >
> > > It's an old bug and happens also in linux 3.0. It's very reproducible
> > > for the attached config. I can send the initrd (yocto-minimal-i386.cgz)
> > > on your request in private email.
> >
> > Odds are a driver without a terminating NULL for the device id list is
> > causing this to fail.
> >
> > What devices are in the system and what drivers are trying to be bound?
>
> The last match is for: drivers/usb/misc/emi62.c
>
> Located down by Tianyu's debug patch:
>
> [ 2.206708] usb_device_match: device 1-1:1.0, driver cytherm
> [ 2.207627] usb_device_match: device 1-1:1.0, driver emi62 - firmware loader
> [ 2.208769] BUG: unable to handle kernel paging request at c1f7478e
> [ 2.209726] IP: [<c14ac1c0>] usb_match_id+0x5b/0xcd
>
> > --- a/drivers/usb/core/driver.c
> > +++ b/drivers/usb/core/driver.c
> > @@ -778,7 +778,8 @@ static int usb_device_match(struct device *dev, struct device_driver *drv)
> >
> > intf = to_usb_interface(dev);
> > usb_drv = to_usb_driver(drv);
> > -
> > +
> > + pr_info("%s: device %s, driver %s \n", dev_name(dev), drv->name);
> > id = usb_match_id(intf, usb_drv->id_table);
> > if (id)
> > return 1;
Odd that it takes so long after you call that function for it to fail.
And that driver has a proper termination, so we aren't walking off the
end of the list, so it must be in the probe function itself.
Care to add some more debugging for that driver?
Also, I don't see the dev_info() message from the driver itself, in the
probe function, so it must not be getting called properly.
Something weird is happening...
greg k-h
On Thu, Aug 16, 2012 at 08:48:25PM -0700, Greg KH wrote:
> On Fri, Aug 17, 2012 at 10:00:46AM +0800, Fengguang Wu wrote:
> > On Sun, Aug 05, 2012 at 09:58:26AM -0700, Greg KH wrote:
> > > On Sun, Aug 05, 2012 at 10:59:38AM +0800, Fengguang Wu wrote:
> > > > Hi all,
> > > >
> > > > This line triggers an oops in kvm boot test:
> > > >
> > > > usb_match_id():
> > > > ==> 748 for (; id->idVendor || id->idProduct || id->bDeviceClass ||
> > > > 749 id->bInterfaceClass || id->driver_info; id++) {
> > > > 750 if (usb_match_one_id(interface, id))
> > > > 751 return id;
> > > > 752 }
> > > >
> > > > It's an old bug and happens also in linux 3.0. It's very reproducible
> > > > for the attached config. I can send the initrd (yocto-minimal-i386.cgz)
> > > > on your request in private email.
> > >
> > > Odds are a driver without a terminating NULL for the device id list is
> > > causing this to fail.
> > >
> > > What devices are in the system and what drivers are trying to be bound?
> >
> > The last match is for: drivers/usb/misc/emi62.c
> >
> > Located down by Tianyu's debug patch:
> >
> > [ 2.206708] usb_device_match: device 1-1:1.0, driver cytherm
> > [ 2.207627] usb_device_match: device 1-1:1.0, driver emi62 - firmware loader
> > [ 2.208769] BUG: unable to handle kernel paging request at c1f7478e
> > [ 2.209726] IP: [<c14ac1c0>] usb_match_id+0x5b/0xcd
> >
> > > --- a/drivers/usb/core/driver.c
> > > +++ b/drivers/usb/core/driver.c
> > > @@ -778,7 +778,8 @@ static int usb_device_match(struct device *dev, struct device_driver *drv)
> > >
> > > intf = to_usb_interface(dev);
> > > usb_drv = to_usb_driver(drv);
> > > -
> > > +
> > > + pr_info("%s: device %s, driver %s \n", dev_name(dev), drv->name);
> > > id = usb_match_id(intf, usb_drv->id_table);
> > > if (id)
> > > return 1;
>
> Odd that it takes so long after you call that function for it to fail.
>
> And that driver has a proper termination, so we aren't walking off the
> end of the list, so it must be in the probe function itself.
>
> Care to add some more debugging for that driver?
>
> Also, I don't see the dev_info() message from the driver itself, in the
> probe function, so it must not be getting called properly.
>
> Something weird is happening...
Very true. I find it related to timing: after adding these debug
printks, it becomes very hard to reproduce. And I confirmed that
emi62_probe() is never called.
Anyway, I managed to get one oops. For comparison, one non-failing
dmesg for the same kernel is also attached.
--- tip.orig/drivers/usb/core/driver.c 2012-08-16 09:27:20.063315545 +0800
+++ tip/drivers/usb/core/driver.c 2012-08-17 12:27:32.441761637 +0800
@@ -747,6 +747,7 @@ const struct usb_device_id *usb_match_id
device and interface. */
for (; id->idVendor || id->idProduct || id->bDeviceClass ||
id->bInterfaceClass || id->driver_info; id++) {
+ printk(KERN_ERR "usb_match_id: id=%p idVendor=%#x idProduct=%#x\n", id, id->idVendor, id->idProduct);
if (usb_match_one_id(interface, id))
return id;
}
@@ -779,6 +780,7 @@ static int usb_device_match(struct devic
intf = to_usb_interface(dev);
usb_drv = to_usb_driver(drv);
+ pr_info("%s: device %s, driver %s \n", __func__, dev_name(dev), drv->name);
id = usb_match_id(intf, usb_drv->id_table);
if (id)
return 1;
--- tip.orig/drivers/usb/misc/emi62.c 2012-07-29 18:22:35.256996861 +0800
+++ tip/drivers/usb/misc/emi62.c 2012-08-17 12:46:49.761743138 +0800
@@ -242,9 +242,9 @@ MODULE_DEVICE_TABLE (usb, id_table);
static int emi62_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
struct usb_device *dev = interface_to_usbdev(intf);
- dev_dbg(&intf->dev, "emi62_probe\n");
+ dev_err(&intf->dev, "emi62_probe\n");
- dev_info(&intf->dev, "%s start\n", __func__);
+ printk(KERN_ERR "%s start\n", __func__);
emi62_load_firmware(dev);
Thanks,
Fengguang
Fengguang Wu <[email protected]> writes:
> @@ -779,6 +780,7 @@ static int usb_device_match(struct devic
> intf = to_usb_interface(dev);
> usb_drv = to_usb_driver(drv);
>
> + pr_info("%s: device %s, driver %s \n", __func__, dev_name(dev), drv->name);
> id = usb_match_id(intf, usb_drv->id_table);
> if (id)
> return 1;
> [ 1.397745] usb_match_id: id=c1d80820 idVendor=0x18f3 idProduct=0x220
> [ 1.398834] usbcore: registered new interface driver dvb_usb_dtt200u
> [ 1.399943] usb_device_match: device 1-0:1.0, driver dvb_usb_nova_t_usb2
> [ 1.401095] usb_match_id: id=c1d81b6c idVendor=0x2040 idProduct=0x9300
> [ 1.402195] usb_match_id: id=c1d81b84 idVendor=0x2040 idProduct=0x9301
> [ 1.403270] usbcore: registered new interface driver dvb_usb_nova_t_usb2
> [ 1.404158] usb_device_match: device 1-0:1.0, driver dvb_usb_umt_010
> [ 1.404861] usb_match_id: id=c1d824f4 idVendor=0x15f4 idProduct=0x1
> [ 1.405547] usb_match_id: id=c1d8250c idVendor=0x15f4 idProduct=0x15
> [ 1.406326] usbcore: registered new interface driver dvb_usb_umt_010
> [ 1.407190] usb_device_match: device 1-0:1.0, driver dvb_usb_gl861
Why is dev_name(dev) constant here? That seems more than odd to me. Is
this really the same interface matching all these drivers?
Bjørn
Bjørn Mork <[email protected]> writes:
> Fengguang Wu <[email protected]> writes:
>
>> @@ -779,6 +780,7 @@ static int usb_device_match(struct devic
>> intf = to_usb_interface(dev);
>> usb_drv = to_usb_driver(drv);
>>
>> + pr_info("%s: device %s, driver %s \n", __func__, dev_name(dev), drv->name);
>> id = usb_match_id(intf, usb_drv->id_table);
>> if (id)
>> return 1;
>> [ 1.397745] usb_match_id: id=c1d80820 idVendor=0x18f3 idProduct=0x220
>> [ 1.398834] usbcore: registered new interface driver dvb_usb_dtt200u
>> [ 1.399943] usb_device_match: device 1-0:1.0, driver dvb_usb_nova_t_usb2
>> [ 1.401095] usb_match_id: id=c1d81b6c idVendor=0x2040 idProduct=0x9300
>> [ 1.402195] usb_match_id: id=c1d81b84 idVendor=0x2040 idProduct=0x9301
>> [ 1.403270] usbcore: registered new interface driver dvb_usb_nova_t_usb2
>> [ 1.404158] usb_device_match: device 1-0:1.0, driver dvb_usb_umt_010
>> [ 1.404861] usb_match_id: id=c1d824f4 idVendor=0x15f4 idProduct=0x1
>> [ 1.405547] usb_match_id: id=c1d8250c idVendor=0x15f4 idProduct=0x15
>> [ 1.406326] usbcore: registered new interface driver dvb_usb_umt_010
>> [ 1.407190] usb_device_match: device 1-0:1.0, driver dvb_usb_gl861
>
> Why is dev_name(dev) constant here? That seems more than odd to me. Is
> this really the same interface matching all these drivers?
Stupid me didn't read your debugging printk's properly. You are of
course logging this *before* testing for a match, so everything is as
expected.
But looking at the emi62 driver, I wonder if this will make any
difference:
diff --git a/drivers/usb/misc/emi62.c b/drivers/usb/misc/emi62.c
index ff08015..ae794b9 100644
--- a/drivers/usb/misc/emi62.c
+++ b/drivers/usb/misc/emi62.c
@@ -232,7 +232,7 @@ wraperr:
return err;
}
-static const struct usb_device_id id_table[] __devinitconst = {
+static const struct usb_device_id id_table[] = {
{ USB_DEVICE(EMI62_VENDOR_ID, EMI62_PRODUCT_ID) },
{ } /* Terminating entry */
};
Bjørn
Hi Bjørn,
> -static const struct usb_device_id id_table[] __devinitconst = {
> +static const struct usb_device_id id_table[] = {
Good catch! It magically fixed the oops. So that id_table was
freed sometime with __devinitconst?
There are some more "usb_device_id .* __devinitconst" users in the system,
Shall they be fixed as well?
Thanks,
Fengguang
On Fri, Aug 17, 2012 at 05:27:08PM +0800, Fengguang Wu wrote:
> Hi Bjørn,
>
> > -static const struct usb_device_id id_table[] __devinitconst = {
> > +static const struct usb_device_id id_table[] = {
>
> Good catch! It magically fixed the oops. So that id_table was
> freed sometime with __devinitconst?
>
> There are some more "usb_device_id .* __devinitconst" users in the system,
> Shall they be fixed as well?
This should be a complete list:
definitions:
static const struct usb_device_id smsusb_id_table[] __devinitconst = {
static const __devinitdata struct usb_device_id device_table[] = {
static const struct usb_device_id device_table[] __devinitconst = {
static struct usb_device_id p54u_table[] __devinitdata = {
static struct usb_device_id rtl8187_table[] __devinitdata = {
static struct usb_device_id vt6656_table[] __devinitdata = {
static const struct usb_device_id wb35_table[] __devinitconst = {
static const struct usb_device_id id_table[] __devinitconst = {
files:
drivers/media/video/gspca/jl2005bcd.c
drivers/net/wireless/p54/p54usb.c
drivers/net/wireless/rtl818x/rtl8187/dev.c
drivers/usb/misc/emi62.c
drivers/media/dvb/siano/smsusb.c
drivers/media/video/gspca/spca506.c
drivers/staging/winbond/wbusb.c
drivers/staging/vt6656/main_usb.c
Thanks,
Fengguang
On Fri, Aug 17, 2012 at 6:24 PM, Fengguang Wu <[email protected]> wrote:
> On Fri, Aug 17, 2012 at 05:27:08PM +0800, Fengguang Wu wrote:
>> Hi Bj?rn,
>>
>> > -static const struct usb_device_id id_table[] __devinitconst = {
>> > +static const struct usb_device_id id_table[] = {
>>
>> Good catch! It magically fixed the oops. So that id_table was
>> freed sometime with __devinitconst?
>>
>> There are some more "usb_device_id .* __devinitconst" users in the system,
>> Shall they be fixed as well?
The issue should be triggered only when HOTPLUG is not set.
But, if HOTPLUG is not enabled, should device_add() trigger driver probe
further after kernel init is completed? Or even devices should be allowed
to add into system?
Thanks,
--
Ming Lei
On Fri, Aug 17, 2012 at 08:16:15PM +0800, Ming Lei wrote:
> On Fri, Aug 17, 2012 at 6:24 PM, Fengguang Wu <[email protected]> wrote:
> > On Fri, Aug 17, 2012 at 05:27:08PM +0800, Fengguang Wu wrote:
> >> Hi Bj?rn,
> >>
> >> > -static const struct usb_device_id id_table[] __devinitconst = {
> >> > +static const struct usb_device_id id_table[] = {
> >>
> >> Good catch! It magically fixed the oops. So that id_table was
> >> freed sometime with __devinitconst?
> >>
> >> There are some more "usb_device_id .* __devinitconst" users in the system,
> >> Shall they be fixed as well?
>
> The issue should be triggered only when HOTPLUG is not set.
>
> But, if HOTPLUG is not enabled, should device_add() trigger driver probe
> further after kernel init is completed? Or even devices should be allowed
> to add into system?
Yes, it can, and should, because you can add them through sysfs.
I'll go fix these up, I really hate the __devinit* markings, they really
are pointless these days, and only cause problems :(
Thanks all for tracking this down.
greg k-h
On Fri, 17 Aug 2012, Ming Lei wrote:
> But, if HOTPLUG is not enabled, should device_add() trigger driver probe
> further after kernel init is completed? Or even devices should be allowed
> to add into system?
Indeed, does it make any sense to have USB support at all if HOTPLUG
isn't enabled? Should USB select HOTPLUG?
Alan Stern
On Fri, Aug 17, 2012 at 10:38:16AM -0400, Alan Stern wrote:
> On Fri, 17 Aug 2012, Ming Lei wrote:
>
> > But, if HOTPLUG is not enabled, should device_add() trigger driver probe
> > further after kernel init is completed? Or even devices should be allowed
> > to add into system?
>
> Indeed, does it make any sense to have USB support at all if HOTPLUG
> isn't enabled? Should USB select HOTPLUG?
Well, a long time ago people wanted to use USB but not have HOTPLUG
enabled in their systems for various (odd) embedded systems. As it's
pretty hard to even turn off HOTPLUG these days, I'd be more likely to
just remove CONFIG_HOTPLUG entirely given the dynamic nature of almost
all systems.
So no, I don't think we should select HOTPLUG for USB.
greg k-h
On Fri, Aug 17, 2012 at 10:42 PM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Fri, Aug 17, 2012 at 10:38:16AM -0400, Alan Stern wrote:
>> On Fri, 17 Aug 2012, Ming Lei wrote:
>>
>> > But, if HOTPLUG is not enabled, should device_add() trigger driver probe
>> > further after kernel init is completed? Or even devices should be allowed
>> > to add into system?
>>
>> Indeed, does it make any sense to have USB support at all if HOTPLUG
>> isn't enabled? Should USB select HOTPLUG?
>
> Well, a long time ago people wanted to use USB but not have HOTPLUG
> enabled in their systems for various (odd) embedded systems. As it's
> pretty hard to even turn off HOTPLUG these days, I'd be more likely to
> just remove CONFIG_HOTPLUG entirely given the dynamic nature of almost
> all systems.
It should make sense, otherwise all device id table should not use
__devinit* markings. There are lots of pci driver usage on it.
Thanks,
--
Ming Lei
Ming Lei <[email protected]> writes:
> On Fri, Aug 17, 2012 at 10:42 PM, Greg Kroah-Hartman
> <[email protected]> wrote:
>> On Fri, Aug 17, 2012 at 10:38:16AM -0400, Alan Stern wrote:
>>> On Fri, 17 Aug 2012, Ming Lei wrote:
>>>
>>> > But, if HOTPLUG is not enabled, should device_add() trigger driver probe
>>> > further after kernel init is completed? Or even devices should be allowed
>>> > to add into system?
>>>
>>> Indeed, does it make any sense to have USB support at all if HOTPLUG
>>> isn't enabled? Should USB select HOTPLUG?
>>
>> Well, a long time ago people wanted to use USB but not have HOTPLUG
>> enabled in their systems for various (odd) embedded systems. As it's
>> pretty hard to even turn off HOTPLUG these days, I'd be more likely to
>> just remove CONFIG_HOTPLUG entirely given the dynamic nature of almost
>> all systems.
>
> It should make sense, otherwise all device id table should not use
> __devinit* markings. There are lots of pci driver usage on it.
You might want to start here then:
/**
* DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table
* @_table: device table name
*
* This macro is used to create a struct pci_device_id array (a device table)
* in a generic manner.
*/
#define DEFINE_PCI_DEVICE_TABLE(_table) \
const struct pci_device_id _table[] __devinitconst
Bjørn
On Sun, Aug 19, 2012 at 12:23:38PM +0200, Bj?rn Mork wrote:
> Ming Lei <[email protected]> writes:
> > On Fri, Aug 17, 2012 at 10:42 PM, Greg Kroah-Hartman
> > <[email protected]> wrote:
> >> On Fri, Aug 17, 2012 at 10:38:16AM -0400, Alan Stern wrote:
> >>> On Fri, 17 Aug 2012, Ming Lei wrote:
> >>>
> >>> > But, if HOTPLUG is not enabled, should device_add() trigger driver probe
> >>> > further after kernel init is completed? Or even devices should be allowed
> >>> > to add into system?
> >>>
> >>> Indeed, does it make any sense to have USB support at all if HOTPLUG
> >>> isn't enabled? Should USB select HOTPLUG?
> >>
> >> Well, a long time ago people wanted to use USB but not have HOTPLUG
> >> enabled in their systems for various (odd) embedded systems. As it's
> >> pretty hard to even turn off HOTPLUG these days, I'd be more likely to
> >> just remove CONFIG_HOTPLUG entirely given the dynamic nature of almost
> >> all systems.
> >
> > It should make sense, otherwise all device id table should not use
> > __devinit* markings. There are lots of pci driver usage on it.
>
> You might want to start here then:
>
> /**
> * DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table
> * @_table: device table name
> *
> * This macro is used to create a struct pci_device_id array (a device table)
> * in a generic manner.
> */
> #define DEFINE_PCI_DEVICE_TABLE(_table) \
> const struct pci_device_id _table[] __devinitconst
>
That's not as big of a problem, as pci drivers are usually left as a
module, and very few people dynamically add and remove pci devices on
systems that do not have CONFIG_HOTPLUG enabled.
Not to say that it couldn't happen, just that it is rare.
And it shows again that __devinitconst just needs to be removed, I'll
work on that soon and see what happens...
greg k-h