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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Sun, 06 Dec 2015 03:46:28 +0200
Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> escreveu:

> 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.


The same rationale as patch 36/55: if one would later add some other
subsystem to the pipeline, the check will be needed. So, better to have
the check here.

I'm changing the description of this patch to:

[media] omap4iss: change the logic that checks if an entity is a subdev

As we're getting rid of an specific number range for the V4L2 subdev,
we need to replace the check for MEDIA_ENT_T_V4L2_SUBDEV by a macro.

Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>

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



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux