[PATCH 0/4] misc cxd2099/Kconfig/ddbridge/ngene improvements

[PATCH 0/4] misc cxd2099/Kconfig/ddbridge/ngene improvements

[Daniel Scheller]

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

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

-- 
2.16.1

[PATCH 1/4] [media] dvb-frontends/cxd2099: Kconfig additions

[Daniel Scheller]

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

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.

Cc: Jasmin Jessich <jasmin@anw.at>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/dvb-frontends/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
index 2ad81907c714..fcfa1135557e 100644
--- a/drivers/media/dvb-frontends/Kconfig
+++ b/drivers/media/dvb-frontends/Kconfig
@@ -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.
 
-- 
2.16.1

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

[Daniel Scheller]

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

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 <olli.salonen@iki.fi>
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/dvb-frontends/Kconfig | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
index fcfa1135557e..687086cdb870 100644
--- a/drivers/media/dvb-frontends/Kconfig
+++ b/drivers/media/dvb-frontends/Kconfig
@@ -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
-- 
2.16.1

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

[Daniel Scheller]

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

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.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ddbridge/ddbridge-ci.c   | 33 ++++++--------------------
 drivers/media/pci/ddbridge/ddbridge-core.c | 37 +++++++-----------------------
 2 files changed, 15 insertions(+), 55 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c 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)
 {
 	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)
diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index f9bee36f1cad..36d2deb1c106 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -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);
-- 
2.16.1

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

[Mauro Carvalho Chehab]

Em Wed,  7 Mar 2018 20:23:49 +0100
Daniel Scheller <d.scheller.oss@gmail.com> escreveu:

> From: Daniel Scheller <d.scheller@gmx.net>
> 
> 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.
> 
> Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
> ---
>  drivers/media/pci/ddbridge/ddbridge-ci.c   | 33 ++++++--------------------
>  drivers/media/pci/ddbridge/ddbridge-core.c | 37 +++++++-----------------------
>  2 files changed, 15 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c 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)
>  {
>  	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);

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

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

[Daniel Scheller]

Am Wed, 7 Mar 2018 16:44:49 -0300
schrieb Mauro Carvalho Chehab <mchehab@kernel.org>:

> Em Wed,  7 Mar 2018 20:23:49 +0100
> Daniel Scheller <d.scheller.oss@gmail.com> escreveu:
> 
> > From: Daniel Scheller <d.scheller@gmx.net>
> > 
> > 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.
> > 
> > Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
> > ---
> >  drivers/media/pci/ddbridge/ddbridge-ci.c   | 33
> > ++++++--------------------
> > drivers/media/pci/ddbridge/ddbridge-core.c | 37
> > +++++++----------------------- 2 files changed, 15 insertions(+),
> > 55 deletions(-)
> > 
> > diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c
> > 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) {
> >  	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);  
> 
> 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.

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

V2 sent, skipped the cover letter though :-)

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst

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

[Daniel Scheller]

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

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.

Signed-off-by: Daniel Scheller <d.scheller@gmx.net>
---
 drivers/media/pci/ngene/ngene-cards.c | 25 ++++----------------
 drivers/media/pci/ngene/ngene-core.c  | 43 ++++++++---------------------------
 2 files changed, 15 insertions(+), 53 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-cards.c b/drivers/media/pci/ngene/ngene-cards.c
index 37e9f0eb6d20..3ae7da2e9858 100644
--- a/drivers/media/pci/ngene/ngene-cards.c
+++ b/drivers/media/pci/ngene/ngene-cards.c
@@ -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;
 
diff --git a/drivers/media/pci/ngene/ngene-core.c b/drivers/media/pci/ngene/ngene-core.c
index f69a8fc1ec2a..bef3c9fd75ce 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -1408,7 +1408,6 @@ static void release_channel(struct ngene_channel *chan)
 {
 	struct dvb_demux *dvbdemux = &chan->demux;
 	struct ngene *dev = chan->dev;
-	struct i2c_client *client;
 
 	if (chan->running)
 		set_transfer(chan, 0);
@@ -1427,12 +1426,9 @@ static void release_channel(struct ngene_channel *chan)
 		dvb_unregister_frontend(chan->fe);
 
 		/* release I2C client (tuner) if needed */
-		client = chan->i2c_client[0];
-		if (chan->i2c_client_fe && client) {
-			module_put(client->dev.driver->owner);
-			i2c_unregister_device(client);
+		if (chan->i2c_client_fe) {
+			dvb_module_release(chan->i2c_client[0]);
 			chan->i2c_client[0] = NULL;
-			client = NULL;
 		}
 
 		dvb_frontend_detach(chan->fe);
@@ -1584,11 +1580,6 @@ static void cxd_attach(struct ngene *dev)
 	struct ngene_ci *ci = &dev->ci;
 	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,
-	};
 	int ret;
 	u8 type;
 
@@ -1605,26 +1596,17 @@ static void cxd_attach(struct ngene *dev)
 	}
 
 	cxd_cfg.en = &ci->en;
-
-	request_module(board_info.type);
-
-	client = i2c_new_device(&dev->channel[0].i2c_adapter, &board_info);
-	if (!client || !client->dev.driver)
-		goto err_ret;
-
-	if (!try_module_get(client->dev.driver->owner))
-		goto err_i2c;
-
-	if (!ci->en)
-		goto err_i2c;
+	client = dvb_module_probe("cxd2099", "cxd2099",
+				  &dev->channel[0].i2c_adapter,
+				  0x40, &cxd_cfg);
+	if (!client)
+		goto err;
 
 	ci->dev = dev;
 	dev->channel[0].i2c_client[0] = client;
 	return;
 
-err_i2c:
-	i2c_unregister_device(client);
-err_ret:
+err:
 	dev_err(pdev, "CXD2099AR attach failed\n");
 	return;
 }
@@ -1632,16 +1614,11 @@ static void cxd_attach(struct ngene *dev)
 static void cxd_detach(struct ngene *dev)
 {
 	struct ngene_ci *ci = &dev->ci;
-	struct i2c_client *client;
 
 	dvb_ca_en50221_release(ci->en);
 
-	client = dev->channel[0].i2c_client[0];
-	if (client) {
-		module_put(client->dev.driver->owner);
-		i2c_unregister_device(client);
-	}
-
+	dvb_module_release(dev->channel[0].i2c_client[0]);
+	dev->channel[0].i2c_client[0] = NULL;
 	ci->en = NULL;
 }
 
-- 
2.16.1