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;
Subject: Re: [PATCH v2 6/6] media: uvcvideo: Set a different name for the
 metadata entity
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Ricardo Ribalda <ribalda@chromium.org>
Cc:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        Tomasz Figa <tfiga@chromium.org>, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org, senozhatsky@chromium.org
References: <20210311221946.1319924-1-ribalda@chromium.org>
 <20210311221946.1319924-7-ribalda@chromium.org>
 <YEqp/EkIUwOQhAN5@pendragon.ideasonboard.com>
From:   Hans Verkuil <hverkuil@xs4all.nl>
Message-ID: <96245ed2-dd6f-ddc3-aa02-a9924f76bc76@xs4all.nl>
Date:   Fri, 12 Mar 2021 08:17:28 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <YEqp/EkIUwOQhAN5@pendragon.ideasonboard.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/03/2021 00:38, Laurent Pinchart wrote:
> Hi Ricardo,
> 
> Thank you for the patch.
> 
> On Thu, Mar 11, 2021 at 11:19:46PM +0100, Ricardo Ribalda wrote:
>> All the entities must have a unique name.
>>
>> Fixes v4l2-compliance:
>> Media Controller ioctls:
>>                 fail: v4l2-test-media.cpp(205): v2_entity_names_set.find(key) != v2_entity_names_set.end()
>>         test MEDIA_IOC_G_TOPOLOGY: FAIL
>>                 fail: v4l2-test-media.cpp(394): num_data_links != num_links
>> 	test MEDIA_IOC_ENUM_ENTITIES/LINKS: FAIL
>>
>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>> ---
>>  drivers/media/usb/uvc/uvc_driver.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
>> index 30ef2a3110f7..47efa9a9be99 100644
>> --- a/drivers/media/usb/uvc/uvc_driver.c
>> +++ b/drivers/media/usb/uvc/uvc_driver.c
>> @@ -2199,7 +2199,10 @@ int uvc_register_video_device(struct uvc_device *dev,
>>  		break;
>>  	}
>>  
>> -	strscpy(vdev->name, dev->name, sizeof(vdev->name));
>> +	if (type == V4L2_BUF_TYPE_META_CAPTURE)
>> +		strscpy(vdev->name, "Metadata Videodev", sizeof(vdev->name));
>> +	else
>> +		strscpy(vdev->name, dev->name, sizeof(vdev->name));
> 
> A UVC device could contain multiple output terminals (either in the same
> chain or in different chains), which would still result in multiple
> entities having the same name. Could this be fixed at the same time ?
> You can use the unit ID of the output terminal to create unique names
> (and it would be nice if the video and metadata nodes has similar names,
> with "video" and "metadata" being the only difference between them).

I agree with Laurent. How about using something like this for the videodevs:

	snprintf(vdev->name, sizeof(vdev->name), "Meta %s", dev->name);

and:

	snprintf(vdev->name, sizeof(vdev->name), "Video %s", dev->name);

Regards,

	Hans

> 
>>  
>>  	/*
>>  	 * Set the driver data before calling video_register_device, otherwise
>