Em Tue, 30 Dec 2014 11:10:51 -0200
Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> escreveu:
> Em Fri, 05 Dec 2014 19:49:33 +0900
> tskd08@xxxxxxxxx escreveu:
>
> > From: Akihiro Tsukada <tskd08@xxxxxxxxx>
> >
> > Define a standard interface for demod/tuner i2c driver modules.
> > A module client calls dvb_i2c_attach_{fe,tuner}(),
> > and a module driver defines struct dvb_i2c_module_param and
> > calls DEFINE_DVB_I2C_MODULE() macro.
> >
> > This template provides implicit module requests and ref-counting,
> > alloc/free's private data structures,
> > fixes the usage of clientdata of i2c devices,
> > and defines the platformdata structures for passing around
> > device specific config/output parameters between drivers and clients.
> > These kinds of code are common to (almost) all dvb i2c drivers/client,
> > but they were scattered over adapter modules and demod/tuner modules.
>
> The idea behind this patch is good. I didn't have time yet to test it on a
> device that I have currently access.
>
> I have a few comments below, mostly regards with naming convention.
>
> By seeing the patches you converted a few drivers to use this, I'm a little
> bit concern about the conversion. We need something that won't be hard to
> convert to the new model without requiring to re-test everything.
>
> Anyway, my plan is to take a deeper look on it next week and convert one
> or two drivers to use the new I2C binding approach you're proposing.
Ok, did some tests and it worked. The issues I commented on my last email
may be fixed latter. I'm working on adding media controller support for
it.
The only thing I noticed is that it is causing some warnings at
dmesg about trying to create already created sysfs nodes, when the
driver is removed/reinserted.
Probably, the remove callback is called too soon or too late.
I'll do more tests latter (either this or the next week).
I used the enclosed patch on my tests.
Regards,
Mauro
[PATCH] mb86a20s: convert it to I2C binding model
Instead of using I2C raw API, use the standard I2C binding API,
with the DVB core support for it.
Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
diff --git a/drivers/media/dvb-frontends/mb86a20s.c b/drivers/media/dvb-frontends/mb86a20s.c
index 8f54c39..8dd608b 100644
--- a/drivers/media/dvb-frontends/mb86a20s.c
+++ b/drivers/media/dvb-frontends/mb86a20s.c
@@ -18,6 +18,7 @@
#include <asm/div64.h>
#include "dvb_frontend.h"
+#include "dvb_i2c.h"
#include "mb86a20s.h"
#define NUM_LAYERS 3
@@ -35,12 +36,10 @@ static u8 mb86a20s_subchannel[] = {
};
struct mb86a20s_state {
- struct i2c_adapter *i2c;
+ struct i2c_client *i2c;
const struct mb86a20s_config *config;
u32 last_frequency;
- struct dvb_frontend frontend;
-
u32 if_freq;
enum mb86a20s_bandwidth bw;
bool inversion;
@@ -234,7 +233,7 @@ static int mb86a20s_i2c_writereg(struct mb86a20s_state *state,
};
int rc;
- rc = i2c_transfer(state->i2c, &msg, 1);
+ rc = i2c_transfer(state->i2c->adapter, &msg, 1);
if (rc != 1) {
dev_err(&state->i2c->dev,
"%s: writereg error (rc == %i, reg == 0x%02x, data == 0x%02x)\n",
@@ -269,7 +268,7 @@ static int mb86a20s_i2c_readreg(struct mb86a20s_state *state,
{ .addr = i2c_addr, .flags = I2C_M_RD, .buf = &val, .len = 1 }
};
- rc = i2c_transfer(state->i2c, msg, 2);
+ rc = i2c_transfer(state->i2c->adapter, msg, 2);
if (rc != 2) {
dev_err(&state->i2c->dev, "%s: reg=0x%x (error=%d)\n",
@@ -281,11 +280,11 @@ static int mb86a20s_i2c_readreg(struct mb86a20s_state *state,
}
#define mb86a20s_readreg(state, reg) \
- mb86a20s_i2c_readreg(state, state->config->demod_address, reg)
+ mb86a20s_i2c_readreg(state, state->i2c->addr, reg)
#define mb86a20s_writereg(state, reg, val) \
- mb86a20s_i2c_writereg(state, state->config->demod_address, reg, val)
+ mb86a20s_i2c_writereg(state, state->i2c->addr, reg, val)
#define mb86a20s_writeregdata(state, regdata) \
- mb86a20s_i2c_writeregdata(state, state->config->demod_address, \
+ mb86a20s_i2c_writeregdata(state, state->i2c->addr, \
regdata, ARRAY_SIZE(regdata))
/*
@@ -2058,41 +2057,34 @@ static int mb86a20s_tune(struct dvb_frontend *fe,
return rc;
}
-static void mb86a20s_release(struct dvb_frontend *fe)
+static int mb86a20s_remove(struct i2c_client *i2c)
{
- struct mb86a20s_state *state = fe->demodulator_priv;
-
- dev_dbg(&state->i2c->dev, "%s called.\n", __func__);
+ dev_dbg(&i2c->dev, "%s called.\n", __func__);
- kfree(state);
+ return 0;
}
static struct dvb_frontend_ops mb86a20s_ops;
-struct dvb_frontend *mb86a20s_attach(const struct mb86a20s_config *config,
- struct i2c_adapter *i2c)
+static int mb86a20s_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
{
+ struct dvb_frontend *fe;
struct mb86a20s_state *state;
u8 rev;
dev_dbg(&i2c->dev, "%s called.\n", __func__);
- /* allocate memory for the internal state */
- state = kzalloc(sizeof(struct mb86a20s_state), GFP_KERNEL);
- if (state == NULL) {
- dev_err(&i2c->dev,
- "%s: unable to allocate memory for state\n", __func__);
- goto error;
- }
+ fe = i2c_get_clientdata(i2c);
+ state = fe->demodulator_priv;
/* setup the state */
- state->config = config;
+ memcpy(&state->config, i2c->dev.platform_data, sizeof(state->config));
state->i2c = i2c;
/* create dvb_frontend */
- memcpy(&state->frontend.ops, &mb86a20s_ops,
+ memcpy(&fe->ops, &mb86a20s_ops,
sizeof(struct dvb_frontend_ops));
- state->frontend.demodulator_priv = state;
/* Check if it is a mb86a20s frontend */
rev = mb86a20s_readreg(state, 0);
@@ -2104,16 +2096,11 @@ struct dvb_frontend *mb86a20s_attach(const struct mb86a20s_config *config,
dev_dbg(&i2c->dev,
"Frontend revision %d is unknown - aborting.\n",
rev);
- goto error;
+ return -ENODEV;
}
- return &state->frontend;
-
-error:
- kfree(state);
- return NULL;
+ return 0;
}
-EXPORT_SYMBOL(mb86a20s_attach);
static struct dvb_frontend_ops mb86a20s_ops = {
.delsys = { SYS_ISDBT },
@@ -2132,8 +2119,6 @@ static struct dvb_frontend_ops mb86a20s_ops = {
.frequency_stepsize = 62500,
},
- .release = mb86a20s_release,
-
.init = mb86a20s_initfe,
.set_frontend = mb86a20s_set_frontend,
.get_frontend = mb86a20s_get_frontend_dummy,
@@ -2142,6 +2127,21 @@ static struct dvb_frontend_ops mb86a20s_ops = {
.tune = mb86a20s_tune,
};
+static const struct i2c_device_id mb86a20s_id[] = {
+ { "mb86a20s", 0 },
+ {}
+};
+
+static const struct dvb_i2c_module_param mb86a20s_param = {
+ .ops.fe_ops = NULL,
+ .priv_probe = mb86a20s_probe,
+ .priv_remove = mb86a20s_remove,
+ .priv_size = sizeof(struct mb86a20s_state),
+ .is_tuner = false,
+};
+
+DEFINE_DVB_I2C_MODULE(mb86a20s, mb86a20s_id, mb86a20s_param);
+
MODULE_DESCRIPTION("DVB Frontend module for Fujitsu mb86A20s hardware");
MODULE_AUTHOR("Mauro Carvalho Chehab");
MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/mb86a20s.h b/drivers/media/dvb-frontends/mb86a20s.h
index cbeb941..18743c3 100644
--- a/drivers/media/dvb-frontends/mb86a20s.h
+++ b/drivers/media/dvb-frontends/mb86a20s.h
@@ -34,23 +34,4 @@ struct mb86a20s_config {
bool is_serial;
};
-#if IS_ENABLED(CONFIG_DVB_MB86A20S)
-extern struct dvb_frontend *mb86a20s_attach(const struct mb86a20s_config *config,
- struct i2c_adapter *i2c);
-extern struct i2c_adapter *mb86a20s_get_tuner_i2c_adapter(struct dvb_frontend *);
-#else
-static inline struct dvb_frontend *mb86a20s_attach(
- const struct mb86a20s_config *config, struct i2c_adapter *i2c)
-{
- printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
- return NULL;
-}
-static struct i2c_adapter *
- mb86a20s_get_tuner_i2c_adapter(struct dvb_frontend *fe)
-{
- printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
- return NULL;
-}
-#endif
-
#endif /* MB86A20S */
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index c47d182..fc23b7a 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -26,6 +26,7 @@
#include "cx23885.h"
#include <media/v4l2-common.h>
+#include "dvb_i2c.h"
#include "dvb_ca_en50221.h"
#include "s5h1409.h"
#include "s5h1411.h"
@@ -542,7 +543,10 @@ static struct xc5000_config mygica_x8506_xc5000_config = {
};
static struct mb86a20s_config mygica_x8507_mb86a20s_config = {
- .demod_address = 0x10,
+};
+
+static const struct i2c_board_info mb86a20s_board_info = {
+ I2C_BOARD_INFO("mb86a20s", 0x10)
};
static struct xc5000_config mygica_x8507_xc5000_config = {
@@ -1503,9 +1507,10 @@ static int dvb_register(struct cx23885_tsport *port)
case CX23885_BOARD_MYGICA_X8507:
i2c_bus = &dev->i2c_bus[0];
i2c_bus2 = &dev->i2c_bus[1];
- fe0->dvb.frontend = dvb_attach(mb86a20s_attach,
- &mygica_x8507_mb86a20s_config,
- &i2c_bus->i2c_adap);
+ fe0->dvb.frontend = dvb_i2c_attach_fe(&i2c_bus->i2c_adap,
+ &mb86a20s_board_info,
+ &mygica_x8507_mb86a20s_config,
+ NULL);
if (fe0->dvb.frontend == NULL)
break;
diff --git a/drivers/media/pci/saa7134/saa7134-dvb.c b/drivers/media/pci/saa7134/saa7134-dvb.c
index 73ffbab..74b5ce0 100644
--- a/drivers/media/pci/saa7134/saa7134-dvb.c
+++ b/drivers/media/pci/saa7134/saa7134-dvb.c
@@ -34,6 +34,7 @@
#include "dvb-pll.h"
#include <dvb_frontend.h>
+#include "dvb_i2c.h"
#include "mt352.h"
#include "mt352_priv.h" /* FIXME */
#include "tda1004x.h"
@@ -245,7 +246,10 @@ static struct tda18271_config kworld_tda18271_config = {
};
static const struct mb86a20s_config kworld_mb86a20s_config = {
- .demod_address = 0x10,
+};
+
+static const struct i2c_board_info mb86a20s_board_info = {
+ I2C_BOARD_INFO("mb86a20s", 0x10)
};
static int kworld_sbtvd_gate_ctrl(struct dvb_frontend* fe, int enable)
@@ -1807,9 +1811,10 @@ static int dvb_init(struct saa7134_dev *dev)
/* Switch to digital mode */
saa7134_tuner_callback(dev, 0,
TDA18271_CALLBACK_CMD_AGC_ENABLE, 1);
- fe0->dvb.frontend = dvb_attach(mb86a20s_attach,
- &kworld_mb86a20s_config,
- &dev->i2c_adap);
+ fe0->dvb.frontend = dvb_i2c_attach_fe(&dev->i2c_adap,
+ &mb86a20s_board_info,
+ &kworld_mb86a20s_config,
+ NULL);
if (fe0->dvb.frontend != NULL) {
dvb_attach(tda829x_attach, fe0->dvb.frontend,
&dev->i2c_adap, 0x4b,
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index dd600b9..27803a8 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -26,6 +26,7 @@
#include <media/v4l2-common.h>
#include <media/videobuf-vmalloc.h>
+#include "dvb_i2c.h"
#include "xc5000.h"
#include "s5h1432.h"
#include "tda18271.h"
@@ -138,10 +139,13 @@ static struct tda18271_config hcw_tda18271_config = {
};
static const struct mb86a20s_config pv_mb86a20s_config = {
- .demod_address = 0x10,
.is_serial = true,
};
+static const struct i2c_board_info mb86a20s_board_info = {
+ I2C_BOARD_INFO("mb86a20s", 0x10)
+};
+
static struct tda18271_config pv_tda18271_config = {
.std_map = &mb86a20s_tda18271_config,
.gate = TDA18271_GATE_DIGITAL,
@@ -815,10 +819,10 @@ static int dvb_init(struct cx231xx *dev)
"%s: looking for demod on i2c bus: %d\n",
__func__, i2c_adapter_id(tuner_i2c));
- dev->dvb->frontend = dvb_attach(mb86a20s_attach,
- &pv_mb86a20s_config,
- demod_i2c);
-
+ dev->dvb->frontend = dvb_i2c_attach_fe(demod_i2c,
+ &mb86a20s_board_info,
+ &pv_mb86a20s_config,
+ NULL);
if (dev->dvb->frontend == NULL) {
dev_err(dev->dev,
"Failed to attach mb86a20s demod\n");
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index aee70d4..6fa4eee 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -34,6 +34,7 @@
#include "tuner-simple.h"
#include <linux/gpio.h>
+#include "dvb_i2c.h"
#include "lgdt330x.h"
#include "lgdt3305.h"
#include "zl10353.h"
@@ -833,10 +834,13 @@ static struct qt1010_config em28xx_qt1010_config = {
};
static const struct mb86a20s_config c3tech_duo_mb86a20s_config = {
- .demod_address = 0x10,
.is_serial = true,
};
+static const struct i2c_board_info mb86a20s_board_info = {
+ I2C_BOARD_INFO("mb86a20s", 0x10)
+};
+
static struct tda18271_std_map mb86a20s_tda18271_config = {
.dvbt_6 = { .if_freq = 4000, .agc_mode = 3, .std = 4,
.if_lvl = 1, .rfagc_top = 0x37, },
@@ -1323,9 +1327,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
break;
case EM2884_BOARD_C3TECH_DIGITAL_DUO:
- dvb->fe[0] = dvb_attach(mb86a20s_attach,
- &c3tech_duo_mb86a20s_config,
- &dev->i2c_adap[dev->def_i2c_bus]);
+ dvb->fe[0] = dvb_i2c_attach_fe(&dev->i2c_adap[dev->def_i2c_bus],
+ &mb86a20s_board_info,
+ &c3tech_duo_mb86a20s_config,
+ NULL);
if (dvb->fe[0] != NULL)
dvb_attach(tda18271_attach, dvb->fe[0], 0x60,
&dev->i2c_adap[dev->def_i2c_bus],
--
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
