Skip to contentSkip to conversation

misc cxd2099/Kconfig/ddbridge/ngene improvements

PATCH In review Daniel Scheller 2 participants 7 messages Mar 7, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover misc cxd2099/Kconfig/ddbridge/ngene improvements

From: Daniel Scheller <[email protected]>

This series improves this:

* The cxd2099 Kconfig block is improved to be more complete and to match those of the other drivers
* The sp2 CI driver is moved down to the new CI EN50221 subsection in the Kconfig of the dvb-frontends/ subdir
* Both ddbridge's and ngene's I2C client handling is cleaned up to make use of the new dvb_module_*() I2C handling helpers.

Daniel Scheller (4):
[media] dvb-frontends/cxd2099: Kconfig additions
[media] dvb-frontends/Kconfig: move the SP2 driver to the CI section [media] ddbridge: use common DVB I2C client handling helpers [media] ngene: use common DVB I2C client handling helpers

 drivers/media/dvb-frontends/Kconfig        | 18 +++++++------
 drivers/media/pci/ddbridge/ddbridge-ci.c   | 33 +++++------------------
 drivers/media/pci/ddbridge/ddbridge-core.c | 37 ++++++-------------------
 drivers/media/pci/ngene/ngene-cards.c      | 25 ++++-------------
 drivers/media/pci/ngene/ngene-core.c       | 43 +++++++-----------------------
 5 files changed, 40 insertions(+), 116 deletions(-)
Signature · 1 line
2.16.1
#1 lore raw

PATCH 1/4 dvb-frontends/cxd2099: Kconfig additions

From: Daniel Scheller <[email protected]>

The cxd2099 driver makes use of the Regmap I2C kernel API, thus add "select REGMAP_I2C" to it's Kconfig block. Also, make it default "m" if !MEDIA_SUBDRV_AUTOSELECT, just like every other dvb-frontend driver. And, while at it, remove the hyphens around the help tag.

---
 drivers/media/dvb-frontends/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/media/dvb-frontends/Kconfig +3 -1
index 2ad81907c714..fcfa1135557e 100644
@@ -912,7 +912,9 @@ comment "Common Interface (EN50221) controller drivers"
 config DVB_CXD2099
 	tristate "CXD2099AR Common Interface driver"
 	depends on DVB_CORE && I2C
-	---help---
+	select REGMAP_I2C
+	default m if !MEDIA_SUBDRV_AUTOSELECT
+	help
 	  A driver for the CI controller currently found mostly on
 	  Digital Devices DuoFlex CI (single) addon modules.
 
Signature · 1 line
2.16.1
#2 lore raw

PATCH 2/4 dvb-frontends/Kconfig: move the SP2 driver to the CI section

In reply to Daniel Scheller

From: Daniel Scheller <[email protected]>

The CIMaX SP2 driver is a EN50221 CI controller I2C driver similar to the cxd2099 driver. Move it's Kconfig block into the newly introduced CI subsection.

Cc Olli Salonen <[email protected]>
Cc Antti Palosaari <[email protected]>
---
 drivers/media/dvb-frontends/Kconfig | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/media/dvb-frontends/Kconfig +7 -7
index fcfa1135557e..687086cdb870 100644
@@ -824,13 +824,6 @@ config DVB_A8293
 	depends on DVB_CORE && I2C
 	default m if !MEDIA_SUBDRV_AUTOSELECT
 
-config DVB_SP2
-	tristate "CIMaX SP2"
-	depends on DVB_CORE && I2C
-	default m if !MEDIA_SUBDRV_AUTOSELECT
-	help
-	  CIMaX SP2/SP2HF Common Interface module.
-
 config DVB_LGS8GL5
 	tristate "Silicon Legend LGS-8GL5 demodulator (OFDM)"
 	depends on DVB_CORE && I2C
@@ -920,6 +913,13 @@ config DVB_CXD2099
 
 	  Say Y when you want to support these devices.
 
+config DVB_SP2
+	tristate "CIMaX SP2"
+	depends on DVB_CORE && I2C
+	default m if !MEDIA_SUBDRV_AUTOSELECT
+	help
+	  CIMaX SP2/SP2HF Common Interface module.
+
 comment "Tools to develop new frontends"
 
 config DVB_DUMMY_FE
Signature · 1 line
2.16.1
#3 lore raw

PATCH 3/4 ddbridge: use common DVB I2C client handling helpers

In reply to Daniel Scheller

From: Daniel Scheller <[email protected]>

Instead of keeping duplicated I2C client handling construct, make use of the newly introduced dvb_module_*() helpers. This not only keeps things way cleaner and removes the need for duplicated I2C client attach code, but even allows to get rid of some variables that won't help in making things look cleaner anymore.

The check on a valid ptr on port->en isn't really needed since the cxd2099 driver will set it at a time where it is going to return successfully from probing.

---
 drivers/media/pci/ddbridge/ddbridge-ci.c   | 33 ++++++--------------------
 drivers/media/pci/ddbridge/ddbridge-core.c | 37 +++++++-----------------------
 2 files changed, 15 insertions(+), 55 deletions(-)
drivers/media/pci/ddbridge/ddbridge-ci.c +7 -26
index 6585ef54ac22..d0ce6a1f1bd0 100644
@@ -324,34 +324,20 @@ static int ci_cxd2099_attach(struct ddb_port *port, u32 bitrate)
 {
 	struct cxd2099_cfg cxd_cfg = cxd_cfgtmpl;
 	struct i2c_client *client;
-	struct i2c_board_info board_info = {
-		.type = "cxd2099",
-		.addr = 0x40,
-		.platform_data = &cxd_cfg,
-	};
 
 	cxd_cfg.bitrate = bitrate;
 	cxd_cfg.en = &port->en;
 
-	request_module(board_info.type);
-
-	client = i2c_new_device(&port->i2c->adap, &board_info);
-	if (!client || !client->dev.driver)
-		goto err_ret;
-
-	if (!try_module_get(client->dev.driver->owner))
-		goto err_i2c;
-
-	if (!port->en)
-		goto err_i2c;
+	client = dvb_module_probe("cxd2099", "cxd2099", &port->i2c->adap,
+				  0x40, &cxd_cfg);
+	if (!client)
+		goto err;
 
 	port->dvb[0].i2c_client[0] = client;
 	port->en_freedata = 0;
 	return 0;
 
-err_i2c:
-	i2c_unregister_device(client);
-err_ret:
+err:
 	dev_err(port->dev->dev, "CXD2099AR attach failed\n");
 	return -ENODEV;
 }
@@ -385,18 +371,13 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate)
 
 void ddb_ci_detach(struct ddb_port *port)
 {
-	struct i2c_client *client;
-
 	if (port->dvb[0].dev)
 		dvb_unregister_device(port->dvb[0].dev);
 	if (port->en) {
 		dvb_ca_en50221_release(port->en);
 
-		client = port->dvb[0].i2c_client[0];
-		if (client) {
-			module_put(client->dev.driver->owner);
-			i2c_unregister_device(client);
-		}
+		dvb_module_release(port->dvb[0].i2c_client[0]);
+		port->dvb[0].i2c_client[0] = NULL;
 
 		/* free alloc'ed memory if needed */
 		if (port->en_freedata)
drivers/media/pci/ddbridge/ddbridge-core.c +8 -29
index f9bee36f1cad..36d2deb1c106 100644
@@ -999,37 +999,22 @@ static int tuner_attach_tda18212(struct ddb_input *input, u32 porttype)
 		.if_dvbt2_8 = 4000,
 		.if_dvbc = 5000,
 	};
-	struct i2c_board_info board_info = {
-		.type = "tda18212",
-		.platform_data = &config,
-	};
-
-	if (input->nr & 1)
-		board_info.addr = 0x63;
-	else
-		board_info.addr = 0x60;
+	u8 addr = (input->nr & 1) ? 0x63 : 0x60;
 
 	/* due to a hardware quirk with the I2C gate on the stv0367+tda18212
 	 * combo, the tda18212 must be probed by reading it's id _twice_ when
 	 * cold started, or it very likely will fail.
 	 */
 	if (porttype == DDB_TUNER_DVBCT_ST)
-		tuner_tda18212_ping(input, board_info.addr);
-
-	request_module(board_info.type);
-
-	/* perform tuner init/attach */
-	client = i2c_new_device(adapter, &board_info);
-	if (!client || !client->dev.driver)
-		goto err;
+		tuner_tda18212_ping(input, addr);
 
-	if (!try_module_get(client->dev.driver->owner)) {
-		i2c_unregister_device(client);
+	/* perform tuner probe/init/attach */
+	client = dvb_module_probe("tda18212", "tda18212",
+				  adapter, addr, &config);
+	if (!client)
 		goto err;
-	}
 
 	dvb->i2c_client[0] = client;
-
 	return 0;
 err:
 	dev_err(dev, "TDA18212 tuner not found. Device is not fully operational.\n");
@@ -1253,7 +1238,6 @@ static void dvb_input_detach(struct ddb_input *input)
 {
 	struct ddb_dvb *dvb = &input->port->dvb[input->nr & 1];
 	struct dvb_demux *dvbdemux = &dvb->demux;
-	struct i2c_client *client;
 
 	switch (dvb->attached) {
 	case 0x31:
@@ -1263,13 +1247,8 @@ 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;
-		}
+		dvb_module_release(dvb->i2c_client[0]);
+		dvb->i2c_client[0] = NULL;
 
 		if (dvb->fe2)
 			dvb_frontend_detach(dvb->fe2);
Signature · 1 line
2.16.1
#4 lore raw

Re: [PATCH 3/4] [media] ddbridge: use common DVB I2C client handling helpers

Em Wed, 7 Mar 2018 20:23:49 +0100
Daniel Scheller <[email protected]> escreveu:

Quoted 38 lines

From: Daniel Scheller <[email protected]>

Instead of keeping duplicated I2C client handling construct, make use of the newly introduced dvb_module_*() helpers. This not only keeps things way cleaner and removes the need for duplicated I2C client attach code, but even allows to get rid of some variables that won't help in making things look cleaner anymore.

The check on a valid ptr on port->en isn't really needed since the cxd2099 driver will set it at a time where it is going to return successfully from probing.

---
 drivers/media/pci/ddbridge/ddbridge-ci.c   | 33 ++++++--------------------
 drivers/media/pci/ddbridge/ddbridge-core.c | 37 +++++++-----------------------
 2 files changed, 15 insertions(+), 55 deletions(-)
drivers/media/pci/ddbridge/ddbridge-ci.c +0 -8
index 6585ef54ac22..d0ce6a1f1bd0 100644
@@ -324,34 +324,20 @@ static int ci_cxd2099_attach(struct ddb_port *port, u32 bitrate)
 {
 	struct cxd2099_cfg cxd_cfg = cxd_cfgtmpl;
 	struct i2c_client *client;
-	struct i2c_board_info board_info = {
-		.type = "cxd2099",
-		.addr = 0x40,
-		.platform_data = &cxd_cfg,
-	};
 
 	cxd_cfg.bitrate = bitrate;
 	cxd_cfg.en = &port->en;
 
-	request_module(board_info.type);
-
-	client = i2c_new_device(&port->i2c->adap, &board_info);

drivers/media/pci/ddbridge/ddbridge-ci.c static int ci_cxd2099_attach(struct ddb_port *port, u32 bitrate) · line 332

Quoted 10 lines
drivers/media/pci/ddbridge/ddbridge-ci.c +2 -8
-	if (!client || !client->dev.driver)
-		goto err_ret;
-
-	if (!try_module_get(client->dev.driver->owner))
-		goto err_i2c;
-
-	if (!port->en)
-		goto err_i2c;
+	client = dvb_module_probe("cxd2099", "cxd2099", &port->i2c->adap,
+				  0x40, &cxd_cfg);

Here and on all similar calls, there's no need to duplicate the name, if they're identical. Just use NULL at the second time, e. g.:

	client = dvb_module_probe("cxd2099", NULL, &port->i2c->adap,
				  0x40, &cxd_cfg);

The dvb_module_probe() will use the same string for both.

Regards,
Mauro

#5 lore raw

Re: [PATCH 3/4] [media] ddbridge: use common DVB I2C client handling helpers

Am Wed, 7 Mar 2018 16:44:49 -0300
schrieb Mauro Carvalho Chehab <[email protected]>:

Quoted 58 lines

Em Wed, 7 Mar 2018 20:23:49 +0100
Daniel Scheller <[email protected]> escreveu:

Quoted 50 lines

From: Daniel Scheller <[email protected]>

Instead of keeping duplicated I2C client handling construct, make use of the newly introduced dvb_module_*() helpers. This not only keeps things way cleaner and removes the need for duplicated I2C client attach code, but even allows to get rid of some variables that won't help in making things look cleaner anymore.

The check on a valid ptr on port->en isn't really needed since the cxd2099 driver will set it at a time where it is going to return successfully from probing.

---
 drivers/media/pci/ddbridge/ddbridge-ci.c   | 33

++++++--------------------
drivers/media/pci/ddbridge/ddbridge-core.c | 37 +++++++----------------------- 2 files changed, 15 insertions(+), 55 deletions(-)

(quoted diff) +0 -0

b/drivers/media/pci/ddbridge/ddbridge-ci.c index 6585ef54ac22..d0ce6a1f1bd0 100644 --- a/drivers/media/pci/ddbridge/ddbridge-ci.c +++ b/drivers/media/pci/ddbridge/ddbridge-ci.c @@ -324,34 +324,20 @@ static int ci_cxd2099_attach(struct ddb_port *port, u32 bitrate) {

drivers/media/pci/ddbridge/ddbridge-ci.c +1 -16
 	struct cxd2099_cfg cxd_cfg = cxd_cfgtmpl;
 	struct i2c_client *client;
-	struct i2c_board_info board_info = {
-		.type = "cxd2099",
-		.addr = 0x40,
-		.platform_data = &cxd_cfg,
-	};
 
 	cxd_cfg.bitrate = bitrate;
 	cxd_cfg.en = &port->en;
 
-	request_module(board_info.type);
-
-	client = i2c_new_device(&port->i2c->adap, &board_info);
-	if (!client || !client->dev.driver)
-		goto err_ret;
-
-	if (!try_module_get(client->dev.driver->owner))
-		goto err_i2c;
-
-	if (!port->en)
-		goto err_i2c;
+	client = dvb_module_probe("cxd2099", "cxd2099",

&port->i2c->adap,

drivers/media/pci/ddbridge/ddbridge-ci.c static int ci_cxd2099_attach(struct ddb_port *port, u32 bitrate) · line 332

Quoted 1 line
drivers/media/pci/ddbridge/ddbridge-ci.c +1 -0
+				  0x40, &cxd_cfg);  

Here and on all similar calls, there's no need to duplicate the name, if they're identical. Just use NULL at the second time, e. g.:

Quoted 4 lines
	client = dvb_module_probe("cxd2099", NULL, &port->i2c->adap,
				  0x40, &cxd_cfg);

The dvb_module_probe() will use the same string for both.

Convinced (wanted to make this look "complete" and "proper").

V2 sent, skipped the cover letter though :-)

Best regards,
Daniel Scheller

Signature · 1 line
https://github.com/herrnst
#6 lore raw

PATCH 4/4 ngene: use common DVB I2C client handling helpers

In reply to Daniel Scheller

From: Daniel Scheller <[email protected]>

Like in ddbridge, get rid of all duplicated I2C client handling constructs and rather make use of the newly added dvb_module_*() helpers. Makes things more clean and removes the (cosmetic) need for some variables.

The check on a valid ptr on ci->en isn't really needed since the cxd2099
driver will set it at a time where it is going to return successfully
from probing.

---
 drivers/media/pci/ngene/ngene-cards.c | 25 ++++----------------
 drivers/media/pci/ngene/ngene-core.c  | 43 ++++++++---------------------------
 2 files changed, 15 insertions(+), 53 deletions(-)
drivers/media/pci/ngene/ngene-cards.c +5 -20
index 37e9f0eb6d20..3ae7da2e9858 100644
@@ -253,15 +253,7 @@ static int tuner_attach_tda18212(struct ngene_channel *chan, u32 dmdtype)
 		.if_dvbt2_8 = 4000,
 		.if_dvbc = 5000,
 	};
-	struct i2c_board_info board_info = {
-		.type = "tda18212",
-		.platform_data = &config,
-	};
-
-	if (chan->number & 1)
-		board_info.addr = 0x63;
-	else
-		board_info.addr = 0x60;
+	u8 addr = (chan->number & 1) ? 0x63 : 0x60;
 
 	/*
 	 * due to a hardware quirk with the I2C gate on the stv0367+tda18212
@@ -269,20 +261,13 @@ static int tuner_attach_tda18212(struct ngene_channel *chan, u32 dmdtype)
 	 * cold started, or it very likely will fail.
 	 */
 	if (dmdtype == DEMOD_TYPE_STV0367)
-		tuner_tda18212_ping(chan, i2c, board_info.addr);
-
-	request_module(board_info.type);
+		tuner_tda18212_ping(chan, i2c, addr);
 
-	/* perform tuner init/attach */
-	client = i2c_new_device(i2c, &board_info);
-	if (!client || !client->dev.driver)
+	/* perform tuner probe/init/attach */
+	client = dvb_module_probe("tda18212", "tda18212", i2c, addr, &config);
+	if (!client)
 		goto err;
 
-	if (!try_module_get(client->dev.driver->owner)) {
-		i2c_unregister_device(client);
-		goto err;
-	}
-
 	chan->i2c_client[0] = client;
 	chan->i2c_client_fe = 1;
 
drivers/media/pci/ngene/ngene-core.c +10 -33

82 lines not on this page · raw message

Signature · 1 line
2.16.1