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@xxxxxxxxxxxxxxx> > > 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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel