Hi,
Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
>> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
>> > On Wed, 7 Sep 2016, Felipe Balbi wrote:
>> >
>> >> > --- a/include/uapi/linux/usb/gadgetfs.h
>> >> > +++ b/include/uapi/linux/usb/gadgetfs.h
>> >> > @@ -85,4 +85,33 @@ struct usb_gadgetfs_event {
>> >> > */
>> >> > #define GADGETFS_CLEAR_HALT _IO('g', 3)
>> >> >
>> >> > +
>> >> > +
>> >> > +struct usb_gadgetfs_features {
>> >> > + uint64_t bitmap[4];
>> >>
>> >> this should really be:
>> >>
>> >> uint64_t features0;
>> >>
>> >> /* Keep 24 bytes reserved for possible future usage */
>> >> uint8_t RESERVED[24];
>> >
>> > Isn't this really extreme overkill? I would be surprised if gadgetfs
>> > ever supports more than 10 features.
>>
>> I'm hoping to be extreme overkill :-) I really don't wanna get into a
>> situation where we run out of bits. What if some features require
>> arguments in the future? We could just pass them here, right?
>>
>> Frankly, I'm afraid of what happened to i2c-dev :-) We can't change
>> (easily) i2c subsystem's write() to return the amount of bytes written
>> because its userspace facing ABI has no reserved bits we could use to
>> return that value. Changing that would be a huge amount of work.
>>
>> What do you think?
>
> Well, I don't care about it all that much, especially since I don't
> think you'll ever need to use it. :-)
>
> Still, there are approaches that have been used in the past when people
> wanted to create an extensible interface. Passing a variable-length
> structure that has a length field at a reserved spot near the beginning
> (perhaps along with one or two other reserved entries), for example.
something along the like below?
/* better names appreciated */
struct usb_gadgetfs_packet {
uint16_t length;
uint8_t reserved[2];
uint8_t *data; /* dynamically allocated by userspace */
}
I can work with that no problem.
--
balbi
Attachment:
signature.asc
Description: PGP signature
