On Mon, Jun 17, 2024 at 09:43:50AM +0200, Ricardo Ribalda wrote:
> Hi Laurent
>
> Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
>
> On Mon, 17 Jun 2024 at 01:14, Laurent Pinchart wrote:
> >
> > Hello,
> >
> > This patch series is a new version of Ricardo's v2 that incorporate my
> > review feedback and squash v2 7/7 into the appropriate commits. I've
> > decided to send it as a new version to speed up merging.
> >
> > As part of the squashing, patch 1/6 now implements a slightly different
> > filtering logic by ignoring mappings whose .filter_mapping() function
> > returns NULL. Apart from that, the series should be functionally
> > equivalento to v2.
> >
> > The patches have been rebased on my UVC -next branch. The base commit
> > can be found in
> > git://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git. If
> > this version is acceptable, I will add it to the branch and send a pull
> > request in the next few days.
>
> For reference this is the diff with v2
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index eb930825c354..79c6dacd516e 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -495,8 +495,8 @@ static const struct uvc_control_mapping
> uvc_ctrl_power_line_mapping_uvc15 = {
> V4L2_CID_POWER_LINE_FREQUENCY_DISABLED),
> };
>
> -static const struct uvc_control_mapping *uvc_ctrl_filter_plf_mapping
> - (struct uvc_video_chain *chain, struct uvc_control *ctrl)
> +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;
> @@ -2408,8 +2408,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,
> - struct uvc_control *ctrl,
> - const struct uvc_control_mapping *mapping)
> + struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
>
> Just curious, do you have a nice vim code formatter that you can
> share? Or is it just "what looks nicer"?
It's the latter I'm afraid, and I'm sure it has changed over time. I
would nowadays use the '-' style, not the '+' style here. The diff shows
a change, but that's only because splitting patch 7/7 and squashing it
in previous patches removed the need to change the
__uvc_ctrl_add_mapping() function, so I ended up dropping the style
change.
> {
> struct uvc_control_mapping *map;
> unsigned int size;
> @@ -2670,14 +2669,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 = 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];
> + const struct uvc_control_mapping *mapping = &uvc_ctrl_mappings[i];
> +
> + /* Let the device provide a custom mapping. */
> + if (mapping->filter_mapping) {
> + mapping = mapping->filter_mapping(chain, ctrl);
> + if (!mapping)
> + continue;
> + }
>
> I guess that if the device is too broken to fail filter_mapping we can
> skip that control.
And I think we *should* skip it in that case, as exposing the control
would lead to trouble.
> > Ricardo Ribalda (6):
> > 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
> >
> > drivers/media/usb/uvc/uvc_ctrl.c | 184 ++++++++++++++++-------------
> > drivers/media/usb/uvc/uvc_driver.c | 131 --------------------
> > drivers/media/usb/uvc/uvcvideo.h | 8 +-
> > 3 files changed, 105 insertions(+), 218 deletions(-)
> >
> >
> > base-commit: 75007ad7544c3a4da6b670983fb41cc4cbe8e9b1
--
Regards,
Laurent Pinchart
