ddbridge improvements and cleanups
cover ddbridge improvements and cleanups
From: Daniel Scheller <[email protected]>
This series improves on a few things in ddbridge:
* Fix up a KASAN report which pops up with all TDA18212-equipped hardware by changing the order of all frontend driver teardown. This was originally thought to be a problem of the tda18212 driver, see
https://patchwork.linuxtv.org/patch/45992/ (though other drivers with that problem may remain).
* Fix up CI resources cleanup handling, and further cosmetics and code move, all CI related
* Frontend cleanup improvements when handling errors (ie. when on one port the device initialisation fails). Whenever a tuner module fails now, everything should be cleaned up properly (and early) now, while all other (working) tuners are being usable. Proper errors are printed to the kernel log about this.
Mauro, I'm pretty sure you like this overall approach better, compared to https://patchwork.linuxtv.org/patch/45810/ :-) In fact, I picked up your idea of counting ports and act accordingly. Partial hardware setup now starts up all working parts properly, while releasing resources early when the nonworking parts are handled. If no ports could be started at all, the driver instance will fail gracefully and report this to upper layers.
I verified this by simply removing tda18212.ko with this DD setup:
* CineCTv6 bridge card (stv0367+tda18212 soldered on it, handled as port 0), one DuoFlex C2T2 connected to port 1 (cxd2841er+tda18212)
* Octopus CI Duo, one DuoFlex C2T2I (cxd2841er+tda18212) connected to port 1, one SingleCI module (cxd2099) connected to port 2
Upon modprobe ddbridge, the CTv6 will completely fail due to the tuner driver not initialising (it's not there, actually). The OctoCIDUO will fail on the C2T2I Flex, but starts up the CI hardware, registers it's en50221 device nodes and things work fine with it. Unload cleans up everything, no leaked usecounts, no KASAN complaints. Putting back tda18212.ko, modprobe ddbridge - registers everything. Unload cleans up everything properly aswell.
Not entirely sure, but patch 1 might be something for stable (ie. 4.14).
Daniel Scheller (8):
[media] ddbridge: unregister I2C tuner client before detaching fe's [media] ddbridge: fix resources cleanup for CI hardware [media] ddbridge: deduplicate calls to dvb_ca_en50221_init() [media] ddbridge: move CI detach code to ddbridge-ci.c [media] ddbridge: completely tear down input resources on failure [media] ddbridge: fix deinit order in case of failure in ddb_init() [media] ddbridge: detach first input if the second one failed to init [media] ddbridge: improve ddb_ports_attach() failure handling
drivers/media/pci/ddbridge/ddbridge-ci.c | 17 +++-- drivers/media/pci/ddbridge/ddbridge-ci.h | 1 + drivers/media/pci/ddbridge/ddbridge-core.c | 108 ++++++++++++++++++----------- 3 files changed, 81 insertions(+), 45 deletions(-)
Signature · 1 line
2.13.6
PATCH 1/8 ddbridge: unregister I2C tuner client before detaching fe's
From: Daniel Scheller <[email protected]>
Currently, rmmod ddbridge on a KASAN enabled kernel yields this report for hardware that utilises the tda18212 tuner driver:
[ 50.355229] ================================================================== [ 50.355271] BUG: KASAN: use-after-free in tda18212_remove+0x5c/0xb0 [tda18212] [ 50.355290] Write of size 288 at addr ffff8800c235cf18 by task rmmod/285
[ 50.355316] CPU: 1 PID: 285 Comm: rmmod Not tainted 4.15.0-rc1-13744-g352a86ad536f #11 [ 50.355318] Hardware name: Gigabyte Technology Co., Ltd. P35-DS3/P35-DS3, BIOS F3 06/11/2007 [ 50.355319] Call Trace: [ 50.355326] dump_stack+0x46/0x61 [ 50.355332] print_address_description+0x79/0x270 [ 50.355336] ? tda18212_remove+0x5c/0xb0 [tda18212] [ 50.355339] kasan_report+0x229/0x340 [ 50.355342] memset+0x1f/0x40 [ 50.355345] tda18212_remove+0x5c/0xb0 [tda18212] [ 50.355350] i2c_device_remove+0x97/0xe0 [ 50.355355] device_release_driver_internal+0x267/0x510 [ 50.355358] bus_remove_device+0x296/0x470 [ 50.355360] device_del+0x35c/0x890 [ 50.355363] ? __device_links_no_driver+0x1c0/0x1c0 [ 50.355367] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] [ 50.355371] ? cxd2841er_get_algo+0x10/0x10 [cxd2841er] [ 50.355374] ? __module_text_address+0xe/0x140 [ 50.355377] device_unregister+0x9/0x20 [ 50.355382] dvb_input_detach.isra.24+0x286/0x480 [ddbridge] [ 50.355388] ddb_ports_detach+0x15f/0x4f0 [ddbridge] [ 50.355393] ddb_remove+0x3c/0xb0 [ddbridge] [ 50.355397] pci_device_remove+0x93/0x1d0 [ 50.355400] device_release_driver_internal+0x267/0x510 [ 50.355403] driver_detach+0xb9/0x1b0 [ 50.355406] bus_remove_driver+0xd0/0x1f0 [ 50.355410] pci_unregister_driver+0x25/0x210 [ 50.355415] module_exit_ddbridge+0xc/0x45 [ddbridge] [ 50.355418] SyS_delete_module+0x314/0x440 [ 50.355420] ? free_module+0x5b0/0x5b0 [ 50.355423] ? exit_to_usermode_loop+0xa9/0xc0 [ 50.355425] ? free_module+0x5b0/0x5b0 [ 50.355428] do_syscall_64+0x179/0x4c0 [ 50.355432] ? do_page_fault+0x1b/0x60 [ 50.355435] entry_SYSCALL64_slow_path+0x25/0x25 [ 50.355438] RIP: 0033:0x7fe65d08ade7 [ 50.355439] RSP: 002b:00007fff5a6a09a8 EFLAGS: 00000202 ORIG_RAX: 00000000000000b0 [ 50.355443] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fe65d08ade7 [ 50.355445] RDX: 000000000000000a RSI: 0000000000000800 RDI: 0000000000f4e268 [ 50.355447] RBP: 0000000000f4e200 R08: 0000000000000000 R09: 1999999999999999 [ 50.355449] R10: 0000000000000891 R11: 0000000000000202 R12: 00007fff5a6a14ef [ 50.355451] R13: 0000000000000000 R14: 0000000000f4e200 R15: 0000000000f4d010
[ 50.355462] Allocated by task 164: [ 50.355477] cxd2841er_attach+0xc3/0x7f0 [cxd2841er] [ 50.355482] demod_attach_cxd28xx+0x14c/0x3f0 [ddbridge] [ 50.355486] dvb_input_attach+0x671/0x1e20 [ddbridge] [ 50.355490] ddb_ports_attach+0x3d7/0xbf0 [ddbridge] [ 50.355495] ddb_init+0x4b3/0xa30 [ddbridge] [ 50.355499] ddb_probe+0xa51/0xfe0 [ddbridge] [ 50.355501] pci_device_probe+0x279/0x480 [ 50.355504] driver_probe_device+0x46f/0x7a0 [ 50.355506] __driver_attach+0x133/0x170 [ 50.355509] bus_for_each_dev+0x10a/0x190 [ 50.355511] bus_add_driver+0x2a3/0x5a0 [ 50.355513] driver_register+0x182/0x3a0 [ 50.355516] arc4_set_key+0x8f/0x2a0 [arc4] [ 50.355518] do_one_initcall+0x77/0x1d0 [ 50.355521] do_init_module+0x1c2/0x548 [ 50.355523] load_module+0x5e61/0x8df0 [ 50.355525] SyS_finit_module+0x142/0x150 [ 50.355527] do_syscall_64+0x179/0x4c0 [ 50.355529] return_from_SYSCALL_64+0x0/0x65
[ 50.355539] Freed by task 285: [ 50.355551] kfree+0x6c/0xa0 [ 50.355558] __dvb_frontend_free+0x81/0xb0 [dvb_core] [ 50.355562] dvb_input_detach.isra.24+0x17c/0x480 [ddbridge] [ 50.355566] ddb_ports_detach+0x15f/0x4f0 [ddbridge] [ 50.355570] ddb_remove+0x3c/0xb0 [ddbridge] [ 50.355573] pci_device_remove+0x93/0x1d0 [ 50.355576] device_release_driver_internal+0x267/0x510 [ 50.355578] driver_detach+0xb9/0x1b0 [ 50.355580] bus_remove_driver+0xd0/0x1f0 [ 50.355583] pci_unregister_driver+0x25/0x210 [ 50.355587] module_exit_ddbridge+0xc/0x45 [ddbridge] [ 50.355590] SyS_delete_module+0x314/0x440 [ 50.355592] do_syscall_64+0x179/0x4c0 [ 50.355594] return_from_SYSCALL_64+0x0/0x65
[ 50.355604] The buggy address belongs to the object at ffff8800c235cd80
which belongs to the cache kmalloc-2048 of size 2048
[ 50.355630] The buggy address is located 408 bytes inside of
2048-byte region [ffff8800c235cd80, ffff8800c235d580)
[ 50.355652] The buggy address belongs to the page:
[ 50.355666] page:ffffea0002a7bc20 count:1 mapcount:0 mapping:ffff8800c235c500 index:0x0 compound_mapcount: 0
[ 50.355688] flags: 0x4000000000008100(slab|head)
[ 50.355703] raw: 4000000000008100 ffff8800c235c500 0000000000000000 0000000100000003
[ 50.355720] raw: ffffea000382b4b0 ffffea0002b91550 ffff88010b000800
[ 50.355734] page dumped because: kasan: bad access detected[ 50.355754] Memory state around the buggy address: [ 50.355767] ffff8800c235ce00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 50.355783] ffff8800c235ce80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 50.355800] >ffff8800c235cf00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 50.355815] ^ [ 50.355827] ffff8800c235cf80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 50.355843] ffff8800c235d000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 50.355858] ==================================================================
This is due to dvb_frontend_detach() being called before i2c_unregister_device() on the TDA18212 tuner client instance, as dvb_frontend_detach() causes the demod drivers to release all their resources, and the tuner driver's _remove method does further cleanup on the now invalid (freed) resources. Fix this by putting the I2C client deregistration in dvb_input_detach() to state/case 0x30, right before the call to dvb_frontend_detach(). This also makes sure that any further (tuner) hardware driven by I2C client drivers unload cleanly.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/media/pci/ddbridge/ddbridge-core.c +8 -6
@@ -1263,6 +1263,14 @@ static void dvb_input_detach(struct ddb_input *input)dvb_unregister_frontend (dvb->fe);/* fallthrough */ case 0x30 : + client = dvb->i2c_client[0 ]; +if (client) { +module_put (client->dev.driver->owner); +i2c_unregister_device (client); + dvb->i2c_client[0 ] =NULL ; + client =NULL ; + } +if (dvb->fe2)dvb_frontend_detach (dvb->fe2);if (dvb->fe)
@@ -1271,12 +1279,6 @@ static void dvb_input_detach(struct ddb_input *input) dvb->fe2 =NULL ;/* fallthrough */ case 0x20 : - client = dvb->i2c_client[0 ]; -if (client) { -module_put (client->dev.driver->owner); -i2c_unregister_device (client); - } -dvb_net_release (&dvb->dvbnet);/* fallthrough */ case 0x12 :
Signature · 1 line
2.13.6
PATCH 2/8 ddbridge: fix resources cleanup for CI hardware
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
Do kfree() on port->en->data instead of port->en. port->en only holds a ptr to a struct dvb_ca_en50221, which is a member either of a memalloc'ed struct ddb_ci (DuoFlex CI, Octopus CI Duo) or a struct cxd (CXD2099AR based Single Flex, allocated by the cxd2099 driver). port->en.data though holds the ptr to the allocated memory, which must rather be kfree()'d. Change this accordingly.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/pci/ddbridge/ddbridge-core.c +1 -1
@@ -1990,7 +1990,7 @@ void ddb_ports_detach(struct ddb *dev)dvb_unregister_device (port->dvb[0 ].dev);if (port->en) {dvb_ca_en50221_release (port->en); -kfree (port->en); +kfree (port->en->data); port->en =NULL ; }break ;
Signature · 1 line
2.13.6
PATCH 3/8 ddbridge: deduplicate calls to dvb_ca_en50221_init()
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
All CI types do dvb_ca_en50221_init() with the same arguments. Move this call after the switch-case to remove the repetition in every case block.
--- drivers/media/pci/ddbridge/ddbridge-ci.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/media/pci/ddbridge/ddbridge-ci.c +2 -4
@@ -327,8 +327,6 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate) port->en =cxd2099_attach (&cxd_cfg, port, &port->i2c->adap);if (!port->en)return -ENODEV ; -dvb_ca_en50221_init (port->dvb[0 ].adap, - port->en,0 ,1 );break ;case DDB_CI_EXTERNAL_XO2 :
@@ -336,15 +334,15 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate)ci_xo2_attach (port);if (!port->en)return -ENODEV ; -dvb_ca_en50221_init (port->dvb[0 ].adap, port->en,0 ,1 );break ;case DDB_CI_INTERNAL :ci_attach (port);if (!port->en)return -ENODEV ; -dvb_ca_en50221_init (port->dvb[0 ].adap, port->en,0 ,1 );break ; } + +dvb_ca_en50221_init (port->dvb[0 ].adap, port->en,0 ,1 );return 0 ; }
Signature · 1 line
2.13.6
PATCH 4/8 ddbridge: move CI detach code to ddbridge-ci.c
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
Move the CI teardown code to ddbridge-ci.c where everything else related to CI hardware lives.
--- drivers/media/pci/ddbridge/ddbridge-ci.c | 11 +++++++++++ drivers/media/pci/ddbridge/ddbridge-ci.h | 1 + drivers/media/pci/ddbridge/ddbridge-core.c | 8 +------- 3 files changed, 13 insertions(+), 7 deletions(-)
drivers/media/pci/ddbridge/ddbridge-ci.c +11 -0
@@ -346,3 +346,14 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate)dvb_ca_en50221_init (port->dvb[0 ].adap, port->en,0 ,1 );return 0 ; } + +void ddb_ci_detach (struct ddb_port *port) +{ +if (port->dvb[0 ].dev) +dvb_unregister_device (port->dvb[0 ].dev); +if (port->en) { +dvb_ca_en50221_release (port->en); +kfree (port->en->data); + port->en =NULL ; + } +}
drivers/media/pci/ddbridge/ddbridge-ci.h +1 -0
@@ -26,5 +26,6 @@/******************************************************************************/ int ddb_ci_attach (struct ddb_port *port,u32 bitrate); +void ddb_ci_detach (struct ddb_port *port);#endif /* __DDBRIDGE_CI_H__ */
drivers/media/pci/ddbridge/ddbridge-core.c +1 -7
@@ -1986,13 +1986,7 @@ void ddb_ports_detach(struct ddb *dev)break ;case DDB_PORT_CI :case DDB_PORT_LOOP : -if (port->dvb[0 ].dev) -dvb_unregister_device (port->dvb[0 ].dev); -if (port->en) { -dvb_ca_en50221_release (port->en); -kfree (port->en->data); - port->en =NULL ; - } +ddb_ci_detach (port);break ; } }
Signature · 1 line
2.13.6
PATCH 5/8 ddbridge: completely tear down input resources on failure
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
In dvb_input_attach(), whenever a demod driver fails to initialise, or if frontend registration fails, perform a full input/frontend teardown using dvb_input_detach() (which can safely be done since the current init state is tracked in the 'attached' struct member). Claimed resources thus are freed which aren't needed when an input or a port is not functional.
While at it, in ddb_ports_detach(), detach the secondary input first. Also increase the kernlog severity of TDA18212 errors and tuner failures in general.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 46 ++++++++++++++++++------------ 1 file changed, 27 insertions(+), 19 deletions(-)
drivers/media/pci/ddbridge/ddbridge-core.c +27 -19
@@ -1032,7 +1032,7 @@ static int tuner_attach_tda18212(struct ddb_input *input, u32 porttype)return 0 ; err: -dev_notice (dev,"TDA18212 tuner not found. Device is not fully operational.\n" ); +dev_err (dev,"TDA18212 tuner not found. Device is not fully operational.\n" );return -ENODEV ; }
@@ -1425,7 +1425,7 @@ static int dvb_input_attach(struct ddb_input *input) dvb->dmxdev.demux = &dvbdemux->dmx; ret =dvb_dmxdev_init (&dvb->dmxdev, adap);if (ret <0 ) -return ret; + goto err_detach; dvb->attached =0x11 ; dvb->mem_frontend.source =DMX_MEMORY_FE ;
@@ -1434,12 +1434,12 @@ static int dvb_input_attach(struct ddb_input *input) dvb->demux.dmx.add_frontend (&dvb->demux.dmx, &dvb->hw_frontend); ret = dvbdemux->dmx.connect_frontend (&dvbdemux->dmx, &dvb->hw_frontend);if (ret <0 ) -return ret; + goto err_detach; dvb->attached =0x12 ; ret =dvb_net_init (adap, &dvb->dvbnet, dvb->dmxdev.demux);if (ret <0 ) -return ret; + goto err_detach; dvb->attached =0x20 ; dvb->fe =NULL ;
@@ -1447,47 +1447,47 @@ static int dvb_input_attach(struct ddb_input *input)switch (port->type) {case DDB_TUNER_MXL5XX :if (ddb_fe_attach_mxl5xx (input) <0 ) -return -ENODEV ; + goto err_detach;break ;case DDB_TUNER_DVBS_ST :if (demod_attach_stv0900 (input,0 ) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_stv6110 (input,0 ) <0 ) goto err_tuner;break ;case DDB_TUNER_DVBS_ST_AA :if (demod_attach_stv0900 (input,1 ) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_stv6110 (input,1 ) <0 ) goto err_tuner;break ;case DDB_TUNER_DVBS_STV0910 :if (demod_attach_stv0910 (input,0 ) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_stv6111 (input,0 ) <0 ) goto err_tuner;break ;case DDB_TUNER_DVBS_STV0910_PR :if (demod_attach_stv0910 (input,1 ) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_stv6111 (input,1 ) <0 ) goto err_tuner;break ;case DDB_TUNER_DVBS_STV0910_P :if (demod_attach_stv0910 (input,0 ) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_stv6111 (input,1 ) <0 ) goto err_tuner;break ;case DDB_TUNER_DVBCT_TR :if (demod_attach_drxk (input) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_tda18271 (input) <0 ) goto err_tuner;break ;case DDB_TUNER_DVBCT_ST :if (demod_attach_stv0367 (input) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_tda18212 (input, port->type) <0 ) goto err_tuner;break ;
@@ -1507,7 +1507,7 @@ static int dvb_input_attach(struct ddb_input *input)else par =1 ;if (demod_attach_cxd28xx (input, par, osc24) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_tda18212 (input, port->type) <0 ) goto err_tuner;break ;
@@ -1518,7 +1518,7 @@ static int dvb_input_attach(struct ddb_input *input)case DDB_TUNER_DVBC2T2_SONY :case DDB_TUNER_ISDBT_SONY :if (demod_attach_cxd28xx (input,0 , osc24) <0 ) -return -ENODEV ; + goto err_detach;if (tuner_attach_tda18212 (input, port->type) <0 ) goto err_tuner;break ;
@@ -1529,11 +1529,13 @@ static int dvb_input_attach(struct ddb_input *input)if (dvb->fe) {if (dvb_register_frontend (adap, dvb->fe) <0 ) -return -ENODEV ; + goto err_detach;if (dvb->fe2) { -if (dvb_register_frontend (adap, dvb->fe2) <0 ) -return -ENODEV ; +if (dvb_register_frontend (adap, dvb->fe2) <0 ) { +dvb_unregister_frontend (dvb->fe); + goto err_detach; + } dvb->fe2->tuner_priv = dvb->fe->tuner_priv; memcpy(&dvb->fe2->ops.tuner_ops, &dvb->fe->ops.tuner_ops,
@@ -1545,12 +1547,18 @@ static int dvb_input_attach(struct ddb_input *input)return 0 ; err_tuner: -dev_warn (port->dev->dev,"tuner attach failed!\n" ); +dev_err (port->dev->dev,"tuner attach failed!\n" );if (dvb->fe2)dvb_frontend_detach (dvb->fe2);if (dvb->fe)dvb_frontend_detach (dvb->fe); +err_detach: +dvb_input_detach (input); + +/* return error from ret if set */ +if (ret <0 ) +return ret;return -ENODEV ; }
@@ -1981,8 +1989,8 @@ void ddb_ports_detach(struct ddb *dev)switch (port->class) {case DDB_PORT_TUNER : -dvb_input_detach (port->input[0 ]);dvb_input_detach (port->input[1 ]); +dvb_input_detach (port->input[0 ]);break ;case DDB_PORT_CI :case DDB_PORT_LOOP :
Signature · 1 line
2.13.6
PATCH 6/8 ddbridge: fix deinit order in case of failure in ddb_init()
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
In ddb_init(), the deinitialization sequence isn't correct when handling errors, and could even lead to a memleak depending on where things failed. Fix the deinit order.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/media/pci/ddbridge/ddbridge-core.c +5 -5
@@ -3273,7 +3273,7 @@ int ddb_init(struct ddb *dev)ddb_init_boards (dev);if (ddb_i2c_init (dev) <0 ) - goto fail; + goto fail1;ddb_ports_init (dev);if (ddb_buffers_alloc (dev) <0 ) {dev_info (dev->dev,"Could not allocate buffer memory\n" );
@@ -3291,14 +3291,14 @@ int ddb_init(struct ddb *dev)return 0 ; fail3: -ddb_ports_detach (dev);dev_err (dev->dev,"fail3\n" ); -ddb_ports_release (dev); +ddb_ports_detach (dev); +ddb_buffers_free (dev); fail2:dev_err (dev->dev,"fail2\n" ); -ddb_buffers_free (dev); +ddb_ports_release (dev);ddb_i2c_release (dev); -fail: +fail1:dev_err (dev->dev,"fail1\n" );return -1 ; }
Signature · 1 line
2.13.6
PATCH 7/8 ddbridge: detach first input if the second one failed to init
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
In ddb_ports_attach(), if the second input of a dual tuner failed to initialise, the first one can be detached (and resources be freed) as this will be counted as the whole port having failed to initialise, thus the first one won't be used anyway.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/media/pci/ddbridge/ddbridge-core.c +3 -1
@@ -1935,8 +1935,10 @@ static int ddb_port_attach(struct ddb_port *port)if (ret <0 )break ; ret =dvb_input_attach (port->input[1 ]); -if (ret <0 ) +if (ret <0 ) { +dvb_input_detach (port->input[0 ]);break ; + } port->input[0 ]->redi = port->input[0 ]; port->input[1 ]->redi = port->input[1 ];break ;
Signature · 1 line
2.13.6
PATCH 8/8 ddbridge: improve ddb_ports_attach() failure handling
In reply to Daniel SchellerFrom: Daniel Scheller <[email protected]>
As all error handling improved quite a bit, don't stop attaching frontends if one of them failed, since - if other tuner modules are connected to the PCIe bridge - other hardware may just work, so don't break on a single port failure, but rather initialise as much as possible. Ie. if there are issues with a C2T2-equipped PCIe bridge card which has additional DuoFlex modules connected and the bridge generally works, the DuoFlex tuners can still work fine.
If all ports failed to initialise where connected hardware was detected on at first, return -ENODEV though to cause this PCI device to fail and free all allocated resources. In any case, leave a kernel log warning (or error, even) if things went wrong.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-)
drivers/media/pci/ddbridge/ddbridge-core.c +23 -3
@@ -1964,7 +1964,7 @@ static int ddb_port_attach(struct ddb_port *port)int ddb_ports_attach (struct ddb *dev) { -int i, ret =0 ; +int i, numports, err_ports =0 , ret =0 ;struct ddb_port *port;if (dev->port_num) {
@@ -1974,11 +1974,31 @@ int ddb_ports_attach(struct ddb *dev)return ret; } } + + numports = dev->port_num; +for (i =0 ; i < dev->port_num; i++) { port = &dev->port[i]; - ret =ddb_port_attach (port); +if (port->class !=DDB_PORT_NONE ) { + ret =ddb_port_attach (port); +if (ret) + err_ports++; + }else { + numports--; + } } -return ret; + +if (err_ports) { +if (err_ports == numports) { +dev_err (dev->dev,"All connected ports failed to initialise!\n" ); +return -ENODEV ; + } + +dev_warn (dev->dev,"%d of %d connected ports failed to initialise!\n" , + err_ports, numports); + } + +return 0 ; }void ddb_ports_detach (struct ddb *dev)
Signature · 1 line
2.13.6
Quoted 1 line · On Sun, 17 Dec 2017 16:40:41 +0100 Daniel Scheller <[email protected]>…
I verified this by simply removing tda18212.ko with this DD setup:
Sorry, I forgot to outline this: I also tested by removing stv0367.ko and cxd2841er.ko of course, which resulted in partially working hardware, either the cxd-based hardware or the stv-based one didn't work, while everything else did. Unload worked cleanly, without leaving any side effects.
All this worked without these patches before of course, but as they touch the attach logic and make failure cleanup resources instantly now, this required retesting.
Best regards,
Daniel Scheller
Signature · 1 line
https://github.com/herrnst