On Sat, 7 Jun 2003, Martin Kittel wrote:
> Hi,
>
> while battling to get my Praktica DC1.3 digital camera to work I noticed
> what seems to be a problem in the usb-storage driver.
>
> I am using kernel 2.4.21-rc6 with the ACPI-patches applied. The camera
> uses transparent SCSI and bulk-only transport,
>
> In function usb_stor_transfer_partial in drivers/usb/storage/transport.c
> the case of
> 'result == -EPIPE'
> does not seem to be handled properly. The pipe gets reinitialized but no
> return code is set, so the catch-all error handler gets invoked and
> returns US_BULK_TRANSFER_FAILED.
You're right about that. Strictly speaking, the return code ought to be
US_BULK_TRANSFER_SHORT. However in your case it doesn't matter, because
the bulk-only transport routine only tests whether the return code is
US_BULK_TRANSFER_ABORTED.
> I noticed that whenever -EPIPE occurs in this method the bcs->Tag
> content gets changed, too, finally resulting in a bus reset:
>
> <taken from logs>
> usb-storage: Command INQUIRY (6 bytes)
> usb-storage: 12 00 00 00 ff 00 00 00 cb 07 00 00
> usb-storage: Invoking Mode Translation
> usb-storage: Bulk command S 0x43425355 T 0x1 Trg 0 LUN 0 L 255 F 128 CL 6
> usb-storage: Bulk command transfer result=0
> usb-storage: usb_stor_transfer_partial(): xfer 255 bytes
> usb-storage: usb_stor_bulk_msg() returned -32 xferred 0/255
> usb-storage: clearing endpoint halt for pipe 0xc0008280
> usb-storage: usb_stor_clear_halt: result=0
> usb-storage: usb_stor_transfer_partial(): unknown error
> usb-storage: Bulk data transfer result 0x2
> usb-storage: Attempting to get CSW...
> usb-storage: Bulk status result = 0
> usb-storage: Bulk status Sig 0x53425355 T 0x143 R 0 Stat 0x1
> usb-storage: Bulk logical error
> usb-storage: -- transport indicates error, resetting
> usb-storage: Bulk reset requested
That bcs->Tag value is the real cause of your problem. It appears to be a
genuine flaw in the digital camera; it's not a result of the EPIPE error.
> It seems to me that the -EPIPE case should try to transfer the data
> propery after taking care of the pipe, however as I don't really have
> any clue about how the protocol is working I was not very successful.
The -EPIPE means that the device doesn't want to transfer any more data.
But the protocol continues correctly, by going on to transfer the status.
> At least I managed to get my camera recognized by adding a return value
> of US_BULK_TRANSFER_GOOD to the -EPIPE case and allowing the changed
> bcs-Tag content. As the data itself was not properly transfered, a mount
> was possible, but the directory content didn't look pretty.
> By the way, I was able to mount the camera successfully with the
> original transport.c, but that took ages since after about every other
> command a bus reset was initiated...
As I said above, the US_BULK_TRANSFER_GOOD wouldn't have made any
difference. Allowing the changed Tag was the key. But I don't think we
want to do that in general, since matching the Tags is an important error
detection technique. And it sounds like doing it just for cameras like
yours wouldn't work very well either, judging from the results you got.
> Any help would be appreciated.
I don't know that I can offer much help; the device doesn't work right.
On the other hand, the EPIPE case ought to return an appropriate code.
Although it doesn't matter for bulk-only, it does affect CB and CBI. The
patch below fixes that, and also removes unnecessary in_interrupt()
tests from the transport routine (which always runs in process context).
Alan Stern
--- usb-2.4/drivers/usb/storage/transport.c.orig Fri Jun 6 23:14:59 2003
+++ usb-2.4/drivers/usb/storage/transport.c Fri Jun 6 23:23:36 2003
@@ -523,6 +523,7 @@
if (result == -EPIPE) {
US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe);
usb_stor_clear_halt(us, pipe);
+ return US_BULK_TRANSFER_SHORT;
}
/* did we abort this command? */
@@ -1109,11 +1110,11 @@
int partial;
int ret = USB_STOR_TRANSPORT_ERROR;
- bcb = kmalloc(sizeof *bcb, in_interrupt() ? GFP_ATOMIC : GFP_NOIO);
+ bcb = kmalloc(sizeof *bcb, GFP_NOIO);
if (!bcb) {
return USB_STOR_TRANSPORT_ERROR;
}
- bcs = kmalloc(sizeof *bcs, in_interrupt() ? GFP_ATOMIC : GFP_NOIO);
+ bcs = kmalloc(sizeof *bcs, GFP_NOIO);
if (!bcs) {
kfree(bcb);
return USB_STOR_TRANSPORT_ERROR;
-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The best
thread debugger on the planet. Designed with thread debugging features
you've never dreamed of, try TotalView 6 free at www.etnus.com.
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
