Re: [PATCH v8 37/55] [media] omap4iss: stop MEDIA_ENT_T_V4L2_SUBDEV abuse

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:48 Mauro Carvalho Chehab wrote:
> This driver is abusing MEDIA_ENT_T_V4L2_SUBDEV, as it uses a
> hack to check if the remote entity is a subdev. Get rid of it.

While I agree with the idea of the patch I don't think this is a hack, it was 
a totally valid implementation with the existing API.

> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/staging/media/omap4iss/iss_ipipe.c
> b/drivers/staging/media/omap4iss/iss_ipipe.c index
> e1a7b7ba7362..eb91ec48a21e 100644
> --- a/drivers/staging/media/omap4iss/iss_ipipe.c
> +++ b/drivers/staging/media/omap4iss/iss_ipipe.c
> @@ -447,8 +447,11 @@ static int ipipe_link_setup(struct media_entity
> *entity, struct iss_ipipe_device *ipipe = v4l2_get_subdevdata(sd);
>  	struct iss_device *iss = to_iss_device(ipipe);
> 
> -	switch (local->index | media_entity_type(remote->entity)) {
> -	case IPIPE_PAD_SINK | MEDIA_ENT_T_V4L2_SUBDEV:
> +	if (!is_media_entity_v4l2_subdev(remote->entity))
> +		return -EINVAL;
> +

Furthermore the ipipe entity is never connected to anything else than a 
subdev, so you can remove this check completely.

I'd rewrite the subject line as "omap4iss: ipipe: Don't check entity type 
needlessly during link setup" and update the commit message accordingly.

> +	switch (local->index) {
> +	case IPIPE_PAD_SINK:
>  		/* Read from IPIPEIF. */
>  		if (!(flags & MEDIA_LNK_FL_ENABLED)) {
>  			ipipe->input = IPIPE_INPUT_NONE;
> @@ -463,7 +466,7 @@ static int ipipe_link_setup(struct media_entity *entity,
> 
>  		break;
> 
> -	case IPIPE_PAD_SOURCE_VP | MEDIA_ENT_T_V4L2_SUBDEV:
> +	case IPIPE_PAD_SOURCE_VP:
>  		/* Send to RESIZER */
>  		if (flags & MEDIA_LNK_FL_ENABLED) {
>  			if (ipipe->output & ~IPIPE_OUTPUT_VP)

-- 
Regards,

Laurent Pinchart