[PATCHv13 09/28] v4l2-ctrls: prepare internal structs for request API
From: Hans Verkuil
Embed and initialize a media_request_object in struct v4l2_ctrl_handler.
Add a p_req field to struct v4l2_ctrl_ref that will store the
request value.
Signed-off-by: Hans Verkuil
Signed-off-by: Alexandre Courbot
---
drivers/media/v4l2-core/v4l2-ctrls.c | 1 +
include/media/v4l2-ctrls.h | 7 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index aa1dd2015e84..d09f49530d9e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1880,6 +1880,7 @@ int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler
*hdl,
sizeof(hdl->buckets[0]),
GFP_KERNEL | __GFP_ZERO);
hdl->error = hdl->buckets ? 0 : -ENOMEM;
+ media_request_object_init(&hdl->req_obj);
return hdl->error;
}
EXPORT_SYMBOL(v4l2_ctrl_handler_init_class);
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index d26b8ddebb56..76352eb59f14 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
/* forward references */
struct file;
@@ -249,6 +250,8 @@ struct v4l2_ctrl {
* ``prepare_ext_ctrls`` function at ``v4l2-ctrl.c``.
* @from_other_dev: If true, then @ctrl was defined in another
* device than the &struct v4l2_ctrl_handler.
+ * @p_req: The request value. Only used if the control handler
+ * is bound to a media request.
*
* Each control handler has a list of these refs. The list_head is used to
* keep a sorted-by-control-ID list of all controls, while the next pointer
@@ -260,6 +263,7 @@ struct v4l2_ctrl_ref {
struct v4l2_ctrl *ctrl;
struct v4l2_ctrl_helper *helper;
bool from_other_dev;
+ union v4l2_ctrl_ptr p_req;
};
/**
@@ -283,6 +287,8 @@ struct v4l2_ctrl_ref {
* @notify_priv: Passed as argument to the v4l2_ctrl notify callback.
* @nr_of_buckets: Total number of buckets in the array.
* @error: The error code of the first failed control addition.
+ * @req_obj: The &struct media_request_object, used to link into a
+ * &struct media_request.
*/
struct v4l2_ctrl_handler {
struct mutex _lock;
@@ -295,6 +301,7 @@ struct v4l2_ctrl_handler {
void *notify_priv;
u16 nr_of_buckets;
int error;
+ struct media_request_object req_obj;
};
/**
--
2.17.0
Re: [PATCHv13 09/28] v4l2-ctrls: prepare internal structs for request API
Em Thu, 3 May 2018 16:52:59 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Embed and initialize a media_request_object in struct v4l2_ctrl_handler.
>
> Add a p_req field to struct v4l2_ctrl_ref that will store the
> request value.
>
> Signed-off-by: Hans Verkuil
> Signed-off-by: Alexandre Courbot
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 1 +
> include/media/v4l2-ctrls.h | 7 +++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index aa1dd2015e84..d09f49530d9e 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1880,6 +1880,7 @@ int v4l2_ctrl_handler_init_class(struct
> v4l2_ctrl_handler *hdl,
> sizeof(hdl->buckets[0]),
> GFP_KERNEL | __GFP_ZERO);
> hdl->error = hdl->buckets ? 0 : -ENOMEM;
> + media_request_object_init(&hdl->req_obj);
> return hdl->error;
> }
> EXPORT_SYMBOL(v4l2_ctrl_handler_init_class);
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index d26b8ddebb56..76352eb59f14 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> /* forward references */
> struct file;
> @@ -249,6 +250,8 @@ struct v4l2_ctrl {
> * ``prepare_ext_ctrls`` function at ``v4l2-ctrl.c``.
> * @from_other_dev: If true, then @ctrl was defined in another
> * device than the &struct v4l2_ctrl_handler.
> + * @p_req: The request value. Only used if the control handler
> + * is bound to a media request.
Could you please better elaborate the description of this field?
I read this patch dozen times to understand what you meant by
"request value", as I would be expecting here a "media_request_"
object or something similar. Instead, what you meant to say is that
@p_req will be used to cache the data passed via a request API call,
while the request is not handled yet, right?
> *
> * Each control handler has a list of these refs. The list_head is used to
> * keep a sorted-by-control-ID list of all controls, while the next pointer
> @@ -260,6 +263,7 @@ struct v4l2_ctrl_ref {
> struct v4l2_ctrl *ctrl;
> struct v4l2_ctrl_helper *helper;
> bool from_other_dev;
> + union v4l2_ctrl_ptr p_req;
> };
>
> /**
> @@ -283,6 +287,8 @@ struct v4l2_ctrl_ref {
> * @notify_priv: Passed as argument to the v4l2_ctrl notify callback.
> * @nr_of_buckets: Total number of buckets in the array.
> * @error: The error code of the first failed control addition.
> + * @req_obj: The &struct media_request_object, used to link into a
> + * &struct media_request.
I would document that there is kref is inside @req_obj, as we can't
add another kref here later.
> */
> struct v4l2_ctrl_handler {
> struct mutex _lock;
> @@ -295,6 +301,7 @@ struct v4l2_ctrl_handler {
> void *notify_priv;
> u16 nr_of_buckets;
> int error;
> + struct media_request_object req_obj;
> };
>
> /**
Thanks,
Mauro
Re: [PATCHv13 09/28] v4l2-ctrls: prepare internal structs for request API
On 05/07/2018 07:35 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 3 May 2018 16:52:59 +0200
> Hans Verkuil escreveu:
>
>> From: Hans Verkuil
>>
>> Embed and initialize a media_request_object in struct v4l2_ctrl_handler.
>>
>> Add a p_req field to struct v4l2_ctrl_ref that will store the
>> request value.
>>
>> Signed-off-by: Hans Verkuil
>> Signed-off-by: Alexandre Courbot
>> ---
>> drivers/media/v4l2-core/v4l2-ctrls.c | 1 +
>> include/media/v4l2-ctrls.h | 7 +++
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index aa1dd2015e84..d09f49530d9e 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -1880,6 +1880,7 @@ int v4l2_ctrl_handler_init_class(struct
>> v4l2_ctrl_handler *hdl,
>>sizeof(hdl->buckets[0]),
>>GFP_KERNEL | __GFP_ZERO);
>> hdl->error = hdl->buckets ? 0 : -ENOMEM;
>> +media_request_object_init(&hdl->req_obj);
>> return hdl->error;
>> }
>> EXPORT_SYMBOL(v4l2_ctrl_handler_init_class);
>> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
>> index d26b8ddebb56..76352eb59f14 100644
>> --- a/include/media/v4l2-ctrls.h
>> +++ b/include/media/v4l2-ctrls.h
>> @@ -20,6 +20,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> /* forward references */
>> struct file;
>> @@ -249,6 +250,8 @@ struct v4l2_ctrl {
>> * ``prepare_ext_ctrls`` function at ``v4l2-ctrl.c``.
>> * @from_other_dev: If true, then @ctrl was defined in another
>> * device than the &struct v4l2_ctrl_handler.
>> + * @p_req: The request value. Only used if the control handler
>> + * is bound to a media request.
>
> Could you please better elaborate the description of this field?
>
> I read this patch dozen times to understand what you meant by
> "request value", as I would be expecting here a "media_request_"
> object or something similar. Instead, what you meant to say is that
> @p_req will be used to cache the data passed via a request API call,
> while the request is not handled yet, right?
The control handler has the request object. If the control handler is
part of a request, then the values of the controls that are owned by that
control handler are stored in p_req. When the request has been completed
p_req will contain the value at completion time.
I'll improve the documentation.
>
>
>> *
>> * Each control handler has a list of these refs. The list_head is used to
>> * keep a sorted-by-control-ID list of all controls, while the next pointer
>> @@ -260,6 +263,7 @@ struct v4l2_ctrl_ref {
>> struct v4l2_ctrl *ctrl;
>> struct v4l2_ctrl_helper *helper;
>> bool from_other_dev;
>> +union v4l2_ctrl_ptr p_req;
>> };
>>
>> /**
>> @@ -283,6 +287,8 @@ struct v4l2_ctrl_ref {
>> * @notify_priv: Passed as argument to the v4l2_ctrl notify callback.
>> * @nr_of_buckets: Total number of buckets in the array.
>> * @error: The error code of the first failed control addition.
>> + * @req_obj:The &struct media_request_object, used to link into a
>> + * &struct media_request.
>
> I would document that there is kref is inside @req_obj, as we can't
> add another kref here later.
OK.
Regards,
Hans
>
>> */
>> struct v4l2_ctrl_handler {
>> struct mutex _lock;
>> @@ -295,6 +301,7 @@ struct v4l2_ctrl_handler {
>> void *notify_priv;
>> u16 nr_of_buckets;
>> int error;
>> +struct media_request_object req_obj;
>> };
>>
>> /**
>
>
>
> Thanks,
> Mauro
>
