Hi,
Binyamin Sharet <bsharet@xxxxxxxxx> writes:
> 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?
sounds good to me, yes :-)
--
balbi
Attachment:
signature.asc
Description: PGP signature
