From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from lb1-smtp-cloud6.xs4all.net ([194.109.24.24]:55872 "EHLO lb1-smtp-cloud6.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752755AbbHaLCZ (ORCPT ); Mon, 31 Aug 2015 07:02:25 -0400 Message-ID: <55E43407.6030400@xs4all.nl> Date: Mon, 31 Aug 2015 13:01:27 +0200 From: Hans Verkuil MIME-Version: 1.0 To: Mauro Carvalho Chehab CC: Linux Media Mailing List , Mauro Carvalho Chehab , Javier Martinez Canillas Subject: Re: [PATCH v8 16/55] [media] media: Don't accept early-created links References: <31329e1be748d26ce5a90fe050ba15b8d1e5aff1.1440902901.git.mchehab@osg.samsung.com> <55E42CB8.6010706@xs4all.nl> <20150831075444.5bc98366@recife.lan> In-Reply-To: <20150831075444.5bc98366@recife.lan> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: On 08/31/2015 12:54 PM, Mauro Carvalho Chehab wrote: > Em Mon, 31 Aug 2015 12:30:16 +0200 > Hans Verkuil 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 >> >> The code is OK, so: >> >> Acked-by: Hans Verkuil >> >> 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >