* Alan Stern | 2011-12-15 11:34:19 [-0500]:
>> Okay. So we do this in dummy because it is a code path in a driver. Fair
>> enough. Looking at net2280 I see one early complete call for ep0 if
>> length == 0 and for OUT ep where the fifo is read during enqueue in case
>> it is not empty. Nothing for IN.
>
>Maybe I'm remembering wrong. It has been many years since I looked at
>this stuff. Perhaps this early-completion path is unique to dummy-hcd.
>
>I still think it makes a good test case. Gadget drivers shouldn't
>assume that request completions will always occur _after_ the
>submission call has returned.
Okay then, will keep it in mind.
>> So if we keep it, this raises two questions:
>> - This looks like every gadget has drop its locks while submitting
>> requests. From a quick grep I see that this is done. So I learned
>> something here.
>
>Right. Or at least, they have to drop any locks that might be taken by
>a completion handler.
>
>> - Isn't it premature to call complete for an IN endpoint before we know
>> it went through? For OUT I would understand, the data may already sit
>> in the FIFO but the IN transfer did not complete yet.
>
>This is an obscure technical point. I asked Dave Brownell about it
>once. He said that the completion callback doesn't necessarily mean
>that the host has received the data; it only means that the UDC driver
>has taken over responsibility for the data, so that the usb_request
>structure and data buffer are now available for the gadget driver to
>reuse.
The driver usually calls complete once the hardware told it, that it is
done with it. So nothing can go wrong with it. If you do this for an IN
endpoint that early you can't tell the gadget if something went wrong
afterwards. Especially if the host side did not enqueue an URB.
I admit that this example not really a problem because the host will
enqueue an URB. And most likely nothing goes wrong with packet on its
away unless someone pulls the cable but this will be noticed by both
sides and nobody cares about that one packet.
So don't worry, will leave it there.
>> >One more thing: Why do you overuse u32 so much? For example, in patch
>> >3/7:
>> >
>> >> @@ -147,6 +148,8 @@ static const char *const ep_name [] = {
>> >> struct urbp {
>> >> struct urb *urb;
>> >> struct list_head urbp_list;
>> >> + struct sg_mapping_iter miter;
>> >> + u32 miter_started;
>> >> };
>> >
>> >There's no reason for miter_started to be a u32. In general, types
>> >that specify a particular size should be used only in situations where
>> >the size really matters -- which basically means whenever the value is
>> >embedded in a structure and a particular alignment is needed, or when
>> >the value is defined according to an external protocol.
>>
>> So here I could use u32 miter_stared:1 to use only one bit. The compiler
>> tends to pad the structure to the next naturally aligned spot so adding
>> u8 here would hardly makes a difference. If it is followed by a pointer
>> for instance it will be padded by 3 or 7 bytes. So this here eats up the
>> same amount of memory.
>> Once a second variable of this kind is required this could change to the
>> bitfield (:1 thingy) notation or rename to flags and add a few defines.
>> I usually prefer the latter because I don't like bitfields like that and
>> they often produce worse code especially if you need to set both of them
>> to the same value at once.
>
>I wasn't asking why you didn't use a bitfield; I was asking why you
>didn't use unsigned or bool. Why go to the trouble of specifying a
>32-bit quantity when the actual length doesn't matter?
I don't like the type "unsigned". It misses the type, it is not
complete. int or unsigned int is something I can live with it. Since I
don't need negative numbers I go for the unsigned int and for the fast
writting u32.
There is nothing wrong with bool. I could use bool but I didn't.
What is the problem with u32? It is a shortcut for unsigned int. This
is not something where you say "give me an unsigned data type compiler,
please" and the compiler gives you someting what fits best here. The
compiler has strict rules it has to follow and it ends up with an
unsigned int. So why not specify u32 in the first place?
>> >When neither of these holds, the best approach is to use a generic type
>> >(like int or unsigned) and allow the compiler the freedom to do what it
>> int is defined to be signed int on all architectures if it is not
>> specified. We don't have this for the char type for instance. This one is
>> different on powerpc vs x86. I like to be explicit here and I am lazy
>> as well so u32 is a good compromise :) Since negative values are not
>> required I pick the unsigned instead of the signed int. One could ask
>> himself why miter_started could be negative. Only "unsigned" looks
>> incomplete. I don't know how it is defined but I think it internally
>> handled as int. Atleast it uses 4 bytes which speaks for int (u32). So
>> if it is u32 why not state it clearly?
>
>Because it confuses people. It implies strongly that there's a good
>reason why the variable has to hold a 32-bit value.
So you are saying that using an int or unsigned is okay but u32 confuses
people beacuase a 32bit value is expected while the former provides the
exact same data type?
>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
