e extra field (like internal_state).
Functionally this does pretty much what we have been discussing. Overall
the changes are pretty minimal, just what is necessary to make things
work.
Anyway, tell me how you like the patch.
Alan Stern
= drivers/usb/core/hcd.c 1.89 vs edited =
--- 1.8
ated note, who gets to decide which
configuration a device should use?
Thanks for any explanations...
Alan Stern
---
This SF.net email is sponsored by: Scholarships for Techies!
Can't afford IT training? All 2003 ictp students receive sc
the completion
handler, it's no longer legal for the handler to free the urb. Hmmm,
maybe I should call usb_get_urb() just before the handler and
usb_free_urb() just after.
Alan Stern
---
This SF.net email is sponsored by: Scholarships fo
ort of coherent plan for how to handle this.
In fact, it ought to be part of the device-driver model that underlies
sysfs. But so far as I am aware, there is currently nothing in the sysfs
documentation to address the problem.
Alan Stern
-
this anyway. (I
know David Brownell will agree with that :-) Given these objections, it's
probably best not to implement (1).
(2) can be done separately. It's just a question of grabbing the
handler_lock in hcd_unlink_urb() before the call to acquire urb->lock and
then releasing handler
g error codes that need to be fixed in files other than
transport.c. Maybe somebody can check and see.
I don't know that this will fix all the problems observed above, but it's
a start.
Alan Stern
# This is a BitKeeper generated patch for the following project:
# Project Name:
.
Alan Stern
= drivers/usb/core/hcd.c 1.89 vs edited =
--- 1.89/drivers/usb/core/hcd.c Thu Jan 16 04:29:28 2003
+++ edited/drivers/usb/core/hcd.c Wed Jan 22 16:25:26 2003
@@ -1116,8 +1116,13 @@
* lock sequence needed for the usb_hcd_giveback_urb() code paths
* (urb lock
On Wed, 22 Jan 2003, Oliver Neukum wrote:
> Am Mittwoch, 22. Januar 2003 22:32 schrieb Alan Stern:
> > Here is my patch to implement proper timing of synchronous unlinks.
> > Basically it just uses a spinlock to cause unlink requests during a
> > completion handler to wait
sn't this exactly what everyone has been asking for and debating about?
Alan Stern
---
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com
___
; The entity which calls scsi_register_host() should by good design call
> scsi_unregister_host().
In usb-storage, it _is_ the LLDD that calls scsi_register_host().
Alan Stern
---
This SF.NET email is sponsored by:
SourceForge Enterpris
bout scsi_unregister_host() or
> scsi_unregister(host)? The former does drivers and the latter
> does hosts. This would mean that my original statement was
> nevertheless correct, how can a LLDD decide to unload itself safely?
I did indeed type it wrong. T
Oliver:
In case my message from last Friday fell through the cracks, I'm resending
it. This patch should do what you want:
Calls to usb_unlink_urb() while the completion handler is running
won't do anything until the handler has returned.
Synchronous calls to usb_unlink
On Tue, 28 Jan 2003, David Brownell wrote:
> Alan Stern wrote:
> >
> > Calls to usb_unlink_urb() while the completion handler is running
> > won't do anything until the handler has returned.
>
> It still violates the API spec: there's no comple
first place -- i.e., had already
completed by the time usb_unlink_urb was called, or had never been
submitted at all. Waiting for a completion handler under those
circumstances would be a disaster.
Alan Stern
---
This SF.NET email is sponsor
d be much more logical to have the
option selectable at the time the unlink is performed, either by passing a
flag to usb_unlink_urb() or by having a separate entry point
usb_async_unlink_urb(). That sounds like something for 2.7...
Alan Stern
o a driver. If we can't guarantee
that the completion function will have finished when a synchronous unlink
returns (with a success code), then there's no reason for a driver _ever_
to use a synchronous unlink.
> With
> HCDs
so unlink returns -EINVAL; leaving the -EBUSY case as indicating only
> the "starting to complete" case.
I wish I had known that before. It means my proposed patch for
synchronous unlinks is wrong. *Sigh* Back to the drawing board.
Alan Stern
---
X_INQUIRY causes the driver not to send
INQUIRY commands to the device but to fake a reply instead. Some devices
don't respond properly to INQUIRYs and cause a timeout, hence the need for
this kludge.
I believe the current plan is to phase out this flag; the driver will
automatically
When David mentioned that unlink_urb tests urb->hcpriv to see whether the
low-level driver has finished processing the urb, I realized that my patch
needed to be fixed. Here's the new version.
Alan Stern
= include/linux/usb.h 1.124 vs edited =
--- 1.124/include/linux/usb.h
?
Are you sure that the unlink is the cause of the crash, not some other
operation?
Alan Stern
---
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www
Here is another version of my patch to provide proper synchronous unlinks.
This one uses a struct completion rather than a spinlock, and it also
eliminates all the awkwardness of the "splice" mechanism currently in use.
Alan Stern
= include/linux/usb.h 1.124 vs edited =
ant to unlink an URB, erase its urb->next field before calling
usb_unlink_urb().
Hope this helps.
Alan Stern
---
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld
>more
> - we are sad
If a driver was written to work this way, that would indeed be sad. I
guess the best advice is: Don't write your drivers like that!
> Or do urbs take a reference to the driver somehow?
No. And in any case, the core does not check for any such refere
s way, that would indeed be sad. I
> > guess the best advice is: Don't write your drivers like that!
>
> Sure. In 2.4 it is clear that you should be using MOD_INC_USE_COUNT.
> But in 2.5 that way of doing things seems to be deprecated.
So what provides an equivalen
ait in disconnect() for the
completion handler to be called. You would end up blocking the khubd
thread and the unload process, but that's better than crashing the kernel.
Alan Stern
---
This SF.NET email is sponsored by:
SourceForge En
like this is pointless.
Not necessarily, it depends on the structure of the driver. Increasing
the use count in this way would prevent unloading while any urbs are in
use, thus making it unnecessary to cancel anything in disconnect().
(Unfortunately, that doesn&
e here is the question of how a synchronous unlink should behave.
Even if it returns an error (either because the HCD refused to unlink the
urb or because the urb had already been unlinked), should it wait until
the completion handler has run before it returns?
Alan Stern
r is
done -- in which case we should make it work like that.
> The above reasoning suggests that sync unlink should be made as
> convenient as possible. However, first you have to find situations in
> which async unlink is particularly inconvenient and ana
Besides, only 3
hunks failed; you could just type in the appropriate changes by hand.
Almost all of the changes just involve replacing -ENOENT with
-ECONNRESET -- although your third failure involves replacing it with
-ENODEV.
Alan Stern
--
is considerably complicated by the need to take into account module
unloading, port resets, configuration and altsetting changes, in addition
to physical disconnects.
Alan Stern
---
This SF.net email is sponsored by: SlickEdit Inc. Develop an edg
b(), and completion handlers. What do
you think is the right thing to do here?
(Incidentally, usb-skeleton.c is a good example of a driver where the
disconnect() routine can exit with urbs still in flight.)
Alan Stern
---
This SF.net
e before and after.
But that's just what would happen in the usb-storage driver, e.g.
The approach that seems most logical to me is to define another entry
point for USB device drivers. This entry point should be called by the
core whenever a device bound to the driver is reset. The driver can
and change the one use -- take your choice.
Alan Stern
= usb-2.5/include/linux/usb.h 1.128 vs edited =
--- 1.128/include/linux/usb.h Tue Feb 18 19:18:58 2003
+++ edited/usb-2.5/include/linux/usb.h Mon Feb 24 10:19:25 2003
@@ -960,8 +960,8 @@
#define usb_rcvbulkpipe(dev,endpoint) ((PIPE_
ers have been replaced with symbolic constants.
New comments discuss the merits of different I/O styles and the
requirements for a driver's disconnect() routine.
Alan Stern
= usb-skeleton.c 1.36 vs edited =
--- 1.36/drivers/usb/usb-skeleton.c Tue Jan 14 12:00
structure; that would never have compiled.
Alan Stern
P.S. to David: I want to look into writing a replacement for the
usb_bulk_msg() family of routines along the lines you described (probably
following the usb_sg_xxx calls as a model). It seems clear that a
prerequisite is a version
ge introduced around the time of 2.4.19-acX in the
usb_stor_Bulk_max_lun() routine, and the change had a bug in it. I don't
know if that bug was the source of Oliver's problem, but it is certainly
possible. Here is a patch to fix the bug.
Alan Stern
--- usb-2.4/drivers/usb/storage/t
On Tue, 25 Feb 2003, David Brownell wrote:
> Alan Stern wrote:
> > Heck, I've barely had a chance to write it, let alone test it. In fact,
> > it seemed clear that nobody had tried any testing recently. One of the
> > minor bugs I fixed was a subroutine call that
On Wed, 26 Feb 2003, Alan Stern wrote:
> As it turns out, this still generates a compiler warning. There's a
> mistake in the min() and max() macros in include/linux/kernel.h. I will
> post something about that on the linux kernel mailing list.
I spoke too soon (don't yo
On Wed, 26 Feb 2003, David Brownell wrote:
> Alan Stern wrote:
> > That sounds like you're saying the macros don't belong in usb.h at all.
> > Shall I send in a patch that takes them out?
>
> I think that'd be a good solution. Or maybe just put them i
send it to
Linus for his tree as soon as reasonably possible.
Mea culpa,
Alan Stern
--- usb-2.5/drivers/usb/core/hcd.h.orig Sat Mar 1 12:27:21 2003
+++ usb-2.5/drivers/usb/core/hcd.h Sat Mar 1 12:30:42 2003
@@ -255,8 +255,8 @@
extern int usb_set_address(struct usb_device *dev);
/* use t
n't
work with the erroneous routine but does work after the patch has been
applied.
Alan Stern
---
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
__
moving trailing whitespace, etc.).
I think it's probably okay to apply this patch. More testing would be
good, but I don't have any suitable hardware. And I think a
software-loopback sort of test wouldn't address many of my changes very
well.
Alan Stern
= drivers/usb/usb-skeleton.c
;s pretty minimal, so it
ought to work on any reasonable Pentium II and up. You might want to turn
off SMP and PM support. You might even want to turn off USB debugging.
Alan Stern
#
# Automatically generated make config: don't edit
#
CONFIG_X86=y
CONFIG_MMU=y
CONFIG_SWAP=y
CONFIG_UID
On Tue, 4 Mar 2003, David Brownell wrote:
> Alan Stern wrote:
> >
> > I think it's probably okay to apply this patch. More testing would be
> > good, but I don't have any suitable hardware. And I think a
> > software-loopback sort of test wouldn'
On Wed, 5 Mar 2003, David Brownell wrote:
> Alan Stern wrote:
> > Here is a slightly updated patch for usb-skeleton.c. It has been tested
> > minimally with real hardware, but David Brownell would like to see some
> > more tests using "gadget zero".
>
> I ju
Greg:
This patch fixes an oversight in usb-storage whereby the command length
and command buffer for an automatically-generated REQUEST-SENSE command
would not be initialized properly.
Please apply both the 2.4 and 2.5 versions.
Alan Stern
-- Forwarded message --
Date: Wed, 5
dure is finished, flag the interface to allow submissions
again (if appropriate).
I realize this would be a big change. But maybe it's worthwhile.
Alan Stern
---
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger
fo
On Thu, 6 Mar 2003, Greg KH wrote:
> On Mon, Mar 03, 2003 at 02:14:54PM -0500, Alan Stern wrote:
> > Here is a slightly updated patch for usb-skeleton.c. It has been tested
> > minimally with real hardware, but David Brownell would like to see some
> > more tests using "
on call.
> > Here is a radical new thought. Suppose we could gain advantage (7)
> > (although not (6)) without changing the synchronous API at all! Since (6)
> > isn't really necessary, this would be among the better, if not the best,
> > of all possible worlds.
Greg:
Further testing uncovered a _very_ minor error in usb-skeleton: there
should not be a debugging log message for a successful write.
This goes on top of the previous patch.
Alan Stern
= drivers/usb/usb-skeleton.c 1.37 vs edited =
--- 1.37/drivers/usb/usb-skeleton.c Wed Mar 5
e this way there is more bandwidth
available in each frame. With a straightforward change to let the
allocator understand how this works, it would be possible to schedule more
interrupt transactions than currently.
Is there any point in me pu
ous urb found */
Shouldn't that be:
*end = (last_urb->start_frame + last_urb->number_of_packets *
last_urb->interval) & 1023;
Also, it wouldn't hurt to replace the 1023 wi
On Tue, 11 Mar 2003, David Brownell wrote:
> Alan Stern wrote:
> > I agree about the need for synchronization. The point is that with the
> > only data structure available to the client being a pointer to an urb,
> > there _is_ no way to carry out this synchroniza
On Wed, 12 Mar 2003, David Brownell wrote:
> Alan Stern wrote:
> > The major purpose of adding the facility to cancel synchronous messages is
> > to allow drivers to unlink the corresponding urbs during disconnect().
> > Okay, so let's keep the existing API pretty
> Unlinking them _before_ is likely to make some drivers unhappy,
> but this is a case where I think the cleaner solution is to be
> preferred ... fewer ways that things can go haywire is Good.
Hmm... I still think it might better be done after. But either way seems
abou
-alien.net/~mdharm/linux-usb> for more information).
2. Try upgrading to 2.4.21-current. Several bug fixes to usb-storage
have been made since 2.4.18.
Alan Stern
---
This SF.net email is sponsored by:Crypto Challenge is now open!
Get cr
Greg:
My update for usb-skeleton seems to have gotten lost in the shuffle, so
here it is again -- all wrapped up in one nice little patch. It's been
tested by three different people and passed with flying colors. Please
apply.
Alan Stern
= usb-skeleton.c 1.37 vs edited =
---
to sleep with
> TASK_INTERRUPTIBLE and unlink in the interruption case?
The problem is that drivers may choose to use the usb_bulk/control_msg()
functions to perform their I/O, and these sleep in TASK_UNINTERRUPTIBLE.
They don't have any support for fielding signals.
Alan Stern
---
work in most cases.
How will your simple solution work in the situation where two drivers are
bound to different interfaces on the same device and one of the drivers is
rmmod'ed? How does your patch prevent the core from accepting urbs for ep
0 from
hing
else? For usb-storage it's true that the synchronous API is used in the
I/O error handling code paths. But those code paths always execute in the
context of a kernel thread -- the SCSI error-handler thread -- with no
semaphores held, so it's perfectly okay for them to us
; > versions?
>
> Aren't those two options identical? Both are API syntax changes. :)
I agree with David's proposal above; just make usb_bulk_msg() and
usb_control_msg() interruptible. But what about synchronous
usb_unlink_urb()?
Alan Stern
--
On Tue, 18 Mar 2003, Oliver Neukum wrote:
> Am Dienstag, 18. März 2003 16:12 schrieb Alan Stern:
> > On Tue, 18 Mar 2003, Oliver Neukum wrote:
> > > Hi,
> > >
> > > some part of the synchronous API is used in the block io error handling
> > > code p
ifferent error codes. This would just add another failure mode,
in which the error code happens to be -EINTR. But the drivers should
treat it much like any other error. That is, unless they try to do some
sort of error recovery based on the return code -- then this would have to
fall u
omplete until the error-handler fixes things up? I
don't know. How do other subsystems handle this?
Alan Stern
---
This SF.net email is sponsored by: Does your code think in ink?
You could win a Tablet PC. Get a free Tablet PC
On Tue, 18 Mar 2003, Oliver Neukum wrote:
> Am Dienstag, 18. März 2003 20:03 schrieb Alan Stern:
> > I'm not convinced there will be so many bugs. usb_bulk_msg() and
> > usb_control_msg() both can fail already, in several different ways with
> > several different err
On Tue, 18 Mar 2003, Alan Stern wrote:
> On Tue, 18 Mar 2003, Oliver Neukum wrote:
>
> > > > How can that be? The SCSI layer guarantees that no further requests are
> > > > issued while the error handler is running.
> > >
> > > Yes. The synchr
A couple of changes
were made to the usb-storage driver fairly recently that might address the
problem you see.
Alan Stern
---
This SF.net email is sponsored by: Does your code think in ink?
You could win a Tablet PC. Get a free Tablet PC hat
On Wed, 19 Mar 2003, Dave Turner wrote:
> On Wed, 19 Mar 2003 at 10:05am, Alan Stern wrote:
>
> > Dave:
> >
> > You didn't say what version of Linux you were running. Try installing
> > 2.4.21-current and see if that makes a difference. A couple of chang
is case, it's very easy to use an asynchronous unlink rather than a
synchronous one. All you have to do is wait for the struct completion
again after the unlink call, with no timeout. Given that this whole
matter revolves around updating the synchronous API interface, it's
probably bes
etter simply to remove the usb_reset_device() call from
usb-storage, or to implement it differently. For example, usb-storage's
function for handling an emulated SCSI bus reset (which is where
usb_reset_device() gets called) might mark the device as bad, fail the
r
On Thu, 20 Mar 2003, Dave Turner wrote:
> On Thu, 20 Mar 2003 at 10:47am, Alan Stern wrote:
>
> > Can you post an extract from the kernel log under 2.4.21-rc5 with
> > usb-storage debugging turned on?
>
> Sure thing.
>
> Linux version 2.4.21-pre5 ([EMAIL P
simply rely on the user
unplugging the USB cable and putting it back in or cycling the power to
the drive. (Yes, there are situations where these resets crop up
regularly -- but they are the result of some other incompatibility that a
device reset won't fix anyway.)
In the long ter
On Thu, 20 Mar 2003, Matthew Dharm wrote:
> On Thu, Mar 20, 2003 at 05:01:46PM -0500, Alan Stern wrote:
> > After inserting a 1Gb Microdrive into the card reader and attempting to
> > mount it (following a successful MODE-SENSE):
> >
> > > usb-storage: queuecomm
On Fri, 21 Mar 2003, Dave Turner wrote:
> On Fri, 21 Mar 2003 at 10:45am, Alan Stern wrote:
>
> > Dave, please try this patch, see what happens, and send another kernel
> > log extract. I don't think it will fix everything because it doesn't
> > address the roo
a warning log message to the path
where there's more than 1 interface.
Alan Stern
---
This SF.net email is sponsored by:Crypto Challenge is now open!
Get cracking and register here for some mind boggling fun and
the chance of winning an
nts about this code. First, setting count = skb->len
+ 1 implies transmitting a random byte of memory, thereby leaking
potentially sensitive information. The byte in question should be set to
0 before it is transmitted. Second, if this is a matter of sending a runt
US
Vetter.
>
> PS.: I have attached the /proc/bus/usb/devices and the /proc/scsi/scsi files!
Try compiling your kernel with usb-storage debugging turned on, and post a
copy of your kernel log with the debugging messages. That will help to
pin
oesn't count as an error).
Based on the kernel log you posted earlier, it looks like your problem may
stem from the use of a START-STOP command. The patch below removes that
command; it's a backport from 2.5. Try installing this and let us know if
it helps.
Alan Stern
--
rmation, at:
>
> http://sourceforge.net/mailarchive/message.php?msg_id=3737890
>
> ...was much more in-depth, but I'll add:
>
Judging from the information in Peter's message, it looks like the problem
stems from the use
f nothing better comes up, here's a suggestion. Compile usb-storage with
debugging turned on and repeat your test using a serial console (and
regular kernel logging turned off!). Of course this will generate lots
and lots of logging output but only the last part will be
st blocked
on a read, that points to a problem somewhere in the intermediate levels
of the block I/O system or the SCSI system. Not much help there, since
you must already be aware of that.
Alan Stern
---
This SF.net email is sponsored by: Va
tty stable by
> now, though the SCSI code was (still) in flux.
I thought so too, and I don't know of any such problems. Maybe this is a
new one?
Alan Stern
---
This SF.net email is sponsored by: ValueWeb:
Dedicated Hosting for ju
On Wed, 2 Apr 2003, David Brownell wrote:
> Alan Stern wrote:
> > What sort of information would it be useful to expose in this way? A few
> > possibilities that would be easy to implement (stored in the per-device
> > data structure in usb-storage) are:
> >
>
fore is protected by the spinlock in
question. In fact, some of it isn't protected by a spinlock at all. But
that's probably all right -- I think things will end up being sufficiently
consistent anyway.
Alan Stern
---
This SF.net e
ng, plus the
ability to perform selective updates, come to mind.
In short, we should permit both types of approach with minimal hassle.
Alan Stern
---
This SF.net email is sponsored by: Etnus, makers of TotalView, The best
thread debu
ten by the CPU, whereas a buffer used for
device output can be shared with other data in a buffer allocated by
kmalloc, but not on the stack.
How does the DMA driver arrange to flush the CPU's cache before starting a
DMA out operation? I didn't notice any particular code to do that in the
U
remental test sequence. Each time increase the size
of your queue by one URB until you trigger the error. Then you would know
that the last URB was the straw that broke the camel's back.
Alan Stern
---
This SF.net email is sponsored by:
it was faintly possible that the problem lay in the
intermediate hub. Though I can't imagine how.
Alan Stern
---
This SF.net email is sponsored by: Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thre
"some configurations of chipset and driver". My question: is it possible to
> write a workaround in the Linux-USB driver for this problem?
Not without some pretty specific information from the vendor explaining
the source of the problem and how to work around it. Is th
the tests? And related to that, did your original command ever
end up writing anything to the md5sums output file?
Alan Stern
---
This SF.net email is sponsored by: Etnus, makers of TotalView, The best
thread debugger on the planet. Desi
ny possibility that some additional kernel messages failed to make
their way into your logfile?
Alan Stern
---
This SF.net email is sponsored by: Etnus, makers of TotalView, The best
thread debugger on the planet. Designed
that's why
> /sbin/hotplug does
> not succeed.
> mount -t ext2 /dev/sda1 /mnt does not return
>
> Any idea ?
Have you tried mounting it read-only? Also, can you tell where the mount
process is hung? Try Alt-SysRq-T (and m
's inefficient about this? It looks like the
decision of whether or not to call dma_cache_wback_inv() has nothing to do
with what other data is stored in the same memory block with the output
buffer. And there's nothing that indicates
e: usb_reset_device returns -19
Could you mail to me directly (offlist) your altered hub.c file, so I can
match up the line numbers in the log with the source code?
Alan Stern
---
This SF.net email is sponsored by: Etnus,
her hand, for DMA_IN it is most likely that the next piece of
data the CPU will access in that memory region is the input buffer itself.
Especially if the memory region is devoted only to input buffers and the
input operations occur sequentially. So nothing is lost by incurring a
cache miss th
.2: GetStatus port 1 status 001002
> > POWER sig=se0 CSC
> > Jun 5 20:55:06 ventus kernel: usb-storage: usb_reset_device returns -19
>From then on, there was nothing to try.
I strongly suspect -- but I don't know for certain -- that the device did
not really disconnec
k of it, what use could the driver make of that information?
If the call failed there's nothing the driver can do, and if it succeeded
the driver will be probed again anyway.
Alan Stern
---
This SF.net email is sponsored by: Etnus
uffers may share a region with other data, they probably
shouldn't.
If there's anything wrong or incomplete about this, please let me know.
Alan Stern
---
This SF.net email is sponsored by: Etnus, makers of TotalView, The best
thread d
n() call is interrupted
> unfinished.
> Without strace the mount call cannot be interrupted by CTRL-C.
Can you try using your device on a regular PC? Can you try using Linux
2.5.70 to see if that helps? Can you enable OHCI_VERBOSE_DEBUG near the
start of the oh
nding. As I recall, the current usage is almost
> exclusively in probe() calls for single-interface devices.
Sorry, I misspoke in my original message. I didn't mean
usb_reset_config(); I was thinking about drivers that want to change to a
different configuration during their probe(). H
1 - 100 of 6560 matches
Mail list logo
