From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Ricardo Ribalda <ribalda@chromium.org>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 1/9] media: uvcvideo: Move guid to entity
Date: Sun, 20 Dec 2020 18:05:39 +0200 [thread overview]
Message-ID: <X992U/SCVGd41fML@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20201215154439.69062-2-ribalda@chromium.org>
Hi Ricardo,
Thank you for the patch.
On Tue, Dec 15, 2020 at 04:44:31PM +0100, Ricardo Ribalda wrote:
> Instead of having multiple copies of the entity guid on the code, move
> it to the entity structure.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 30 ++++--------------------------
> drivers/media/usb/uvc/uvc_driver.c | 26 ++++++++++++++++++++++++--
> drivers/media/usb/uvc/uvcvideo.h | 2 +-
> 3 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 011e69427b7c..9f6174a10e73 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -826,31 +826,10 @@ static void uvc_set_le_value(struct uvc_control_mapping *mapping,
> * Terminal and unit management
> */
>
> -static const u8 uvc_processing_guid[16] = UVC_GUID_UVC_PROCESSING;
> -static const u8 uvc_camera_guid[16] = UVC_GUID_UVC_CAMERA;
> -static const u8 uvc_media_transport_input_guid[16] =
> - UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT;
> -
> static int uvc_entity_match_guid(const struct uvc_entity *entity,
> - const u8 guid[16])
> + const u8 guid[16])
> {
> - switch (UVC_ENTITY_TYPE(entity)) {
> - case UVC_ITT_CAMERA:
> - return memcmp(uvc_camera_guid, guid, 16) == 0;
> -
> - case UVC_ITT_MEDIA_TRANSPORT_INPUT:
> - return memcmp(uvc_media_transport_input_guid, guid, 16) == 0;
> -
> - case UVC_VC_PROCESSING_UNIT:
> - return memcmp(uvc_processing_guid, guid, 16) == 0;
> -
> - case UVC_VC_EXTENSION_UNIT:
> - return memcmp(entity->extension.guidExtensionCode,
> - guid, 16) == 0;
> -
> - default:
> - return 0;
> - }
> + return memcmp(entity->guid, guid, sizeof(entity->guid)) == 0;
> }
>
> /* ------------------------------------------------------------------------
> @@ -1776,8 +1755,7 @@ static int uvc_ctrl_fill_xu_info(struct uvc_device *dev,
> if (data == NULL)
> return -ENOMEM;
>
> - memcpy(info->entity, ctrl->entity->extension.guidExtensionCode,
> - sizeof(info->entity));
> + memcpy(info->entity, ctrl->entity->guid, sizeof(info->entity));
> info->index = ctrl->index;
> info->selector = ctrl->index + 1;
>
> @@ -1883,7 +1861,7 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>
> if (!found) {
> uvc_trace(UVC_TRACE_CONTROL, "Control %pUl/%u not found.\n",
> - entity->extension.guidExtensionCode, xqry->selector);
> + entity->guid, xqry->selector);
> return -ENOENT;
> }
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index ddb9eaa11be7..4cdd65d252d9 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1019,6 +1019,11 @@ static int uvc_parse_streaming(struct uvc_device *dev,
> return ret;
> }
>
> +static const u8 uvc_camera_guid[16] = UVC_GUID_UVC_CAMERA;
> +static const u8 uvc_media_transport_input_guid[16] =
> + UVC_GUID_UVC_MEDIA_TRANSPORT_INPUT;
> +static const u8 uvc_processing_guid[16] = UVC_GUID_UVC_PROCESSING;
> +
> static struct uvc_entity *uvc_alloc_entity(u16 type, u8 id,
> unsigned int num_pads, unsigned int extra_size)
> {
> @@ -1038,6 +1043,23 @@ static struct uvc_entity *uvc_alloc_entity(u16 type, u8 id,
> entity->id = id;
> entity->type = type;
>
> +
Nearly there, just one blank line to remove :-) I'll fix this when
applying.
> + /*
> + * Set the GUID for standard entity types. For extension units, the GUID
> + * is initialized by the caller.
> + */
> + switch (type) {
> + case UVC_ITT_CAMERA:
> + memcpy(entity->guid, uvc_camera_guid, 16);
> + break;
> + case UVC_ITT_MEDIA_TRANSPORT_INPUT:
> + memcpy(entity->guid, uvc_media_transport_input_guid, 16);
> + break;
> + case UVC_VC_PROCESSING_UNIT:
> + memcpy(entity->guid, uvc_processing_guid, 16);
> + break;
> + }
> +
> entity->num_links = 0;
> entity->num_pads = num_pads;
> entity->pads = ((void *)(entity + 1)) + extra_size;
> @@ -1109,7 +1131,7 @@ static int uvc_parse_vendor_control(struct uvc_device *dev,
> if (unit == NULL)
> return -ENOMEM;
>
> - memcpy(unit->extension.guidExtensionCode, &buffer[4], 16);
> + memcpy(unit->guid, &buffer[4], 16);
> unit->extension.bNumControls = buffer[20];
> memcpy(unit->baSourceID, &buffer[22], p);
> unit->extension.bControlSize = buffer[22+p];
> @@ -1368,7 +1390,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev,
> if (unit == NULL)
> return -ENOMEM;
>
> - memcpy(unit->extension.guidExtensionCode, &buffer[4], 16);
> + memcpy(unit->guid, &buffer[4], 16);
> unit->extension.bNumControls = buffer[20];
> memcpy(unit->baSourceID, &buffer[22], p);
> unit->extension.bControlSize = buffer[22+p];
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index a3dfacf069c4..df7bf2d104a3 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -304,6 +304,7 @@ struct uvc_entity {
> u8 id;
> u16 type;
> char name[64];
> + u8 guid[16];
>
> /* Media controller-related fields. */
> struct video_device *vdev;
> @@ -342,7 +343,6 @@ struct uvc_entity {
> } selector;
>
> struct {
> - u8 guidExtensionCode[16];
> u8 bNumControls;
> u8 bControlSize;
> u8 *bmControls;
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2020-12-20 16:06 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-15 15:44 [PATCH v4 0/9] Show privacy_gpio as a v4l2_ctrl Ricardo Ribalda
2020-12-15 15:44 ` [PATCH v4 1/9] media: uvcvideo: Move guid to entity Ricardo Ribalda
2020-12-20 16:05 ` Laurent Pinchart [this message]
2020-12-15 15:44 ` [PATCH v4 2/9] media: uvcvideo: Allow external entities Ricardo Ribalda
2020-12-20 16:53 ` Laurent Pinchart
2020-12-15 15:44 ` [PATCH v4 3/9] media: uvcvideo: Allow entities with no pads Ricardo Ribalda
2020-12-20 16:08 ` Laurent Pinchart
2020-12-15 15:44 ` [PATCH v4 4/9] media: uvcvideo: Entity defined get_info and get_cur Ricardo Ribalda
2020-12-20 16:15 ` Laurent Pinchart
2020-12-15 15:44 ` [PATCH v4 5/9] media: uvcvideo: Implement UVC_EXT_GPIO_UNIT Ricardo Ribalda
2020-12-20 16:49 ` Laurent Pinchart
2020-12-20 22:56 ` Ricardo Ribalda
2020-12-20 23:24 ` Laurent Pinchart
2020-12-21 0:30 ` Ricardo Ribalda
2020-12-15 15:44 ` [PATCH v4 6/9] media: uvcvideo: Add Privacy control based on EXT_GPIO Ricardo Ribalda
2020-12-20 16:55 ` Laurent Pinchart
2020-12-15 15:44 ` [PATCH v4 7/9] media: uvcvideo: Use dev_ printk aliases Ricardo Ribalda
2020-12-20 17:11 ` Laurent Pinchart
2020-12-15 15:44 ` [PATCH v4 8/9] media: uvcvideo: New macro uvc_trace_cont Ricardo Ribalda
2020-12-20 17:13 ` Laurent Pinchart
2020-12-15 15:44 ` [PATCH v4 9/9] media: uvcvideo: Implement UVC_QUIRK_PRIVACY_DURING_STREAM Ricardo Ribalda
2020-12-20 17:21 ` Laurent Pinchart
2020-12-21 1:10 ` Ricardo Ribalda
2020-12-21 2:08 ` Laurent Pinchart
2020-12-21 2:12 ` Ricardo Ribalda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=X992U/SCVGd41fML@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=ribalda@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).