Re: [RFCv11 PATCH 02/29] uapi/linux/media.h: add request API

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

 



On 04/10/2018 01:00 PM, Sakari Ailus wrote:
> On Tue, Apr 10, 2018 at 06:38:56AM -0300, Mauro Carvalho Chehab wrote:
>> Em Mon,  9 Apr 2018 16:19:59 +0200
>> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
>>
>>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>>
>>> Define the public request API.
>>>
>>> This adds the new MEDIA_IOC_REQUEST_ALLOC ioctl to allocate a request
>>> and two ioctls that operate on a request in order to queue the
>>> contents of the request to the driver and to re-initialize the
>>> request.
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>> ---
>>>  include/uapi/linux/media.h | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
>>> index c7e9a5cba24e..f8769e74f847 100644
>>> --- a/include/uapi/linux/media.h
>>> +++ b/include/uapi/linux/media.h
>>> @@ -342,11 +342,19 @@ struct media_v2_topology {
>>>  
>>>  /* ioctls */
>>>  
>>> +struct __attribute__ ((packed)) media_request_alloc {
>>> +	__s32 fd;
>>> +};
>>> +
>>>  #define MEDIA_IOC_DEVICE_INFO	_IOWR('|', 0x00, struct media_device_info)
>>>  #define MEDIA_IOC_ENUM_ENTITIES	_IOWR('|', 0x01, struct media_entity_desc)
>>>  #define MEDIA_IOC_ENUM_LINKS	_IOWR('|', 0x02, struct media_links_enum)
>>>  #define MEDIA_IOC_SETUP_LINK	_IOWR('|', 0x03, struct media_link_desc)
>>>  #define MEDIA_IOC_G_TOPOLOGY	_IOWR('|', 0x04, struct media_v2_topology)
>>> +#define MEDIA_IOC_REQUEST_ALLOC	_IOWR('|', 0x05, struct media_request_alloc)
>>> +
>>
>> Why use a struct here? Just declare it as:
>>
>> 	#define MEDIA_IOC_REQUEST_ALLOC	_IOWR('|', 0x05, int)
> 
> I'd say it's easier to extend it if it's a struct. All other IOCTLs also
> have a struct as an argument. As a struct member, the parameter (fd) also
> has a name; this is a plus.

While I do not have a very strong opinion on this, I do agree with Sakari here.

Regards,

	Hans

> 
>>
>>> +#define MEDIA_REQUEST_IOC_QUEUE		_IO('|',  0x80)
>>> +#define MEDIA_REQUEST_IOC_REINIT	_IO('|',  0x81)
>>>  
>>>  #if !defined(__KERNEL__) || defined(__NEED_MEDIA_LEGACY_API)
>>>  
>>
>> Thanks,
>> Mauro
> 




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux