Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 20 Dec 2020 18:53:04 +0200
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 2/9] media: uvcvideo: Allow external entities
Message-ID: <X9+BcPNyFs0oWxQN@pendragon.ideasonboard.com>
References: <20201215154439.69062-1-ribalda@chromium.org>
 <20201215154439.69062-3-ribalda@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20201215154439.69062-3-ribalda@chromium.org>
Precedence: bulk

Hi Ricardo,

Thank you for the patch.

On Tue, Dec 15, 2020 at 04:44:32PM +0100, Ricardo Ribalda wrote:
> Increase the size of the id, to avoid collisions with external entities.

Could you expand the commit message a bit to explain what external
entities are ?

> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 2 +-
>  drivers/media/usb/uvc/uvcvideo.h   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 4cdd65d252d9..9f4451a2e0a6 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1024,7 +1024,7 @@ 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,
> +static struct uvc_entity *uvc_alloc_entity(u16 type, u16 id,
>  		unsigned int num_pads, unsigned int extra_size)
>  {
>  	struct uvc_entity *entity;
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index df7bf2d104a3..00f985001c1d 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -301,7 +301,7 @@ struct uvc_entity {
>  					 * chain. */
>  	unsigned int flags;
>  
> -	u8 id;
> +	u16 id; /* 0-255: usb entity. 256-65535: external entities */

How about

	/*
	 * Entities exposed by the UVC device use IDs 0-255, extra entities
	 * implemented by the driver (such as the GPIO entity) use IDs 256 and
	 * up.
	 */

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

>  	u16 type;
>  	char name[64];
>  	u8 guid[16];

-- 
Regards,

Laurent Pinchart