[PATCH 0/8] ddbridge improvements and cleanups

[PATCH 0/8] ddbridge improvements and cleanups

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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

-- 
2.13.6

[PATCH 1/8] [media] ddbridge: unregister I2C tuner client before detaching fe's

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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.

Fixes: 1502efd2d59 ("media: ddbridge: fix teardown/deregistration order in ddb_input_detach()")

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index eb43dcb4e5d2..eda004398316 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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:
-- 
2.13.6

[PATCH 2/8] [media] ddbridge: fix resources cleanup for CI hardware

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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.

Cc: Ralph Metzler <rjkm@metzlerbros.de>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index eda004398316..a81125d492ff 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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;
-- 
2.13.6

[PATCH 3/8] [media] ddbridge: deduplicate calls to dvb_ca_en50221_init()

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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.

Cc: Ralph Metzler <rjkm@metzlerbros.de>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-ci.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c b/drivers/media/pci/ddbridge/ddbridge-ci.c
index a4fd747763a0..8dfbc3bbd86d 100644
--- a/drivers/media/pci/ddbridge/ddbridge-ci.c
+++ b/drivers/media/pci/ddbridge/ddbridge-ci.c
@@ -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;
 }
-- 
2.13.6

[PATCH 4/8] [media] ddbridge: move CI detach code to ddbridge-ci.c

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

Move the CI teardown code to ddbridge-ci.c where everything else related
to CI hardware lives.

Cc: Ralph Metzler <rjkm@metzlerbros.de>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 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(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c b/drivers/media/pci/ddbridge/ddbridge-ci.c
index 8dfbc3bbd86d..5828111487b0 100644
--- a/drivers/media/pci/ddbridge/ddbridge-ci.c
+++ b/drivers/media/pci/ddbridge/ddbridge-ci.c
@@ -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;
+	}
+}
diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.h b/drivers/media/pci/ddbridge/ddbridge-ci.h
index 3a5d7ffab7b7..35a39182dd83 100644
--- a/drivers/media/pci/ddbridge/ddbridge-ci.h
+++ b/drivers/media/pci/ddbridge/ddbridge-ci.h
@@ -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__ */
diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index a81125d492ff..c2f028152388 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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;
 		}
 	}
-- 
2.13.6

[PATCH 5/8] [media] ddbridge: completely tear down input resources on failure

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 46 ++++++++++++++++++------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index c2f028152388..07f3e37a0fca 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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:
-- 
2.13.6

[PATCH 6/8] [media] ddbridge: fix deinit order in case of failure in ddb_init()

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 07f3e37a0fca..548b7047ca09 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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;
 }
-- 
2.13.6

[PATCH 7/8] [media] ddbridge: detach first input if the second one failed to init

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 548b7047ca09..940371067346 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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;
-- 
2.13.6

[PATCH 8/8] [media] ddbridge: improve ddb_ports_attach() failure handling

[Daniel Scheller]

From: Daniel Scheller <d.scheller@gmx.net>

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.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 940371067346..c7d923e0e21a 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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)
-- 
2.13.6

Re: [PATCH 0/8] ddbridge improvements and cleanups

[Daniel Scheller]

On Sun, 17 Dec 2017 16:40:41 +0100
Daniel Scheller <d.scheller.oss@gmail.com> wrote:

> 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
-- 
https://github.com/herrnst