On Tue, Jan 24, 2012 at 05:50:53PM +0100, Sebastian Andrzej Siewior wrote: > * 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. Yeah, ok, that sounds like the right behavior then. They probably didn't want a USB 3.0 device to stall on unknown commands because it would shut down any other commands in flight on different stream IDs. > >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? Either way, I think we'll end up ripping it out later, so I guess I don't care, as long as the non-streams case is taken care of. However, what will the uas driver do if the device completes the data and status phases in the correct order, but the host controller gives them back in the opposite order? Since the data and status phases can be sent from different endpoints, there's no guarantee of ordering of completed transfers. The xHC host could chose to put the event for the status transfer on the event ring before the data transfer event. (In practice it probably won't, but it could happen.) So receiving a status URB completion before the data URB completion doesn't necessarily mean the device has skipped the data phase. I think your patch will still work in this case. The status URB completion will attempt to unlink the data URBs, but the xHCI driver will probably give back the data URB before the stop endpoint command will be serviced by the host controller. Will the data URB completion see the URB canceled status in urb->status? What should it do in that case? Hopefully it won't give back the SCSI command with an error, since technically the command did complete. Sarah Sharp -- 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
