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

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

 



On 01/18/2012 04:46 PM, Alan Stern wrote:
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.

Okay. So you prefer to have both things in one place. Seems reasonable.

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

Now imagine you receive VERIFY with length of 16.

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.

I must have said it bad. I _must_ consider the direction & length
fields from cbw in order to know what to do in order to complete the
command. But I still have to check if the command makes sense and I can
deal with it. Take the example from above: VERIFY with length 16 is bad.
That would be case 9 from spec's table 6.1 (host expects to send data
to device, device intends no data transfer).

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.

Okay. So embedding the direction flag into the cdb handling is a
possible way.

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.

yes, you have a check_command(length, direction) in every cdb, that is
what I mean by "kind of lookup 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...

yes, looking into it.

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?

The host side sets residue via

   sdb->resid = sdb->length - urb->actual_length;

once the data urb completes which is before the status urb (end)
arrives.

What if you want to receive too little data?

Don't get it. But you send a smaller URB then.


Alan Stern

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


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

  Powered by Linux