* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
@ 2002-03-12 0:20 ` David Brownell
2002-03-12 20:27 ` Greg KH
` (9 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: David Brownell @ 2002-03-12 0:20 UTC (permalink / raw)
To: linux-hotplug
Johannes replied to Greg:
> > Since the latest USB patch went in that added a proper delay to the
> > hub connection sequence, a lot of people have been reporting odd
> > problems with some USB hubs, keyboards, and mice. I've seen this
> > problem too. It is usually fixed by just replugging the device in, but
> > not all of the time.
>
> .... deleted patch (adds delay in userland) and comments ....
>
> Is it that devices don't like how quickly we let the driver talk to it
> after enumeration?
I'm likewise puzzled by this. Were these devices that worked
previously? If so it'd seem to be the kernel "delay" patch
that's at fault somehow. And if not, it'd seem like that "delay"
patch doesn't work quite well enough on those devices (which
might be more marginal than it seems at first).
It doesn't make sense to me that if more delays are added in
the kernel code, userland needs to see even more.
- Dave
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
2002-03-12 0:20 ` David Brownell
@ 2002-03-12 20:27 ` Greg KH
2002-03-12 20:29 ` Greg KH
` (8 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2002-03-12 20:27 UTC (permalink / raw)
To: linux-hotplug
On Mon, Mar 11, 2002 at 05:08:37PM -0500, Johannes Erdfelt wrote:
>
> The hub driver doesn't talk to devices, so I'll assume you mean the hub
> thread, which is actually calling the core for enumeration (setting
> address, fetching descriptors, etc)
Sorry, yes this is what I meant.
> But the enumeration should be finished by the time the hotplug stuff
> finishes.
>
> In that case, nothing should be stomping over each other.
In looking at this some more I agree, it doesn't look like this could
race with the hub thread.
> Is it that devices don't like how quickly we let the driver talk to it
> after enumeration?
Ah, this is probably more likely.
The problem looks like the usbdevfs calls are happening at the same time
the hid driver is also talking to the device.
Olaf, is this what your log messages show?
thanks,
greg k-h
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
2002-03-12 0:20 ` David Brownell
2002-03-12 20:27 ` Greg KH
@ 2002-03-12 20:29 ` Greg KH
2002-03-12 22:15 ` David Brownell
` (7 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2002-03-12 20:29 UTC (permalink / raw)
To: linux-hotplug
On Mon, Mar 11, 2002 at 04:20:30PM -0800, David Brownell wrote:
>
> I'm likewise puzzled by this. Were these devices that worked
> previously? If so it'd seem to be the kernel "delay" patch
> that's at fault somehow. And if not, it'd seem like that "delay"
> patch doesn't work quite well enough on those devices (which
> might be more marginal than it seems at first).
>
> It doesn't make sense to me that if more delays are added in
> the kernel code, userland needs to see even more.
I agree, but Olaf and others prove that this works.
I'm more puzzled why usbmodules is needing to talk to a device. I
thought usbmodules was only for cold-plug stuff. Have any ideas?
thanks,
greg k-h
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (2 preceding siblings ...)
2002-03-12 20:29 ` Greg KH
@ 2002-03-12 22:15 ` David Brownell
2002-03-12 22:52 ` Greg KH
` (6 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: David Brownell @ 2002-03-12 22:15 UTC (permalink / raw)
To: linux-hotplug
> > It doesn't make sense to me that if more delays are added in
> > the kernel code, userland needs to see even more.
>
> I agree, but Olaf and others prove that this works.
There was that hypothesis that it's really control traffic
interference ... I notice that usb_control_msg() chose
against the "keep the setup data in struct usb_device
and lock it with a semaphore" strategy (it kmallocs
the setup buffer), so I could see how that'd happen.
"usbmodules" asks the device for its config descriptor,
rather than reading /proc/bus/usb/BBB/DDD, so HCDs
that don't handle control queuing (I think OHCI behaves
there, and EHCI, but not the UHCIs) might suffer from
concurrent control messages from it and from HID ...
I think the best that can be done for now would be a
hack: add a lock in usb_device that must be held
by whoever submits a control request, and released
when that request completes. The non-hack solution
is to support control queuing in all HCDs.
What kind of lock? Maybe that binary semaphore.
Code that needs to issue control requests in_interrupt()
would need to be modified, as would usb_control_msg().
> I'm more puzzled why usbmodules is needing to talk to a device. I
> thought usbmodules was only for cold-plug stuff. Have any ideas?
Usbmodules is used to cope with /sbin/hotplug not providing
a complete interface descriptor dump: it only shows the first
of N inferface descriptors. With coldplug, it doesn't run, and
doesn't provide any descriptors at all -- so it must be used.
However, even for the hotplug case "usbmodules" gives a
more complete module list. In the case of an audio device
with a sound/ISO interface and a control/HID interface, it'll
pick up the HID.
- Dave
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (3 preceding siblings ...)
2002-03-12 22:15 ` David Brownell
@ 2002-03-12 22:52 ` Greg KH
2002-03-13 1:52 ` David Brownell
` (5 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2002-03-12 22:52 UTC (permalink / raw)
To: linux-hotplug
On Tue, Mar 12, 2002 at 02:15:05PM -0800, David Brownell wrote:
> > > It doesn't make sense to me that if more delays are added in
> > > the kernel code, userland needs to see even more.
> >
> > I agree, but Olaf and others prove that this works.
>
> There was that hypothesis that it's really control traffic
> interference ... I notice that usb_control_msg() chose
> against the "keep the setup data in struct usb_device
> and lock it with a semaphore" strategy (it kmallocs
> the setup buffer), so I could see how that'd happen.
It's always been this way, right?
> "usbmodules" asks the device for its config descriptor,
> rather than reading /proc/bus/usb/BBB/DDD, so HCDs
> that don't handle control queuing (I think OHCI behaves
> there, and EHCI, but not the UHCIs) might suffer from
> concurrent control messages from it and from HID ...
>
> I think the best that can be done for now would be a
> hack: add a lock in usb_device that must be held
> by whoever submits a control request, and released
> when that request completes. The non-hack solution
> is to support control queuing in all HCDs.
>
> What kind of lock? Maybe that binary semaphore.
> Code that needs to issue control requests in_interrupt()
> would need to be modified, as would usb_control_msg().
Yes, this would be the best way. I don't think it's a hack, as some
drivers (like the usb-storage one) have said that they really need to
lock the control pipe for a while to prevent things from happening out
of order (for very dumb devices.)
> Usbmodules is used to cope with /sbin/hotplug not providing
> a complete interface descriptor dump: it only shows the first
> of N inferface descriptors. With coldplug, it doesn't run, and
> doesn't provide any descriptors at all -- so it must be used.
>
> However, even for the hotplug case "usbmodules" gives a
> more complete module list. In the case of an audio device
> with a sound/ISO interface and a control/HID interface, it'll
> pick up the HID.
Ah, thanks for explaining this, makes more sense now.
greg k-h
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (4 preceding siblings ...)
2002-03-12 22:52 ` Greg KH
@ 2002-03-13 1:52 ` David Brownell
2002-03-14 13:21 ` Olaf Hering
` (4 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: David Brownell @ 2002-03-13 1:52 UTC (permalink / raw)
To: linux-hotplug
> > > > It doesn't make sense to me that if more delays are added in
> > > > the kernel code, userland needs to see even more.
> > >
> > > I agree, but Olaf and others prove that this works.
> >
> > There was that hypothesis that it's really control traffic
> > interference ... I notice that usb_control_msg() chose
> > against the "keep the setup data in struct usb_device
> > and lock it with a semaphore" strategy (it kmallocs
> > the setup buffer), so I could see how that'd happen.
>
> It's always been this way, right?
No, the kmalloc() is newish. Previously it did stack
allocation (bad since dma-from-stack isn't sure to work
on all systems). I wonder whether adding semaphore plus
eight bytes of buffer (vs just semaphore) would even
make the "struct usb_device" objects go into a different
kmalloc cache ... :)
> > ...
> >
> > I think the best that can be done for now would be a
> > hack: add a lock in usb_device that must be held
> > by whoever submits a control request, and released
> > when that request completes. The non-hack solution
> > is to support control queuing in all HCDs.
> >
> > What kind of lock? Maybe that binary semaphore.
> > Code that needs to issue control requests in_interrupt()
> > would need to be modified, as would usb_control_msg().
>
> Yes, this would be the best way. I don't think it's a hack, as some
> drivers (like the usb-storage one) have said that they really need to
> lock the control pipe for a while to prevent things from happening out
> of order (for very dumb devices.)
Seems like a hack to me since it pessimizes driver
performance for conforming devices in favor of an
unknown (and presumed small :) set of nonconforming
ones. Plus, I seem to recall Matt saying that the original
motivation was a reading that the storage spec required
that -- but a later reading said it didn't require that.
That said -- I think we'd be interested in a patch prepared
to implement that "hack", since it's the best that's likely
to show up near-term. I presume Olaf (and others) could
verify whether it really solved the problem.
- Dave
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (5 preceding siblings ...)
2002-03-13 1:52 ` David Brownell
@ 2002-03-14 13:21 ` Olaf Hering
2002-03-15 18:28 ` Greg KH
` (3 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Olaf Hering @ 2002-03-14 13:21 UTC (permalink / raw)
To: linux-hotplug
On Tue, Mar 12, Greg KH wrote:
> On Mon, Mar 11, 2002 at 05:08:37PM -0500, Johannes Erdfelt wrote:
> >
> > The hub driver doesn't talk to devices, so I'll assume you mean the hub
> > thread, which is actually calling the core for enumeration (setting
> > address, fetching descriptors, etc)
>
> Sorry, yes this is what I meant.
>
> > But the enumeration should be finished by the time the hotplug stuff
> > finishes.
> >
> > In that case, nothing should be stomping over each other.
>
> In looking at this some more I agree, it doesn't look like this could
> race with the hub thread.
>
> > Is it that devices don't like how quickly we let the driver talk to it
> > after enumeration?
>
> Ah, this is probably more likely.
>
> The problem looks like the usbdevfs calls are happening at the same time
> the hid driver is also talking to the device.
>
> Olaf, is this what your log messages show?
It seems so. I think a more correct fix would be a lock in the usbcore
code to hold usbdevfs requests when "some bus action" is going on, like
this renumbering. You said there is no lock right now for this case?
Gruss Olaf
--
$ man clone
BUGS
Main feature not yet implemented...
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (6 preceding siblings ...)
2002-03-14 13:21 ` Olaf Hering
@ 2002-03-15 18:28 ` Greg KH
2002-05-15 20:25 ` David Brownell
` (2 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2002-03-15 18:28 UTC (permalink / raw)
To: linux-hotplug
On Thu, Mar 14, 2002 at 02:21:47PM +0100, Olaf Hering wrote:
> > The problem looks like the usbdevfs calls are happening at the same time
> > the hid driver is also talking to the device.
> >
> > Olaf, is this what your log messages show?
>
> It seems so. I think a more correct fix would be a lock in the usbcore
> code to hold usbdevfs requests when "some bus action" is going on, like
> this renumbering. You said there is no lock right now for this case?
This is correct and has been on the list of things to fix in the usb
code for a while.
thanks,
greg k-h
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (7 preceding siblings ...)
2002-03-15 18:28 ` Greg KH
@ 2002-05-15 20:25 ` David Brownell
2002-05-15 20:37 ` Olaf Hering
2002-05-15 21:37 ` Johannes Erdfelt
10 siblings, 0 replies; 13+ messages in thread
From: David Brownell @ 2002-05-15 20:25 UTC (permalink / raw)
To: linux-hotplug
> What is the status of this bug in the hotplug related functions? We
> added a "sleep 3" in the add routing in usb.rc, but that can only be a
> workaround.
Have you verified that it only happens with the UHCI drivers?
To my understanding, HCDs that queue control traffic will not
have this problem. So OHCI or EHCI shouldn't need that
timeout. Or said differently, the issue isn't with hotplug, but
generally with control traffic.
- Dave
_______________________________________________________________
Have big pipes? SourceForge.net is looking for download mirrors. We supply
the hardware. You get the recognition. Email Us: bandwidth@sourceforge.net
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (8 preceding siblings ...)
2002-05-15 20:25 ` David Brownell
@ 2002-05-15 20:37 ` Olaf Hering
2002-05-15 21:37 ` Johannes Erdfelt
10 siblings, 0 replies; 13+ messages in thread
From: Olaf Hering @ 2002-05-15 20:37 UTC (permalink / raw)
To: linux-hotplug
On Wed, May 15, David Brownell wrote:
> > What is the status of this bug in the hotplug related functions? We
> > added a "sleep 3" in the add routing in usb.rc, but that can only be a
> > workaround.
>
> Have you verified that it only happens with the UHCI drivers?
I see it only on intel with uhci, yes.
Gruss Olaf
--
$ man clone
BUGS
Main feature not yet implemented...
_______________________________________________________________
Have big pipes? SourceForge.net is looking for download mirrors. We supply
the hardware. You get the recognition. Email Us: bandwidth@sourceforge.net
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup
2002-03-11 22:08 [linux-usb-devel] [PATCH] fix some hubs and hid devices at startup Johannes Erdfelt
` (9 preceding siblings ...)
2002-05-15 20:37 ` Olaf Hering
@ 2002-05-15 21:37 ` Johannes Erdfelt
10 siblings, 0 replies; 13+ messages in thread
From: Johannes Erdfelt @ 2002-05-15 21:37 UTC (permalink / raw)
To: linux-hotplug
On Wed, May 15, 2002, Olaf Hering <olh@suse.de> wrote:
> On Fri, Mar 15, Greg KH wrote:
>
> > On Thu, Mar 14, 2002 at 02:21:47PM +0100, Olaf Hering wrote:
> > > > The problem looks like the usbdevfs calls are happening at the same time
> > > > the hid driver is also talking to the device.
> > > >
> > > > Olaf, is this what your log messages show?
> > >
> > > It seems so. I think a more correct fix would be a lock in the usbcore
> > > code to hold usbdevfs requests when "some bus action" is going on, like
> > > this renumbering. You said there is no lock right now for this case?
> >
> > This is correct and has been on the list of things to fix in the usb
> > code for a while.
>
> What is the status of this bug in the hotplug related functions? We
> added a "sleep 3" in the add routing in usb.rc, but that can only be a
> workaround.
>
> My idea was that we queue the hotplug events somehow and wait for all
> "bus traffic" to complete, the stuff that occours when the usb-uhci
> driver is loaded. Something like
> insmod usbcore;insmod usb-uhci;modprobe hid;insmod keybdev;insmod mousedev
> should work. And it did not with 2.4.19pre2/3, I got timeouts. However,
> typing the commands manually to get some delay, and it worked. So I
> think that it is not strictly related to usbdevfs, that might be another
> bug.
>
> However, after reading the whole thread again, it seems that queuing the
> events is maybe not the best solution. What should be done to get this
> fixed? (The delay and locking should be in the kernel and not in the
> hotplug package).
Well, it shouldn't be giving you back ETIMEDOUT. That's indicitive of
another bug, not the queueing.
However, it looks like uhci.c (and probably usb-uhci.c too since the
code was based on it) didn't correctly match control pipe's and prevent
2 URB's from being queued, 1 in and 1 out.
Can you try this patch?
It should atleast start returning -ENXIO (-6 on x86).
JE
--- linux-2.4.19-pre8.orig/drivers/usb/uhci.c Wed May 15 13:29:27 2002
+++ linux-2.4.19-pre8/drivers/usb/uhci.c Wed May 15 13:27:52 2002
@@ -1470,9 +1470,14 @@
tmp = tmp->next;
- if (u->dev = urb->dev && u->pipe = urb->pipe &&
- u->status = -EINPROGRESS)
- return u;
+ if (u->dev = urb->dev && u->status = -EINPROGRESS) {
+ /* For control, ignore the direction */
+ if (usb_pipecontrol(urb->pipe) &&
+ (u->pipe & ~USB_DIR_IN) = (urb->pipe & ~USB_DIR_IN))
+ return u;
+ else if (u->pipe = urb->pipe)
+ return u;
+ }
}
return NULL;
_______________________________________________________________
Have big pipes? SourceForge.net is looking for download mirrors. We supply
the hardware. You get the recognition. Email Us: bandwidth@sourceforge.net
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 13+ messages in thread