Re: [PATCH v6 09/17] media: uvcvideo: refactor __uvc_ctrl_add_mapping
On 17/03/2021 17:45, Ricardo Ribalda wrote:
> Pass the chain instead of the device. We want to keed the reference to
keed -> keep
With that typo fixed you can add my:
Reviewed-by: Hans Verkuil
Thanks!
Hans
> the chain that controls belong to.
>
> We need to delay the initialization of the controls after the chains
> have been initialized.
>
> This is a cleanup needed for the next patches.
>
> Reviewed-by: Laurent Pinchart
> Signed-off-by: Ricardo Ribalda
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 41 --
> drivers/media/usb/uvc/uvc_driver.c | 8 +++---
> 2 files changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c
> index b3dde98499f4..b75da65115ef 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2057,7 +2057,7 @@ static int uvc_ctrl_add_info(struct uvc_device *dev,
> struct uvc_control *ctrl,
> /*
> * Add a control mapping to a given control.
> */
> -static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
> +static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
> {
> struct uvc_control_mapping *map;
> @@ -2086,7 +2086,7 @@ static int __uvc_ctrl_add_mapping(struct uvc_device
> *dev,
> map->set = uvc_set_le_value;
>
> list_add_tail(>list, >info.mappings);
> - uvc_dbg(dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
> + uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
> map->name, ctrl->info.entity, ctrl->info.selector);
>
> return 0;
> @@ -2168,7 +2168,7 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> goto done;
> }
>
> - ret = __uvc_ctrl_add_mapping(dev, ctrl, mapping);
> + ret = __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> if (ret < 0)
> atomic_dec(>nmappings);
>
> @@ -2244,7 +2244,8 @@ static void uvc_ctrl_prune_entity(struct uvc_device
> *dev,
> * Add control information and hardcoded stock control mappings to the given
> * device.
> */
> -static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control
> *ctrl)
> +static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> +struct uvc_control *ctrl)
> {
> const struct uvc_control_info *info = uvc_ctrls;
> const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> @@ -2263,14 +2264,14 @@ static void uvc_ctrl_init_ctrl(struct uvc_device
> *dev, struct uvc_control *ctrl)
> for (; info < iend; ++info) {
> if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
> ctrl->index == info->index) {
> - uvc_ctrl_add_info(dev, ctrl, info);
> + uvc_ctrl_add_info(chain->dev, ctrl, info);
> /*
>* Retrieve control flags from the device. Ignore errors
>* and work with default flag values from the uvc_ctrl
>* array when the device doesn't properly implement
>* GET_INFO on standard controls.
>*/
> - uvc_ctrl_get_flags(dev, ctrl, >info);
> + uvc_ctrl_get_flags(chain->dev, ctrl, >info);
> break;
>}
> }
> @@ -2281,22 +2282,20 @@ static void uvc_ctrl_init_ctrl(struct uvc_device
> *dev, struct uvc_control *ctrl)
> for (; mapping < mend; ++mapping) {
> if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> ctrl->info.selector == mapping->selector)
> - __uvc_ctrl_add_mapping(dev, ctrl, mapping);
> + __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> }
> }
>
> /*
> * Initialize device controls.
> */
> -int uvc_ctrl_init_device(struct uvc_device *dev)
> +static int uvc_ctrl_init_chain(struct uvc_video_chain *chain)
> {
> struct uvc_entity *entity;
> unsigned int i;
>
> - INIT_WORK(>async_ctrl.work, uvc_ctrl_status_event_work);
> -
> /* Walk the entities list and instantiate controls */
> - list_for_each_entry(entity, >entities, list) {
> + list_for_each_entry(entity, >entities, chain) {
> struct uvc_control *ctrl;
> unsigned int bControlSize = 0, ncontrols;
> u8 *bmControls = NULL;
> @@ -2316,7 +2315,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
> }
>
> /* Remove bogus/blacklisted controls */
> - uvc_ctrl_prune_entity(dev, entity);
> + uvc_ctrl_prune_entity(chain->dev, entity);
>
> /* Count supported controls and allocate the controls array */
> ncontrols = memweight(bmControls, bControlSize);
> @@ -2338,7 +2337,7 @@ int
[PATCH v6 09/17] media: uvcvideo: refactor __uvc_ctrl_add_mapping
Pass the chain instead of the device. We want to keed the reference to
the chain that controls belong to.
We need to delay the initialization of the controls after the chains
have been initialized.
This is a cleanup needed for the next patches.
Reviewed-by: Laurent Pinchart
Signed-off-by: Ricardo Ribalda
---
drivers/media/usb/uvc/uvc_ctrl.c | 41 --
drivers/media/usb/uvc/uvc_driver.c | 8 +++---
2 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index b3dde98499f4..b75da65115ef 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2057,7 +2057,7 @@ static int uvc_ctrl_add_info(struct uvc_device *dev,
struct uvc_control *ctrl,
/*
* Add a control mapping to a given control.
*/
-static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
+static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
{
struct uvc_control_mapping *map;
@@ -2086,7 +2086,7 @@ static int __uvc_ctrl_add_mapping(struct uvc_device *dev,
map->set = uvc_set_le_value;
list_add_tail(>list, >info.mappings);
- uvc_dbg(dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
+ uvc_dbg(chain->dev, CONTROL, "Adding mapping '%s' to control %pUl/%u\n",
map->name, ctrl->info.entity, ctrl->info.selector);
return 0;
@@ -2168,7 +2168,7 @@ int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
goto done;
}
- ret = __uvc_ctrl_add_mapping(dev, ctrl, mapping);
+ ret = __uvc_ctrl_add_mapping(chain, ctrl, mapping);
if (ret < 0)
atomic_dec(>nmappings);
@@ -2244,7 +2244,8 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev,
* Add control information and hardcoded stock control mappings to the given
* device.
*/
-static void uvc_ctrl_init_ctrl(struct uvc_device *dev, struct uvc_control
*ctrl)
+static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
+ struct uvc_control *ctrl)
{
const struct uvc_control_info *info = uvc_ctrls;
const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
@@ -2263,14 +2264,14 @@ static void uvc_ctrl_init_ctrl(struct uvc_device *dev,
struct uvc_control *ctrl)
for (; info < iend; ++info) {
if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
ctrl->index == info->index) {
- uvc_ctrl_add_info(dev, ctrl, info);
+ uvc_ctrl_add_info(chain->dev, ctrl, info);
/*
* Retrieve control flags from the device. Ignore errors
* and work with default flag values from the uvc_ctrl
* array when the device doesn't properly implement
* GET_INFO on standard controls.
*/
- uvc_ctrl_get_flags(dev, ctrl, >info);
+ uvc_ctrl_get_flags(chain->dev, ctrl, >info);
break;
}
}
@@ -2281,22 +2282,20 @@ static void uvc_ctrl_init_ctrl(struct uvc_device *dev,
struct uvc_control *ctrl)
for (; mapping < mend; ++mapping) {
if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
ctrl->info.selector == mapping->selector)
- __uvc_ctrl_add_mapping(dev, ctrl, mapping);
+ __uvc_ctrl_add_mapping(chain, ctrl, mapping);
}
}
/*
* Initialize device controls.
*/
-int uvc_ctrl_init_device(struct uvc_device *dev)
+static int uvc_ctrl_init_chain(struct uvc_video_chain *chain)
{
struct uvc_entity *entity;
unsigned int i;
- INIT_WORK(>async_ctrl.work, uvc_ctrl_status_event_work);
-
/* Walk the entities list and instantiate controls */
- list_for_each_entry(entity, >entities, list) {
+ list_for_each_entry(entity, >entities, chain) {
struct uvc_control *ctrl;
unsigned int bControlSize = 0, ncontrols;
u8 *bmControls = NULL;
@@ -2316,7 +2315,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
}
/* Remove bogus/blacklisted controls */
- uvc_ctrl_prune_entity(dev, entity);
+ uvc_ctrl_prune_entity(chain->dev, entity);
/* Count supported controls and allocate the controls array */
ncontrols = memweight(bmControls, bControlSize);
@@ -2338,7 +2337,7 @@ int uvc_ctrl_init_device(struct uvc_device *dev)
ctrl->entity = entity;
ctrl->index = i;
- uvc_ctrl_init_ctrl(dev, ctrl);
+ uvc_ctrl_init_ctrl(chain, ctrl);
ctrl++;
