Re: [PATCH v2] target: add usb gadget fabric module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 18 Jan 2012, Sebastian Andrzej Siewior wrote:

> On 01/17/2012 07:29 PM, Alan Stern wrote:
> >> I have a database where I lookup the transfer direction for each
> >> command.
> >
> > That's not robust.  You might have to fail a command simply because it
> > isn't in your table, even though the target layer could have handled
> 
> This is also the case with the current BOT storage gadget which does
> everything on its own.

That's not how I would describe it.  With the BOT storage gadget, there
isn't a separate lookup table.  The direction and amount are computed
by the code that processes a command.  Hence it's not at all easy for
the two of them to get out of sync.

With your driver, on the other hand, somebody could modify the target 
core to handle a new command but leave your table unchanged.

> > it.  With BOT this isn't so critical; the CBW includes a flag for the
> > data direction.
> 
> According to the spec, the device should not consider it while
> evaluating the command. If the host attempts to do VERIFY in OUT
> direction (which is wrong because VERIFY should not move data) the

(Bad example.  Commands with no data phase always use the OUT
direction, with a data length of 0.)

> device should abort it instead of reading the data and throwing it away.

And how does the device know to abort the command?  Answer: by checking
the direction flag and data length.  Thus the device _must_ consider
these things while evaluating a command.

> > In fact, this sounds like a weakness in the target layer.  A transport
> > driver should be able to ask the target layer how much data should be
> > transferred for a command, and in which direction.
> 
> Yes, and this is why I have this lookup table. It was initially
> submitted as an extension to target but since I'm the only user it was
> moved into the gadget. Maybe it can be moved into drivers/scsi since
> there are other drivers having another lookup table.

I think there are a few commands which have variant meanings, depending
on the type of the device.  It would be difficult to express that in a
simple table.

> BTW: the storage gadget has a similar kind of lookup table :)

No, it doesn't.  It has a lot of code for processing the various 
commands, and the expected values for direction and data length are 
embedded in that code.  There is no separate table.


> So I wired that part up now. So Windows retries the first command
> twice. This also happens with the mass_storage gadget but somehow it
> answers the second request properly. Anyway, the MODE_SENSE response is 
> little complicated. Target response always with KEY=UNIT_ATTENTION and 
> Windows somehow ignores it. With KEY=ILLEGAL_REQUEST this request is not 
> re-retried. Let me see how I tell target this....

UNIT ATTENTION does not sound like the right response.  Maybe it's a 
bug in the target layer...

> > The real question is what happens when the amount of data you want to
> > send/receive is different from what the other side expects.  It sounds
> > like the only way to recover will be for the host to cancel the command
> > and possibly reset the device.
> 
> Interesting. If I get the direction wrong, it will stall and will have
> to cancel the command because it times out. If I send too much data it
> will timeout because it does not receive the status urb.

What if you want to receive too much data?  Probably another timeout.

>  If I send not
> enough data then it will notice it soon because the status URB arrives
> before the data URB completes.

And then what happens?  Does the host driver compute the difference and 
report it as the residue?

What if you want to receive too little data?

Alan Stern

--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux