* Sarah Sharp | 2012-01-23 18:13:01 [-0800]: >On Fri, Jan 20, 2012 at 08:57:20PM +0100, Sebastian Andrzej Siewior wrote: >> Just run into the following: >> - new disk arrived in the system >> - udev couldn't wait to get its hands on to to run ata_id /dev/sda >> - this sent the cdb 0xa1 to the device. >> - my UAS-gadget recevied the cdb and had no idea what to do with it. It >> decided to send a status URB back with sense set to invalid opcode. > >Oh, right, UAS devices aren't supposed to stall transfers. Was skipping >to the status URB the correct behavior? Good point. I can't find anything that suggest that I have to stall the pipe. All UAS transfer diagrams describing the data flow assume that I know what I have to do. UAS-2 says in "4.10 USB error handling" that |No condition defined in this standard results in a stall (see USB-2) on |any pipe. prior that they were talking about short packets on command/status pipe. In USB-2 it is suggested sometimes that the device may stall IN pipe. However, the unknown-command cdb is not a big thing there because I get the expected size _and_ data direction together with the cdb. So I just fill the pipe and set the status code that the command was not handled properly. >> - the host side received it status and completed the scsi command. >> - the host sent another scsi with 4kib data buffer >> - Now I was confused why the data transfer is only 512 bytes instead of >> 4kib since the host is always allocating the complete transfer in one >> go. >> - Finally the system crashed while walking through the sg list. >> >> This patch adds three new flags in order to distinguish between DATA >> URB completed and outstanding. If we receive status before data, we >> cancel data and let data complete the command. >> This solves the problem for IN and OUT transfers but does not work for >> BIDI. > >I'm not sure the current patch will work with USB 2.0 UAS devices. The No it does not. I will fix that. >device receives the setup URB. Later it responds with an error on the >status pipe about the legal request. But the data URBs would have never >been submitted because we never got the read/write ready response. I >think that means the call to usb_unlink_urb() will just fail without >calling the data URBs' completion functions. Then the SCSI command will >never get completed for USB 2.0 UAS devices with the type of error you >described. The previous code would have worked just fine though. :) yeah. >Why not have an atomic count in the uas_cmd_info of the number of URBs >that need to be completed? The status completion function can decrement >and test the count, and if it's non-zero, it should call >usb_unlink_urb() for the data URBs. If it's zero, we know the data URBs >(including both data URBs in the bidirectional sense) completed and we >can free all the URBs and complete the SCSI command. It might work. >Actually it sounds sort of like we just need real cancellation code with >USB anchors. Then we'd just mark the uas_cmd_info as being "dead" >because we got the status URB with the legal request, and ask some sort >of cancellation workqueue to go about killing and waiting for the >anchored data URBs to complete. But the atomic count might do until we >can get some real cancellation code in there. You want the atomic counters for now or is this patch fixed for the non-stream case enough? Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
