Re: [PATCH] media: uvcvideo: Set media controller entity functions

[Kieran Bingham <kieran.bingham@ideasonboard.com>]

Hi Laurent,

On 28/07/2020 01:13, Laurent Pinchart wrote:
> Kieran, would you have time to give this a review ?
> 
> On Sun, Jun 07, 2020 at 05:20:47AM +0300, Laurent Pinchart wrote:
>> The media controller core prints a warning when an entity is registered
>> without a function being set. This affect the uvcvideo driver, as the

s/affect/affects/

>> warning was added without first addressing the issue in existing
>> drivers. The problem is harmless, but unnecessarily worries users. Fix
>> it by mapping UVC entity types to MC entity functions as accurately as
>> possible using the existing functions.
>>
>> Fixes: b50bde4e476d ("[media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs")
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Looks fine to me.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>> ---
>>  drivers/media/usb/uvc/uvc_entity.c | 35 ++++++++++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c
>> index b4499cddeffe..ca3a9c2eec27 100644
>> --- a/drivers/media/usb/uvc/uvc_entity.c
>> +++ b/drivers/media/usb/uvc/uvc_entity.c
>> @@ -73,10 +73,45 @@ static int uvc_mc_init_entity(struct uvc_video_chain *chain,
>>  	int ret;
>>  
>>  	if (UVC_ENTITY_TYPE(entity) != UVC_TT_STREAMING) {
>> +		u32 function;
>> +
>>  		v4l2_subdev_init(&entity->subdev, &uvc_subdev_ops);
>>  		strscpy(entity->subdev.name, entity->name,
>>  			sizeof(entity->subdev.name));
>>  
>> +		switch (UVC_ENTITY_TYPE(entity)) {
>> +		case UVC_VC_SELECTOR_UNIT:
>> +			function = MEDIA_ENT_F_VID_MUX;
>> +			break;
>> +		case UVC_VC_PROCESSING_UNIT:
>> +		case UVC_VC_EXTENSION_UNIT:
>> +			/* For lack of a better option. */
>> +			function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
>> +			break;
>> +		case UVC_COMPOSITE_CONNECTOR:
>> +		case UVC_COMPONENT_CONNECTOR:
>> +			function = MEDIA_ENT_F_CONN_COMPOSITE;
>> +			break;
>> +		case UVC_SVIDEO_CONNECTOR:
>> +			function = MEDIA_ENT_F_CONN_SVIDEO;
>> +			break;
>> +		case UVC_ITT_CAMERA:
>> +			function = MEDIA_ENT_F_CAM_SENSOR;
>> +			break;
>> +		case UVC_TT_VENDOR_SPECIFIC:
>> +		case UVC_ITT_VENDOR_SPECIFIC:
>> +		case UVC_ITT_MEDIA_TRANSPORT_INPUT:
>> +		case UVC_OTT_VENDOR_SPECIFIC:
>> +		case UVC_OTT_DISPLAY:
>> +		case UVC_OTT_MEDIA_TRANSPORT_OUTPUT:
>> +		case UVC_EXTERNAL_VENDOR_SPECIFIC:
>> +		default:
>> +			function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN;
>> +			break;
>> +		}
>> +
>> +		entity->subdev.entity.function = function;
>> +
>>  		ret = media_entity_pads_init(&entity->subdev.entity,
>>  					entity->num_pads, entity->pads);
>>  
> 

-- 
Regards
--
Kieran