[PATCH v2 0/7] media: uvc: Probe PLF limits at start-up

[PATCH v2 0/7] media: uvc: Probe PLF limits at start-up

[Ricardo Ribalda]

The UVC standard descries the values for the PLF control for its
versions 1.1 and and 1.5, but it does not describe which values MUST be
implemented.

So far, we have been adding "device quirks" to provide proper limits for
the control, but this is failing to scale.

Add a function that during probe-time checks the capability of the
control.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v2: Thanks Laurent
- Cleanup mapings from uvc_device_info
- Use __free()
- Implement filter_mapping
- Link to v1: https://lore.kernel.org/r/20240318-billion-v1-0-2f7bc0ee2030@chromium.org

---
Ricardo Ribalda (7):
      media: uvcvideo: Allow custom control mapping
      media: uvcvideo: Refactor Power Line Frequency limit selection
      media: uvcvideo: Probe the PLF characteristics
      media: uvcvideo: Cleanup version-specific mapping
      media: uvcvideo: Remove PLF device quirking
      media: uvcvideo: Remove mappings form uvc_device_info
      media: uvcvideo: Replace get_mapping with filter_mapping

 drivers/media/usb/uvc/uvc_ctrl.c   | 191 ++++++++++++++++++++-----------------
 drivers/media/usb/uvc/uvc_driver.c | 122 -----------------------
 drivers/media/usb/uvc/uvcvideo.h   |   8 +-
 3 files changed, 109 insertions(+), 212 deletions(-)
---
base-commit: b14257abe7057def6127f6fb2f14f9adc8acabdb
change-id: 20240313-billion-5b2a45fa86f4

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

[PATCH v2 1/7] media: uvcvideo: Allow custom control mapping

[Ricardo Ribalda]

Some advanced controls might not be completely implemented by vendors.

If the controls are a enumeration, UVC does not gives a way to probe
what is implemented and what is not.

Let's create a new callback function where heuristics can be implemented
to detect what is implemented and what not.

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 10 +++++++++-
 drivers/media/usb/uvc/uvcvideo.h |  5 +++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index e59a463c2761..44ec185a8c8b 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2360,7 +2360,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_video_chain *chain,
+static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
 	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
 {
 	struct uvc_control_mapping *map;
@@ -2434,6 +2434,14 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
 	return -ENOMEM;
 }
 
+static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
+	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
+{
+	if (mapping && mapping->add_mapping)
+		return mapping->add_mapping(chain, ctrl, mapping);
+	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, mapping);
+}
+
 int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
 	const struct uvc_control_mapping *mapping)
 {
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 6fb0a78b1b00..fa0396dd5b35 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -84,7 +84,9 @@
 
 struct gpio_desc;
 struct sg_table;
+struct uvc_control;
 struct uvc_device;
+struct uvc_video_chain;
 
 /*
  * TODO: Put the most frequently accessed fields at the beginning of
@@ -123,6 +125,9 @@ struct uvc_control_mapping {
 	s32 master_manual;
 	u32 slave_ids[2];
 
+	int (*add_mapping)(struct uvc_video_chain *chain,
+			   struct uvc_control *ctrl,
+			   const struct uvc_control_mapping *mapping);
 	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
 		   const u8 *data);
 	void (*set)(struct uvc_control_mapping *mapping, s32 value,

-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH v2 1/7] media: uvcvideo: Allow custom control mapping

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Mon, Jun 10, 2024 at 11:09:52PM +0000, Ricardo Ribalda wrote:
> Some advanced controls might not be completely implemented by vendors.
> 
> If the controls are a enumeration, UVC does not gives a way to probe
> what is implemented and what is not.
> 
> Let's create a new callback function where heuristics can be implemented
> to detect what is implemented and what not.
> 
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 10 +++++++++-
>  drivers/media/usb/uvc/uvcvideo.h |  5 +++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index e59a463c2761..44ec185a8c8b 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2360,7 +2360,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_video_chain *chain,
> +static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
>  	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
>  {
>  	struct uvc_control_mapping *map;
> @@ -2434,6 +2434,14 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>  	return -ENOMEM;
>  }
>  
> +static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> +	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
> +{
> +	if (mapping && mapping->add_mapping)
> +		return mapping->add_mapping(chain, ctrl, mapping);
> +	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, mapping);
> +}
> +
>  int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>  	const struct uvc_control_mapping *mapping)
>  {
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 6fb0a78b1b00..fa0396dd5b35 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -84,7 +84,9 @@
>  
>  struct gpio_desc;
>  struct sg_table;
> +struct uvc_control;
>  struct uvc_device;
> +struct uvc_video_chain;
>  
>  /*
>   * TODO: Put the most frequently accessed fields at the beginning of
> @@ -123,6 +125,9 @@ struct uvc_control_mapping {
>  	s32 master_manual;
>  	u32 slave_ids[2];
>  
> +	int (*add_mapping)(struct uvc_video_chain *chain,
> +			   struct uvc_control *ctrl,
> +			   const struct uvc_control_mapping *mapping);
>  	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
>  		   const u8 *data);
>  	void (*set)(struct uvc_control_mapping *mapping, s32 value,

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/7] media: uvcvideo: Refactor Power Line Frequency limit selection

[Ricardo Ribalda]

Move the PLF mapping logic to its own function. This patch does not
introduce any new functionality to the logic, it is just a preparation
patch.

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 93 ++++++++++++++++++++++++----------------
 1 file changed, 55 insertions(+), 38 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 44ec185a8c8b..d82cfc56dfd5 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -459,6 +459,56 @@ static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping,
 	data[first+1] = min_t(int, abs(value), 0xff);
 }
 
+const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
+	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
+	.entity		= UVC_GUID_UVC_PROCESSING,
+	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
+	.size		= 2,
+	.offset		= 0,
+	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
+	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
+	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
+				  V4L2_CID_POWER_LINE_FREQUENCY_50HZ),
+};
+
+const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
+	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
+	.entity		= UVC_GUID_UVC_PROCESSING,
+	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
+	.size		= 2,
+	.offset		= 0,
+	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
+	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
+	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
+				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
+};
+
+static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
+	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
+	.entity		= UVC_GUID_UVC_PROCESSING,
+	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
+	.size		= 2,
+	.offset		= 0,
+	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
+	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
+	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO,
+				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
+};
+
+static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
+	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping);
+
+static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
+	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
+{
+	if (chain->dev->uvc_version < 0x150)
+		return __uvc_ctrl_add_mapping(chain, ctrl,
+					      &uvc_ctrl_power_line_mapping_uvc11);
+
+	return __uvc_ctrl_add_mapping(chain, ctrl,
+				      &uvc_ctrl_power_line_mapping_uvc15);
+}
+
 static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
 	{
 		.id		= V4L2_CID_BRIGHTNESS,
@@ -748,51 +798,18 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
 	},
-};
-
-const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
-	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
-	.entity		= UVC_GUID_UVC_PROCESSING,
-	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
-	.size		= 2,
-	.offset		= 0,
-	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
-	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
-	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
-				  V4L2_CID_POWER_LINE_FREQUENCY_50HZ),
-};
-
-const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
-	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
-	.entity		= UVC_GUID_UVC_PROCESSING,
-	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
-	.size		= 2,
-	.offset		= 0,
-	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
-	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
-	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
-				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
+	{
+		.entity		= UVC_GUID_UVC_PROCESSING,
+		.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
+		.add_mapping	= uvc_ctrl_add_plf_mapping,
+	},
 };
 
 static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc11[] = {
-	&uvc_ctrl_power_line_mapping_uvc11,
 	NULL, /* Sentinel */
 };
 
-static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
-	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
-	.entity		= UVC_GUID_UVC_PROCESSING,
-	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
-	.size		= 2,
-	.offset		= 0,
-	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
-	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
-	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO,
-				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
-};
-
 static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc15[] = {
-	&uvc_ctrl_power_line_mapping_uvc15,
 	NULL, /* Sentinel */
 };
 

-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH v2 2/7] media: uvcvideo: Refactor Power Line Frequency limit selection

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Mon, Jun 10, 2024 at 11:09:53PM +0000, Ricardo Ribalda wrote:
> Move the PLF mapping logic to its own function. This patch does not
> introduce any new functionality to the logic, it is just a preparation
> patch.
> 
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 93 ++++++++++++++++++++++++----------------
>  1 file changed, 55 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 44ec185a8c8b..d82cfc56dfd5 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -459,6 +459,56 @@ static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping,
>  	data[first+1] = min_t(int, abs(value), 0xff);
>  }
>  
> +const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
> +	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
> +	.entity		= UVC_GUID_UVC_PROCESSING,
> +	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> +	.size		= 2,
> +	.offset		= 0,
> +	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> +	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> +	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
> +				  V4L2_CID_POWER_LINE_FREQUENCY_50HZ),
> +};
> +
> +const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
> +	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
> +	.entity		= UVC_GUID_UVC_PROCESSING,
> +	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> +	.size		= 2,
> +	.offset		= 0,
> +	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> +	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> +	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
> +				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
> +};
> +
> +static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
> +	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
> +	.entity		= UVC_GUID_UVC_PROCESSING,
> +	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> +	.size		= 2,
> +	.offset		= 0,
> +	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> +	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> +	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO,
> +				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
> +};
> +
> +static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> +	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping);
> +
> +static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
> +	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
> +{
> +	if (chain->dev->uvc_version < 0x150)
> +		return __uvc_ctrl_add_mapping(chain, ctrl,
> +					      &uvc_ctrl_power_line_mapping_uvc11);
> +
> +	return __uvc_ctrl_add_mapping(chain, ctrl,
> +				      &uvc_ctrl_power_line_mapping_uvc15);
> +}
> +
>  static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
>  	{
>  		.id		= V4L2_CID_BRIGHTNESS,
> @@ -748,51 +798,18 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
>  		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
>  		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
>  	},
> -};
> -
> -const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
> -	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
> -	.entity		= UVC_GUID_UVC_PROCESSING,
> -	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> -	.size		= 2,
> -	.offset		= 0,
> -	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> -	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> -	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
> -				  V4L2_CID_POWER_LINE_FREQUENCY_50HZ),
> -};
> -
> -const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
> -	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
> -	.entity		= UVC_GUID_UVC_PROCESSING,
> -	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> -	.size		= 2,
> -	.offset		= 0,
> -	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> -	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> -	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ,
> -				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
> +	{
> +		.entity		= UVC_GUID_UVC_PROCESSING,
> +		.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> +		.add_mapping	= uvc_ctrl_add_plf_mapping,
> +	},
>  };
>  
>  static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc11[] = {
> -	&uvc_ctrl_power_line_mapping_uvc11,
>  	NULL, /* Sentinel */
>  };
>  
> -static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
> -	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
> -	.entity		= UVC_GUID_UVC_PROCESSING,
> -	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> -	.size		= 2,
> -	.offset		= 0,
> -	.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> -	.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> -	.menu_mask	= GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO,
> -				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
> -};
> -
>  static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc15[] = {
> -	&uvc_ctrl_power_line_mapping_uvc15,
>  	NULL, /* Sentinel */
>  };
>  

-- 
Regards,

Laurent Pinchart

[PATCH v2 3/7] media: uvcvideo: Probe the PLF characteristics

[Ricardo Ribalda]

The UVC 1.5 standard defines 4 values for the PLF control: Off, 50Hz,
60Hz and Auto. But it does not clearly define if all the values must be
implemented or not.

Instead of just using the UVC version to determine what the PLF control
can do, probe it.

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 52 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index d82cfc56dfd5..5b77ac308c84 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -495,18 +495,60 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
 				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
 };
 
-static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
+static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
 	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping);
 
 static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
 	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
 {
+	const struct uvc_control_mapping *out_mapping =
+					&uvc_ctrl_power_line_mapping_uvc11;
+	u8 *buf __free(kfree) = NULL;
+	u8 init_val;
+	int ret;
+
+	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/* Save the default PLF value, so we can restore it. */
+	ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, ctrl->entity->id,
+			     chain->dev->intfnum, ctrl->info.selector,
+			     buf, sizeof(*buf));
+	/* If we cannot read the control skip it. */
+	if (ret)
+		return ret;
+	init_val = *buf;
+
+	/* If PLF value cannot be set to off, it is limited. */
+	*buf = V4L2_CID_POWER_LINE_FREQUENCY_DISABLED;
+	ret = uvc_query_ctrl(chain->dev, UVC_SET_CUR, ctrl->entity->id,
+			     chain->dev->intfnum, ctrl->info.selector,
+			     buf, sizeof(*buf));
+	if (ret)
+		return __uvc_ctrl_add_mapping_to_list(chain, ctrl,
+					&uvc_ctrl_power_line_mapping_limited);
+
+	/* UVC 1.1 does not define auto, we can exit. */
 	if (chain->dev->uvc_version < 0x150)
-		return __uvc_ctrl_add_mapping(chain, ctrl,
-					      &uvc_ctrl_power_line_mapping_uvc11);
+		goto end;
+
+	/* Check if the device supports auto. */
+	*buf = V4L2_CID_POWER_LINE_FREQUENCY_AUTO;
+	ret = uvc_query_ctrl(chain->dev, UVC_SET_CUR, ctrl->entity->id,
+			     chain->dev->intfnum, ctrl->info.selector,
+			     buf, sizeof(*buf));
+	if (!ret)
+		out_mapping = &uvc_ctrl_power_line_mapping_uvc15;
+
+end:
+	/* Restore initial value and add mapping. */
+	*buf = init_val;
+	uvc_query_ctrl(chain->dev, UVC_SET_CUR, ctrl->entity->id,
+		       chain->dev->intfnum, ctrl->info.selector,
+		       buf, sizeof(*buf));
 
-	return __uvc_ctrl_add_mapping(chain, ctrl,
-				      &uvc_ctrl_power_line_mapping_uvc15);
+	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, out_mapping);
 }
 
 static const struct uvc_control_mapping uvc_ctrl_mappings[] = {

-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH v2 3/7] media: uvcvideo: Probe the PLF characteristics

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Mon, Jun 10, 2024 at 11:09:54PM +0000, Ricardo Ribalda wrote:
> The UVC 1.5 standard defines 4 values for the PLF control: Off, 50Hz,
> 60Hz and Auto. But it does not clearly define if all the values must be
> implemented or not.
> 
> Instead of just using the UVC version to determine what the PLF control
> can do, probe it.
> 
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 52 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index d82cfc56dfd5..5b77ac308c84 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -495,18 +495,60 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
>  				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
>  };
>  
> -static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> +static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
>  	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping);
>  
>  static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
>  	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
>  {
> +	const struct uvc_control_mapping *out_mapping =
> +					&uvc_ctrl_power_line_mapping_uvc11;
> +	u8 *buf __free(kfree) = NULL;
> +	u8 init_val;
> +	int ret;
> +
> +	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	/* Save the default PLF value, so we can restore it. */
> +	ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, ctrl->entity->id,
> +			     chain->dev->intfnum, ctrl->info.selector,
> +			     buf, sizeof(*buf));
> +	/* If we cannot read the control skip it. */
> +	if (ret)
> +		return ret;
> +	init_val = *buf;
> +
> +	/* If PLF value cannot be set to off, it is limited. */
> +	*buf = V4L2_CID_POWER_LINE_FREQUENCY_DISABLED;
> +	ret = uvc_query_ctrl(chain->dev, UVC_SET_CUR, ctrl->entity->id,
> +			     chain->dev->intfnum, ctrl->info.selector,
> +			     buf, sizeof(*buf));
> +	if (ret)
> +		return __uvc_ctrl_add_mapping_to_list(chain, ctrl,
> +					&uvc_ctrl_power_line_mapping_limited);
> +
> +	/* UVC 1.1 does not define auto, we can exit. */
>  	if (chain->dev->uvc_version < 0x150)
> -		return __uvc_ctrl_add_mapping(chain, ctrl,
> -					      &uvc_ctrl_power_line_mapping_uvc11);
> +		goto end;
> +
> +	/* Check if the device supports auto. */
> +	*buf = V4L2_CID_POWER_LINE_FREQUENCY_AUTO;
> +	ret = uvc_query_ctrl(chain->dev, UVC_SET_CUR, ctrl->entity->id,
> +			     chain->dev->intfnum, ctrl->info.selector,
> +			     buf, sizeof(*buf));
> +	if (!ret)
> +		out_mapping = &uvc_ctrl_power_line_mapping_uvc15;
> +
> +end:
> +	/* Restore initial value and add mapping. */
> +	*buf = init_val;
> +	uvc_query_ctrl(chain->dev, UVC_SET_CUR, ctrl->entity->id,
> +		       chain->dev->intfnum, ctrl->info.selector,
> +		       buf, sizeof(*buf));
>  
> -	return __uvc_ctrl_add_mapping(chain, ctrl,
> -				      &uvc_ctrl_power_line_mapping_uvc15);
> +	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, out_mapping);
>  }
>  
>  static const struct uvc_control_mapping uvc_ctrl_mappings[] = {

-- 
Regards,

Laurent Pinchart

[PATCH v2 4/7] media: uvcvideo: Cleanup version-specific mapping

[Ricardo Ribalda]

We do not have more version specific mappings. Let's remove this code
for now. It can be easily reverted later if needed.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 5b77ac308c84..efc46f53ac81 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -847,14 +847,6 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
 	},
 };
 
-static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc11[] = {
-	NULL, /* Sentinel */
-};
-
-static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc15[] = {
-	NULL, /* Sentinel */
-};
-
 /* ------------------------------------------------------------------------
  * Utility functions
  */
@@ -2656,7 +2648,6 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev,
 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 			       struct uvc_control *ctrl)
 {
-	const struct uvc_control_mapping **mappings;
 	unsigned int i;
 
 	/*
@@ -2721,18 +2712,6 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 		    ctrl->info.selector == mapping->selector)
 			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
 	}
-
-	/* Finally process version-specific mappings. */
-	mappings = chain->dev->uvc_version < 0x0150
-		 ? uvc_ctrl_mappings_uvc11 : uvc_ctrl_mappings_uvc15;
-
-	for (i = 0; mappings[i]; ++i) {
-		const struct uvc_control_mapping *mapping = mappings[i];
-
-		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
-		    ctrl->info.selector == mapping->selector)
-			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
-	}
 }
 
 /*

-- 
2.45.2.505.gda0bf45e8d-goog

[PATCH v2 5/7] media: uvcvideo: Remove PLF device quirking

[Ricardo Ribalda]

We can use heuristics to figure out the proper range of the control
instead of quirking every single device.

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c   |   4 +-
 drivers/media/usb/uvc/uvc_driver.c | 122 -------------------------------------
 drivers/media/usb/uvc/uvcvideo.h   |   2 -
 3 files changed, 2 insertions(+), 126 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index efc46f53ac81..d74019cb27fe 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -459,7 +459,7 @@ static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping,
 	data[first+1] = min_t(int, abs(value), 0xff);
 }
 
-const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
+static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
 	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
 	.entity		= UVC_GUID_UVC_PROCESSING,
 	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
@@ -471,7 +471,7 @@ const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
 				  V4L2_CID_POWER_LINE_FREQUENCY_50HZ),
 };
 
-const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
+static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
 	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
 	.entity		= UVC_GUID_UVC_PROCESSING,
 	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index bbd90123a4e7..5f689fee60a9 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2383,20 +2383,6 @@ MODULE_PARM_DESC(timeout, "Streaming control requests timeout");
  * Driver initialization and cleanup
  */
 
-static const struct uvc_device_info uvc_ctrl_power_line_limited = {
-	.mappings = (const struct uvc_control_mapping *[]) {
-		&uvc_ctrl_power_line_mapping_limited,
-		NULL, /* Sentinel */
-	},
-};
-
-static const struct uvc_device_info uvc_ctrl_power_line_uvc11 = {
-	.mappings = (const struct uvc_control_mapping *[]) {
-		&uvc_ctrl_power_line_mapping_uvc11,
-		NULL, /* Sentinel */
-	},
-};
-
 static const struct uvc_device_info uvc_quirk_probe_minmax = {
 	.quirks = UVC_QUIRK_PROBE_MINMAX,
 };
@@ -2427,33 +2413,6 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
  * though they are compliant.
  */
 static const struct usb_device_id uvc_ids[] = {
-	/* Quanta USB2.0 HD UVC Webcam */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x0408,
-	  .idProduct		= 0x3090,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
-	/* Quanta USB2.0 HD UVC Webcam */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x0408,
-	  .idProduct		= 0x4030,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
-	/* Quanta USB2.0 HD UVC Webcam */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x0408,
-	  .idProduct		= 0x4034,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
 	/* LogiLink Wireless Webcam */
 	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
 				| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -2583,42 +2542,6 @@ static const struct usb_device_id uvc_ids[] = {
 	  .bInterfaceSubClass	= 1,
 	  .bInterfaceProtocol	= 0,
 	  .driver_info		= UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) },
-	/* Chicony EasyCamera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x04f2,
-	  .idProduct		= 0xb5eb,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
-	/* Chicony Electronics Co., Ltd Integrated Camera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x04f2,
-	  .idProduct		= 0xb67c,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_uvc11 },
-	/* Chicony EasyCamera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x04f2,
-	  .idProduct		= 0xb6ba,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
-	/* Chicony EasyCamera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x04f2,
-	  .idProduct		= 0xb746,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
 	/* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
 	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
 				| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -3003,51 +2926,6 @@ static const struct usb_device_id uvc_ids[] = {
 	  .bInterfaceSubClass	= 1,
 	  .bInterfaceProtocol	= 0,
 	  .driver_info		= UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) },
-	/* SunplusIT Inc HD Camera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x2b7e,
-	  .idProduct		= 0xb752,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_uvc11 },
-	/* Lenovo Integrated Camera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x30c9,
-	  .idProduct		= 0x0093,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_uvc11 },
-	/* Sonix Technology USB 2.0 Camera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x3277,
-	  .idProduct		= 0x0072,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
-	/* Acer EasyCamera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x5986,
-	  .idProduct		= 0x1172,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
-	/* Acer EasyCamera */
-	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
-				| USB_DEVICE_ID_MATCH_INT_INFO,
-	  .idVendor		= 0x5986,
-	  .idProduct		= 0x1180,
-	  .bInterfaceClass	= USB_CLASS_VIDEO,
-	  .bInterfaceSubClass	= 1,
-	  .bInterfaceProtocol	= 0,
-	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
 	/* Intel D410/ASR depth camera */
 	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
 				| USB_DEVICE_ID_MATCH_INT_INFO,
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index fa0396dd5b35..4df02a40c74f 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -753,8 +753,6 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags);
 void uvc_status_stop(struct uvc_device *dev);
 
 /* Controls */
-extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited;
-extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11;
 extern const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops;
 
 int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,

-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH v2 5/7] media: uvcvideo: Remove PLF device quirking

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Mon, Jun 10, 2024 at 11:09:56PM +0000, Ricardo Ribalda wrote:
> We can use heuristics to figure out the proper range of the control
> instead of quirking every single device.
> 
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c   |   4 +-
>  drivers/media/usb/uvc/uvc_driver.c | 122 -------------------------------------
>  drivers/media/usb/uvc/uvcvideo.h   |   2 -
>  3 files changed, 2 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index efc46f53ac81..d74019cb27fe 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -459,7 +459,7 @@ static void uvc_ctrl_set_rel_speed(struct uvc_control_mapping *mapping,
>  	data[first+1] = min_t(int, abs(value), 0xff);
>  }
>  
> -const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
> +static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
>  	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
>  	.entity		= UVC_GUID_UVC_PROCESSING,
>  	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> @@ -471,7 +471,7 @@ const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = {
>  				  V4L2_CID_POWER_LINE_FREQUENCY_50HZ),
>  };
>  
> -const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
> +static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = {
>  	.id		= V4L2_CID_POWER_LINE_FREQUENCY,
>  	.entity		= UVC_GUID_UVC_PROCESSING,
>  	.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index bbd90123a4e7..5f689fee60a9 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2383,20 +2383,6 @@ MODULE_PARM_DESC(timeout, "Streaming control requests timeout");
>   * Driver initialization and cleanup
>   */
>  
> -static const struct uvc_device_info uvc_ctrl_power_line_limited = {
> -	.mappings = (const struct uvc_control_mapping *[]) {
> -		&uvc_ctrl_power_line_mapping_limited,
> -		NULL, /* Sentinel */
> -	},
> -};
> -
> -static const struct uvc_device_info uvc_ctrl_power_line_uvc11 = {
> -	.mappings = (const struct uvc_control_mapping *[]) {
> -		&uvc_ctrl_power_line_mapping_uvc11,
> -		NULL, /* Sentinel */
> -	},
> -};
> -
>  static const struct uvc_device_info uvc_quirk_probe_minmax = {
>  	.quirks = UVC_QUIRK_PROBE_MINMAX,
>  };
> @@ -2427,33 +2413,6 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
>   * though they are compliant.
>   */
>  static const struct usb_device_id uvc_ids[] = {
> -	/* Quanta USB2.0 HD UVC Webcam */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x0408,
> -	  .idProduct		= 0x3090,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> -	/* Quanta USB2.0 HD UVC Webcam */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x0408,
> -	  .idProduct		= 0x4030,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> -	/* Quanta USB2.0 HD UVC Webcam */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x0408,
> -	  .idProduct		= 0x4034,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
>  	/* LogiLink Wireless Webcam */
>  	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
>  				| USB_DEVICE_ID_MATCH_INT_INFO,
> @@ -2583,42 +2542,6 @@ static const struct usb_device_id uvc_ids[] = {
>  	  .bInterfaceSubClass	= 1,
>  	  .bInterfaceProtocol	= 0,
>  	  .driver_info		= UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE) },
> -	/* Chicony EasyCamera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x04f2,
> -	  .idProduct		= 0xb5eb,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> -	/* Chicony Electronics Co., Ltd Integrated Camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x04f2,
> -	  .idProduct		= 0xb67c,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_uvc11 },
> -	/* Chicony EasyCamera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x04f2,
> -	  .idProduct		= 0xb6ba,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> -	/* Chicony EasyCamera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x04f2,
> -	  .idProduct		= 0xb746,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
>  	/* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
>  	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
>  				| USB_DEVICE_ID_MATCH_INT_INFO,
> @@ -3003,51 +2926,6 @@ static const struct usb_device_id uvc_ids[] = {
>  	  .bInterfaceSubClass	= 1,
>  	  .bInterfaceProtocol	= 0,
>  	  .driver_info		= UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) },
> -	/* SunplusIT Inc HD Camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x2b7e,
> -	  .idProduct		= 0xb752,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_uvc11 },
> -	/* Lenovo Integrated Camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x30c9,
> -	  .idProduct		= 0x0093,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= UVC_PC_PROTOCOL_15,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_uvc11 },
> -	/* Sonix Technology USB 2.0 Camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x3277,
> -	  .idProduct		= 0x0072,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> -	/* Acer EasyCamera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x5986,
> -	  .idProduct		= 0x1172,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
> -	/* Acer EasyCamera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x5986,
> -	  .idProduct		= 0x1180,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= (kernel_ulong_t)&uvc_ctrl_power_line_limited },
>  	/* Intel D410/ASR depth camera */
>  	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
>  				| USB_DEVICE_ID_MATCH_INT_INFO,
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index fa0396dd5b35..4df02a40c74f 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -753,8 +753,6 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags);
>  void uvc_status_stop(struct uvc_device *dev);
>  
>  /* Controls */
> -extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited;
> -extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11;
>  extern const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops;
>  
>  int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,

-- 
Regards,

Laurent Pinchart

[PATCH v2 6/7] media: uvcvideo: Remove mappings form uvc_device_info

[Ricardo Ribalda]

We do not have any quirk device making us of this. Remove from now. It
can be easily reverted later if needed.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 27 +--------------------------
 drivers/media/usb/uvc/uvcvideo.h |  1 -
 2 files changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index d74019cb27fe..1c1710e3c486 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2679,32 +2679,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 	if (!ctrl->initialized)
 		return;
 
-	/*
-	 * First check if the device provides a custom mapping for this control,
-	 * used to override standard mappings for non-conformant devices. Don't
-	 * process standard mappings if a custom mapping is found. This
-	 * mechanism doesn't support combining standard and custom mappings for
-	 * a single control.
-	 */
-	if (chain->dev->info->mappings) {
-		bool custom = false;
-
-		for (i = 0; chain->dev->info->mappings[i]; ++i) {
-			const struct uvc_control_mapping *mapping =
-				chain->dev->info->mappings[i];
-
-			if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
-			    ctrl->info.selector == mapping->selector) {
-				__uvc_ctrl_add_mapping(chain, ctrl, mapping);
-				custom = true;
-			}
-		}
-
-		if (custom)
-			return;
-	}
-
-	/* Process common mappings next. */
+	/* Process common mappings. */
 	for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); ++i) {
 		const struct uvc_control_mapping *mapping = &uvc_ctrl_mappings[i];
 
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 4df02a40c74f..ff9545dcf716 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -527,7 +527,6 @@ struct uvc_device_info {
 	u32	quirks;
 	u32	meta_format;
 	u16	uvc_version;
-	const struct uvc_control_mapping **mappings;
 };
 
 struct uvc_status_streaming {

-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH v2 6/7] media: uvcvideo: Remove mappings form uvc_device_info

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Mon, Jun 10, 2024 at 11:09:57PM +0000, Ricardo Ribalda wrote:
> We do not have any quirk device making us of this. Remove from now. It
> can be easily reverted later if needed.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 27 +--------------------------
>  drivers/media/usb/uvc/uvcvideo.h |  1 -
>  2 files changed, 1 insertion(+), 27 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index d74019cb27fe..1c1710e3c486 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2679,32 +2679,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  	if (!ctrl->initialized)
>  		return;
>  
> -	/*
> -	 * First check if the device provides a custom mapping for this control,
> -	 * used to override standard mappings for non-conformant devices. Don't
> -	 * process standard mappings if a custom mapping is found. This
> -	 * mechanism doesn't support combining standard and custom mappings for
> -	 * a single control.
> -	 */
> -	if (chain->dev->info->mappings) {
> -		bool custom = false;
> -
> -		for (i = 0; chain->dev->info->mappings[i]; ++i) {
> -			const struct uvc_control_mapping *mapping =
> -				chain->dev->info->mappings[i];
> -
> -			if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> -			    ctrl->info.selector == mapping->selector) {
> -				__uvc_ctrl_add_mapping(chain, ctrl, mapping);
> -				custom = true;
> -			}
> -		}
> -
> -		if (custom)
> -			return;
> -	}
> -
> -	/* Process common mappings next. */
> +	/* Process common mappings. */
>  	for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); ++i) {
>  		const struct uvc_control_mapping *mapping = &uvc_ctrl_mappings[i];
>  
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 4df02a40c74f..ff9545dcf716 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -527,7 +527,6 @@ struct uvc_device_info {
>  	u32	quirks;
>  	u32	meta_format;
>  	u16	uvc_version;
> -	const struct uvc_control_mapping **mappings;
>  };
>  
>  struct uvc_status_streaming {

-- 
Regards,

Laurent Pinchart

[PATCH v2 7/7] media: uvcvideo: Replace get_mapping with filter_mapping

[Ricardo Ribalda]

If the callback returns a mapping instead of adding it, the codeflow is
more clean and we do not need a forward declaration of
__uvc_ctrl_add_mapping_to_list().

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 40 ++++++++++++++++++----------------------
 drivers/media/usb/uvc/uvcvideo.h |  6 +++---
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 1c1710e3c486..4a13f2685d9e 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -495,11 +495,8 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
 				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
 };
 
-static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
-	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping);
-
-static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
-	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
+static const struct uvc_control_mapping *uvc_ctrl_filter_plf_mapping
+		(struct uvc_video_chain *chain, struct uvc_control *ctrl)
 {
 	const struct uvc_control_mapping *out_mapping =
 					&uvc_ctrl_power_line_mapping_uvc11;
@@ -509,7 +506,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
 
 	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
 	if (!buf)
-		return -ENOMEM;
+		return NULL;
 
 	/* Save the default PLF value, so we can restore it. */
 	ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, ctrl->entity->id,
@@ -517,7 +514,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
 			     buf, sizeof(*buf));
 	/* If we cannot read the control skip it. */
 	if (ret)
-		return ret;
+		return NULL;
 	init_val = *buf;
 
 	/* If PLF value cannot be set to off, it is limited. */
@@ -526,8 +523,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
 			     chain->dev->intfnum, ctrl->info.selector,
 			     buf, sizeof(*buf));
 	if (ret)
-		return __uvc_ctrl_add_mapping_to_list(chain, ctrl,
-					&uvc_ctrl_power_line_mapping_limited);
+		return &uvc_ctrl_power_line_mapping_limited;
 
 	/* UVC 1.1 does not define auto, we can exit. */
 	if (chain->dev->uvc_version < 0x150)
@@ -548,7 +544,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
 		       chain->dev->intfnum, ctrl->info.selector,
 		       buf, sizeof(*buf));
 
-	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, out_mapping);
+	return out_mapping;
 }
 
 static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
@@ -843,7 +839,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
 	{
 		.entity		= UVC_GUID_UVC_PROCESSING,
 		.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
-		.add_mapping	= uvc_ctrl_add_plf_mapping,
+		.filter_mapping	= uvc_ctrl_filter_plf_mapping,
 	},
 };
 
@@ -2411,8 +2407,9 @@ 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_to_list(struct uvc_video_chain *chain,
-	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
+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;
 	unsigned int size;
@@ -2485,14 +2482,6 @@ static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
 	return -ENOMEM;
 }
 
-static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
-	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
-{
-	if (mapping && mapping->add_mapping)
-		return mapping->add_mapping(chain, ctrl, mapping);
-	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, mapping);
-}
-
 int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
 	const struct uvc_control_mapping *mapping)
 {
@@ -2681,7 +2670,14 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 
 	/* Process common mappings. */
 	for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); ++i) {
-		const struct uvc_control_mapping *mapping = &uvc_ctrl_mappings[i];
+		const struct uvc_control_mapping *mapping = NULL;
+
+		/* Try to get a custom mapping from the device. */
+		if (uvc_ctrl_mappings[i].filter_mapping)
+			mapping = uvc_ctrl_mappings[i].filter_mapping(chain,
+								      ctrl);
+		if (!mapping)
+			mapping = &uvc_ctrl_mappings[i];
 
 		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
 		    ctrl->info.selector == mapping->selector)
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index ff9545dcf716..a9547795fe22 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -125,9 +125,9 @@ struct uvc_control_mapping {
 	s32 master_manual;
 	u32 slave_ids[2];
 
-	int (*add_mapping)(struct uvc_video_chain *chain,
-			   struct uvc_control *ctrl,
-			   const struct uvc_control_mapping *mapping);
+	const struct uvc_control_mapping *(*filter_mapping)
+				(struct uvc_video_chain *chain,
+				struct uvc_control *ctrl);
 	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
 		   const u8 *data);
 	void (*set)(struct uvc_control_mapping *mapping, s32 value,

-- 
2.45.2.505.gda0bf45e8d-goog

Re: [PATCH v2 7/7] media: uvcvideo: Replace get_mapping with filter_mapping

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Mon, Jun 10, 2024 at 11:09:58PM +0000, Ricardo Ribalda wrote:
> If the callback returns a mapping instead of adding it, the codeflow is
> more clean and we do not need a forward declaration of
> __uvc_ctrl_add_mapping_to_list().
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

This should have been squashed in the previous patches as appropriate.
It's hard to review the new version this way.

The diff with v1 looks good, so I don't expect to have further comments.

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 40 ++++++++++++++++++----------------------
>  drivers/media/usb/uvc/uvcvideo.h |  6 +++---
>  2 files changed, 21 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 1c1710e3c486..4a13f2685d9e 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -495,11 +495,8 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = {
>  				  V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
>  };
>  
> -static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
> -	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping);
> -
> -static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
> -	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
> +static const struct uvc_control_mapping *uvc_ctrl_filter_plf_mapping
> +		(struct uvc_video_chain *chain, struct uvc_control *ctrl)
>  {
>  	const struct uvc_control_mapping *out_mapping =
>  					&uvc_ctrl_power_line_mapping_uvc11;
> @@ -509,7 +506,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
>  
>  	buf = kmalloc(sizeof(*buf), GFP_KERNEL);
>  	if (!buf)
> -		return -ENOMEM;
> +		return NULL;
>  
>  	/* Save the default PLF value, so we can restore it. */
>  	ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, ctrl->entity->id,
> @@ -517,7 +514,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
>  			     buf, sizeof(*buf));
>  	/* If we cannot read the control skip it. */
>  	if (ret)
> -		return ret;
> +		return NULL;
>  	init_val = *buf;
>  
>  	/* If PLF value cannot be set to off, it is limited. */
> @@ -526,8 +523,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
>  			     chain->dev->intfnum, ctrl->info.selector,
>  			     buf, sizeof(*buf));
>  	if (ret)
> -		return __uvc_ctrl_add_mapping_to_list(chain, ctrl,
> -					&uvc_ctrl_power_line_mapping_limited);
> +		return &uvc_ctrl_power_line_mapping_limited;
>  
>  	/* UVC 1.1 does not define auto, we can exit. */
>  	if (chain->dev->uvc_version < 0x150)
> @@ -548,7 +544,7 @@ static int uvc_ctrl_add_plf_mapping(struct uvc_video_chain *chain,
>  		       chain->dev->intfnum, ctrl->info.selector,
>  		       buf, sizeof(*buf));
>  
> -	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, out_mapping);
> +	return out_mapping;
>  }
>  
>  static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> @@ -843,7 +839,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
>  	{
>  		.entity		= UVC_GUID_UVC_PROCESSING,
>  		.selector	= UVC_PU_POWER_LINE_FREQUENCY_CONTROL,
> -		.add_mapping	= uvc_ctrl_add_plf_mapping,
> +		.filter_mapping	= uvc_ctrl_filter_plf_mapping,
>  	},
>  };
>  
> @@ -2411,8 +2407,9 @@ 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_to_list(struct uvc_video_chain *chain,
> -	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
> +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;
>  	unsigned int size;
> @@ -2485,14 +2482,6 @@ static int __uvc_ctrl_add_mapping_to_list(struct uvc_video_chain *chain,
>  	return -ENOMEM;
>  }
>  
> -static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
> -	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
> -{
> -	if (mapping && mapping->add_mapping)
> -		return mapping->add_mapping(chain, ctrl, mapping);
> -	return __uvc_ctrl_add_mapping_to_list(chain, ctrl, mapping);
> -}
> -
>  int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>  	const struct uvc_control_mapping *mapping)
>  {
> @@ -2681,7 +2670,14 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  
>  	/* Process common mappings. */
>  	for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); ++i) {
> -		const struct uvc_control_mapping *mapping = &uvc_ctrl_mappings[i];
> +		const struct uvc_control_mapping *mapping = NULL;
> +
> +		/* Try to get a custom mapping from the device. */
> +		if (uvc_ctrl_mappings[i].filter_mapping)
> +			mapping = uvc_ctrl_mappings[i].filter_mapping(chain,
> +								      ctrl);
> +		if (!mapping)
> +			mapping = &uvc_ctrl_mappings[i];
>  
>  		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
>  		    ctrl->info.selector == mapping->selector)
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index ff9545dcf716..a9547795fe22 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -125,9 +125,9 @@ struct uvc_control_mapping {
>  	s32 master_manual;
>  	u32 slave_ids[2];
>  
> -	int (*add_mapping)(struct uvc_video_chain *chain,
> -			   struct uvc_control *ctrl,
> -			   const struct uvc_control_mapping *mapping);
> +	const struct uvc_control_mapping *(*filter_mapping)
> +				(struct uvc_video_chain *chain,
> +				struct uvc_control *ctrl);
>  	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
>  		   const u8 *data);
>  	void (*set)(struct uvc_control_mapping *mapping, s32 value,
> 

-- 
Regards,

Laurent Pinchart