diff mbox

[v7,15/44,media] media: get rid of an unused code

Message ID 5ccb3df9166af331070f546a7d3c522d65964919.1440359643.git.mchehab@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mauro Carvalho Chehab Aug. 23, 2015, 8:17 p.m. UTC 
This code is not used in practice. Get rid of it before
start converting links to lists.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Comments

Hans Verkuil Aug. 25, 2015, 7:10 a.m. UTC | #1 
On 08/23/2015 10:17 PM, Mauro Carvalho Chehab wrote:
> This code is not used in practice. Get rid of it before
> start converting links to lists.

I assume the reason is that links are always created *after*
entities are registered?

Can you add that to this commit log?

With that change:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 138b18416460..0d85c6c28004 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -443,13 +443,6 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  	media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
>  	list_add_tail(&entity->list, &mdev->entities);
>  
> -	/*
> -	 * Initialize objects at the links
> -	 * in the case where links got created before entity register
> -	 */
> -	for (i = 0; i < entity->num_links; i++)
> -		media_gobj_init(mdev, MEDIA_GRAPH_LINK,
> -				&entity->links[i].graph_obj);
>  	/* Initialize objects at the pads */
>  	for (i = 0; i < entity->num_pads; i++)
>  		media_gobj_init(mdev, MEDIA_GRAPH_PAD,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab Aug. 25, 2015, 10:10 a.m. UTC | #2 
Em Tue, 25 Aug 2015 09:10:57 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 08/23/2015 10:17 PM, Mauro Carvalho Chehab wrote:
> > This code is not used in practice. Get rid of it before
> > start converting links to lists.
> 
> I assume the reason is that links are always created *after*
> entities are registered?

That was the assumption. However, Javier found some cases where drivers
are creating links before. 

So, we should either drop this patch and add some additional logic
on the next one to handle late graph object init or to fix the
drivers before.

I'll work on the delayed graph object init, as it sounds the
easiest way, but let's see how such change will actually work.

> 
> Can you add that to this commit log?
> 
> With that change:
> 
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index 138b18416460..0d85c6c28004 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -443,13 +443,6 @@ int __must_check media_device_register_entity(struct media_device *mdev,
> >  	media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
> >  	list_add_tail(&entity->list, &mdev->entities);
> >  
> > -	/*
> > -	 * Initialize objects at the links
> > -	 * in the case where links got created before entity register
> > -	 */
> > -	for (i = 0; i < entity->num_links; i++)
> > -		media_gobj_init(mdev, MEDIA_GRAPH_LINK,
> > -				&entity->links[i].graph_obj);
> >  	/* Initialize objects at the pads */
> >  	for (i = 0; i < entity->num_pads; i++)
> >  		media_gobj_init(mdev, MEDIA_GRAPH_PAD,
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shuah Khan Aug. 25, 2015, 10:32 p.m. UTC | #3 
On Tue, Aug 25, 2015 at 4:10 AM, Mauro Carvalho Chehab
<mchehab@osg.samsung.com> wrote:
> Em Tue, 25 Aug 2015 09:10:57 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
>> On 08/23/2015 10:17 PM, Mauro Carvalho Chehab wrote:
>> > This code is not used in practice. Get rid of it before
>> > start converting links to lists.
>>
>> I assume the reason is that links are always created *after*
>> entities are registered?
>
> That was the assumption. However, Javier found some cases where drivers
> are creating links before.
>
> So, we should either drop this patch and add some additional logic
> on the next one to handle late graph object init or to fix the
> drivers before.
>
> I'll work on the delayed graph object init, as it sounds the
> easiest way, but let's see how such change will actually work.
>

I think we should drop this patch for now. I also would like to see
this new code
in action on a driver that has DVB and V4L modules and creates entities during
probe and maybe even links during probe with no specific probe ordering between
individual module probes. This way we are sure we need this code and know that
it is correct.

thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mauro Carvalho Chehab Aug. 26, 2015, 3:17 p.m. UTC | #4 
Em Tue, 25 Aug 2015 16:32:19 -0600
Shuah Khan <shuahkhan@gmail.com> escreveu:

> On Tue, Aug 25, 2015 at 4:10 AM, Mauro Carvalho Chehab
> <mchehab@osg.samsung.com> wrote:
> > Em Tue, 25 Aug 2015 09:10:57 +0200
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >
> >> On 08/23/2015 10:17 PM, Mauro Carvalho Chehab wrote:
> >> > This code is not used in practice. Get rid of it before
> >> > start converting links to lists.
> >>
> >> I assume the reason is that links are always created *after*
> >> entities are registered?
> >
> > That was the assumption. However, Javier found some cases where drivers
> > are creating links before.
> >
> > So, we should either drop this patch and add some additional logic
> > on the next one to handle late graph object init or to fix the
> > drivers before.
> >
> > I'll work on the delayed graph object init, as it sounds the
> > easiest way, but let's see how such change will actually work.
> >
> 
> I think we should drop this patch for now.

We can't, as otherwise it will break compilation on patch 16/44.

What we need to do is to ensure that all drivers will be doing the
right thing before this one.

> I also would like to see
> this new code
> in action on a driver that has DVB and V4L modules and creates entities during
> probe and maybe even links during probe with no specific probe ordering between
> individual module probes. This way we are sure we need this code and know that
> it is correct.

See media-ctl --print-t output:
	http://pastebin.com/ckxafiJB

Please notice that the media-ctl version I'm using doesn't know the DVB
entity types yet, as it needs to be patched to be aware of the new API.
So, it reports the DVB stuff as "unknow entities".

And that's the output of the new tool that uses the new API:
	http://pastebin.com/fRhMcTue

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 138b18416460..0d85c6c28004 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -443,13 +443,6 @@  int __must_check media_device_register_entity(struct media_device *mdev,
 	media_gobj_init(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
 	list_add_tail(&entity->list, &mdev->entities);
 
-	/*
-	 * Initialize objects at the links
-	 * in the case where links got created before entity register
-	 */
-	for (i = 0; i < entity->num_links; i++)
-		media_gobj_init(mdev, MEDIA_GRAPH_LINK,
-				&entity->links[i].graph_obj);
 	/* Initialize objects at the pads */
 	for (i = 0; i < entity->num_pads; i++)
 		media_gobj_init(mdev, MEDIA_GRAPH_PAD,