[1/1] v4l: subdev: Register the entity before calling subdev's registered op
diff mbox

Message ID 1451259900-9295-1-git-send-email-sakari.ailus@iki.fi
State New
Headers show

Commit Message

Sakari Ailus Dec. 27, 2015, 11:45 p.m. UTC
Registering a V4L2 sub-device includes, among other things, registering
the related media entity and calling the sub-device's registered op. Since
patch "media: convert links from array to list", creating a link between
two pads requires registering the entity first. If the registered() op
involves link creation, the link list head will not be initialised before
it is used.

Resolve this by first registering the entity, then calling its
registered() op.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Hi Mauro,

You seem to be missing this patch from your media-controller-rc4 branch.
Not having it breaks at least the smiapp driver. I was pretty sure to have
sent it but I can't find it on linux-media. Oh well.

Speaking of the entity function warnings --- I'd omit them until we've
agreed that this is what we should really have (I don't think so). I can
submit a patch to remove them if you like.

----------8<------------
[  108.919189] omap3isp 480bc000.isp: Entity type for entity jt8ew9 binner 1-0010 was not initialized!
[  108.929046] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[  108.937652] pgd = ed0b8000
[  108.940521] [00000000] *pgd=aefc3831, *pte=00000000, *ppte=00000000
[  108.947204] Internal error: Oops: 817 [#1] ARM
[  108.951904] Modules linked in: smiapp(+) smiapp_pll omap3_isp videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_common videodev media
[  108.966735] CPU: 0 PID: 1163 Comm: modprobe Not tainted 4.4.0-rc2-00328-g40e950d #507
[  108.975006] Hardware name: Generic OMAP36xx (Flattened Device Tree)
[  108.981597] task: eeb91340 ti: eec6e000 task.ti: eec6e000
[  108.987335] PC is at media_add_link+0x34/0x44 [media]
[  108.992645] LR is at 0x0
[  108.995330] pc : [<bf001850>]    lr : [<00000000>]    psr: a0000013
[  108.995330] sp : eec6fc78  ip : 00000000  fp : 00000000
[  109.007415] r10: 00000000  r9 : 00000003  r8 : eeee1c40
[  109.012939] r7 : 00000000  r6 : 0000001c  r5 : ee9a7248  r4 : ee9a70c4
[  109.019805] r3 : 00000000  r2 : eeee1c10  r1 : ee8000c0  r0 : eeee1c00
[  109.026672] Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[  109.034210] Control: 10c5387d  Table: ad0b8019  DAC: 00000051
[  109.040252] Process modprobe (pid: 1163, stack limit = 0xeec6e208)
[  109.046752] Stack: (0xeec6fc78 to 0xeec70000)
[  109.051361] fc60:                                                       ee9a7098 bf0021b4
[  109.059997] fc80: 00000000 ee9a7010 eec6fcbc eea1fc00 ee9a7248 00000000 eec6fcc0 ee9a7098
[  109.068634] fca0: 00000136 bf09c520 00000003 ee9a7140 eeb91340 ee9a7098 ee9a7248 ee9a73f8
[  109.077270] fcc0: ee9a7010 ee9a7098 eefd0010 ed08f620 00000000 bf024d8c 0000035a 1dc13000
[  109.085906] fce0: 00000000 bf014488 eefd0084 ee9a7098 ed08f620 00000000 bf024d8c bf01d384
[  109.094543] fd00: ee9a7140 eefd0090 ed08f620 bf024d48 ee9a7098 eefd0084 ee9a7140 bf01d444
[  109.103179] fd20: eeee6650 eea1fc00 ee9a7010 eeee66c0 00000003 bf09c3f8 1dc13000 00000000
[  109.111785] fd40: 00000002 c02bab44 ef7e2994 00000000 eea1fc00 bf09c13c bf09eba4 eea1fc04
[  109.120422] fd60: 00000000 eea1fc20 eec6ff0c c028b7dc eea1fc20 bf09ebc0 00000000 c0dce564
[  109.129058] fd80: 00000002 00000000 00000000 c02112dc bf09ebc0 eea1fc20 00000000 00000002
[  109.137695] fda0: eea1fc20 eea1fc54 bf09ebc0 00000000 c05908e0 c02115c8 bf09ebc0 eec6fdc8
[  109.146331] fdc0: c0211560 c020f8ac ee92c6a4 eea78410 bf09ebc0 bf09ebc0 ee9ec240 c05d1e74
[  109.154968] fde0: c05908e0 c0210110 bf09d8ec eec6fd90 bf09ebc0 bf0a3000 00000000 c0211cc4
[  109.163604] fe00: bf09eba4 bf0a3000 00000000 c028bd94 eeee6780 bf0a3000 00000000 c0009784
[  109.172241] fe20: efdd6ca0 efdd6cc0 00000000 00000001 00000000 c009709c eeb91340 efdd6ca0
[  109.180877] fe40: 00000000 c0063428 00000000 00000000 eeb91340 00000001 c00c4f1c eedb13c0
[  109.189514] fe60: 00000018 c005fdc8 024000c0 ee8000c0 60000013 bf09f340 eec6ff48 eedb13c0
[  109.198150] fe80: 00000001 00000018 00000000 00000000 eec6ff0c c008ac94 bf09f340 00000000
[  109.206756] fea0: efd99ff4 bf09f340 eec6ff48 bf09f34c 00000001 c008c634 ffff8000 00007fff
[  109.215393] fec0: 00000000 c008a8f4 f163a300 f163a448 000007d0 00000186 0001cfc0 f16b9e80
[  109.224029] fee0: 00000000 00000000 00000000 00000000 00000000 00000000 bf09f344 eec6ff1c
[  109.232666] ff00: 00001c02 c019cf24 20000013 c050e0b8 00000055 00000001 00010000 b6dfdea8
[  109.241302] ff20: 00006ea8 0001cfc0 00000000 f16b9ea8 eec6e000 00000051 0001cf48 c008cb04
[  109.249938] ff40: 60000013 c005dcf4 f1633000 00086ea8 f16b96b0 f1697f05 f1699958 00007560
[  109.258575] ff60: 00008670 bf09ef90 00000025 00000000 00000031 00000032 00000017 0000001b
[  109.267211] ff80: 00000010 00000000 0001b070 0001b070 00000000 00000000 00000080 c000fa44
[  109.275848] ffa0: 00000000 c000f8a0 0001b070 00000000 b6d77000 00086ea8 0001cfc0 00000000
[  109.284484] ffc0: 0001b070 00000000 00000000 00000080 00000000 0001cfe0 00000000 0001cf48
[  109.293121] ffe0: 0001cfc0 bea866fc 0000b720 b6ec6ed4 60000010 b6d77000 afffd861 afffdc61
[  109.301788] [<bf001850>] (media_add_link [media]) from [<bf0021b4>] (media_create_pad_link+0xb0/0x134 [media])
[  109.312408] [<bf0021b4>] (media_create_pad_link [media]) from [<bf09c520>] (smiapp_registered+0xd0/0x114 [smiapp])
[  109.323455] [<bf09c520>] (smiapp_registered [smiapp]) from [<bf014488>] (v4l2_device_register_subdev+0xb8/0x17c [videodev])
[  109.335327] [<bf014488>] (v4l2_device_register_subdev [videodev]) from [<bf01d384>] (v4l2_async_test_notify+0x90/0xf0 [videodev])
[  109.347778] [<bf01d384>] (v4l2_async_test_notify [videodev]) from [<bf01d444>] (v4l2_async_register_subdev+0x60/0xbc [videodev])
[  109.360046] [<bf01d444>] (v4l2_async_register_subdev [videodev]) from [<bf09c3f8>] (smiapp_probe+0x2bc/0x314 [smiapp])
[  109.371368] [<bf09c3f8>] (smiapp_probe [smiapp]) from [<c028b7dc>] (i2c_device_probe+0x1b8/0x214)
[  109.380737] [<c028b7dc>] (i2c_device_probe) from [<c02112dc>] (driver_probe_device+0x18c/0x410)
[  109.389923] [<c02112dc>] (driver_probe_device) from [<c02115c8>] (__driver_attach+0x68/0x8c)
[  109.398834] [<c02115c8>] (__driver_attach) from [<c020f8ac>] (bus_for_each_dev+0x4c/0x90)
[  109.407470] [<c020f8ac>] (bus_for_each_dev) from [<c0210110>] (bus_add_driver+0x118/0x238)
[  109.416198] [<c0210110>] (bus_add_driver) from [<c0211cc4>] (driver_register+0xa0/0xe4)
[  109.424652] [<c0211cc4>] (driver_register) from [<c028bd94>] (i2c_register_driver+0x40/0x9c)
[  109.433563] [<c028bd94>] (i2c_register_driver) from [<c0009784>] (do_one_initcall+0x118/0x1e0)
[  109.442657] [<c0009784>] (do_one_initcall) from [<c008ac94>] (do_init_module+0x58/0x1b0)
[  109.451202] [<c008ac94>] (do_init_module) from [<c008c634>] (load_module+0x1240/0x1584)
[  109.459655] [<c008c634>] (load_module) from [<c008cb04>] (SyS_init_module+0x18c/0x1a4)
[  109.468017] [<c008cb04>] (SyS_init_module) from [<c000f8a0>] (ret_fast_syscall+0x0/0x1c)
[  109.476562] Code: e2802010 e5842004 e5804010 e5803014 (e5832000) 
[  109.483062] ---[ end trace aa9de464363318da ]---
----------8<------------

Kind regards,
Sakari

 drivers/media/v4l2-core/v4l2-device.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

Comments

kernel test robot Dec. 27, 2015, 11:56 p.m. UTC | #1
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
Hi Sakari,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.4-rc6 next-20151223]

url:    https://github.com/0day-ci/linux/commits/Sakari-Ailus/v4l-subdev-Register-the-entity-before-calling-subdev-s-registered-op/20151228-074927
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x016-201552 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/v4l2-core/v4l2-device.c: In function 'v4l2_device_register_subdev':
>> drivers/media/v4l2-core/v4l2-device.c:210:33: error: 'entity' undeclared (first use in this function)
     media_device_unregister_entity(entity);
                                    ^
   drivers/media/v4l2-core/v4l2-device.c:210:33: note: each undeclared identifier is reported only once for each function it appears in

vim +/entity +210 drivers/media/v4l2-core/v4l2-device.c

   204		list_add_tail(&sd->list, &v4l2_dev->subdevs);
   205		spin_unlock(&v4l2_dev->lock);
   206	
   207		return 0;
   208	
   209	error_unregister:
 > 210		media_device_unregister_entity(entity);
   211	error_module:
   212		if (!sd->owner_v4l2_dev)
   213			module_put(sd->owner);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox

diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 85b1e98..b50fb8d 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -180,26 +180,26 @@  int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 		return -ENODEV;
 
 	sd->v4l2_dev = v4l2_dev;
-	if (sd->internal_ops && sd->internal_ops->registered) {
-		err = sd->internal_ops->registered(sd);
-		if (err)
-			goto error_module;
-	}
-
 	/* This just returns 0 if either of the two args is NULL */
 	err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, sd->ctrl_handler, NULL);
 	if (err)
-		goto error_unregister;
+		goto error_module;
 
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	/* Register the entity. */
 	if (v4l2_dev->mdev) {
 		err = media_device_register_entity(v4l2_dev->mdev, entity);
 		if (err < 0)
-			goto error_unregister;
+			goto error_module;
 	}
 #endif
 
+	if (sd->internal_ops && sd->internal_ops->registered) {
+		err = sd->internal_ops->registered(sd);
+		if (err)
+			goto error_unregister;
+	}
+
 	spin_lock(&v4l2_dev->lock);
 	list_add_tail(&sd->list, &v4l2_dev->subdevs);
 	spin_unlock(&v4l2_dev->lock);
@@ -207,8 +207,7 @@  int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 	return 0;
 
 error_unregister:
-	if (sd->internal_ops && sd->internal_ops->unregistered)
-		sd->internal_ops->unregistered(sd);
+	media_device_unregister_entity(entity);
 error_module:
 	if (!sd->owner_v4l2_dev)
 		module_put(sd->owner);