On 08/31/2015 12:54 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 31 Aug 2015 12:30:16 +0200
> Hans Verkuil <hverk...@xs4all.nl> escreveu:
> 
>> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
>>> Links are graph objects that represent the links of two already
>>> existing objects in the graph.
>>>
>>> While with the current implementation, it is possible to create
>>> the links earlier, It doesn't make any sense to allow linking
>>> two objects when they are not both created.
>>>
>>> So, remove the code that would be handling those early-created
>>> links and add a BUG_ON() to ensure that.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
>>
>> The code is OK, so:
>>
>> Acked-by: Hans Verkuil <hans.verk...@cisco.com>
>>
>> But shouldn't this go *after* the omap3isp fixes? After this patch the
>> omap3isp will call BUG_ON, and that's not what you want.
> 
> Yes. I'll change the order on my git tree.
> 
>> It is also not clear if the omap3isp driver is the only one that has this
>> 'create link before objects' problem. I would expect that the omap4 staging
>> driver has the same issue and possibly others as well.
>>
>> Did someone look at that?
> 
> I guess other drivers are doing the same.
> 
> Javier's planning to review the other platform drivers in order to add the 
> needed fixes there too, and to do more tests with some other platform
> drivers that he has hardware for testing.

OK, good. Just wanted to know that.

Perhaps it is a good idea to add a TODO list to the cover letter. This would
be one item on that list.

Regards,

        Hans

> 
>>
>> Regards,
>>
>>      Hans
>>
>>>
>>> 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,
>>> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
>>> index 01946baa32d5..9f8e0145db7a 100644
>>> --- a/drivers/media/media-entity.c
>>> +++ b/drivers/media/media-entity.c
>>> @@ -161,6 +161,8 @@ void media_gobj_init(struct media_device *mdev,
>>>                        enum media_gobj_type type,
>>>                        struct media_gobj *gobj)
>>>  {
>>> +   BUG_ON(!mdev);
>>> +
>>>     gobj->mdev = mdev;
>>>  
>>>     /* Create a per-type unique object ID */
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

Reply via email to