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

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

 



Em Mon, 28 Dec 2015 01:45:00 +0200
Sakari Ailus <sakari.ailus@xxxxxx> escreveu:

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

Perhaps it was on some of your replies. There were too many OOT media
controller patches floating around ;)

Anyway, I applied the patches at:

	https://git.linuxtv.org/mchehab/experimental.git/log/?h=media-controller-rc6

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

There were some discussions about that and I was thinking that we had an
agreement about that. Anyway, I don't object to remove the warning for
now, and discuss that a little more.

> 
> ----------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(-)
> 
> 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);

This breaks if !CONFIG_MEDIA_CONTROLLER. So, I'm folding the following fixup:

diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index b50fb8d7d62e..06fa5f1b2cff 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -207,7 +207,9 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 	return 0;
 
 error_unregister:
+#if defined(CONFIG_MEDIA_CONTROLLER)
 	media_device_unregister_entity(entity);
+#endif
 error_module:
 	if (!sd->owner_v4l2_dev)
 		module_put(sd->owner);




>  error_module:
>  	if (!sd->owner_v4l2_dev)
>  		module_put(sd->owner);
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux