* [PATCH v2 1/8] tda18212: add support for slave chip version
@ 2014-09-07 1:59 Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 2/8] tda18212: prepare for I2C client conversion Antti Palosaari
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 1:59 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
There is 2 different versions of that chip available, master and
slave. Slave is used only on dual tuner devices with master tuner.
Laser printing top of chip is 18212/M or 18212/S according to chip
version.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/tuners/tda18212.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
index 05a4ac9..15b09f8 100644
--- a/drivers/media/tuners/tda18212.c
+++ b/drivers/media/tuners/tda18212.c
@@ -306,7 +306,8 @@ struct dvb_frontend *tda18212_attach(struct dvb_frontend *fe,
{
struct tda18212_priv *priv = NULL;
int ret;
- u8 val;
+ u8 chip_id = chip_id;
+ char *version;
priv = kzalloc(sizeof(struct tda18212_priv), GFP_KERNEL);
if (priv == NULL)
@@ -320,26 +321,38 @@ struct dvb_frontend *tda18212_attach(struct dvb_frontend *fe,
fe->ops.i2c_gate_ctrl(fe, 1); /* open I2C-gate */
/* check if the tuner is there */
- ret = tda18212_rd_reg(priv, 0x00, &val);
+ ret = tda18212_rd_reg(priv, 0x00, &chip_id);
+ dev_dbg(&priv->i2c->dev, "%s: chip_id=%02x\n", __func__, chip_id);
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 0); /* close I2C-gate */
- if (!ret)
- dev_dbg(&priv->i2c->dev, "%s: chip id=%02x\n", __func__, val);
- if (ret || val != 0xc7) {
- kfree(priv);
- return NULL;
+ if (ret)
+ goto err;
+
+ switch (chip_id) {
+ case 0xc7:
+ version = "M"; /* master */
+ break;
+ case 0x47:
+ version = "S"; /* slave */
+ break;
+ default:
+ goto err;
}
dev_info(&priv->i2c->dev,
- "%s: NXP TDA18212HN successfully identified\n",
- KBUILD_MODNAME);
+ "%s: NXP TDA18212HN/%s successfully identified\n",
+ KBUILD_MODNAME, version);
memcpy(&fe->ops.tuner_ops, &tda18212_tuner_ops,
sizeof(struct dvb_tuner_ops));
return fe;
+err:
+ dev_dbg(&i2c->dev, "%s: failed=%d\n", __func__, ret);
+ kfree(priv);
+ return NULL;
}
EXPORT_SYMBOL(tda18212_attach);
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/8] tda18212: prepare for I2C client conversion
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
@ 2014-09-07 1:59 ` Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client Antti Palosaari
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 1:59 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
We need carry pointer to frontend via config struct
(I2C platform_data ptr) when I2C model is used. Add that pointer
first in order to keep build unbreakable during conversion.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/tuners/tda18212.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/media/tuners/tda18212.h b/drivers/media/tuners/tda18212.h
index c36b49e..265559a 100644
--- a/drivers/media/tuners/tda18212.h
+++ b/drivers/media/tuners/tda18212.h
@@ -37,6 +37,11 @@ struct tda18212_config {
u16 if_dvbc;
u16 if_atsc_vsb;
u16 if_atsc_qam;
+
+ /*
+ * pointer to DVB frontend
+ */
+ struct dvb_frontend *fe;
};
#if IS_ENABLED(CONFIG_MEDIA_TUNER_TDA18212)
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 2/8] tda18212: prepare for I2C client conversion Antti Palosaari
@ 2014-09-07 1:59 ` Antti Palosaari
2014-09-18 12:31 ` Mauro Carvalho Chehab
2014-09-07 1:59 ` [PATCH v2 4/8] em28xx: " Antti Palosaari
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 1:59 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
Used tda18212 tuner is implemented as I2C driver. Implement I2C
client to anysee and use it for tda18212.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/usb/dvb-usb-v2/anysee.c | 185 +++++++++++++++++++++++++++-------
drivers/media/usb/dvb-usb-v2/anysee.h | 3 +
2 files changed, 152 insertions(+), 36 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c b/drivers/media/usb/dvb-usb-v2/anysee.c
index e4a2382..d3c5f23 100644
--- a/drivers/media/usb/dvb-usb-v2/anysee.c
+++ b/drivers/media/usb/dvb-usb-v2/anysee.c
@@ -332,7 +332,6 @@ static struct tda10023_config anysee_tda10023_tda18212_config = {
};
static struct tda18212_config anysee_tda18212_config = {
- .i2c_address = (0xc0 >> 1),
.if_dvbt_6 = 4150,
.if_dvbt_7 = 4150,
.if_dvbt_8 = 4150,
@@ -340,7 +339,6 @@ static struct tda18212_config anysee_tda18212_config = {
};
static struct tda18212_config anysee_tda18212_config2 = {
- .i2c_address = 0x60 /* (0xc0 >> 1) */,
.if_dvbt_6 = 3550,
.if_dvbt_7 = 3700,
.if_dvbt_8 = 4150,
@@ -632,6 +630,92 @@ error:
return ret;
}
+static int anysee_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
+ void *platform_data)
+{
+ int ret, num;
+ struct anysee_state *state = d_to_priv(d);
+ struct i2c_client *client;
+ struct i2c_adapter *adapter = &d->i2c_adap;
+ struct i2c_board_info board_info = {
+ .addr = addr,
+ .platform_data = platform_data,
+ };
+
+ strlcpy(board_info.type, type, I2C_NAME_SIZE);
+
+ /* find first free client */
+ for (num = 0; num < ANYSEE_I2C_CLIENT_MAX; num++) {
+ if (state->i2c_client[num] == NULL)
+ break;
+ }
+
+ dev_dbg(&d->udev->dev, "%s: num=%d\n", __func__, num);
+
+ if (num == ANYSEE_I2C_CLIENT_MAX) {
+ dev_err(&d->udev->dev, "%s: I2C client out of index\n",
+ KBUILD_MODNAME);
+ ret = -ENODEV;
+ goto err;
+ }
+
+ request_module(board_info.type);
+
+ /* register I2C device */
+ client = i2c_new_device(adapter, &board_info);
+ if (client == NULL || client->dev.driver == NULL) {
+ ret = -ENODEV;
+ goto err;
+ }
+
+ /* increase I2C driver usage count */
+ if (!try_module_get(client->dev.driver->owner)) {
+ i2c_unregister_device(client);
+ ret = -ENODEV;
+ goto err;
+ }
+
+ state->i2c_client[num] = client;
+ return 0;
+err:
+ dev_dbg(&d->udev->dev, "%s: failed=%d\n", __func__, ret);
+ return ret;
+}
+
+static void anysee_del_i2c_dev(struct dvb_usb_device *d)
+{
+ int num;
+ struct anysee_state *state = d_to_priv(d);
+ struct i2c_client *client;
+
+ /* find last used client */
+ num = ANYSEE_I2C_CLIENT_MAX;
+ while (num--) {
+ if (state->i2c_client[num] != NULL)
+ break;
+ }
+
+ dev_dbg(&d->udev->dev, "%s: num=%d\n", __func__, num);
+
+ if (num == -1) {
+ dev_err(&d->udev->dev, "%s: I2C client out of index\n",
+ KBUILD_MODNAME);
+ goto err;
+ }
+
+ client = state->i2c_client[num];
+
+ /* decrease I2C driver usage count */
+ module_put(client->dev.driver->owner);
+
+ /* unregister I2C device */
+ i2c_unregister_device(client);
+
+ state->i2c_client[num] = NULL;
+err:
+ dev_dbg(&d->udev->dev, "%s: failed\n", __func__);
+}
+
static int anysee_frontend_attach(struct dvb_usb_adapter *adap)
{
struct anysee_state *state = adap_to_priv(adap);
@@ -640,12 +724,12 @@ static int anysee_frontend_attach(struct dvb_usb_adapter *adap)
u8 tmp;
struct i2c_msg msg[2] = {
{
- .addr = anysee_tda18212_config.i2c_address,
+ .addr = 0x60,
.flags = 0,
.len = 1,
.buf = "\x00",
}, {
- .addr = anysee_tda18212_config.i2c_address,
+ .addr = 0x60,
.flags = I2C_M_RD,
.len = 1,
.buf = &tmp,
@@ -723,9 +807,11 @@ static int anysee_frontend_attach(struct dvb_usb_adapter *adap)
/* probe TDA18212 */
tmp = 0;
ret = i2c_transfer(&d->i2c_adap, msg, 2);
- if (ret == 2 && tmp == 0xc7)
+ if (ret == 2 && tmp == 0xc7) {
dev_dbg(&d->udev->dev, "%s: TDA18212 found\n",
__func__);
+ state->has_tda18212 = true;
+ }
else
tmp = 0;
@@ -939,46 +1025,63 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
* fails attach old simple PLL. */
/* attach tuner */
- fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
- &anysee_tda18212_config);
+ if (state->has_tda18212) {
+ struct tda18212_config tda18212_config =
+ anysee_tda18212_config;
- if (fe && adap->fe[1]) {
- /* attach tuner for 2nd FE */
- fe = dvb_attach(tda18212_attach, adap->fe[1],
- &d->i2c_adap, &anysee_tda18212_config);
- break;
- } else if (fe) {
- break;
- }
-
- /* attach tuner */
- fe = dvb_attach(dvb_pll_attach, adap->fe[0], (0xc0 >> 1),
- &d->i2c_adap, DVB_PLL_SAMSUNG_DTOS403IH102A);
+ tda18212_config.fe = adap->fe[0];
+ ret = anysee_add_i2c_dev(d, "tda18212", 0x60,
+ &tda18212_config);
+ if (ret)
+ goto err;
+
+ /* copy tuner ops for 2nd FE as tuner is shared */
+ if (adap->fe[1]) {
+ adap->fe[1]->tuner_priv =
+ adap->fe[0]->tuner_priv;
+ memcpy(&adap->fe[1]->ops.tuner_ops,
+ &adap->fe[0]->ops.tuner_ops,
+ sizeof(struct dvb_tuner_ops));
+ }
- if (fe && adap->fe[1]) {
- /* attach tuner for 2nd FE */
- fe = dvb_attach(dvb_pll_attach, adap->fe[1],
+ return 0;
+ } else {
+ /* attach tuner */
+ fe = dvb_attach(dvb_pll_attach, adap->fe[0],
(0xc0 >> 1), &d->i2c_adap,
DVB_PLL_SAMSUNG_DTOS403IH102A);
+
+ if (fe && adap->fe[1]) {
+ /* attach tuner for 2nd FE */
+ fe = dvb_attach(dvb_pll_attach, adap->fe[1],
+ (0xc0 >> 1), &d->i2c_adap,
+ DVB_PLL_SAMSUNG_DTOS403IH102A);
+ }
}
break;
case ANYSEE_HW_508TC: /* 18 */
case ANYSEE_HW_508PTC: /* 21 */
+ {
/* E7 TC */
/* E7 PTC */
+ struct tda18212_config tda18212_config = anysee_tda18212_config;
- /* attach tuner */
- fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
- &anysee_tda18212_config);
-
- if (fe) {
- /* attach tuner for 2nd FE */
- fe = dvb_attach(tda18212_attach, adap->fe[1],
- &d->i2c_adap, &anysee_tda18212_config);
+ tda18212_config.fe = adap->fe[0];
+ ret = anysee_add_i2c_dev(d, "tda18212", 0x60, &tda18212_config);
+ if (ret)
+ goto err;
+
+ /* copy tuner ops for 2nd FE as tuner is shared */
+ if (adap->fe[1]) {
+ adap->fe[1]->tuner_priv = adap->fe[0]->tuner_priv;
+ memcpy(&adap->fe[1]->ops.tuner_ops,
+ &adap->fe[0]->ops.tuner_ops,
+ sizeof(struct dvb_tuner_ops));
}
- break;
+ return 0;
+ }
case ANYSEE_HW_508S2: /* 19 */
case ANYSEE_HW_508PS2: /* 22 */
/* E7 S2 */
@@ -997,13 +1100,18 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
break;
case ANYSEE_HW_508T2C: /* 20 */
+ {
/* E7 T2C */
+ struct tda18212_config tda18212_config =
+ anysee_tda18212_config2;
- /* attach tuner */
- fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
- &anysee_tda18212_config2);
+ tda18212_config.fe = adap->fe[0];
+ ret = anysee_add_i2c_dev(d, "tda18212", 0x60, &tda18212_config);
+ if (ret)
+ goto err;
- break;
+ return 0;
+ }
default:
fe = NULL;
}
@@ -1012,7 +1120,7 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
ret = 0;
else
ret = -ENODEV;
-
+err:
return ret;
}
@@ -1270,6 +1378,11 @@ static int anysee_init(struct dvb_usb_device *d)
static void anysee_exit(struct dvb_usb_device *d)
{
+ struct anysee_state *state = d_to_priv(d);
+
+ if (state->i2c_client[0])
+ anysee_del_i2c_dev(d);
+
return anysee_ci_release(d);
}
diff --git a/drivers/media/usb/dvb-usb-v2/anysee.h b/drivers/media/usb/dvb-usb-v2/anysee.h
index 8f426d9..3ca2bca 100644
--- a/drivers/media/usb/dvb-usb-v2/anysee.h
+++ b/drivers/media/usb/dvb-usb-v2/anysee.h
@@ -55,8 +55,11 @@ struct anysee_state {
u8 buf[64];
u8 seq;
u8 hw; /* PCB ID */
+ #define ANYSEE_I2C_CLIENT_MAX 1
+ struct i2c_client *i2c_client[ANYSEE_I2C_CLIENT_MAX];
u8 fe_id:1; /* frondend ID */
u8 has_ci:1;
+ u8 has_tda18212:1;
u8 ci_attached:1;
struct dvb_ca_en50221 ci;
unsigned long ci_cam_ready; /* jiffies */
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/8] em28xx: convert tda18212 tuner to I2C client
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 2/8] tda18212: prepare for I2C client conversion Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client Antti Palosaari
@ 2014-09-07 1:59 ` Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 5/8] tda18212: convert driver to I2C binding Antti Palosaari
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 1:59 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
Used tda18212 tuner is implemented as a I2C driver. Use em28xx
tuner I2C client for tda18212 driver.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/usb/em28xx/em28xx-dvb.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 0645793..9682c52 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -373,7 +373,6 @@ static struct tda18271_config kworld_ub435q_v2_config = {
};
static struct tda18212_config kworld_ub435q_v3_config = {
- .i2c_address = 0x60,
.if_atsc_vsb = 3600,
.if_atsc_qam = 3600,
};
@@ -1437,6 +1436,15 @@ static int em28xx_dvb_init(struct em28xx *dev)
}
break;
case EM2874_BOARD_KWORLD_UB435Q_V3:
+ {
+ struct i2c_client *client;
+ struct i2c_adapter *adapter = &dev->i2c_adap[dev->def_i2c_bus];
+ struct i2c_board_info board_info = {
+ .type = "tda18212",
+ .addr = 0x60,
+ .platform_data = &kworld_ub435q_v3_config,
+ };
+
dvb->fe[0] = dvb_attach(lgdt3305_attach,
&em2874_lgdt3305_nogate_dev,
&dev->i2c_adap[dev->def_i2c_bus]);
@@ -1445,14 +1453,26 @@ static int em28xx_dvb_init(struct em28xx *dev)
goto out_free;
}
- /* Attach the demodulator. */
- if (!dvb_attach(tda18212_attach, dvb->fe[0],
- &dev->i2c_adap[dev->def_i2c_bus],
- &kworld_ub435q_v3_config)) {
- result = -EINVAL;
+ /* attach tuner */
+ kworld_ub435q_v3_config.fe = dvb->fe[0];
+ request_module("tda18212");
+ client = i2c_new_device(adapter, &board_info);
+ if (client == NULL || client->dev.driver == NULL) {
+ dvb_frontend_detach(dvb->fe[0]);
+ result = -ENODEV;
goto out_free;
}
+
+ if (!try_module_get(client->dev.driver->owner)) {
+ i2c_unregister_device(client);
+ dvb_frontend_detach(dvb->fe[0]);
+ result = -ENODEV;
+ goto out_free;
+ }
+
+ dvb->i2c_client_tuner = client;
break;
+ }
case EM2874_BOARD_PCTV_HD_MINI_80E:
dvb->fe[0] = dvb_attach(drx39xxj_attach, &dev->i2c_adap[dev->def_i2c_bus]);
if (dvb->fe[0] != NULL) {
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/8] tda18212: convert driver to I2C binding
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
` (2 preceding siblings ...)
2014-09-07 1:59 ` [PATCH v2 4/8] em28xx: " Antti Palosaari
@ 2014-09-07 1:59 ` Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 6/8] tda18212: clean logging Antti Palosaari
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 1:59 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
Convert driver from DVB proprietary model to common I2C model.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/tuners/tda18212.c | 129 ++++++++++++++++++++++++----------------
drivers/media/tuners/tda18212.h | 14 -----
2 files changed, 79 insertions(+), 64 deletions(-)
diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
index 15b09f8..659787b 100644
--- a/drivers/media/tuners/tda18212.c
+++ b/drivers/media/tuners/tda18212.c
@@ -24,8 +24,8 @@
#define MAX_XFER_SIZE 64
struct tda18212_priv {
- struct tda18212_config *cfg;
- struct i2c_adapter *i2c;
+ struct tda18212_config cfg;
+ struct i2c_client *client;
u32 if_frequency;
};
@@ -38,7 +38,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[1] = {
{
- .addr = priv->cfg->i2c_address,
+ .addr = priv->client->addr,
.flags = 0,
.len = 1 + len,
.buf = buf,
@@ -46,7 +46,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
};
if (1 + len > sizeof(buf)) {
- dev_warn(&priv->i2c->dev,
+ dev_warn(&priv->client->dev,
"%s: i2c wr reg=%04x: len=%d is too big!\n",
KBUILD_MODNAME, reg, len);
return -EINVAL;
@@ -55,12 +55,12 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
buf[0] = reg;
memcpy(&buf[1], val, len);
- ret = i2c_transfer(priv->i2c, msg, 1);
+ ret = i2c_transfer(priv->client->adapter, msg, 1);
if (ret == 1) {
ret = 0;
} else {
- dev_warn(&priv->i2c->dev, "%s: i2c wr failed=%d reg=%02x " \
- "len=%d\n", KBUILD_MODNAME, ret, reg, len);
+ dev_warn(&priv->client->dev, "%s: i2c wr failed=%d reg=%02x len=%d\n",
+ KBUILD_MODNAME, ret, reg, len);
ret = -EREMOTEIO;
}
return ret;
@@ -74,12 +74,12 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[2] = {
{
- .addr = priv->cfg->i2c_address,
+ .addr = priv->client->addr,
.flags = 0,
.len = 1,
.buf = ®,
}, {
- .addr = priv->cfg->i2c_address,
+ .addr = priv->client->addr,
.flags = I2C_M_RD,
.len = len,
.buf = buf,
@@ -87,19 +87,19 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
};
if (len > sizeof(buf)) {
- dev_warn(&priv->i2c->dev,
+ dev_warn(&priv->client->dev,
"%s: i2c rd reg=%04x: len=%d is too big!\n",
KBUILD_MODNAME, reg, len);
return -EINVAL;
}
- ret = i2c_transfer(priv->i2c, msg, 2);
+ ret = i2c_transfer(priv->client->adapter, msg, 2);
if (ret == 2) {
memcpy(val, buf, len);
ret = 0;
} else {
- dev_warn(&priv->i2c->dev, "%s: i2c rd failed=%d reg=%02x " \
- "len=%d\n", KBUILD_MODNAME, ret, reg, len);
+ dev_warn(&priv->client->dev, "%s: i2c rd failed=%d reg=%02x len=%d\n",
+ KBUILD_MODNAME, ret, reg, len);
ret = -EREMOTEIO;
}
@@ -166,7 +166,7 @@ static int tda18212_set_params(struct dvb_frontend *fe)
[ATSC_QAM] = { 0x7d, 0x20, 0x63 },
};
- dev_dbg(&priv->i2c->dev,
+ dev_dbg(&priv->client->dev,
"%s: delivery_system=%d frequency=%d bandwidth_hz=%d\n",
__func__, c->delivery_system, c->frequency,
c->bandwidth_hz);
@@ -176,25 +176,25 @@ static int tda18212_set_params(struct dvb_frontend *fe)
switch (c->delivery_system) {
case SYS_ATSC:
- if_khz = priv->cfg->if_atsc_vsb;
+ if_khz = priv->cfg.if_atsc_vsb;
i = ATSC_VSB;
break;
case SYS_DVBC_ANNEX_B:
- if_khz = priv->cfg->if_atsc_qam;
+ if_khz = priv->cfg.if_atsc_qam;
i = ATSC_QAM;
break;
case SYS_DVBT:
switch (c->bandwidth_hz) {
case 6000000:
- if_khz = priv->cfg->if_dvbt_6;
+ if_khz = priv->cfg.if_dvbt_6;
i = DVBT_6;
break;
case 7000000:
- if_khz = priv->cfg->if_dvbt_7;
+ if_khz = priv->cfg.if_dvbt_7;
i = DVBT_7;
break;
case 8000000:
- if_khz = priv->cfg->if_dvbt_8;
+ if_khz = priv->cfg.if_dvbt_8;
i = DVBT_8;
break;
default:
@@ -205,15 +205,15 @@ static int tda18212_set_params(struct dvb_frontend *fe)
case SYS_DVBT2:
switch (c->bandwidth_hz) {
case 6000000:
- if_khz = priv->cfg->if_dvbt2_6;
+ if_khz = priv->cfg.if_dvbt2_6;
i = DVBT2_6;
break;
case 7000000:
- if_khz = priv->cfg->if_dvbt2_7;
+ if_khz = priv->cfg.if_dvbt2_7;
i = DVBT2_7;
break;
case 8000000:
- if_khz = priv->cfg->if_dvbt2_8;
+ if_khz = priv->cfg.if_dvbt2_8;
i = DVBT2_8;
break;
default:
@@ -223,7 +223,7 @@ static int tda18212_set_params(struct dvb_frontend *fe)
break;
case SYS_DVBC_ANNEX_A:
case SYS_DVBC_ANNEX_C:
- if_khz = priv->cfg->if_dvbc;
+ if_khz = priv->cfg.if_dvbc;
i = DVBC_8;
break;
default:
@@ -266,7 +266,7 @@ exit:
return ret;
error:
- dev_dbg(&priv->i2c->dev, "%s: failed=%d\n", __func__, ret);
+ dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
goto exit;
}
@@ -279,13 +279,6 @@ static int tda18212_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
return 0;
}
-static int tda18212_release(struct dvb_frontend *fe)
-{
- kfree(fe->tuner_priv);
- fe->tuner_priv = NULL;
- return 0;
-}
-
static const struct dvb_tuner_ops tda18212_tuner_ops = {
.info = {
.name = "NXP TDA18212",
@@ -295,34 +288,36 @@ static const struct dvb_tuner_ops tda18212_tuner_ops = {
.frequency_step = 1000,
},
- .release = tda18212_release,
-
.set_params = tda18212_set_params,
.get_if_frequency = tda18212_get_if_frequency,
};
-struct dvb_frontend *tda18212_attach(struct dvb_frontend *fe,
- struct i2c_adapter *i2c, struct tda18212_config *cfg)
+static int tda18212_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
{
- struct tda18212_priv *priv = NULL;
+ struct tda18212_config *cfg = client->dev.platform_data;
+ struct dvb_frontend *fe = cfg->fe;
+ struct tda18212_priv *priv;
int ret;
u8 chip_id = chip_id;
char *version;
- priv = kzalloc(sizeof(struct tda18212_priv), GFP_KERNEL);
- if (priv == NULL)
- return NULL;
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ ret = -ENOMEM;
+ dev_err(&client->dev, "%s: kzalloc() failed\n", KBUILD_MODNAME);
+ goto err;
+ }
- priv->cfg = cfg;
- priv->i2c = i2c;
- fe->tuner_priv = priv;
+ memcpy(&priv->cfg, cfg, sizeof(struct tda18212_config));
+ priv->client = client;
+ /* check if the tuner is there */
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 1); /* open I2C-gate */
- /* check if the tuner is there */
ret = tda18212_rd_reg(priv, 0x00, &chip_id);
- dev_dbg(&priv->i2c->dev, "%s: chip_id=%02x\n", __func__, chip_id);
+ dev_dbg(&priv->client->dev, "%s: chip_id=%02x\n", __func__, chip_id);
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 0); /* close I2C-gate */
@@ -338,23 +333,57 @@ struct dvb_frontend *tda18212_attach(struct dvb_frontend *fe,
version = "S"; /* slave */
break;
default:
+ ret = -ENODEV;
goto err;
}
- dev_info(&priv->i2c->dev,
+ dev_info(&priv->client->dev,
"%s: NXP TDA18212HN/%s successfully identified\n",
KBUILD_MODNAME, version);
+ fe->tuner_priv = priv;
memcpy(&fe->ops.tuner_ops, &tda18212_tuner_ops,
- sizeof(struct dvb_tuner_ops));
+ sizeof(struct dvb_tuner_ops));
+ i2c_set_clientdata(client, priv);
- return fe;
+ return 0;
err:
- dev_dbg(&i2c->dev, "%s: failed=%d\n", __func__, ret);
+ dev_dbg(&client->dev, "%s: failed=%d\n", __func__, ret);
kfree(priv);
- return NULL;
+ return ret;
}
-EXPORT_SYMBOL(tda18212_attach);
+
+static int tda18212_remove(struct i2c_client *client)
+{
+ struct tda18212_priv *priv = i2c_get_clientdata(client);
+ struct dvb_frontend *fe = priv->cfg.fe;
+
+ dev_dbg(&client->dev, "%s:\n", __func__);
+
+ memset(&fe->ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops));
+ fe->tuner_priv = NULL;
+ kfree(priv);
+
+ return 0;
+}
+
+static const struct i2c_device_id tda18212_id[] = {
+ {"tda18212", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, tda18212_id);
+
+static struct i2c_driver tda18212_driver = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "tda18212",
+ },
+ .probe = tda18212_probe,
+ .remove = tda18212_remove,
+ .id_table = tda18212_id,
+};
+
+module_i2c_driver(tda18212_driver);
MODULE_DESCRIPTION("NXP TDA18212HN silicon tuner driver");
MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
diff --git a/drivers/media/tuners/tda18212.h b/drivers/media/tuners/tda18212.h
index 265559a..e58c909 100644
--- a/drivers/media/tuners/tda18212.h
+++ b/drivers/media/tuners/tda18212.h
@@ -25,8 +25,6 @@
#include "dvb_frontend.h"
struct tda18212_config {
- u8 i2c_address;
-
u16 if_dvbt_6;
u16 if_dvbt_7;
u16 if_dvbt_8;
@@ -44,16 +42,4 @@ struct tda18212_config {
struct dvb_frontend *fe;
};
-#if IS_ENABLED(CONFIG_MEDIA_TUNER_TDA18212)
-extern struct dvb_frontend *tda18212_attach(struct dvb_frontend *fe,
- struct i2c_adapter *i2c, struct tda18212_config *cfg);
-#else
-static inline struct dvb_frontend *tda18212_attach(struct dvb_frontend *fe,
- struct i2c_adapter *i2c, struct tda18212_config *cfg)
-{
- printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
- return NULL;
-}
-#endif
-
#endif
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 6/8] tda18212: clean logging
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
` (3 preceding siblings ...)
2014-09-07 1:59 ` [PATCH v2 5/8] tda18212: convert driver to I2C binding Antti Palosaari
@ 2014-09-07 1:59 ` Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 7/8] tda18212: rename state from 'priv' to 'dev' Antti Palosaari
2014-09-07 2:00 ` [PATCH v2 8/8] tda18212: convert to RegMap API Antti Palosaari
6 siblings, 0 replies; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 1:59 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
There is no need to print module name nor function name as those
are done by kernel logging system when dev_xxx logging is used and
driver is proper I2C driver.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/tuners/tda18212.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
index 659787b..5d1d785 100644
--- a/drivers/media/tuners/tda18212.c
+++ b/drivers/media/tuners/tda18212.c
@@ -47,8 +47,8 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
if (1 + len > sizeof(buf)) {
dev_warn(&priv->client->dev,
- "%s: i2c wr reg=%04x: len=%d is too big!\n",
- KBUILD_MODNAME, reg, len);
+ "i2c wr reg=%04x: len=%d is too big!\n",
+ reg, len);
return -EINVAL;
}
@@ -59,8 +59,9 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
if (ret == 1) {
ret = 0;
} else {
- dev_warn(&priv->client->dev, "%s: i2c wr failed=%d reg=%02x len=%d\n",
- KBUILD_MODNAME, ret, reg, len);
+ dev_warn(&priv->client->dev,
+ "i2c wr failed=%d reg=%02x len=%d\n",
+ ret, reg, len);
ret = -EREMOTEIO;
}
return ret;
@@ -88,8 +89,8 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
if (len > sizeof(buf)) {
dev_warn(&priv->client->dev,
- "%s: i2c rd reg=%04x: len=%d is too big!\n",
- KBUILD_MODNAME, reg, len);
+ "i2c rd reg=%04x: len=%d is too big!\n",
+ reg, len);
return -EINVAL;
}
@@ -98,8 +99,9 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
memcpy(val, buf, len);
ret = 0;
} else {
- dev_warn(&priv->client->dev, "%s: i2c rd failed=%d reg=%02x len=%d\n",
- KBUILD_MODNAME, ret, reg, len);
+ dev_warn(&priv->client->dev,
+ "i2c rd failed=%d reg=%02x len=%d\n",
+ ret, reg, len);
ret = -EREMOTEIO;
}
@@ -167,8 +169,8 @@ static int tda18212_set_params(struct dvb_frontend *fe)
};
dev_dbg(&priv->client->dev,
- "%s: delivery_system=%d frequency=%d bandwidth_hz=%d\n",
- __func__, c->delivery_system, c->frequency,
+ "delivery_system=%d frequency=%d bandwidth_hz=%d\n",
+ c->delivery_system, c->frequency,
c->bandwidth_hz);
if (fe->ops.i2c_gate_ctrl)
@@ -266,7 +268,7 @@ exit:
return ret;
error:
- dev_dbg(&priv->client->dev, "%s: failed=%d\n", __func__, ret);
+ dev_dbg(&priv->client->dev, "failed=%d\n", ret);
goto exit;
}
@@ -305,7 +307,7 @@ static int tda18212_probe(struct i2c_client *client,
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
ret = -ENOMEM;
- dev_err(&client->dev, "%s: kzalloc() failed\n", KBUILD_MODNAME);
+ dev_err(&client->dev, "kzalloc() failed\n");
goto err;
}
@@ -317,7 +319,7 @@ static int tda18212_probe(struct i2c_client *client,
fe->ops.i2c_gate_ctrl(fe, 1); /* open I2C-gate */
ret = tda18212_rd_reg(priv, 0x00, &chip_id);
- dev_dbg(&priv->client->dev, "%s: chip_id=%02x\n", __func__, chip_id);
+ dev_dbg(&priv->client->dev, "chip_id=%02x\n", chip_id);
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 0); /* close I2C-gate */
@@ -338,8 +340,7 @@ static int tda18212_probe(struct i2c_client *client,
}
dev_info(&priv->client->dev,
- "%s: NXP TDA18212HN/%s successfully identified\n",
- KBUILD_MODNAME, version);
+ "NXP TDA18212HN/%s successfully identified\n", version);
fe->tuner_priv = priv;
memcpy(&fe->ops.tuner_ops, &tda18212_tuner_ops,
@@ -348,7 +349,7 @@ static int tda18212_probe(struct i2c_client *client,
return 0;
err:
- dev_dbg(&client->dev, "%s: failed=%d\n", __func__, ret);
+ dev_dbg(&client->dev, "failed=%d\n", ret);
kfree(priv);
return ret;
}
@@ -358,7 +359,7 @@ static int tda18212_remove(struct i2c_client *client)
struct tda18212_priv *priv = i2c_get_clientdata(client);
struct dvb_frontend *fe = priv->cfg.fe;
- dev_dbg(&client->dev, "%s:\n", __func__);
+ dev_dbg(&client->dev, "\n");
memset(&fe->ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops));
fe->tuner_priv = NULL;
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 7/8] tda18212: rename state from 'priv' to 'dev'
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
` (4 preceding siblings ...)
2014-09-07 1:59 ` [PATCH v2 6/8] tda18212: clean logging Antti Palosaari
@ 2014-09-07 1:59 ` Antti Palosaari
2014-09-07 2:00 ` [PATCH v2 8/8] tda18212: convert to RegMap API Antti Palosaari
6 siblings, 0 replies; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 1:59 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
foo_dev seems to be most correct term for the structure holding data
of each device instance. It is most used term in Kernel codebase and also
examples from book Linux Device Drivers, Third Edition, uses it.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/tuners/tda18212.c | 104 ++++++++++++++++++++--------------------
1 file changed, 51 insertions(+), 53 deletions(-)
diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
index 5d1d785..24948c7 100644
--- a/drivers/media/tuners/tda18212.c
+++ b/drivers/media/tuners/tda18212.c
@@ -23,7 +23,7 @@
/* Max transfer size done by I2C transfer functions */
#define MAX_XFER_SIZE 64
-struct tda18212_priv {
+struct tda18212_dev {
struct tda18212_config cfg;
struct i2c_client *client;
@@ -31,14 +31,13 @@ struct tda18212_priv {
};
/* write multiple registers */
-static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
- int len)
+static int tda18212_wr_regs(struct tda18212_dev *dev, u8 reg, u8 *val, int len)
{
int ret;
u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[1] = {
{
- .addr = priv->client->addr,
+ .addr = dev->client->addr,
.flags = 0,
.len = 1 + len,
.buf = buf,
@@ -46,7 +45,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
};
if (1 + len > sizeof(buf)) {
- dev_warn(&priv->client->dev,
+ dev_warn(&dev->client->dev,
"i2c wr reg=%04x: len=%d is too big!\n",
reg, len);
return -EINVAL;
@@ -55,11 +54,11 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
buf[0] = reg;
memcpy(&buf[1], val, len);
- ret = i2c_transfer(priv->client->adapter, msg, 1);
+ ret = i2c_transfer(dev->client->adapter, msg, 1);
if (ret == 1) {
ret = 0;
} else {
- dev_warn(&priv->client->dev,
+ dev_warn(&dev->client->dev,
"i2c wr failed=%d reg=%02x len=%d\n",
ret, reg, len);
ret = -EREMOTEIO;
@@ -68,19 +67,18 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
}
/* read multiple registers */
-static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
- int len)
+static int tda18212_rd_regs(struct tda18212_dev *dev, u8 reg, u8 *val, int len)
{
int ret;
u8 buf[MAX_XFER_SIZE];
struct i2c_msg msg[2] = {
{
- .addr = priv->client->addr,
+ .addr = dev->client->addr,
.flags = 0,
.len = 1,
.buf = ®,
}, {
- .addr = priv->client->addr,
+ .addr = dev->client->addr,
.flags = I2C_M_RD,
.len = len,
.buf = buf,
@@ -88,18 +86,18 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
};
if (len > sizeof(buf)) {
- dev_warn(&priv->client->dev,
+ dev_warn(&dev->client->dev,
"i2c rd reg=%04x: len=%d is too big!\n",
reg, len);
return -EINVAL;
}
- ret = i2c_transfer(priv->client->adapter, msg, 2);
+ ret = i2c_transfer(dev->client->adapter, msg, 2);
if (ret == 2) {
memcpy(val, buf, len);
ret = 0;
} else {
- dev_warn(&priv->client->dev,
+ dev_warn(&dev->client->dev,
"i2c rd failed=%d reg=%02x len=%d\n",
ret, reg, len);
ret = -EREMOTEIO;
@@ -109,26 +107,26 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val,
}
/* write single register */
-static int tda18212_wr_reg(struct tda18212_priv *priv, u8 reg, u8 val)
+static int tda18212_wr_reg(struct tda18212_dev *dev, u8 reg, u8 val)
{
- return tda18212_wr_regs(priv, reg, &val, 1);
+ return tda18212_wr_regs(dev, reg, &val, 1);
}
/* read single register */
-static int tda18212_rd_reg(struct tda18212_priv *priv, u8 reg, u8 *val)
+static int tda18212_rd_reg(struct tda18212_dev *dev, u8 reg, u8 *val)
{
- return tda18212_rd_regs(priv, reg, val, 1);
+ return tda18212_rd_regs(dev, reg, val, 1);
}
#if 0 /* keep, useful when developing driver */
-static void tda18212_dump_regs(struct tda18212_priv *priv)
+static void tda18212_dump_regs(struct tda18212_dev *dev)
{
int i;
u8 buf[256];
#define TDA18212_RD_LEN 32
for (i = 0; i < sizeof(buf); i += TDA18212_RD_LEN)
- tda18212_rd_regs(priv, i, &buf[i], TDA18212_RD_LEN);
+ tda18212_rd_regs(dev, i, &buf[i], TDA18212_RD_LEN);
print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 32, 1, buf,
sizeof(buf), true);
@@ -139,7 +137,7 @@ static void tda18212_dump_regs(struct tda18212_priv *priv)
static int tda18212_set_params(struct dvb_frontend *fe)
{
- struct tda18212_priv *priv = fe->tuner_priv;
+ struct tda18212_dev *dev = fe->tuner_priv;
struct dtv_frontend_properties *c = &fe->dtv_property_cache;
int ret, i;
u32 if_khz;
@@ -168,7 +166,7 @@ static int tda18212_set_params(struct dvb_frontend *fe)
[ATSC_QAM] = { 0x7d, 0x20, 0x63 },
};
- dev_dbg(&priv->client->dev,
+ dev_dbg(&dev->client->dev,
"delivery_system=%d frequency=%d bandwidth_hz=%d\n",
c->delivery_system, c->frequency,
c->bandwidth_hz);
@@ -178,25 +176,25 @@ static int tda18212_set_params(struct dvb_frontend *fe)
switch (c->delivery_system) {
case SYS_ATSC:
- if_khz = priv->cfg.if_atsc_vsb;
+ if_khz = dev->cfg.if_atsc_vsb;
i = ATSC_VSB;
break;
case SYS_DVBC_ANNEX_B:
- if_khz = priv->cfg.if_atsc_qam;
+ if_khz = dev->cfg.if_atsc_qam;
i = ATSC_QAM;
break;
case SYS_DVBT:
switch (c->bandwidth_hz) {
case 6000000:
- if_khz = priv->cfg.if_dvbt_6;
+ if_khz = dev->cfg.if_dvbt_6;
i = DVBT_6;
break;
case 7000000:
- if_khz = priv->cfg.if_dvbt_7;
+ if_khz = dev->cfg.if_dvbt_7;
i = DVBT_7;
break;
case 8000000:
- if_khz = priv->cfg.if_dvbt_8;
+ if_khz = dev->cfg.if_dvbt_8;
i = DVBT_8;
break;
default:
@@ -207,15 +205,15 @@ static int tda18212_set_params(struct dvb_frontend *fe)
case SYS_DVBT2:
switch (c->bandwidth_hz) {
case 6000000:
- if_khz = priv->cfg.if_dvbt2_6;
+ if_khz = dev->cfg.if_dvbt2_6;
i = DVBT2_6;
break;
case 7000000:
- if_khz = priv->cfg.if_dvbt2_7;
+ if_khz = dev->cfg.if_dvbt2_7;
i = DVBT2_7;
break;
case 8000000:
- if_khz = priv->cfg.if_dvbt2_8;
+ if_khz = dev->cfg.if_dvbt2_8;
i = DVBT2_8;
break;
default:
@@ -225,7 +223,7 @@ static int tda18212_set_params(struct dvb_frontend *fe)
break;
case SYS_DVBC_ANNEX_A:
case SYS_DVBC_ANNEX_C:
- if_khz = priv->cfg.if_dvbc;
+ if_khz = dev->cfg.if_dvbc;
i = DVBC_8;
break;
default:
@@ -233,15 +231,15 @@ static int tda18212_set_params(struct dvb_frontend *fe)
goto error;
}
- ret = tda18212_wr_reg(priv, 0x23, bw_params[i][2]);
+ ret = tda18212_wr_reg(dev, 0x23, bw_params[i][2]);
if (ret)
goto error;
- ret = tda18212_wr_reg(priv, 0x06, 0x00);
+ ret = tda18212_wr_reg(dev, 0x06, 0x00);
if (ret)
goto error;
- ret = tda18212_wr_reg(priv, 0x0f, bw_params[i][0]);
+ ret = tda18212_wr_reg(dev, 0x0f, bw_params[i][0]);
if (ret)
goto error;
@@ -254,12 +252,12 @@ static int tda18212_set_params(struct dvb_frontend *fe)
buf[6] = ((c->frequency / 1000) >> 0) & 0xff;
buf[7] = 0xc1;
buf[8] = 0x01;
- ret = tda18212_wr_regs(priv, 0x12, buf, sizeof(buf));
+ ret = tda18212_wr_regs(dev, 0x12, buf, sizeof(buf));
if (ret)
goto error;
/* actual IF rounded as it is on register */
- priv->if_frequency = buf[3] * 50 * 1000;
+ dev->if_frequency = buf[3] * 50 * 1000;
exit:
if (fe->ops.i2c_gate_ctrl)
@@ -268,15 +266,15 @@ exit:
return ret;
error:
- dev_dbg(&priv->client->dev, "failed=%d\n", ret);
+ dev_dbg(&dev->client->dev, "failed=%d\n", ret);
goto exit;
}
static int tda18212_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
{
- struct tda18212_priv *priv = fe->tuner_priv;
+ struct tda18212_dev *dev = fe->tuner_priv;
- *frequency = priv->if_frequency;
+ *frequency = dev->if_frequency;
return 0;
}
@@ -299,27 +297,27 @@ static int tda18212_probe(struct i2c_client *client,
{
struct tda18212_config *cfg = client->dev.platform_data;
struct dvb_frontend *fe = cfg->fe;
- struct tda18212_priv *priv;
+ struct tda18212_dev *dev;
int ret;
u8 chip_id = chip_id;
char *version;
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv) {
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (dev == NULL) {
ret = -ENOMEM;
dev_err(&client->dev, "kzalloc() failed\n");
goto err;
}
- memcpy(&priv->cfg, cfg, sizeof(struct tda18212_config));
- priv->client = client;
+ memcpy(&dev->cfg, cfg, sizeof(struct tda18212_config));
+ dev->client = client;
/* check if the tuner is there */
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 1); /* open I2C-gate */
- ret = tda18212_rd_reg(priv, 0x00, &chip_id);
- dev_dbg(&priv->client->dev, "chip_id=%02x\n", chip_id);
+ ret = tda18212_rd_reg(dev, 0x00, &chip_id);
+ dev_dbg(&dev->client->dev, "chip_id=%02x\n", chip_id);
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 0); /* close I2C-gate */
@@ -339,31 +337,31 @@ static int tda18212_probe(struct i2c_client *client,
goto err;
}
- dev_info(&priv->client->dev,
+ dev_info(&dev->client->dev,
"NXP TDA18212HN/%s successfully identified\n", version);
- fe->tuner_priv = priv;
+ fe->tuner_priv = dev;
memcpy(&fe->ops.tuner_ops, &tda18212_tuner_ops,
sizeof(struct dvb_tuner_ops));
- i2c_set_clientdata(client, priv);
+ i2c_set_clientdata(client, dev);
return 0;
err:
dev_dbg(&client->dev, "failed=%d\n", ret);
- kfree(priv);
+ kfree(dev);
return ret;
}
static int tda18212_remove(struct i2c_client *client)
{
- struct tda18212_priv *priv = i2c_get_clientdata(client);
- struct dvb_frontend *fe = priv->cfg.fe;
+ struct tda18212_dev *dev = i2c_get_clientdata(client);
+ struct dvb_frontend *fe = dev->cfg.fe;
dev_dbg(&client->dev, "\n");
memset(&fe->ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops));
fe->tuner_priv = NULL;
- kfree(priv);
+ kfree(dev);
return 0;
}
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 8/8] tda18212: convert to RegMap API
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
` (5 preceding siblings ...)
2014-09-07 1:59 ` [PATCH v2 7/8] tda18212: rename state from 'priv' to 'dev' Antti Palosaari
@ 2014-09-07 2:00 ` Antti Palosaari
6 siblings, 0 replies; 11+ messages in thread
From: Antti Palosaari @ 2014-09-07 2:00 UTC (permalink / raw
To: linux-media; +Cc: Antti Palosaari
Use RegMap API to handle all the boring I2C register access
boilerplate stuff.
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
drivers/media/tuners/Kconfig | 1 +
drivers/media/tuners/tda18212.c | 131 ++++++----------------------------------
2 files changed, 18 insertions(+), 114 deletions(-)
diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig
index d79fd1c..483963d 100644
--- a/drivers/media/tuners/Kconfig
+++ b/drivers/media/tuners/Kconfig
@@ -204,6 +204,7 @@ config MEDIA_TUNER_FC0013
config MEDIA_TUNER_TDA18212
tristate "NXP TDA18212 silicon tuner"
depends on MEDIA_SUPPORT && I2C
+ select REGMAP_I2C
default m if !MEDIA_SUBDRV_AUTOSELECT
help
NXP TDA18212 silicon tuner driver.
diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
index 24948c7..d93e066 100644
--- a/drivers/media/tuners/tda18212.c
+++ b/drivers/media/tuners/tda18212.c
@@ -19,122 +19,16 @@
*/
#include "tda18212.h"
-
-/* Max transfer size done by I2C transfer functions */
-#define MAX_XFER_SIZE 64
+#include <linux/regmap.h>
struct tda18212_dev {
struct tda18212_config cfg;
struct i2c_client *client;
+ struct regmap *regmap;
u32 if_frequency;
};
-/* write multiple registers */
-static int tda18212_wr_regs(struct tda18212_dev *dev, u8 reg, u8 *val, int len)
-{
- int ret;
- u8 buf[MAX_XFER_SIZE];
- struct i2c_msg msg[1] = {
- {
- .addr = dev->client->addr,
- .flags = 0,
- .len = 1 + len,
- .buf = buf,
- }
- };
-
- if (1 + len > sizeof(buf)) {
- dev_warn(&dev->client->dev,
- "i2c wr reg=%04x: len=%d is too big!\n",
- reg, len);
- return -EINVAL;
- }
-
- buf[0] = reg;
- memcpy(&buf[1], val, len);
-
- ret = i2c_transfer(dev->client->adapter, msg, 1);
- if (ret == 1) {
- ret = 0;
- } else {
- dev_warn(&dev->client->dev,
- "i2c wr failed=%d reg=%02x len=%d\n",
- ret, reg, len);
- ret = -EREMOTEIO;
- }
- return ret;
-}
-
-/* read multiple registers */
-static int tda18212_rd_regs(struct tda18212_dev *dev, u8 reg, u8 *val, int len)
-{
- int ret;
- u8 buf[MAX_XFER_SIZE];
- struct i2c_msg msg[2] = {
- {
- .addr = dev->client->addr,
- .flags = 0,
- .len = 1,
- .buf = ®,
- }, {
- .addr = dev->client->addr,
- .flags = I2C_M_RD,
- .len = len,
- .buf = buf,
- }
- };
-
- if (len > sizeof(buf)) {
- dev_warn(&dev->client->dev,
- "i2c rd reg=%04x: len=%d is too big!\n",
- reg, len);
- return -EINVAL;
- }
-
- ret = i2c_transfer(dev->client->adapter, msg, 2);
- if (ret == 2) {
- memcpy(val, buf, len);
- ret = 0;
- } else {
- dev_warn(&dev->client->dev,
- "i2c rd failed=%d reg=%02x len=%d\n",
- ret, reg, len);
- ret = -EREMOTEIO;
- }
-
- return ret;
-}
-
-/* write single register */
-static int tda18212_wr_reg(struct tda18212_dev *dev, u8 reg, u8 val)
-{
- return tda18212_wr_regs(dev, reg, &val, 1);
-}
-
-/* read single register */
-static int tda18212_rd_reg(struct tda18212_dev *dev, u8 reg, u8 *val)
-{
- return tda18212_rd_regs(dev, reg, val, 1);
-}
-
-#if 0 /* keep, useful when developing driver */
-static void tda18212_dump_regs(struct tda18212_dev *dev)
-{
- int i;
- u8 buf[256];
-
- #define TDA18212_RD_LEN 32
- for (i = 0; i < sizeof(buf); i += TDA18212_RD_LEN)
- tda18212_rd_regs(dev, i, &buf[i], TDA18212_RD_LEN);
-
- print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 32, 1, buf,
- sizeof(buf), true);
-
- return;
-}
-#endif
-
static int tda18212_set_params(struct dvb_frontend *fe)
{
struct tda18212_dev *dev = fe->tuner_priv;
@@ -231,15 +125,15 @@ static int tda18212_set_params(struct dvb_frontend *fe)
goto error;
}
- ret = tda18212_wr_reg(dev, 0x23, bw_params[i][2]);
+ ret = regmap_write(dev->regmap, 0x23, bw_params[i][2]);
if (ret)
goto error;
- ret = tda18212_wr_reg(dev, 0x06, 0x00);
+ ret = regmap_write(dev->regmap, 0x06, 0x00);
if (ret)
goto error;
- ret = tda18212_wr_reg(dev, 0x0f, bw_params[i][0]);
+ ret = regmap_write(dev->regmap, 0x0f, bw_params[i][0]);
if (ret)
goto error;
@@ -252,7 +146,7 @@ static int tda18212_set_params(struct dvb_frontend *fe)
buf[6] = ((c->frequency / 1000) >> 0) & 0xff;
buf[7] = 0xc1;
buf[8] = 0x01;
- ret = tda18212_wr_regs(dev, 0x12, buf, sizeof(buf));
+ ret = regmap_bulk_write(dev->regmap, 0x12, buf, sizeof(buf));
if (ret)
goto error;
@@ -299,8 +193,12 @@ static int tda18212_probe(struct i2c_client *client,
struct dvb_frontend *fe = cfg->fe;
struct tda18212_dev *dev;
int ret;
- u8 chip_id = chip_id;
+ unsigned int chip_id;
char *version;
+ static const struct regmap_config regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ };
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (dev == NULL) {
@@ -311,12 +209,17 @@ static int tda18212_probe(struct i2c_client *client,
memcpy(&dev->cfg, cfg, sizeof(struct tda18212_config));
dev->client = client;
+ dev->regmap = devm_regmap_init_i2c(client, ®map_config);
+ if (IS_ERR(dev->regmap)) {
+ ret = PTR_ERR(dev->regmap);
+ goto err;
+ }
/* check if the tuner is there */
if (fe->ops.i2c_gate_ctrl)
fe->ops.i2c_gate_ctrl(fe, 1); /* open I2C-gate */
- ret = tda18212_rd_reg(dev, 0x00, &chip_id);
+ ret = regmap_read(dev->regmap, 0x00, &chip_id);
dev_dbg(&dev->client->dev, "chip_id=%02x\n", chip_id);
if (fe->ops.i2c_gate_ctrl)
--
http://palosaari.fi/
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client
2014-09-07 1:59 ` [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client Antti Palosaari
@ 2014-09-18 12:31 ` Mauro Carvalho Chehab
2014-09-18 13:01 ` Antti Palosaari
0 siblings, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2014-09-18 12:31 UTC (permalink / raw
To: Antti Palosaari; +Cc: linux-media
Em Sun, 7 Sep 2014 04:59:55 +0300
Antti Palosaari <crope@iki.fi> escreveu:
> Used tda18212 tuner is implemented as I2C driver. Implement I2C
> client to anysee and use it for tda18212.
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> drivers/media/usb/dvb-usb-v2/anysee.c | 185 +++++++++++++++++++++++++++-------
> drivers/media/usb/dvb-usb-v2/anysee.h | 3 +
> 2 files changed, 152 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c b/drivers/media/usb/dvb-usb-v2/anysee.c
> index e4a2382..d3c5f23 100644
> --- a/drivers/media/usb/dvb-usb-v2/anysee.c
> +++ b/drivers/media/usb/dvb-usb-v2/anysee.c
> @@ -332,7 +332,6 @@ static struct tda10023_config anysee_tda10023_tda18212_config = {
> };
>
> static struct tda18212_config anysee_tda18212_config = {
> - .i2c_address = (0xc0 >> 1),
> .if_dvbt_6 = 4150,
> .if_dvbt_7 = 4150,
> .if_dvbt_8 = 4150,
> @@ -340,7 +339,6 @@ static struct tda18212_config anysee_tda18212_config = {
> };
>
> static struct tda18212_config anysee_tda18212_config2 = {
> - .i2c_address = 0x60 /* (0xc0 >> 1) */,
> .if_dvbt_6 = 3550,
> .if_dvbt_7 = 3700,
> .if_dvbt_8 = 4150,
> @@ -632,6 +630,92 @@ error:
> return ret;
> }
>
> +static int anysee_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
> + void *platform_data)
> +{
> + int ret, num;
> + struct anysee_state *state = d_to_priv(d);
> + struct i2c_client *client;
> + struct i2c_adapter *adapter = &d->i2c_adap;
> + struct i2c_board_info board_info = {
> + .addr = addr,
> + .platform_data = platform_data,
> + };
> +
> + strlcpy(board_info.type, type, I2C_NAME_SIZE);
> +
> + /* find first free client */
> + for (num = 0; num < ANYSEE_I2C_CLIENT_MAX; num++) {
> + if (state->i2c_client[num] == NULL)
> + break;
> + }
> +
> + dev_dbg(&d->udev->dev, "%s: num=%d\n", __func__, num);
> +
> + if (num == ANYSEE_I2C_CLIENT_MAX) {
> + dev_err(&d->udev->dev, "%s: I2C client out of index\n",
> + KBUILD_MODNAME);
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + request_module(board_info.type);
> +
> + /* register I2C device */
> + client = i2c_new_device(adapter, &board_info);
> + if (client == NULL || client->dev.driver == NULL) {
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + /* increase I2C driver usage count */
> + if (!try_module_get(client->dev.driver->owner)) {
> + i2c_unregister_device(client);
> + ret = -ENODEV;
> + goto err;
> + }
> +
> + state->i2c_client[num] = client;
> + return 0;
> +err:
> + dev_dbg(&d->udev->dev, "%s: failed=%d\n", __func__, ret);
> + return ret;
> +}
> +
> +static void anysee_del_i2c_dev(struct dvb_usb_device *d)
> +{
> + int num;
> + struct anysee_state *state = d_to_priv(d);
> + struct i2c_client *client;
> +
> + /* find last used client */
> + num = ANYSEE_I2C_CLIENT_MAX;
> + while (num--) {
> + if (state->i2c_client[num] != NULL)
> + break;
> + }
> +
> + dev_dbg(&d->udev->dev, "%s: num=%d\n", __func__, num);
> +
> + if (num == -1) {
> + dev_err(&d->udev->dev, "%s: I2C client out of index\n",
> + KBUILD_MODNAME);
> + goto err;
> + }
> +
> + client = state->i2c_client[num];
> +
> + /* decrease I2C driver usage count */
> + module_put(client->dev.driver->owner);
> +
> + /* unregister I2C device */
> + i2c_unregister_device(client);
> +
> + state->i2c_client[num] = NULL;
> +err:
> + dev_dbg(&d->udev->dev, "%s: failed\n", __func__);
> +}
> +
Please, instead of adding a function to insert/remove an I2C driver on every
place, put them into a common place.
I would actually very much prefer if you could reuse the same code that
are already at the media subsystem (see v4l2_i2c_new_subdev_board &
friends at drivers/media/v4l2-core/v4l2-common.c), eventually making it
more generic.
Btw, as we want to use the media controller also for DVB, we'll end
by needing to use a call similar to v4l2_device_register_subdev().
So, having this code all on just one place will make easier for us to
go to this next step.
> static int anysee_frontend_attach(struct dvb_usb_adapter *adap)
> {
> struct anysee_state *state = adap_to_priv(adap);
> @@ -640,12 +724,12 @@ static int anysee_frontend_attach(struct dvb_usb_adapter *adap)
> u8 tmp;
> struct i2c_msg msg[2] = {
> {
> - .addr = anysee_tda18212_config.i2c_address,
> + .addr = 0x60,
> .flags = 0,
> .len = 1,
> .buf = "\x00",
> }, {
> - .addr = anysee_tda18212_config.i2c_address,
> + .addr = 0x60,
> .flags = I2C_M_RD,
> .len = 1,
> .buf = &tmp,
> @@ -723,9 +807,11 @@ static int anysee_frontend_attach(struct dvb_usb_adapter *adap)
> /* probe TDA18212 */
> tmp = 0;
> ret = i2c_transfer(&d->i2c_adap, msg, 2);
> - if (ret == 2 && tmp == 0xc7)
> + if (ret == 2 && tmp == 0xc7) {
> dev_dbg(&d->udev->dev, "%s: TDA18212 found\n",
> __func__);
> + state->has_tda18212 = true;
> + }
> else
> tmp = 0;
>
> @@ -939,46 +1025,63 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
> * fails attach old simple PLL. */
>
> /* attach tuner */
> - fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
> - &anysee_tda18212_config);
> + if (state->has_tda18212) {
> + struct tda18212_config tda18212_config =
> + anysee_tda18212_config;
>
> - if (fe && adap->fe[1]) {
> - /* attach tuner for 2nd FE */
> - fe = dvb_attach(tda18212_attach, adap->fe[1],
> - &d->i2c_adap, &anysee_tda18212_config);
> - break;
> - } else if (fe) {
> - break;
> - }
> -
> - /* attach tuner */
> - fe = dvb_attach(dvb_pll_attach, adap->fe[0], (0xc0 >> 1),
> - &d->i2c_adap, DVB_PLL_SAMSUNG_DTOS403IH102A);
> + tda18212_config.fe = adap->fe[0];
> + ret = anysee_add_i2c_dev(d, "tda18212", 0x60,
> + &tda18212_config);
> + if (ret)
> + goto err;
> +
> + /* copy tuner ops for 2nd FE as tuner is shared */
> + if (adap->fe[1]) {
> + adap->fe[1]->tuner_priv =
> + adap->fe[0]->tuner_priv;
> + memcpy(&adap->fe[1]->ops.tuner_ops,
> + &adap->fe[0]->ops.tuner_ops,
> + sizeof(struct dvb_tuner_ops));
> + }
>
> - if (fe && adap->fe[1]) {
> - /* attach tuner for 2nd FE */
> - fe = dvb_attach(dvb_pll_attach, adap->fe[1],
> + return 0;
> + } else {
> + /* attach tuner */
> + fe = dvb_attach(dvb_pll_attach, adap->fe[0],
> (0xc0 >> 1), &d->i2c_adap,
> DVB_PLL_SAMSUNG_DTOS403IH102A);
Please don't use dvb_attach() for those converted modules. The
dvb_attach() is a very dirty hack that was created as an alternative
to provide an abstraction similar to the one that the I2C core already
provides. See how V4L calls the subdev callbacks at
include/media/v4l2-subdev.h.
One of the big disadvantages of the dvb_attach() is that it allows just
_one_ entry point function on a sub-device. This only works for very
simple demods that don't provide, for example, hardware filtering.
> +
> + if (fe && adap->fe[1]) {
> + /* attach tuner for 2nd FE */
> + fe = dvb_attach(dvb_pll_attach, adap->fe[1],
> + (0xc0 >> 1), &d->i2c_adap,
> + DVB_PLL_SAMSUNG_DTOS403IH102A);
> + }
> }
>
> break;
> case ANYSEE_HW_508TC: /* 18 */
> case ANYSEE_HW_508PTC: /* 21 */
> + {
> /* E7 TC */
> /* E7 PTC */
> + struct tda18212_config tda18212_config = anysee_tda18212_config;
>
> - /* attach tuner */
> - fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
> - &anysee_tda18212_config);
> -
> - if (fe) {
> - /* attach tuner for 2nd FE */
> - fe = dvb_attach(tda18212_attach, adap->fe[1],
> - &d->i2c_adap, &anysee_tda18212_config);
> + tda18212_config.fe = adap->fe[0];
> + ret = anysee_add_i2c_dev(d, "tda18212", 0x60, &tda18212_config);
> + if (ret)
> + goto err;
> +
> + /* copy tuner ops for 2nd FE as tuner is shared */
> + if (adap->fe[1]) {
> + adap->fe[1]->tuner_priv = adap->fe[0]->tuner_priv;
> + memcpy(&adap->fe[1]->ops.tuner_ops,
> + &adap->fe[0]->ops.tuner_ops,
> + sizeof(struct dvb_tuner_ops));
> }
>
> - break;
> + return 0;
> + }
> case ANYSEE_HW_508S2: /* 19 */
> case ANYSEE_HW_508PS2: /* 22 */
> /* E7 S2 */
> @@ -997,13 +1100,18 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
> break;
>
> case ANYSEE_HW_508T2C: /* 20 */
> + {
> /* E7 T2C */
> + struct tda18212_config tda18212_config =
> + anysee_tda18212_config2;
>
> - /* attach tuner */
> - fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
> - &anysee_tda18212_config2);
> + tda18212_config.fe = adap->fe[0];
> + ret = anysee_add_i2c_dev(d, "tda18212", 0x60, &tda18212_config);
> + if (ret)
> + goto err;
>
> - break;
> + return 0;
> + }
> default:
> fe = NULL;
> }
> @@ -1012,7 +1120,7 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
> ret = 0;
> else
> ret = -ENODEV;
> -
> +err:
> return ret;
> }
>
> @@ -1270,6 +1378,11 @@ static int anysee_init(struct dvb_usb_device *d)
>
> static void anysee_exit(struct dvb_usb_device *d)
> {
> + struct anysee_state *state = d_to_priv(d);
> +
> + if (state->i2c_client[0])
> + anysee_del_i2c_dev(d);
> +
> return anysee_ci_release(d);
> }
>
> diff --git a/drivers/media/usb/dvb-usb-v2/anysee.h b/drivers/media/usb/dvb-usb-v2/anysee.h
> index 8f426d9..3ca2bca 100644
> --- a/drivers/media/usb/dvb-usb-v2/anysee.h
> +++ b/drivers/media/usb/dvb-usb-v2/anysee.h
> @@ -55,8 +55,11 @@ struct anysee_state {
> u8 buf[64];
> u8 seq;
> u8 hw; /* PCB ID */
> + #define ANYSEE_I2C_CLIENT_MAX 1
> + struct i2c_client *i2c_client[ANYSEE_I2C_CLIENT_MAX];
> u8 fe_id:1; /* frondend ID */
> u8 has_ci:1;
> + u8 has_tda18212:1;
> u8 ci_attached:1;
> struct dvb_ca_en50221 ci;
> unsigned long ci_cam_ready; /* jiffies */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client
2014-09-18 12:31 ` Mauro Carvalho Chehab
@ 2014-09-18 13:01 ` Antti Palosaari
2014-09-21 21:15 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 11+ messages in thread
From: Antti Palosaari @ 2014-09-18 13:01 UTC (permalink / raw
To: Mauro Carvalho Chehab; +Cc: linux-media
On 09/18/2014 03:31 PM, Mauro Carvalho Chehab wrote:
> Em Sun, 7 Sep 2014 04:59:55 +0300
> Antti Palosaari <crope@iki.fi> escreveu:
>
>> Used tda18212 tuner is implemented as I2C driver. Implement I2C
>> client to anysee and use it for tda18212.
>> +static int anysee_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
>> + void *platform_data)
>> +
>> +static void anysee_del_i2c_dev(struct dvb_usb_device *d)
>
> Please, instead of adding a function to insert/remove an I2C driver on every
> place, put them into a common place.
>
> I would actually very much prefer if you could reuse the same code that
> are already at the media subsystem (see v4l2_i2c_new_subdev_board &
> friends at drivers/media/v4l2-core/v4l2-common.c), eventually making it
> more generic.
>
> Btw, as we want to use the media controller also for DVB, we'll end
> by needing to use a call similar to v4l2_device_register_subdev().
> So, having this code all on just one place will make easier for us to
> go to this next step.
I am just learning and finding out best practices to use I2C drivers.
That was one implementation solution and IMHO not so bad even. Sure
those 2 functions could be replaced some more common at some phase, but
currently, when there is only few drivers, I don't see need for common
implementation. Let it happen when best practices are clear. And I
really wonder why there is no such general implementation provided by
I2C framework?
If you look how I have improved that in a long ran; 1st implementation
is in em28xx driver, 2nd test was dd-bridge driver, then this anysee and
eventually there is af9035 (which is almost same than this anysee).
>> @@ -939,46 +1025,63 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
>> * fails attach old simple PLL. */
>>
>> /* attach tuner */
>> - fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
>> - &anysee_tda18212_config);
>> + if (state->has_tda18212) {
>> + struct tda18212_config tda18212_config =
>> + anysee_tda18212_config;
>>
>> - if (fe && adap->fe[1]) {
>> - /* attach tuner for 2nd FE */
>> - fe = dvb_attach(tda18212_attach, adap->fe[1],
>> - &d->i2c_adap, &anysee_tda18212_config);
>> - break;
>> - } else if (fe) {
>> - break;
>> - }
>> -
>> - /* attach tuner */
>> - fe = dvb_attach(dvb_pll_attach, adap->fe[0], (0xc0 >> 1),
>> - &d->i2c_adap, DVB_PLL_SAMSUNG_DTOS403IH102A);
>> + tda18212_config.fe = adap->fe[0];
>> + ret = anysee_add_i2c_dev(d, "tda18212", 0x60,
>> + &tda18212_config);
>> + if (ret)
>> + goto err;
>> +
>> + /* copy tuner ops for 2nd FE as tuner is shared */
>> + if (adap->fe[1]) {
>> + adap->fe[1]->tuner_priv =
>> + adap->fe[0]->tuner_priv;
>> + memcpy(&adap->fe[1]->ops.tuner_ops,
>> + &adap->fe[0]->ops.tuner_ops,
>> + sizeof(struct dvb_tuner_ops));
>> + }
>>
>> - if (fe && adap->fe[1]) {
>> - /* attach tuner for 2nd FE */
>> - fe = dvb_attach(dvb_pll_attach, adap->fe[1],
>> + return 0;
>> + } else {
>> + /* attach tuner */
>> + fe = dvb_attach(dvb_pll_attach, adap->fe[0],
>> (0xc0 >> 1), &d->i2c_adap,
>> DVB_PLL_SAMSUNG_DTOS403IH102A);
>
> Please don't use dvb_attach() for those converted modules. The
> dvb_attach() is a very dirty hack that was created as an alternative
> to provide an abstraction similar to the one that the I2C core already
> provides. See how V4L calls the subdev callbacks at
> include/media/v4l2-subdev.h.
You looked it wrong, it is dvb_pll_attach. tda18212 attach is replaced
here with I2C driver. It is tda18212 which is converted here to I2C
driver, whilst dvb-pll leaves old.
>
> One of the big disadvantages of the dvb_attach() is that it allows just
> _one_ entry point function on a sub-device. This only works for very
> simple demods that don't provide, for example, hardware filtering.
>
>> +
>> + if (fe && adap->fe[1]) {
>> + /* attach tuner for 2nd FE */
>> + fe = dvb_attach(dvb_pll_attach, adap->fe[1],
>> + (0xc0 >> 1), &d->i2c_adap,
>> + DVB_PLL_SAMSUNG_DTOS403IH102A);
>> + }
That patch has nothing wrong as I explained :)
It could be improved by implementing general I2C sub-driver loading
functions, but these driver specific implementations are just fine until
more common solution is developed.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client
2014-09-18 13:01 ` Antti Palosaari
@ 2014-09-21 21:15 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2014-09-21 21:15 UTC (permalink / raw
To: Antti Palosaari; +Cc: linux-media
Em Thu, 18 Sep 2014 16:01:59 +0300
Antti Palosaari <crope@iki.fi> escreveu:
>
>
> On 09/18/2014 03:31 PM, Mauro Carvalho Chehab wrote:
> > Em Sun, 7 Sep 2014 04:59:55 +0300
> > Antti Palosaari <crope@iki.fi> escreveu:
> >
> >> Used tda18212 tuner is implemented as I2C driver. Implement I2C
> >> client to anysee and use it for tda18212.
>
> >> +static int anysee_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
> >> + void *platform_data)
>
> >> +
> >> +static void anysee_del_i2c_dev(struct dvb_usb_device *d)
>
> >
> > Please, instead of adding a function to insert/remove an I2C driver on every
> > place, put them into a common place.
> >
> > I would actually very much prefer if you could reuse the same code that
> > are already at the media subsystem (see v4l2_i2c_new_subdev_board &
> > friends at drivers/media/v4l2-core/v4l2-common.c), eventually making it
> > more generic.
> >
> > Btw, as we want to use the media controller also for DVB, we'll end
> > by needing to use a call similar to v4l2_device_register_subdev().
> > So, having this code all on just one place will make easier for us to
> > go to this next step.
>
> I am just learning and finding out best practices to use I2C drivers.
> That was one implementation solution and IMHO not so bad even. Sure
> those 2 functions could be replaced some more common at some phase, but
> currently, when there is only few drivers, I don't see need for common
> implementation. Let it happen when best practices are clear.
> And I
> really wonder why there is no such general implementation provided by
> I2C framework?
It used to have on Kernel 2.4 and 2.6 (up to 2.6.1x or 2.6.2x - don't
remember the exact Kernel where this changed). The previous binding model
got deprecated and removed, as different subsystems might need to do
different things. So, it was opted to have one solution per subsystem.
>
> If you look how I have improved that in a long ran; 1st implementation
> is in em28xx driver, 2nd test was dd-bridge driver, then this anysee and
> eventually there is af9035 (which is almost same than this anysee).
So, now we have 3 different implementations (4, if we count the generic
one at v4l-core). That's bad. We should really stick with the original
idea of having just one per subsystem.
So, please write a patch unifying them into just one.
For now, I'll be merging it, but please let's avoid all those code
duplication, as this makes harder to maintain.
>
>
> >> @@ -939,46 +1025,63 @@ static int anysee_tuner_attach(struct dvb_usb_adapter *adap)
> >> * fails attach old simple PLL. */
> >>
> >> /* attach tuner */
> >> - fe = dvb_attach(tda18212_attach, adap->fe[0], &d->i2c_adap,
> >> - &anysee_tda18212_config);
> >> + if (state->has_tda18212) {
> >> + struct tda18212_config tda18212_config =
> >> + anysee_tda18212_config;
> >>
> >> - if (fe && adap->fe[1]) {
> >> - /* attach tuner for 2nd FE */
> >> - fe = dvb_attach(tda18212_attach, adap->fe[1],
> >> - &d->i2c_adap, &anysee_tda18212_config);
> >> - break;
> >> - } else if (fe) {
> >> - break;
> >> - }
> >> -
> >> - /* attach tuner */
> >> - fe = dvb_attach(dvb_pll_attach, adap->fe[0], (0xc0 >> 1),
> >> - &d->i2c_adap, DVB_PLL_SAMSUNG_DTOS403IH102A);
> >> + tda18212_config.fe = adap->fe[0];
> >> + ret = anysee_add_i2c_dev(d, "tda18212", 0x60,
> >> + &tda18212_config);
> >> + if (ret)
> >> + goto err;
> >> +
> >> + /* copy tuner ops for 2nd FE as tuner is shared */
> >> + if (adap->fe[1]) {
> >> + adap->fe[1]->tuner_priv =
> >> + adap->fe[0]->tuner_priv;
> >> + memcpy(&adap->fe[1]->ops.tuner_ops,
> >> + &adap->fe[0]->ops.tuner_ops,
> >> + sizeof(struct dvb_tuner_ops));
> >> + }
> >>
> >> - if (fe && adap->fe[1]) {
> >> - /* attach tuner for 2nd FE */
> >> - fe = dvb_attach(dvb_pll_attach, adap->fe[1],
> >> + return 0;
> >> + } else {
> >> + /* attach tuner */
> >> + fe = dvb_attach(dvb_pll_attach, adap->fe[0],
> >> (0xc0 >> 1), &d->i2c_adap,
> >> DVB_PLL_SAMSUNG_DTOS403IH102A);
> >
> > Please don't use dvb_attach() for those converted modules. The
> > dvb_attach() is a very dirty hack that was created as an alternative
> > to provide an abstraction similar to the one that the I2C core already
> > provides. See how V4L calls the subdev callbacks at
> > include/media/v4l2-subdev.h.
>
> You looked it wrong, it is dvb_pll_attach. tda18212 attach is replaced
> here with I2C driver. It is tda18212 which is converted here to I2C
> driver, whilst dvb-pll leaves old.
Ah, OK.
>
> >
> > One of the big disadvantages of the dvb_attach() is that it allows just
> > _one_ entry point function on a sub-device. This only works for very
> > simple demods that don't provide, for example, hardware filtering.
> >
> >> +
> >> + if (fe && adap->fe[1]) {
> >> + /* attach tuner for 2nd FE */
> >> + fe = dvb_attach(dvb_pll_attach, adap->fe[1],
> >> + (0xc0 >> 1), &d->i2c_adap,
> >> + DVB_PLL_SAMSUNG_DTOS403IH102A);
> >> + }
>
> That patch has nothing wrong as I explained :)
> It could be improved by implementing general I2C sub-driver loading
> functions, but these driver specific implementations are just fine until
> more common solution is developed.
>
> regards
> Antti
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-09-21 21:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-07 1:59 [PATCH v2 1/8] tda18212: add support for slave chip version Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 2/8] tda18212: prepare for I2C client conversion Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 3/8] anysee: convert tda18212 tuner to I2C client Antti Palosaari
2014-09-18 12:31 ` Mauro Carvalho Chehab
2014-09-18 13:01 ` Antti Palosaari
2014-09-21 21:15 ` Mauro Carvalho Chehab
2014-09-07 1:59 ` [PATCH v2 4/8] em28xx: " Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 5/8] tda18212: convert driver to I2C binding Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 6/8] tda18212: clean logging Antti Palosaari
2014-09-07 1:59 ` [PATCH v2 7/8] tda18212: rename state from 'priv' to 'dev' Antti Palosaari
2014-09-07 2:00 ` [PATCH v2 8/8] tda18212: convert to RegMap API Antti Palosaari
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.