Re: [PATCH v14 07/28] rcar-vin: Use generic parser for parsing fwnode endpoints
Hejssan, On 10/02/17 15:14, Niklas Söderlund wrote: > Hi Sakari, > > On 2017-10-02 14:58:10 +0300, Sakari Ailus wrote: >> Hi Niklas, >> >> On 09/30/17 16:17, Niklas Söderlund wrote: >>> Hi Sakari, >>> >>> Thanks for your patch, I like it. Unfortunately it causes issues :-( >>> >>> I picked the first 7 patches of this series on top of media-next and it >>> produce problems when tested on Koelsch with CONFIG_OF_DYNAMIC=y. >>> >>> 1. It print's 'OF: ERROR: Bad of_node_put() on /video@e6ef/port' >>>messages during boot. >> >> Do you have your own patch to fix fwnode_graph_get_port_parent() >> applied? I noticed it doesn't seem to be in Rob's tree; let's continue >> in the other thread. >> >> https://www.mail-archive.com/linux-media@vger.kernel.org/msg117450.html> > > To produce this issue the fix is not applied. But as I try to describe > at the end of my email applying it fixes both issues. So I think this > patch is correct (and that is why I Acked it) but my concern is that if > it's picked up before the fwnode_graph_get_port_parent() issue is sorted > out there will be problems for rcar-vin, and if possible I would like to > avoid that. Oops. I missed that between the oops log and the patch. X-) Well, good to hear that this isn't an actual bug in this set. I'll try to be careful in sending pull requests. :-) The same issue would be present in any other driver using the new convenience functions. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v14 07/28] rcar-vin: Use generic parser for parsing fwnode endpoints
Hi Sakari, On 2017-10-02 14:58:10 +0300, Sakari Ailus wrote: > Hi Niklas, > > On 09/30/17 16:17, Niklas Söderlund wrote: > > Hi Sakari, > > > > Thanks for your patch, I like it. Unfortunately it causes issues :-( > > > > I picked the first 7 patches of this series on top of media-next and it > > produce problems when tested on Koelsch with CONFIG_OF_DYNAMIC=y. > > > > 1. It print's 'OF: ERROR: Bad of_node_put() on /video@e6ef/port' > >messages during boot. > > Do you have your own patch to fix fwnode_graph_get_port_parent() > applied? I noticed it doesn't seem to be in Rob's tree; let's continue > in the other thread. > > https://www.mail-archive.com/linux-media@vger.kernel.org/msg117450.html> To produce this issue the fix is not applied. But as I try to describe at the end of my email applying it fixes both issues. So I think this patch is correct (and that is why I Acked it) but my concern is that if it's picked up before the fwnode_graph_get_port_parent() issue is sorted out there will be problems for rcar-vin, and if possible I would like to avoid that. > > > > >OF: ERROR: Bad of_node_put() on /video@e6ef/port > >CPU: 1 PID: 1 Comm: swapper/0 Not tainted > > 4.13.0-rc4-00632-gfae12f9c98a8c567 #7 > >Hardware name: Generic R8A7791 (Flattened Device Tree) [snip] > > Could fixing the other issue fix this one as well? > > I'll see how the rest works at my end with CONFIG_OF_DYNAMIC enabled. Yes, as I try to describe bellow, applying the fix for fwnode_graph_get_port_parent() solves both issues :-) What is troublesome for me is that I don't understand why cdev_alloc() would change/corrupt the subdevice pointer if CONFIG_OF_DYNAMIC=y or the fix is _not_ applied. > > >[] (v4l2_async_match_notify) from [] > > (v4l2_async_notifier_register+0x11c/0x150) > > r7:eb3cda50 r6:ea9e2868 r5:c0a5ff18 r4:eaa56bd8 > >[] (v4l2_async_notifier_register) from [] > > (rcar_vin_probe+0x104/0x178) > > r9:eaa56bd8 r8: r7:eb251a10 r6:eb251a00 r5: r4:eaa56810 > >[] (rcar_vin_probe) from [] > >(platform_drv_probe+0x58/0xa4) > > r9: r8:c0a6c504 r7: r6:c0a6c504 r5:eb251a10 r4:c05432ec > >[] (platform_drv_probe) from [] > > (driver_probe_device+0x210/0x2d8) > > r7: r6:c0ac72d4 r5:c0ac72c8 r4:eb251a10 > >[] (driver_probe_device) from [] > > (__driver_attach+0x84/0xb0) > > r10: r9:c096d224 r8: r7:c0a50cf0 r6:c0a6c504 r5:eb251a44 > > r4:eb251a10 r3: > >[] (__driver_attach) from [] > > (bus_for_each_dev+0x88/0x98) > > r7:c0a50cf0 r6:c041f3e8 r5:c0a6c504 r4: > >[] (bus_for_each_dev) from [] > > (driver_attach+0x20/0x28) > > r6: r5:eaa55f80 r4:c0a6c504 > >[] (driver_attach) from [] > > (bus_add_driver+0x170/0x1e0) > >[] (bus_add_driver) from [] > > (driver_register+0xa8/0xe8) > > r7:c095883c r6:00cb r5:e000 r4:c0a6c504 > >[] (driver_register) from [] > > (__platform_driver_register+0x38/0x4c) > > r5:e000 r4:c0935328 > >[] (__platform_driver_register) from [] > > (rcar_vin_driver_init+0x18/0x20) > >[] (rcar_vin_driver_init) from [] > > (do_one_initcall+0x12c/0x154) > >[] (do_one_initcall) from [] > > (kernel_init_freeable+0x18c/0x1d0) > > r8:c0a8a700 r7:c095883c r6:00cb r5:c0a8a700 r4:0007 > >[] (kernel_init_freeable) from [] > > (kernel_init+0x10/0x110) > > r9: r8: r7: r6: r5:c06eac54 r4: > >[] (kernel_init) from [] (ret_from_fork+0x14/0x3c) > > r5:c06eac54 r4: > >Code: 03e05012 e5803368 0a0a e5963068 (e593300c) > >---[ end trace 82aa2a1c6173a5f6 ]--- > > > > > > Oddly enough setting CONFIG_OF_DYNAMIC=n or applying the patch > > '[PATCH v2] device property: preserve usecount for node passed to > > of_fwnode_graph_get_port_parent()' fixes _both_ issues. It obviously > > would fix the 'Bad of_node_put() on ...' messages that it also fixes the > > OOPS is strange, so I did some digging. > > > > The problem is introduced when rcar-vin in its complete callback calls > > v4l2_device_register_subdev_nodes(). Before the call > > vin->digital->subdev pointer is correct but after the call the > > vin->digital->subdev pointer is changed to a for me random value. And > > this is what is causing the OOPS in rvin_v4l2_probe() once it tries to > > operate on the subdevice using v4l2_subdev_call() using this bad > > pointer. > > > > I tried to track down the issue but I can't understand what is causing > > it, but I managed to narrow it down. The callchain is: > > > > - rvin_digital_notify_complete > > - pr_dbg("sd: %p\n", vin->digital->subdev); # prints good pointer > > - v4l2_device_register_subdev_nodes() > > - __video_register_device() > > - cdev_alloc() # Here the pointer gets corrupted > > - pr_dbg("sd: %p\n", vin->digital->subdev); # prints bad pointer > > > > I can't figure
Re: [PATCH v14 07/28] rcar-vin: Use generic parser for parsing fwnode endpoints
Hi Niklas, On 09/30/17 16:17, Niklas Söderlund wrote: > Hi Sakari, > > Thanks for your patch, I like it. Unfortunately it causes issues :-( > > I picked the first 7 patches of this series on top of media-next and it > produce problems when tested on Koelsch with CONFIG_OF_DYNAMIC=y. > > 1. It print's 'OF: ERROR: Bad of_node_put() on /video@e6ef/port' >messages during boot. Do you have your own patch to fix fwnode_graph_get_port_parent() applied? I noticed it doesn't seem to be in Rob's tree; let's continue in the other thread. https://www.mail-archive.com/linux-media@vger.kernel.org/msg117450.html> > >OF: ERROR: Bad of_node_put() on /video@e6ef/port >CPU: 1 PID: 1 Comm: swapper/0 Not tainted > 4.13.0-rc4-00632-gfae12f9c98a8c567 #7 >Hardware name: Generic R8A7791 (Flattened Device Tree) >Backtrace: >[] (dump_backtrace) from [] (show_stack+0x18/0x1c) > r7:0001 r6:6013 r5: r4:c0a7c88c >[] (show_stack) from [] (dump_stack+0x84/0xa0) >[] (dump_stack) from [] (of_node_release+0x2c/0x94) > r7:0001 r6:c0a6fc4c r5:eb1284c0 r4:eb7c7a00 >[] (of_node_release) from [] (kobject_put+0xbc/0xdc) > r7:0001 r6:c0a6fc4c r5:eb1284c0 r4:eb7c7a00 >[] (kobject_put) from [] (of_node_put+0x1c/0x20) > r6:eb7c7af8 r5:eb7c79e0 r4:eb7c7798 >[] (of_node_put) from [] (of_fwnode_put+0x38/0x44) >[] (of_fwnode_put) from [] > (fwnode_handle_put+0x30/0x34) >[] (fwnode_handle_put) from [] > (fwnode_graph_get_port_parent+0x44/0x4c) >[] (fwnode_graph_get_port_parent) from [] > (__v4l2_async_notifier_parse_fwnode_endpoints+0x1dc/0x2d8) > r5:eaa56bd8 r4: >[] (__v4l2_async_notifier_parse_fwnode_endpoints) from > [] (v4l2_async_notifier_parse_fwnode_endpoints+0x20/0x28) > r10: r9:eaa56bd8 r8:c0a6c504 r7:eb251a10 r6:eb251a00 r5: > r4:eaa56810 >[] (v4l2_async_notifier_parse_fwnode_endpoints) from > [] (rcar_vin_probe+0xcc/0x178) >[] (rcar_vin_probe) from [] > (platform_drv_probe+0x58/0xa4) > r9: r8:c0a6c504 r7: r6:c0a6c504 r5:eb251a10 r4:c05432ec >[] (platform_drv_probe) from [] > (driver_probe_device+0x210/0x2d8) > r7: r6:c0ac72d4 r5:c0ac72c8 r4:eb251a10 >[] (driver_probe_device) from [] > (__driver_attach+0x84/0xb0) > r10: r9:c096d224 r8: r7:c0a50cf0 r6:c0a6c504 r5:eb251a44 > r4:eb251a10 r3: >[] (__driver_attach) from [] > (bus_for_each_dev+0x88/0x98) > r7:c0a50cf0 r6:c041f3e8 r5:c0a6c504 r4: >[] (bus_for_each_dev) from [] (driver_attach+0x20/0x28) > r6: r5:eaa55f80 r4:c0a6c504 >[] (driver_attach) from [] (bus_add_driver+0x170/0x1e0) >[] (bus_add_driver) from [] (driver_register+0xa8/0xe8) > r7:c095883c r6:00cb r5:e000 r4:c0a6c504 >[] (driver_register) from [] > (__platform_driver_register+0x38/0x4c) > r5:e000 r4:c0935328 >[] (__platform_driver_register) from [] > (rcar_vin_driver_init+0x18/0x20) >[] (rcar_vin_driver_init) from [] > (do_one_initcall+0x12c/0x154) >[] (do_one_initcall) from [] > (kernel_init_freeable+0x18c/0x1d0) > r8:c0a8a700 r7:c095883c r6:00cb r5:c0a8a700 r4:0007 >[] (kernel_init_freeable) from [] > (kernel_init+0x10/0x110) > r9: r8: r7: r6: r5:c06eac54 r4: >[] (kernel_init) from [] (ret_from_fork+0x14/0x3c) > r5:c06eac54 r4: > > 2. It then proceeds to OOPS. > >Unable to handle kernel NULL pointer dereference at virtual address > 000c >pgd = c0004000 >[000c] *pgd= >Internal error: Oops: 5 [#1] SMP ARM >CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 4.13.0-rc4-00632-gfae12f9c98a8c567 #7 >Hardware name: Generic R8A7791 (Flattened Device Tree) >task: eb09b840 task.stack: eb09c000 >PC is at rvin_v4l2_probe+0x38/0x1ec >LR is at rvin_digital_notify_complete+0x10c/0x11c >pc : []lr : []psr: a013 >sp : eb09dcf8 ip : eb09dd20 fp : eb09dd1c >r10: r9 : eaa56bd8 r8 : 2006 >r7 : eaa56810 r6 : c0a1e6dc r5 : r4 : eaa56810 >r3 : r2 : 0001 r1 : eaa57ec0 r0 : eaa56810 >Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none >Control: 10c5387d Table: 4000406a DAC: 0051 >Process swapper/0 (pid: 1, stack limit = 0xeb09c210) >Stack: (0xeb09dcf8 to 0xeb09e000) >dce0: ea9e2868 > eb1284c0 >dd00: eaa56bd8 eb1284c0 eaa56810 eb09dd74 eb09dd20 c0543614 > c0545920 >dd20: 0001 0001 100a 0001 > >dd40: c051c17c eaa56bd8 c0543508 > ea9e2868 >dd60: eb1284c0 c0a5ff2c eb09dd94 eb09dd78 c0525164 c0543514 eaa56bd8 > c0a5ff18 >dd80: ea9e2868 eb3cda50 eb09ddbc eb09dd98 c0525288 c0525088 eaa56810 > >dda0: eb251a00 eb251a10 000
Re: [PATCH v14 07/28] rcar-vin: Use generic parser for parsing fwnode endpoints
Hi Sakari, Thanks for your patch, I like it. Unfortunately it causes issues :-( I picked the first 7 patches of this series on top of media-next and it produce problems when tested on Koelsch with CONFIG_OF_DYNAMIC=y. 1. It print's 'OF: ERROR: Bad of_node_put() on /video@e6ef/port' messages during boot. OF: ERROR: Bad of_node_put() on /video@e6ef/port CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc4-00632-gfae12f9c98a8c567 #7 Hardware name: Generic R8A7791 (Flattened Device Tree) Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:0001 r6:6013 r5: r4:c0a7c88c [] (show_stack) from [] (dump_stack+0x84/0xa0) [] (dump_stack) from [] (of_node_release+0x2c/0x94) r7:0001 r6:c0a6fc4c r5:eb1284c0 r4:eb7c7a00 [] (of_node_release) from [] (kobject_put+0xbc/0xdc) r7:0001 r6:c0a6fc4c r5:eb1284c0 r4:eb7c7a00 [] (kobject_put) from [] (of_node_put+0x1c/0x20) r6:eb7c7af8 r5:eb7c79e0 r4:eb7c7798 [] (of_node_put) from [] (of_fwnode_put+0x38/0x44) [] (of_fwnode_put) from [] (fwnode_handle_put+0x30/0x34) [] (fwnode_handle_put) from [] (fwnode_graph_get_port_parent+0x44/0x4c) [] (fwnode_graph_get_port_parent) from [] (__v4l2_async_notifier_parse_fwnode_endpoints+0x1dc/0x2d8) r5:eaa56bd8 r4: [] (__v4l2_async_notifier_parse_fwnode_endpoints) from [] (v4l2_async_notifier_parse_fwnode_endpoints+0x20/0x28) r10: r9:eaa56bd8 r8:c0a6c504 r7:eb251a10 r6:eb251a00 r5: r4:eaa56810 [] (v4l2_async_notifier_parse_fwnode_endpoints) from [] (rcar_vin_probe+0xcc/0x178) [] (rcar_vin_probe) from [] (platform_drv_probe+0x58/0xa4) r9: r8:c0a6c504 r7: r6:c0a6c504 r5:eb251a10 r4:c05432ec [] (platform_drv_probe) from [] (driver_probe_device+0x210/0x2d8) r7: r6:c0ac72d4 r5:c0ac72c8 r4:eb251a10 [] (driver_probe_device) from [] (__driver_attach+0x84/0xb0) r10: r9:c096d224 r8: r7:c0a50cf0 r6:c0a6c504 r5:eb251a44 r4:eb251a10 r3: [] (__driver_attach) from [] (bus_for_each_dev+0x88/0x98) r7:c0a50cf0 r6:c041f3e8 r5:c0a6c504 r4: [] (bus_for_each_dev) from [] (driver_attach+0x20/0x28) r6: r5:eaa55f80 r4:c0a6c504 [] (driver_attach) from [] (bus_add_driver+0x170/0x1e0) [] (bus_add_driver) from [] (driver_register+0xa8/0xe8) r7:c095883c r6:00cb r5:e000 r4:c0a6c504 [] (driver_register) from [] (__platform_driver_register+0x38/0x4c) r5:e000 r4:c0935328 [] (__platform_driver_register) from [] (rcar_vin_driver_init+0x18/0x20) [] (rcar_vin_driver_init) from [] (do_one_initcall+0x12c/0x154) [] (do_one_initcall) from [] (kernel_init_freeable+0x18c/0x1d0) r8:c0a8a700 r7:c095883c r6:00cb r5:c0a8a700 r4:0007 [] (kernel_init_freeable) from [] (kernel_init+0x10/0x110) r9: r8: r7: r6: r5:c06eac54 r4: [] (kernel_init) from [] (ret_from_fork+0x14/0x3c) r5:c06eac54 r4: 2. It then proceeds to OOPS. Unable to handle kernel NULL pointer dereference at virtual address 000c pgd = c0004000 [000c] *pgd= Internal error: Oops: 5 [#1] SMP ARM CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc4-00632-gfae12f9c98a8c567 #7 Hardware name: Generic R8A7791 (Flattened Device Tree) task: eb09b840 task.stack: eb09c000 PC is at rvin_v4l2_probe+0x38/0x1ec LR is at rvin_digital_notify_complete+0x10c/0x11c pc : []lr : []psr: a013 sp : eb09dcf8 ip : eb09dd20 fp : eb09dd1c r10: r9 : eaa56bd8 r8 : 2006 r7 : eaa56810 r6 : c0a1e6dc r5 : r4 : eaa56810 r3 : r2 : 0001 r1 : eaa57ec0 r0 : eaa56810 Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000406a DAC: 0051 Process swapper/0 (pid: 1, stack limit = 0xeb09c210) Stack: (0xeb09dcf8 to 0xeb09e000) dce0: ea9e2868 eb1284c0 dd00: eaa56bd8 eb1284c0 eaa56810 eb09dd74 eb09dd20 c0543614 c0545920 dd20: 0001 0001 100a 0001 dd40: c051c17c eaa56bd8 c0543508 ea9e2868 dd60: eb1284c0 c0a5ff2c eb09dd94 eb09dd78 c0525164 c0543514 eaa56bd8 c0a5ff18 dd80: ea9e2868 eb3cda50 eb09ddbc eb09dd98 c0525288 c0525088 eaa56810 dda0: eb251a00 eb251a10 eaa56bd8 eb09dde4 eb09ddc0 c05433f0 c0525178 ddc0: c05432ec eb251a10 c0a6c504 c0a6c504 eb09de04 eb09dde8 dde0: c0420a3c c05432f8 eb251a10 c0ac72c8 c0ac72d4 eb09de34 eb09de08 de00: c041f320 c04209f0 eb251a10 eb251a44 c0a6c504 c0a50cf0 de20: c096d224 eb09de54 eb09de38 c041f46c c041f11c c0a6c504 de40: c041f3e8 c0a50cf0 eb09de7c eb09de58 c041da08 c041f3f4 eb0eff58 eb2259b4 de60: eb0eff6c c0a6c504 eaa55f80 eb09de8c eb09d