On 09/12/2016 04:11 PM, Alan Stern wrote:
> On Mon, 12 Sep 2016, Binyamin Sharet (bsharet) wrote:
>
>>> On 8 Sep 2016, at 23:24, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>>>
>>> On Thu, 8 Sep 2016, Binyamin Sharet (bsharet) wrote:
>>>
>>>>> On 8 Sep 2016, at 22:20, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>>>>>
>>>>> On Thu, 8 Sep 2016, Binyamin Sharet (bsharet) wrote:
>>>>>
>>>>>>> I was thinking more like:
>>>>>>>
>>>>>>> struct usb_gadgetfs_ioctl_arg {
>>>>>>> uint16_t length;
>>>>>>> uint8_t reserved[2];
>>>>>>>
>>>>>>> uint8_t data[0];
>>>>>>> }
>>>>>>>
>>>>>>> but the principle is pretty much the same.
>>>>>>>
>>>>>>> Alan Stern
>>>>>>>
>>>>>> Won’t the user lose the relevant information (e.g. feature
>>>>>> structure) by using this model?
>>>>> What feature structure? Aren't your feature lists just vectors of 64
>>>>> bits? They can be stored in the .data field above.
>>>>>
>>>>> Alan Stern
>>>>>
>>>> Not “just” - they are platform-dependant uint64_t. which means they
>>>> won’t look the same on systems with different endianness. If the
>>>> user is unaware of this, it can cause confusion w/r/t which bit is which.
>>>>
>>>> We can use 8-bit vectors instead and skip the endianness issue,
>>>> but why define a generic usb_gadgetfs_ioctl_args structure instead
>>>> of “features struct” for feature-related ioctls and different structs for
>>>> other types of ioctl (if we’ll have such in the future)?
>>> Good point. Yes, you can have different interfaces for different
>>> ioctls.
>>>
>>> This whole question arose because I complained that the features API
>>> looked too extravagant, and Felipe said that he didn't want to run out
>>> of available fields in case any features in the future need them.
>>>
>>> But if you're worried about that, why limit yourself to 64 features and
>>> 24 bytes of extra data? It doesn't make sense to think that some fixed
>>> limit might end up being too small, but then restrict yourself to a
>>> different fixed limit.
>>>
>>> Alan Stern
>> What about changing the ioctl API for the “features” handling, and instead
>> of passing an entire bitmap from user to kernel and vice versa, the user
>> will only operate on a single feature (by feature id)?
>>
>> This way we’ll have a much simpler (and intuitive) API with the user -
>> is_feature_supported(u64), is_feature_enabled(u64), enable_feature(u64)
>> and disabled_feature(u64).
>>
>> So the internal representation of the supported/enabled features will not
>> matter much to the user.
> That's fine with me. But since you're passing feature ID numbers to
> the kernel, instead of feature bitmaps, the argument type should be
> plain int, not u64. Unless you think somebody will implement more than
> 2 billion features... :-)
>
> 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
Felipe - is it OK with you?
--
Binyamin Sharet,
Cisco, STARE-C
--
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
