* [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
@ 2013-03-05 10:55 Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 1/3] em28xx: Prepare to support 2 different I2C buses Mauro Carvalho Chehab
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-05 10:55 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Frank Schäfer
The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
currently used only by eeprom, and bus 1 for the rest. Add support to
register both buses.
Tested with 2 devices: HVR-950C (em2883 - 1 bus) and Terratec Cinergy HTC
(em2884 - 2 buses):
1) HVR-950C (1 bus only)
[ 7476.154903] em28xx: New device WinTV HVR-980 @ 480 Mbps (2040:6513, interface 0, class 0)
[ 7476.163146] em28xx: Audio interface 0 found (Vendor Class)
[ 7476.168609] em28xx: Video interface 0 found: isoc
[ 7476.173293] em28xx: DVB interface 0 found: isoc
[ 7476.177946] em28xx: chip ID is em2882/3
[ 7476.319920] em2882/3 #0: i2c eeprom 00: 1a eb 67 95 40 20 13 65 d0 12 5c 03 82 1e 6a 18
[ 7476.328118] em2882/3 #0: i2c eeprom 10: 00 00 24 57 66 07 01 00 00 00 00 00 00 00 00 00
[ 7476.336468] em2882/3 #0: i2c eeprom 20: 46 00 01 00 f0 10 02 00 b8 00 00 00 5b 1c 00 00
[ 7476.344871] em2882/3 #0: i2c eeprom 30: 00 00 20 40 20 80 02 20 01 01 01 01 00 00 00 00
[ 7476.353269] em2882/3 #0: i2c eeprom 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 7476.361615] em2882/3 #0: i2c eeprom 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 7476.370070] em2882/3 #0: i2c eeprom 60: 00 00 00 00 00 00 00 00 00 00 18 03 34 00 30 00
[ 7476.378421] em2882/3 #0: i2c eeprom 70: 32 00 38 00 34 00 34 00 39 00 30 00 31 00 38 00
[ 7476.386633] em2882/3 #0: i2c eeprom 80: 00 00 1e 03 57 00 69 00 6e 00 54 00 56 00 20 00
[ 7476.394833] em2882/3 #0: i2c eeprom 90: 48 00 56 00 52 00 2d 00 39 00 38 00 30 00 00 00
[ 7476.402986] em2882/3 #0: i2c eeprom a0: 84 12 00 00 05 50 1a 7f d4 78 23 b1 fe d0 18 85
[ 7476.411167] em2882/3 #0: i2c eeprom b0: ff 00 00 00 04 84 0a 00 01 01 20 77 00 40 fa 40
[ 7476.419353] em2882/3 #0: i2c eeprom c0: 1d f0 74 02 01 00 01 79 4f 00 00 00 00 00 00 00
[ 7476.427497] em2882/3 #0: i2c eeprom d0: 84 12 00 00 05 50 1a 7f d4 78 23 b1 fe d0 18 85
[ 7476.435673] em2882/3 #0: i2c eeprom e0: ff 00 00 00 04 84 0a 00 01 01 20 77 00 40 fa 40
[ 7476.443878] em2882/3 #0: i2c eeprom f0: 1d f0 74 02 01 00 01 79 4f 00 00 00 00 00 00 00
[ 7476.452071] em2882/3 #0: EEPROM ID = 1a eb 67 95, EEPROM hash = 0x994b2bdd
[ 7476.458922] em2882/3 #0: EEPROM info:
[ 7476.462617] em2882/3 #0: No audio on board.
[ 7476.466872] em2882/3 #0: 500mA max power
[ 7476.470867] em2882/3 #0: Table at offset 0x00, strings=0x0000, 0x0000, 0x0000
[ 7476.508416] em2882/3 #0: found i2c device @ 0xa0 on bus 0 [eeprom]
[ 7476.519284] em2882/3 #0: found i2c device @ 0xb8 on bus 0 [tvp5150a]
[ 7476.527531] em2882/3 #0: found i2c device @ 0xc2 on bus 0 [tuner (analog)]
[ 7476.545647] em2882/3 #0: Identified as Hauppauge WinTV HVR 950 (card=16)
2) Terratec Cinergy HTC (2 buses)
[ 7598.430202] em28xx: New device TERRATEC Cinergy HTC Stick @ 480 Mbps (0ccd:00b2, interface 0, class 0)
[ 7598.439579] em28xx: Audio interface 0 found (Vendor Class)
[ 7598.445053] em28xx: Video interface 0 found: isoc
[ 7598.449745] em28xx: DVB interface 0 found: isoc
[ 7598.454428] em28xx: chip ID is em2884
[ 7598.517046] em2884 #0: i2c eeprom 0000: 26 00 00 00 02 0b 0f e5 f5 64 01 60 09 e5 f5 64
[ 7598.525292] em2884 #0: i2c eeprom 0010: 09 60 03 c2 c6 22 e5 f7 b4 03 13 e5 f6 b4 87 03
[ 7598.533649] em2884 #0: i2c eeprom 0020: 02 0a b9 e5 f6 b4 93 03 02 09 46 c2 c6 22 c2 c6
[ 7598.542086] em2884 #0: i2c eeprom 0030: 22 00 60 00 ef 70 08 85 3d 82 85 3c 83 93 ff ef
[ 7598.550422] em2884 #0: i2c eeprom 0040: 60 19 85 3d 82 85 3c 83 e4 93 12 07 a3 12 0a fe
[ 7598.558817] em2884 #0: i2c eeprom 0050: 05 3d e5 3d 70 02 05 3c 1f 80 e4 22 12 0b 06 02
[ 7598.567156] em2884 #0: i2c eeprom 0060: 07 e2 01 00 1a eb 67 95 cd 0c b2 00 f0 13 6b 03
[ 7598.575498] em2884 #0: i2c eeprom 0070: 9a 24 6a 1c 86 14 27 57 4e 16 29 00 60 00 00 00
[ 7598.583939] em2884 #0: i2c eeprom 0080: 02 00 00 00 5e 00 13 00 f0 10 44 82 82 00 00 00
[ 7598.592275] em2884 #0: i2c eeprom 0090: 5b 53 c0 00 00 00 20 40 20 80 02 20 01 01 00 00
[ 7598.600618] em2884 #0: i2c eeprom 00a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 7598.609058] em2884 #0: i2c eeprom 00b0: c6 40 00 00 81 00 00 00 00 00 00 00 00 c4 00 00
[ 7598.617373] em2884 #0: i2c eeprom 00c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 1c 03
[ 7598.625758] em2884 #0: i2c eeprom 00d0: 31 00 32 00 33 00 34 00 35 00 36 00 37 00 38 00
[ 7598.634123] em2884 #0: i2c eeprom 00e0: 39 00 41 00 42 00 43 00 44 00 14 03 54 00 45 00
[ 7598.642464] em2884 #0: i2c eeprom 00f0: 52 00 52 00 41 00 54 00 45 00 43 00 20 00 24 03
[ 7598.650900] em2884 #0: i2c eeprom 0100: ... (skipped)
[ 7598.655937] em2884 #0: EEPROM ID = 26 00 00 00, EEPROM hash = 0xfebadddc
[ 7598.662613] em2884 #0: EEPROM info:
[ 7598.666090] em2884 #0: microcode start address = 0x0004, boot configuration = 0x00
[ 7598.681080] em2884 #0: No audio on board.
[ 7598.685175] em2884 #0: 500mA max power
[ 7598.689002] em2884 #0: Table at offset 0x00, strings=0x0000, 0x0000, 0x0000
[ 7598.696181] em2884 #0: 1 bytes requested from i2c device at 0x0, but 0 bytes received
(lots of message similar to above - supressing them to shorten this email)
...
[ 7599.346459] em2884 #0: found i2c device @ 0xa0 on bus 0 [eeprom]
...
[ 7600.057963] em2884 #0: found i2c device @ 0x52 on bus 1 [drxk]
...
[ 7600.249717] em2884 #0: found i2c device @ 0x82 on bus 1 [???]
...
[ 7600.496836] em2884 #0: found i2c device @ 0xc0 on bus 1 [tuner (analog)]
...
[ 7600.760725] em2884 #0: Identified as Terratec Cinergy HTC Stick (card=82)
Mauro Carvalho Chehab (3):
em28xx: Prepare to support 2 different I2C buses
em28xx: Add a separate config dir for secondary bus
em28xx: add support for registering multiple i2c buses
drivers/media/usb/em28xx/em28xx-cards.c | 100 ++++++++++++++++++--------------
drivers/media/usb/em28xx/em28xx-dvb.c | 86 +++++++++++++--------------
drivers/media/usb/em28xx/em28xx-i2c.c | 93 +++++++++++++++++++----------
drivers/media/usb/em28xx/em28xx-input.c | 5 +-
drivers/media/usb/em28xx/em28xx.h | 26 +++++++--
5 files changed, 187 insertions(+), 123 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] em28xx: Prepare to support 2 different I2C buses
2013-03-05 10:55 [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Mauro Carvalho Chehab
@ 2013-03-05 10:55 ` Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 2/3] em28xx: Add a separate config dir for secondary bus Mauro Carvalho Chehab
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-05 10:55 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Frank Schäfer
Newer em28xx devices have 2 buses. Change the logic to allow
using both buses.
This patch was generated by this small script:
for i in drivers/media/usb/em28xx/*.c; do
sed 's,->i2c_adap,->i2c_adap[dev->def_i2c_bus],g;s,->i2c_client,->i2c_client[dev->def_i2c_bus],'
done
Of course, em28xx.h needed manual edit.
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
drivers/media/usb/em28xx/em28xx-cards.c | 32 ++++++------
drivers/media/usb/em28xx/em28xx-dvb.c | 86 ++++++++++++++++-----------------
drivers/media/usb/em28xx/em28xx-i2c.c | 30 ++++++------
drivers/media/usb/em28xx/em28xx-input.c | 5 +-
drivers/media/usb/em28xx/em28xx.h | 10 +++-
5 files changed, 85 insertions(+), 78 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 4dcef9d..d81f7ee 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2249,7 +2249,7 @@ static int em28xx_initialize_mt9m111(struct em28xx *dev)
};
for (i = 0; i < ARRAY_SIZE(regs); i++)
- i2c_master_send(&dev->i2c_client, ®s[i][0], 3);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], ®s[i][0], 3);
return 0;
}
@@ -2276,7 +2276,7 @@ static int em28xx_initialize_mt9m001(struct em28xx *dev)
};
for (i = 0; i < ARRAY_SIZE(regs); i++)
- i2c_master_send(&dev->i2c_client, ®s[i][0], 3);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], ®s[i][0], 3);
return 0;
}
@@ -2294,10 +2294,10 @@ static int em28xx_hint_sensor(struct em28xx *dev)
u16 version;
/* Micron sensor detection */
- dev->i2c_client.addr = 0xba >> 1;
+ dev->i2c_client[dev->def_i2c_bus].addr = 0xba >> 1;
cmd = 0;
- i2c_master_send(&dev->i2c_client, &cmd, 1);
- rc = i2c_master_recv(&dev->i2c_client, (char *)&version_be, 2);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], &cmd, 1);
+ rc = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], (char *)&version_be, 2);
if (rc != 2)
return -EINVAL;
@@ -2748,8 +2748,8 @@ static void em28xx_card_setup(struct em28xx *dev)
#endif
/* Call first TVeeprom */
- dev->i2c_client.addr = 0xa0 >> 1;
- tveeprom_hauppauge_analog(&dev->i2c_client, &tv, dev->eedata);
+ dev->i2c_client[dev->def_i2c_bus].addr = 0xa0 >> 1;
+ tveeprom_hauppauge_analog(&dev->i2c_client[dev->def_i2c_bus], &tv, dev->eedata);
dev->tuner_type = tv.tuner_type;
@@ -2841,15 +2841,15 @@ static void em28xx_card_setup(struct em28xx *dev)
/* request some modules */
if (dev->board.has_msp34xx)
- v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
+ v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
"msp3400", 0, msp3400_addrs);
if (dev->board.decoder == EM28XX_SAA711X)
- v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
+ v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
"saa7115_auto", 0, saa711x_addrs);
if (dev->board.decoder == EM28XX_TVP5150)
- v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
+ v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
"tvp5150", 0, tvp5150_addrs);
if (dev->em28xx_sensor == EM28XX_MT9V011) {
@@ -2861,25 +2861,25 @@ static void em28xx_card_setup(struct em28xx *dev)
};
pdata.xtal = dev->sensor_xtal;
- v4l2_i2c_new_subdev_board(&dev->v4l2_dev, &dev->i2c_adap,
+ v4l2_i2c_new_subdev_board(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
&mt9v011_info, NULL);
}
if (dev->board.adecoder == EM28XX_TVAUDIO)
- v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
+ v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
"tvaudio", dev->board.tvaudio_addr, NULL);
if (dev->board.tuner_type != TUNER_ABSENT) {
int has_demod = (dev->tda9887_conf & TDA9887_PRESENT);
if (dev->board.radio.type)
- v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
+ v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
"tuner", dev->board.radio_addr, NULL);
if (has_demod)
v4l2_i2c_new_subdev(&dev->v4l2_dev,
- &dev->i2c_adap, "tuner",
+ &dev->i2c_adap[dev->def_i2c_bus], "tuner",
0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
if (dev->tuner_addr == 0) {
enum v4l2_i2c_tuner_type type =
@@ -2887,13 +2887,13 @@ static void em28xx_card_setup(struct em28xx *dev)
struct v4l2_subdev *sd;
sd = v4l2_i2c_new_subdev(&dev->v4l2_dev,
- &dev->i2c_adap, "tuner",
+ &dev->i2c_adap[dev->def_i2c_bus], "tuner",
0, v4l2_i2c_tuner_addrs(type));
if (sd)
dev->tuner_addr = v4l2_i2c_subdev_addr(sd);
} else {
- v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
+ v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
"tuner", dev->tuner_addr, NULL);
}
}
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 7200dfe..98b95be 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -463,10 +463,10 @@ static void hauppauge_hvr930c_init(struct em28xx *dev)
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
msleep(10);
- dev->i2c_client.addr = 0x82 >> 1;
+ dev->i2c_client[dev->def_i2c_bus].addr = 0x82 >> 1;
for (i = 0; i < ARRAY_SIZE(regs); i++)
- i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], regs[i].r, regs[i].len);
em28xx_gpio_set(dev, hauppauge_hvr930c_end);
msleep(100);
@@ -520,10 +520,10 @@ static void terratec_h5_init(struct em28xx *dev)
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
msleep(10);
- dev->i2c_client.addr = 0x82 >> 1;
+ dev->i2c_client[dev->def_i2c_bus].addr = 0x82 >> 1;
for (i = 0; i < ARRAY_SIZE(regs); i++)
- i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], regs[i].r, regs[i].len);
em28xx_gpio_set(dev, terratec_h5_end);
};
@@ -573,10 +573,10 @@ static void terratec_htc_stick_init(struct em28xx *dev)
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
msleep(10);
- dev->i2c_client.addr = 0x82 >> 1;
+ dev->i2c_client[dev->def_i2c_bus].addr = 0x82 >> 1;
for (i = 0; i < ARRAY_SIZE(regs); i++)
- i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], regs[i].r, regs[i].len);
em28xx_gpio_set(dev, terratec_htc_stick_end);
};
@@ -631,10 +631,10 @@ static void terratec_htc_usb_xs_init(struct em28xx *dev)
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
msleep(10);
- dev->i2c_client.addr = 0x82 >> 1;
+ dev->i2c_client[dev->def_i2c_bus].addr = 0x82 >> 1;
for (i = 0; i < ARRAY_SIZE(regs); i++)
- i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], regs[i].r, regs[i].len);
em28xx_gpio_set(dev, terratec_htc_usb_xs_end);
};
@@ -660,10 +660,10 @@ static void pctv_520e_init(struct em28xx *dev)
{{ 0x01, 0x00, 0x73, 0xaf }, 4},
};
- dev->i2c_client.addr = 0x82 >> 1; /* 0x41 */
+ dev->i2c_client[dev->def_i2c_bus].addr = 0x82 >> 1; /* 0x41 */
for (i = 0; i < ARRAY_SIZE(regs); i++)
- i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
+ i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], regs[i].r, regs[i].len);
};
static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
@@ -777,7 +777,7 @@ static int em28xx_attach_xc3028(u8 addr, struct em28xx *dev)
struct xc2028_config cfg;
memset(&cfg, 0, sizeof(cfg));
- cfg.i2c_adap = &dev->i2c_adap;
+ cfg.i2c_adap = &dev->i2c_adap[dev->def_i2c_bus];
cfg.i2c_addr = addr;
if (!dev->dvb->fe[0]) {
@@ -960,7 +960,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
switch (dev->model) {
case EM2874_BOARD_LEADERSHIP_ISDBT:
dvb->fe[0] = dvb_attach(s921_attach,
- &sharp_isdbt, &dev->i2c_adap);
+ &sharp_isdbt, &dev->i2c_adap[dev->def_i2c_bus]);
if (!dvb->fe[0]) {
result = -EINVAL;
@@ -974,7 +974,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2880_BOARD_AMD_ATI_TV_WONDER_HD_600:
dvb->fe[0] = dvb_attach(lgdt330x_attach,
&em2880_lgdt3303_dev,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (em28xx_attach_xc3028(0x61, dev) < 0) {
result = -EINVAL;
goto out_free;
@@ -983,7 +983,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2880_BOARD_KWORLD_DVB_310U:
dvb->fe[0] = dvb_attach(zl10353_attach,
&em28xx_zl10353_with_xc3028,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (em28xx_attach_xc3028(0x61, dev) < 0) {
result = -EINVAL;
goto out_free;
@@ -994,7 +994,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2880_BOARD_EMPIRE_DUAL_TV:
dvb->fe[0] = dvb_attach(zl10353_attach,
&em28xx_zl10353_xc3028_no_i2c_gate,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (em28xx_attach_xc3028(0x61, dev) < 0) {
result = -EINVAL;
goto out_free;
@@ -1007,13 +1007,13 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2882_BOARD_KWORLD_VS_DVBT:
dvb->fe[0] = dvb_attach(zl10353_attach,
&em28xx_zl10353_xc3028_no_i2c_gate,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (dvb->fe[0] == NULL) {
/* This board could have either a zl10353 or a mt352.
If the chip id isn't for zl10353, try mt352 */
dvb->fe[0] = dvb_attach(mt352_attach,
&terratec_xs_mt352_cfg,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
}
if (em28xx_attach_xc3028(0x61, dev) < 0) {
@@ -1024,16 +1024,16 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2870_BOARD_KWORLD_355U:
dvb->fe[0] = dvb_attach(zl10353_attach,
&em28xx_zl10353_no_i2c_gate_dev,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (dvb->fe[0] != NULL)
dvb_attach(qt1010_attach, dvb->fe[0],
- &dev->i2c_adap, &em28xx_qt1010_config);
+ &dev->i2c_adap[dev->def_i2c_bus], &em28xx_qt1010_config);
break;
case EM2883_BOARD_KWORLD_HYBRID_330U:
case EM2882_BOARD_EVGA_INDTUBE:
dvb->fe[0] = dvb_attach(s5h1409_attach,
&em28xx_s5h1409_with_xc3028,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (em28xx_attach_xc3028(0x61, dev) < 0) {
result = -EINVAL;
goto out_free;
@@ -1042,10 +1042,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2882_BOARD_KWORLD_ATSC_315U:
dvb->fe[0] = dvb_attach(lgdt330x_attach,
&em2880_lgdt3303_dev,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (dvb->fe[0] != NULL) {
if (!dvb_attach(simple_tuner_attach, dvb->fe[0],
- &dev->i2c_adap, 0x61, TUNER_THOMSON_DTT761X)) {
+ &dev->i2c_adap[dev->def_i2c_bus], 0x61, TUNER_THOMSON_DTT761X)) {
result = -EINVAL;
goto out_free;
}
@@ -1054,7 +1054,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900_R2:
case EM2882_BOARD_PINNACLE_HYBRID_PRO_330E:
dvb->fe[0] = dvb_attach(drxd_attach, &em28xx_drxd, NULL,
- &dev->i2c_adap, &dev->udev->dev);
+ &dev->i2c_adap[dev->def_i2c_bus], &dev->udev->dev);
if (em28xx_attach_xc3028(0x61, dev) < 0) {
result = -EINVAL;
goto out_free;
@@ -1064,10 +1064,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* Philips CU1216L NIM (Philips TDA10023 + Infineon TUA6034) */
dvb->fe[0] = dvb_attach(tda10023_attach,
&em28xx_tda10023_config,
- &dev->i2c_adap, 0x48);
+ &dev->i2c_adap[dev->def_i2c_bus], 0x48);
if (dvb->fe[0]) {
if (!dvb_attach(simple_tuner_attach, dvb->fe[0],
- &dev->i2c_adap, 0x60, TUNER_PHILIPS_CU1216L)) {
+ &dev->i2c_adap[dev->def_i2c_bus], 0x60, TUNER_PHILIPS_CU1216L)) {
result = -EINVAL;
goto out_free;
}
@@ -1076,10 +1076,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2870_BOARD_KWORLD_A340:
dvb->fe[0] = dvb_attach(lgdt3305_attach,
&em2870_lgdt3304_dev,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (dvb->fe[0] != NULL)
dvb_attach(tda18271_attach, dvb->fe[0], 0x60,
- &dev->i2c_adap, &kworld_a340_config);
+ &dev->i2c_adap[dev->def_i2c_bus], &kworld_a340_config);
break;
case EM28174_BOARD_PCTV_290E:
/* set default GPIO0 for LNA, used if GPIOLIB is undefined */
@@ -1087,14 +1087,14 @@ static int em28xx_dvb_init(struct em28xx *dev)
CXD2820R_GPIO_L;
dvb->fe[0] = dvb_attach(cxd2820r_attach,
&em28xx_cxd2820r_config,
- &dev->i2c_adap,
+ &dev->i2c_adap[dev->def_i2c_bus],
&dvb->lna_gpio);
if (dvb->fe[0]) {
/* FE 0 attach tuner */
if (!dvb_attach(tda18271_attach,
dvb->fe[0],
0x60,
- &dev->i2c_adap,
+ &dev->i2c_adap[dev->def_i2c_bus],
&em28xx_cxd2820r_tda18271_config)) {
dvb_frontend_detach(dvb->fe[0]);
@@ -1124,7 +1124,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
hauppauge_hvr930c_init(dev);
dvb->fe[0] = dvb_attach(drxk_attach,
- &hauppauge_930c_drxk, &dev->i2c_adap);
+ &hauppauge_930c_drxk, &dev->i2c_adap[dev->def_i2c_bus]);
if (!dvb->fe[0]) {
result = -EINVAL;
goto out_free;
@@ -1142,7 +1142,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
if (dvb->fe[0]->ops.i2c_gate_ctrl)
dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
- if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap,
+ if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus],
&cfg)) {
result = -EINVAL;
goto out_free;
@@ -1155,7 +1155,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM2884_BOARD_TERRATEC_H5:
terratec_h5_init(dev);
- dvb->fe[0] = dvb_attach(drxk_attach, &terratec_h5_drxk, &dev->i2c_adap);
+ dvb->fe[0] = dvb_attach(drxk_attach, &terratec_h5_drxk, &dev->i2c_adap[dev->def_i2c_bus]);
if (!dvb->fe[0]) {
result = -EINVAL;
goto out_free;
@@ -1169,7 +1169,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* Attach tda18271 to DVB-C frontend */
if (dvb->fe[0]->ops.i2c_gate_ctrl)
dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
- if (!dvb_attach(tda18271c2dd_attach, dvb->fe[0], &dev->i2c_adap, 0x60)) {
+ if (!dvb_attach(tda18271c2dd_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus], 0x60)) {
result = -EINVAL;
goto out_free;
}
@@ -1180,17 +1180,17 @@ static int em28xx_dvb_init(struct em28xx *dev)
case EM28174_BOARD_PCTV_460E:
/* attach demod */
dvb->fe[0] = dvb_attach(tda10071_attach,
- &em28xx_tda10071_config, &dev->i2c_adap);
+ &em28xx_tda10071_config, &dev->i2c_adap[dev->def_i2c_bus]);
/* attach SEC */
if (dvb->fe[0])
- dvb_attach(a8293_attach, dvb->fe[0], &dev->i2c_adap,
+ dvb_attach(a8293_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus],
&em28xx_a8293_config);
break;
case EM2874_BOARD_MAXMEDIA_UB425_TC:
/* attach demodulator */
dvb->fe[0] = dvb_attach(drxk_attach, &maxmedia_ub425_tc_drxk,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (dvb->fe[0]) {
/* disable I2C-gate */
@@ -1198,7 +1198,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach tuner */
if (!dvb_attach(tda18271c2dd_attach, dvb->fe[0],
- &dev->i2c_adap, 0x60)) {
+ &dev->i2c_adap[dev->def_i2c_bus], 0x60)) {
dvb_frontend_detach(dvb->fe[0]);
result = -EINVAL;
goto out_free;
@@ -1216,12 +1216,12 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach demodulator */
dvb->fe[0] = dvb_attach(drxk_attach, &pctv_520e_drxk,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (dvb->fe[0]) {
/* attach tuner */
if (!dvb_attach(tda18271_attach, dvb->fe[0], 0x60,
- &dev->i2c_adap,
+ &dev->i2c_adap[dev->def_i2c_bus],
&em28xx_cxd2820r_tda18271_config)) {
dvb_frontend_detach(dvb->fe[0]);
result = -EINVAL;
@@ -1234,7 +1234,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach demodulator */
dvb->fe[0] = dvb_attach(drxk_attach, &terratec_htc_stick_drxk,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (!dvb->fe[0]) {
result = -EINVAL;
goto out_free;
@@ -1242,7 +1242,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* Attach the demodulator. */
if (!dvb_attach(tda18271_attach, dvb->fe[0], 0x60,
- &dev->i2c_adap,
+ &dev->i2c_adap[dev->def_i2c_bus],
&em28xx_cxd2820r_tda18271_config)) {
result = -EINVAL;
goto out_free;
@@ -1253,7 +1253,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* attach demodulator */
dvb->fe[0] = dvb_attach(drxk_attach, &terratec_htc_stick_drxk,
- &dev->i2c_adap);
+ &dev->i2c_adap[dev->def_i2c_bus]);
if (!dvb->fe[0]) {
result = -EINVAL;
goto out_free;
@@ -1261,7 +1261,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
/* Attach the demodulator. */
if (!dvb_attach(tda18271_attach, dvb->fe[0], 0x60,
- &dev->i2c_adap,
+ &dev->i2c_adap[dev->def_i2c_bus],
&em28xx_cxd2820r_tda18271_config)) {
result = -EINVAL;
goto out_free;
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 6152423..9086e57 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -384,7 +384,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
/* Select address */
buf[0] = addr >> 8;
buf[1] = addr & 0xff;
- ret = i2c_master_send(&dev->i2c_client, buf + !addr_w16, 1 + addr_w16);
+ ret = i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], buf + !addr_w16, 1 + addr_w16);
if (ret < 0)
return ret;
/* Read data */
@@ -398,7 +398,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
else
rsize = remain;
- ret = i2c_master_recv(&dev->i2c_client, data, rsize);
+ ret = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], data, rsize);
if (ret < 0)
return ret;
@@ -422,10 +422,10 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
*eedata = NULL;
*eedata_len = 0;
- dev->i2c_client.addr = 0xa0 >> 1;
+ dev->i2c_client[dev->def_i2c_bus].addr = 0xa0 >> 1;
/* Check if board has eeprom */
- err = i2c_master_recv(&dev->i2c_client, &buf, 0);
+ err = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
if (err < 0) {
em28xx_info("board has no eeprom\n");
return -ENODEV;
@@ -652,8 +652,8 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
memset(i2c_devicelist, 0, ARRAY_SIZE(i2c_devicelist));
for (i = 0; i < ARRAY_SIZE(i2c_devs); i++) {
- dev->i2c_client.addr = i;
- rc = i2c_master_recv(&dev->i2c_client, &buf, 0);
+ dev->i2c_client[dev->def_i2c_bus].addr = i;
+ rc = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
if (rc < 0)
continue;
i2c_devicelist[i] = i;
@@ -675,21 +675,21 @@ int em28xx_i2c_register(struct em28xx *dev)
BUG_ON(!dev->em28xx_write_regs || !dev->em28xx_read_reg);
BUG_ON(!dev->em28xx_write_regs_req || !dev->em28xx_read_reg_req);
- dev->i2c_adap = em28xx_adap_template;
- dev->i2c_adap.dev.parent = &dev->udev->dev;
- strcpy(dev->i2c_adap.name, dev->name);
- dev->i2c_adap.algo_data = dev;
- i2c_set_adapdata(&dev->i2c_adap, &dev->v4l2_dev);
+ dev->i2c_adap[dev->def_i2c_bus] = em28xx_adap_template;
+ dev->i2c_adap[dev->def_i2c_bus].dev.parent = &dev->udev->dev;
+ strcpy(dev->i2c_adap[dev->def_i2c_bus].name, dev->name);
+ dev->i2c_adap[dev->def_i2c_bus].algo_data = dev;
+ i2c_set_adapdata(&dev->i2c_adap[dev->def_i2c_bus], &dev->v4l2_dev);
- retval = i2c_add_adapter(&dev->i2c_adap);
+ retval = i2c_add_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
if (retval < 0) {
em28xx_errdev("%s: i2c_add_adapter failed! retval [%d]\n",
__func__, retval);
return retval;
}
- dev->i2c_client = em28xx_client_template;
- dev->i2c_client.adapter = &dev->i2c_adap;
+ dev->i2c_client[dev->def_i2c_bus] = em28xx_client_template;
+ dev->i2c_client[dev->def_i2c_bus].adapter = &dev->i2c_adap[dev->def_i2c_bus];
retval = em28xx_i2c_eeprom(dev, &dev->eedata, &dev->eedata_len);
if ((retval < 0) && (retval != -ENODEV)) {
@@ -711,6 +711,6 @@ int em28xx_i2c_register(struct em28xx *dev)
*/
int em28xx_i2c_unregister(struct em28xx *dev)
{
- i2c_del_adapter(&dev->i2c_adap);
+ i2c_del_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
return 0;
}
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 1bef990..466b19d 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -280,11 +280,12 @@ static int em2874_polling_getkey(struct em28xx_IR *ir,
static int em28xx_i2c_ir_handle_key(struct em28xx_IR *ir)
{
+ struct em28xx *dev = ir->dev;
static u32 ir_key;
int rc;
struct i2c_client client;
- client.adapter = &ir->dev->i2c_adap;
+ client.adapter = &ir->dev->i2c_adap[dev->def_i2c_bus];
client.addr = ir->i2c_dev_addr;
rc = ir->get_key_i2c(&client, &ir_key);
@@ -461,7 +462,7 @@ static int em28xx_probe_i2c_ir(struct em28xx *dev)
};
while (addr_list[i] != I2C_CLIENT_END) {
- if (i2c_probe_func_quick_read(&dev->i2c_adap, addr_list[i]) == 1)
+ if (i2c_probe_func_quick_read(&dev->i2c_adap[dev->def_i2c_bus], addr_list[i]) == 1)
return addr_list[i];
i++;
}
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 2d6d31a..5de7b6c 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -157,6 +157,9 @@
#define EM28XX_NUM_BUFS 5
#define EM28XX_DVB_NUM_BUFS 5
+/* max number of I2C buses on em28xx devices */
+#define NUM_I2C_BUSES 2
+
/* isoc transfers: number of packets for each buffer
windows requests only 64 packets .. so we better do the same
this is what I found out for all alternate numbers there!
@@ -507,10 +510,13 @@ struct em28xx {
int tuner_type; /* type of the tuner */
int tuner_addr; /* tuner address */
int tda9887_conf;
+
/* i2c i/o */
- struct i2c_adapter i2c_adap;
- struct i2c_client i2c_client;
+ struct i2c_adapter i2c_adap[NUM_I2C_BUSES];
+ struct i2c_client i2c_client[NUM_I2C_BUSES];
unsigned char eeprom_addrwidth_16bit:1;
+ int def_i2c_bus; /* Default I2C bus */
+
/* video for linux */
int users; /* user count for exclusive use */
int streaming_users; /* Number of actively streaming users */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/3] em28xx: Add a separate config dir for secondary bus
2013-03-05 10:55 [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 1/3] em28xx: Prepare to support 2 different I2C buses Mauro Carvalho Chehab
@ 2013-03-05 10:55 ` Mauro Carvalho Chehab
2013-03-06 16:40 ` Frank Schäfer
2013-03-05 10:55 ` [PATCH 3/3] em28xx: add support for registering multiple i2c buses Mauro Carvalho Chehab
2013-03-05 15:43 ` [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Devin Heitmueller
3 siblings, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-05 10:55 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Frank Schäfer
Prepare to register a separate bus for the second bus.
For now, just add a new field. A latter patch will add the
bits to make it work.
This patch was generated by this script:
perl -e 'while (<>) { if (s/EM2874_I2C_SECONDARY_BUS_SELECT.*\n//) {
printf "\t\t.def_i2c_bus = 1,\n"; $found = 1; print $_ } else { if ($found) { s/^\s+// }; $found = 0; print $_; } }' \
drivers/media/usb/em28xx/em28xx-cards.c >a && mv a drivers/media/usb/em28xx/em28xx-cards.c
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
drivers/media/usb/em28xx/em28xx-cards.c | 43 ++++++++++++++++++---------------
drivers/media/usb/em28xx/em28xx.h | 1 +
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index d81f7ee..16ab4d7 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -958,8 +958,8 @@ struct em28xx_board em28xx_boards[] = {
#else
.tuner_type = TUNER_ABSENT,
#endif
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
[EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C] = {
@@ -974,8 +974,8 @@ struct em28xx_board em28xx_boards[] = {
.tuner_type = TUNER_ABSENT,
#endif
.ir_codes = RC_MAP_HAUPPAUGE,
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
[EM2884_BOARD_CINERGY_HTC_STICK] = {
@@ -983,8 +983,8 @@ struct em28xx_board em28xx_boards[] = {
.has_dvb = 1,
.ir_codes = RC_MAP_NEC_TERRATEC_CINERGY_XS,
.tuner_type = TUNER_ABSENT,
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
[EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900] = {
@@ -1404,8 +1404,8 @@ struct em28xx_board em28xx_boards[] = {
},
[EM2874_BOARD_LEADERSHIP_ISDBT] = {
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_100_KHZ,
.xclk = EM28XX_XCLK_FREQUENCY_10MHZ,
.name = "EM2874 Leadership ISDBT",
@@ -1917,8 +1917,8 @@ struct em28xx_board em28xx_boards[] = {
* Empia EM28174, Sony CXD2820R and NXP TDA18271HD/C2 */
[EM28174_BOARD_PCTV_290E] = {
.name = "PCTV nanoStick T2 290e",
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_100_KHZ,
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_100_KHZ,
.tuner_type = TUNER_ABSENT,
.tuner_gpio = pctv_290e,
.has_dvb = 1,
@@ -1927,8 +1927,8 @@ struct em28xx_board em28xx_boards[] = {
/* 2013:024f PCTV DVB-S2 Stick 460e
* Empia EM28174, NXP TDA10071, Conexant CX24118A and Allegro A8293 */
[EM28174_BOARD_PCTV_460E] = {
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ,
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ,
.name = "PCTV DVB-S2 Stick (460e)",
.tuner_type = TUNER_ABSENT,
.tuner_gpio = pctv_460e,
@@ -1958,8 +1958,8 @@ struct em28xx_board em28xx_boards[] = {
.tuner_type = TUNER_ABSENT,
.tuner_gpio = maxmedia_ub425_tc,
.has_dvb = 1,
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
/* 2304:0242 PCTV QuatroStick (510e)
@@ -1970,8 +1970,8 @@ struct em28xx_board em28xx_boards[] = {
.tuner_gpio = pctv_510e,
.has_dvb = 1,
.ir_codes = RC_MAP_PINNACLE_PCTV_HD,
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
/* 2013:0251 PCTV QuatroStick nano (520e)
@@ -1982,8 +1982,8 @@ struct em28xx_board em28xx_boards[] = {
.tuner_gpio = pctv_520e,
.has_dvb = 1,
.ir_codes = RC_MAP_PINNACLE_PCTV_HD,
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
[EM2884_BOARD_TERRATEC_HTC_USB_XS] = {
@@ -1991,8 +1991,8 @@ struct em28xx_board em28xx_boards[] = {
.has_dvb = 1,
.ir_codes = RC_MAP_NEC_TERRATEC_CINERGY_XS,
.tuner_type = TUNER_ABSENT,
- .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
- EM28XX_I2C_CLK_WAIT_ENABLE |
+ .def_i2c_bus = 1,
+ .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_400_KHZ,
},
};
@@ -2234,6 +2234,9 @@ static inline void em28xx_set_model(struct em28xx *dev)
if (!dev->board.i2c_speed)
dev->board.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_100_KHZ;
+
+ if (dev->board.def_i2c_bus == 1)
+ dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
}
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 5de7b6c..43eb1c6 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -375,6 +375,7 @@ struct em28xx_board {
int vchannels;
int tuner_type;
int tuner_addr;
+ int def_i2c_bus; /* Default I2C bus */
/* i2c flags */
unsigned int tda9887_conf;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/3] em28xx: add support for registering multiple i2c buses
2013-03-05 10:55 [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 1/3] em28xx: Prepare to support 2 different I2C buses Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 2/3] em28xx: Add a separate config dir for secondary bus Mauro Carvalho Chehab
@ 2013-03-05 10:55 ` Mauro Carvalho Chehab
2013-03-06 16:53 ` Frank Schäfer
2013-03-05 15:43 ` [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Devin Heitmueller
3 siblings, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-05 10:55 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Linux Media Mailing List, Frank Schäfer
Register both buses 0 and 1 via I2C API. For now, bus 0 is used
only by eeprom on all known devices. Later patches will be needed
if this changes in the future.
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
drivers/media/usb/em28xx/em28xx-cards.c | 29 +++++++---
drivers/media/usb/em28xx/em28xx-i2c.c | 93 ++++++++++++++++++++++-----------
drivers/media/usb/em28xx/em28xx.h | 19 +++++--
3 files changed, 97 insertions(+), 44 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 16ab4d7..496b938 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2235,8 +2235,8 @@ static inline void em28xx_set_model(struct em28xx *dev)
dev->board.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_100_KHZ;
- if (dev->board.def_i2c_bus == 1)
- dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
+ /* Should be initialized early, for I2C to work */
+ dev->def_i2c_bus = dev->board.def_i2c_bus;
}
@@ -2642,7 +2642,7 @@ static int em28xx_hint_board(struct em28xx *dev)
/* user did not request i2c scanning => do it now */
if (!dev->i2c_hash)
- em28xx_do_i2c_scan(dev);
+ em28xx_do_i2c_scan(dev, dev->def_i2c_bus);
for (i = 0; i < ARRAY_SIZE(em28xx_i2c_hash); i++) {
if (dev->i2c_hash == em28xx_i2c_hash[i].hash) {
@@ -2953,7 +2953,9 @@ void em28xx_release_resources(struct em28xx *dev)
em28xx_release_analog_resources(dev);
- em28xx_i2c_unregister(dev);
+ if (dev->def_i2c_bus)
+ em28xx_i2c_unregister(dev, 1);
+ em28xx_i2c_unregister(dev, 0);
v4l2_ctrl_handler_free(&dev->ctrl_handler);
@@ -3109,14 +3111,23 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
v4l2_ctrl_handler_init(hdl, 8);
dev->v4l2_dev.ctrl_handler = hdl;
- /* register i2c bus */
- retval = em28xx_i2c_register(dev);
+ /* register i2c bus 0 */
+ retval = em28xx_i2c_register(dev, 0);
if (retval < 0) {
- em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
+ em28xx_errdev("%s: em28xx_i2c_register bus 0 - error [%d]!\n",
__func__, retval);
goto unregister_dev;
}
+ if (dev->def_i2c_bus) {
+ retval = em28xx_i2c_register(dev, 1);
+ if (retval < 0) {
+ em28xx_errdev("%s: em28xx_i2c_register bus 1 - error [%d]!\n",
+ __func__, retval);
+ goto unregister_dev;
+ }
+ }
+
/*
* Default format, used for tvp5150 or saa711x output formats
*/
@@ -3186,7 +3197,9 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
return 0;
fail:
- em28xx_i2c_unregister(dev);
+ if (dev->def_i2c_bus)
+ em28xx_i2c_unregister(dev, 1);
+ em28xx_i2c_unregister(dev, 0);
v4l2_ctrl_handler_free(&dev->ctrl_handler);
unregister_dev:
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 9086e57..ea63ac4 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -280,9 +280,22 @@ static int em28xx_i2c_check_for_device(struct em28xx *dev, u16 addr)
static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
struct i2c_msg msgs[], int num)
{
- struct em28xx *dev = i2c_adap->algo_data;
+ struct em28xx_i2c_bus *i2c_bus = i2c_adap->algo_data;
+ struct em28xx *dev = i2c_bus->dev;
+ unsigned bus = i2c_bus->bus, last_bus;
int addr, rc, i, byte;
+ /* Switch I2C bus if needed */
+ last_bus = (dev->board.i2c_speed & EM2874_I2C_SECONDARY_BUS_SELECT) ?
+ 1 : 0;
+ if (bus != last_bus) {
+ if (bus == 1)
+ dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
+ else
+ dev->board.i2c_speed &= ~EM2874_I2C_SECONDARY_BUS_SELECT;
+ em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
+ }
+
if (num <= 0)
return 0;
for (i = 0; i < num; i++) {
@@ -384,7 +397,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
/* Select address */
buf[0] = addr >> 8;
buf[1] = addr & 0xff;
- ret = i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], buf + !addr_w16, 1 + addr_w16);
+ ret = i2c_master_send(&dev->i2c_client[0], buf + !addr_w16, 1 + addr_w16);
if (ret < 0)
return ret;
/* Read data */
@@ -398,7 +411,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
else
rsize = remain;
- ret = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], data, rsize);
+ ret = i2c_master_recv(&dev->i2c_client[0], data, rsize);
if (ret < 0)
return ret;
@@ -422,10 +435,12 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
*eedata = NULL;
*eedata_len = 0;
- dev->i2c_client[dev->def_i2c_bus].addr = 0xa0 >> 1;
+ /* EEPROM is always on i2c bus 0 on all known devices. */
+
+ dev->i2c_client[0].addr = 0xa0 >> 1;
/* Check if board has eeprom */
- err = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
+ err = i2c_master_recv(&dev->i2c_client[0], &buf, 0);
if (err < 0) {
em28xx_info("board has no eeprom\n");
return -ENODEV;
@@ -590,9 +605,11 @@ error:
/*
* functionality()
*/
-static u32 functionality(struct i2c_adapter *adap)
+static u32 functionality(struct i2c_adapter *i2c_adap)
{
- struct em28xx *dev = adap->algo_data;
+ struct em28xx_i2c_bus *i2c_bus = i2c_adap->algo_data;
+ struct em28xx *dev = i2c_bus->dev;
+
u32 func_flags = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
if (dev->board.is_em2800)
func_flags &= ~I2C_FUNC_SMBUS_WRITE_BLOCK_DATA;
@@ -643,7 +660,7 @@ static char *i2c_devs[128] = {
* do_i2c_scan()
* check i2c address range for devices
*/
-void em28xx_do_i2c_scan(struct em28xx *dev)
+void em28xx_do_i2c_scan(struct em28xx *dev, unsigned bus)
{
u8 i2c_devicelist[128];
unsigned char buf;
@@ -652,55 +669,66 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
memset(i2c_devicelist, 0, ARRAY_SIZE(i2c_devicelist));
for (i = 0; i < ARRAY_SIZE(i2c_devs); i++) {
- dev->i2c_client[dev->def_i2c_bus].addr = i;
- rc = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
+ dev->i2c_client[bus].addr = i;
+ rc = i2c_master_recv(&dev->i2c_client[bus], &buf, 0);
if (rc < 0)
continue;
i2c_devicelist[i] = i;
- em28xx_info("found i2c device @ 0x%x [%s]\n",
- i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
+ em28xx_info("found i2c device @ 0x%x on bus %d [%s]\n",
+ i << 1, bus, i2c_devs[i] ? i2c_devs[i] : "???");
}
- dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
- ARRAY_SIZE(i2c_devicelist), 32);
+ if (bus == dev->def_i2c_bus)
+ dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
+ ARRAY_SIZE(i2c_devicelist), 32);
}
/*
* em28xx_i2c_register()
* register i2c bus
*/
-int em28xx_i2c_register(struct em28xx *dev)
+int em28xx_i2c_register(struct em28xx *dev, unsigned bus)
{
int retval;
BUG_ON(!dev->em28xx_write_regs || !dev->em28xx_read_reg);
BUG_ON(!dev->em28xx_write_regs_req || !dev->em28xx_read_reg_req);
- dev->i2c_adap[dev->def_i2c_bus] = em28xx_adap_template;
- dev->i2c_adap[dev->def_i2c_bus].dev.parent = &dev->udev->dev;
- strcpy(dev->i2c_adap[dev->def_i2c_bus].name, dev->name);
- dev->i2c_adap[dev->def_i2c_bus].algo_data = dev;
- i2c_set_adapdata(&dev->i2c_adap[dev->def_i2c_bus], &dev->v4l2_dev);
- retval = i2c_add_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
+ if (bus >= NUM_I2C_BUSES)
+ return -ENODEV;
+
+ dev->i2c_adap[bus] = em28xx_adap_template;
+ dev->i2c_adap[bus].dev.parent = &dev->udev->dev;
+ strcpy(dev->i2c_adap[bus].name, dev->name);
+
+ dev->i2c_bus[bus].bus = bus;
+ dev->i2c_bus[bus].dev = dev;
+ dev->i2c_adap[bus].algo_data = &dev->i2c_bus[bus];
+ i2c_set_adapdata(&dev->i2c_adap[bus], &dev->v4l2_dev);
+
+ retval = i2c_add_adapter(&dev->i2c_adap[bus]);
if (retval < 0) {
em28xx_errdev("%s: i2c_add_adapter failed! retval [%d]\n",
__func__, retval);
return retval;
}
- dev->i2c_client[dev->def_i2c_bus] = em28xx_client_template;
- dev->i2c_client[dev->def_i2c_bus].adapter = &dev->i2c_adap[dev->def_i2c_bus];
+ dev->i2c_client[bus] = em28xx_client_template;
+ dev->i2c_client[bus].adapter = &dev->i2c_adap[bus];
- retval = em28xx_i2c_eeprom(dev, &dev->eedata, &dev->eedata_len);
- if ((retval < 0) && (retval != -ENODEV)) {
- em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
- __func__, retval);
+ /* Up to now, all eeproms are at bus 0 */
+ if (!bus) {
+ retval = em28xx_i2c_eeprom(dev, &dev->eedata, &dev->eedata_len);
+ if ((retval < 0) && (retval != -ENODEV)) {
+ em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
+ __func__, retval);
- return retval;
+ return retval;
+ }
}
if (i2c_scan)
- em28xx_do_i2c_scan(dev);
+ em28xx_do_i2c_scan(dev, bus);
return 0;
}
@@ -709,8 +737,11 @@ int em28xx_i2c_register(struct em28xx *dev)
* em28xx_i2c_unregister()
* unregister i2c_bus
*/
-int em28xx_i2c_unregister(struct em28xx *dev)
+int em28xx_i2c_unregister(struct em28xx *dev, unsigned bus)
{
- i2c_del_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
+ if (bus >= NUM_I2C_BUSES)
+ return -ENODEV;
+
+ i2c_del_adapter(&dev->i2c_adap[bus]);
return 0;
}
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 43eb1c6..6800992 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -375,7 +375,7 @@ struct em28xx_board {
int vchannels;
int tuner_type;
int tuner_addr;
- int def_i2c_bus; /* Default I2C bus */
+ unsigned def_i2c_bus; /* Default I2C bus */
/* i2c flags */
unsigned int tda9887_conf;
@@ -460,6 +460,13 @@ struct em28xx_fh {
enum v4l2_buf_type type;
};
+struct em28xx_i2c_bus {
+ struct em28xx *dev;
+
+ unsigned bus;
+};
+
+
/* main device struct */
struct em28xx {
/* generic device properties */
@@ -515,8 +522,10 @@ struct em28xx {
/* i2c i/o */
struct i2c_adapter i2c_adap[NUM_I2C_BUSES];
struct i2c_client i2c_client[NUM_I2C_BUSES];
+ struct em28xx_i2c_bus i2c_bus[NUM_I2C_BUSES];
+
unsigned char eeprom_addrwidth_16bit:1;
- int def_i2c_bus; /* Default I2C bus */
+ unsigned def_i2c_bus; /* Default I2C bus */
/* video for linux */
int users; /* user count for exclusive use */
@@ -638,9 +647,9 @@ struct em28xx_ops {
};
/* Provided by em28xx-i2c.c */
-void em28xx_do_i2c_scan(struct em28xx *dev);
-int em28xx_i2c_register(struct em28xx *dev);
-int em28xx_i2c_unregister(struct em28xx *dev);
+void em28xx_do_i2c_scan(struct em28xx *dev, unsigned bus);
+int em28xx_i2c_register(struct em28xx *dev, unsigned bus);
+int em28xx_i2c_unregister(struct em28xx *dev, unsigned bus);
/* Provided by em28xx-core.c */
int em28xx_read_reg_req_len(struct em28xx *dev, u8 req, u16 reg,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-03-05 10:55 [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Mauro Carvalho Chehab
` (2 preceding siblings ...)
2013-03-05 10:55 ` [PATCH 3/3] em28xx: add support for registering multiple i2c buses Mauro Carvalho Chehab
@ 2013-03-05 15:43 ` Devin Heitmueller
2013-03-06 17:44 ` Frank Schäfer
2013-03-18 21:17 ` Mauro Carvalho Chehab
3 siblings, 2 replies; 21+ messages in thread
From: Devin Heitmueller @ 2013-03-05 15:43 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Frank Schäfer
2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
> currently used only by eeprom, and bus 1 for the rest. Add support to
> register both buses.
Did you add a mutex to ensure that both buses cannot be used at the
same time? Because using the bus requires you to toggle a register
(thus you cannot be using both busses at the same time), you cannot
rely on the existing i2c adapter lock anymore.
You don't want a situation where something is actively talking on bus
0, and then something else tries to talk on bus 1, flips the register
bit and then the thread talking on bus 0 starts failing.
Devin
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] em28xx: Add a separate config dir for secondary bus
2013-03-05 10:55 ` [PATCH 2/3] em28xx: Add a separate config dir for secondary bus Mauro Carvalho Chehab
@ 2013-03-06 16:40 ` Frank Schäfer
2013-03-18 21:22 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 21+ messages in thread
From: Frank Schäfer @ 2013-03-06 16:40 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List
Am 05.03.2013 11:55, schrieb Mauro Carvalho Chehab:
> Prepare to register a separate bus for the second bus.
>
> For now, just add a new field. A latter patch will add the
> bits to make it work.
>
> This patch was generated by this script:
>
> perl -e 'while (<>) { if (s/EM2874_I2C_SECONDARY_BUS_SELECT.*\n//) {
> printf "\t\t.def_i2c_bus = 1,\n"; $found = 1; print $_ } else { if ($found) { s/^\s+// }; $found = 0; print $_; } }' \
> drivers/media/usb/em28xx/em28xx-cards.c >a && mv a drivers/media/usb/em28xx/em28xx-cards.c
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> ---
> drivers/media/usb/em28xx/em28xx-cards.c | 43 ++++++++++++++++++---------------
> drivers/media/usb/em28xx/em28xx.h | 1 +
> 2 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index d81f7ee..16ab4d7 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -958,8 +958,8 @@ struct em28xx_board em28xx_boards[] = {
> #else
> .tuner_type = TUNER_ABSENT,
> #endif
> - .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
> - EM28XX_I2C_CLK_WAIT_ENABLE |
> + .def_i2c_bus = 1,
> + .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
> EM28XX_I2C_FREQ_400_KHZ,
...
Looks good, we need a separate field like this for em2765 and Co., too
(which don't use reg 0x06 for bus switching).
Regards,
Frank
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] em28xx: add support for registering multiple i2c buses
2013-03-05 10:55 ` [PATCH 3/3] em28xx: add support for registering multiple i2c buses Mauro Carvalho Chehab
@ 2013-03-06 16:53 ` Frank Schäfer
2013-03-18 22:36 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 21+ messages in thread
From: Frank Schäfer @ 2013-03-06 16:53 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Hi Mauro,
I'm basically fine this patch, just a few comments/thoughts:
Am 05.03.2013 11:55, schrieb Mauro Carvalho Chehab:
> Register both buses 0 and 1 via I2C API. For now, bus 0 is used
> only by eeprom on all known devices. Later patches will be needed
> if this changes in the future.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> ---
> drivers/media/usb/em28xx/em28xx-cards.c | 29 +++++++---
> drivers/media/usb/em28xx/em28xx-i2c.c | 93 ++++++++++++++++++++++-----------
> drivers/media/usb/em28xx/em28xx.h | 19 +++++--
> 3 files changed, 97 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 16ab4d7..496b938 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -2235,8 +2235,8 @@ static inline void em28xx_set_model(struct em28xx *dev)
> dev->board.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
> EM28XX_I2C_FREQ_100_KHZ;
>
> - if (dev->board.def_i2c_bus == 1)
> - dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
> + /* Should be initialized early, for I2C to work */
> + dev->def_i2c_bus = dev->board.def_i2c_bus;
> }
>
>
> @@ -2642,7 +2642,7 @@ static int em28xx_hint_board(struct em28xx *dev)
>
> /* user did not request i2c scanning => do it now */
> if (!dev->i2c_hash)
> - em28xx_do_i2c_scan(dev);
> + em28xx_do_i2c_scan(dev, dev->def_i2c_bus);
>
> for (i = 0; i < ARRAY_SIZE(em28xx_i2c_hash); i++) {
> if (dev->i2c_hash == em28xx_i2c_hash[i].hash) {
> @@ -2953,7 +2953,9 @@ void em28xx_release_resources(struct em28xx *dev)
>
> em28xx_release_analog_resources(dev);
>
> - em28xx_i2c_unregister(dev);
> + if (dev->def_i2c_bus)
> + em28xx_i2c_unregister(dev, 1);
> + em28xx_i2c_unregister(dev, 0);
>
> v4l2_ctrl_handler_free(&dev->ctrl_handler);
>
> @@ -3109,14 +3111,23 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
> v4l2_ctrl_handler_init(hdl, 8);
> dev->v4l2_dev.ctrl_handler = hdl;
>
> - /* register i2c bus */
> - retval = em28xx_i2c_register(dev);
> + /* register i2c bus 0 */
> + retval = em28xx_i2c_register(dev, 0);
> if (retval < 0) {
> - em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
> + em28xx_errdev("%s: em28xx_i2c_register bus 0 - error [%d]!\n",
> __func__, retval);
> goto unregister_dev;
> }
>
> + if (dev->def_i2c_bus) {
> + retval = em28xx_i2c_register(dev, 1);
Hmm, ok, so you don't want to register unused busses ?
What about bus A when all subdevices are on bus B ?
You don't have to register the adapter for eeprom reading, the i2c
transfer functions work with unregistered i2c_adapters, too.
> + if (retval < 0) {
> + em28xx_errdev("%s: em28xx_i2c_register bus 1 - error [%d]!\n",
> + __func__, retval);
> + goto unregister_dev;
> + }
> + }
> +
> /*
> * Default format, used for tvp5150 or saa711x output formats
> */
> @@ -3186,7 +3197,9 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
> return 0;
>
> fail:
> - em28xx_i2c_unregister(dev);
> + if (dev->def_i2c_bus)
> + em28xx_i2c_unregister(dev, 1);
> + em28xx_i2c_unregister(dev, 0);
> v4l2_ctrl_handler_free(&dev->ctrl_handler);
>
> unregister_dev:
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
> index 9086e57..ea63ac4 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -280,9 +280,22 @@ static int em28xx_i2c_check_for_device(struct em28xx *dev, u16 addr)
> static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
> struct i2c_msg msgs[], int num)
> {
> - struct em28xx *dev = i2c_adap->algo_data;
> + struct em28xx_i2c_bus *i2c_bus = i2c_adap->algo_data;
> + struct em28xx *dev = i2c_bus->dev;
> + unsigned bus = i2c_bus->bus, last_bus;
> int addr, rc, i, byte;
>
> + /* Switch I2C bus if needed */
> + last_bus = (dev->board.i2c_speed & EM2874_I2C_SECONDARY_BUS_SELECT) ?
> + 1 : 0;
> + if (bus != last_bus) {
> + if (bus == 1)
> + dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
> + else
> + dev->board.i2c_speed &= ~EM2874_I2C_SECONDARY_BUS_SELECT;
> + em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
> + }
> +
FYI: although it seems to be not necessary for the devices we've seen so
far, the Windows driver clear/sets bit EM2874_I2C_SECONDARY_BUS_SELECT
at the beginning of each i2c operation.
Anyway, we shouldn't abuse the board data struct for saving the
currently selected bus.
This will also not work for briges which switch the bus by using a
different algorithm instead of modifying register EM28XX_R06_I2C_CLK
(like the em2765).
So better save the currently used bus number to a variable in the device
struct.
> if (num <= 0)
> return 0;
> for (i = 0; i < num; i++) {
> @@ -384,7 +397,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
> /* Select address */
> buf[0] = addr >> 8;
> buf[1] = addr & 0xff;
> - ret = i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], buf + !addr_w16, 1 + addr_w16);
> + ret = i2c_master_send(&dev->i2c_client[0], buf + !addr_w16, 1 + addr_w16);
There is no need to restrict this function to bus 0.
Just change dev->i2c_client[0] to dev->i2c_client[dev->current_i2c_bus]
(or however you call this variable) or (even better) add a function
parameter for bus selection (like you did it with em28xx_do_i2c_scan() ).
It's currently used for eeprom reading only, but that may change in the
future.
> if (ret < 0)
> return ret;
> /* Read data */
> @@ -398,7 +411,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
> else
> rsize = remain;
>
> - ret = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], data, rsize);
> + ret = i2c_master_recv(&dev->i2c_client[0], data, rsize);
The same here.
The rest looks good and I think it's a step in the right direction. :)
Regards,
Frank
> if (ret < 0)
> return ret;
> @@ -422,10 +435,12 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
> *eedata = NULL;
> *eedata_len = 0;
>
> - dev->i2c_client[dev->def_i2c_bus].addr = 0xa0 >> 1;
> + /* EEPROM is always on i2c bus 0 on all known devices. */
> +
> + dev->i2c_client[0].addr = 0xa0 >> 1;
>
> /* Check if board has eeprom */
> - err = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
> + err = i2c_master_recv(&dev->i2c_client[0], &buf, 0);
> if (err < 0) {
> em28xx_info("board has no eeprom\n");
> return -ENODEV;
> @@ -590,9 +605,11 @@ error:
> /*
> * functionality()
> */
> -static u32 functionality(struct i2c_adapter *adap)
> +static u32 functionality(struct i2c_adapter *i2c_adap)
> {
> - struct em28xx *dev = adap->algo_data;
> + struct em28xx_i2c_bus *i2c_bus = i2c_adap->algo_data;
> + struct em28xx *dev = i2c_bus->dev;
> +
> u32 func_flags = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> if (dev->board.is_em2800)
> func_flags &= ~I2C_FUNC_SMBUS_WRITE_BLOCK_DATA;
> @@ -643,7 +660,7 @@ static char *i2c_devs[128] = {
> * do_i2c_scan()
> * check i2c address range for devices
> */
> -void em28xx_do_i2c_scan(struct em28xx *dev)
> +void em28xx_do_i2c_scan(struct em28xx *dev, unsigned bus)
> {
> u8 i2c_devicelist[128];
> unsigned char buf;
> @@ -652,55 +669,66 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
> memset(i2c_devicelist, 0, ARRAY_SIZE(i2c_devicelist));
>
> for (i = 0; i < ARRAY_SIZE(i2c_devs); i++) {
> - dev->i2c_client[dev->def_i2c_bus].addr = i;
> - rc = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
> + dev->i2c_client[bus].addr = i;
> + rc = i2c_master_recv(&dev->i2c_client[bus], &buf, 0);
> if (rc < 0)
> continue;
> i2c_devicelist[i] = i;
> - em28xx_info("found i2c device @ 0x%x [%s]\n",
> - i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
> + em28xx_info("found i2c device @ 0x%x on bus %d [%s]\n",
> + i << 1, bus, i2c_devs[i] ? i2c_devs[i] : "???");
> }
>
> - dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
> - ARRAY_SIZE(i2c_devicelist), 32);
> + if (bus == dev->def_i2c_bus)
> + dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
> + ARRAY_SIZE(i2c_devicelist), 32);
> }
>
> /*
> * em28xx_i2c_register()
> * register i2c bus
> */
> -int em28xx_i2c_register(struct em28xx *dev)
> +int em28xx_i2c_register(struct em28xx *dev, unsigned bus)
> {
> int retval;
>
> BUG_ON(!dev->em28xx_write_regs || !dev->em28xx_read_reg);
> BUG_ON(!dev->em28xx_write_regs_req || !dev->em28xx_read_reg_req);
> - dev->i2c_adap[dev->def_i2c_bus] = em28xx_adap_template;
> - dev->i2c_adap[dev->def_i2c_bus].dev.parent = &dev->udev->dev;
> - strcpy(dev->i2c_adap[dev->def_i2c_bus].name, dev->name);
> - dev->i2c_adap[dev->def_i2c_bus].algo_data = dev;
> - i2c_set_adapdata(&dev->i2c_adap[dev->def_i2c_bus], &dev->v4l2_dev);
>
> - retval = i2c_add_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
> + if (bus >= NUM_I2C_BUSES)
> + return -ENODEV;
> +
> + dev->i2c_adap[bus] = em28xx_adap_template;
> + dev->i2c_adap[bus].dev.parent = &dev->udev->dev;
> + strcpy(dev->i2c_adap[bus].name, dev->name);
> +
> + dev->i2c_bus[bus].bus = bus;
> + dev->i2c_bus[bus].dev = dev;
> + dev->i2c_adap[bus].algo_data = &dev->i2c_bus[bus];
> + i2c_set_adapdata(&dev->i2c_adap[bus], &dev->v4l2_dev);
> +
> + retval = i2c_add_adapter(&dev->i2c_adap[bus]);
> if (retval < 0) {
> em28xx_errdev("%s: i2c_add_adapter failed! retval [%d]\n",
> __func__, retval);
> return retval;
> }
>
> - dev->i2c_client[dev->def_i2c_bus] = em28xx_client_template;
> - dev->i2c_client[dev->def_i2c_bus].adapter = &dev->i2c_adap[dev->def_i2c_bus];
> + dev->i2c_client[bus] = em28xx_client_template;
> + dev->i2c_client[bus].adapter = &dev->i2c_adap[bus];
>
> - retval = em28xx_i2c_eeprom(dev, &dev->eedata, &dev->eedata_len);
> - if ((retval < 0) && (retval != -ENODEV)) {
> - em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
> - __func__, retval);
> + /* Up to now, all eeproms are at bus 0 */
> + if (!bus) {
> + retval = em28xx_i2c_eeprom(dev, &dev->eedata, &dev->eedata_len);
> + if ((retval < 0) && (retval != -ENODEV)) {
> + em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
> + __func__, retval);
>
> - return retval;
> + return retval;
> + }
> }
>
> if (i2c_scan)
> - em28xx_do_i2c_scan(dev);
> + em28xx_do_i2c_scan(dev, bus);
>
> return 0;
> }
> @@ -709,8 +737,11 @@ int em28xx_i2c_register(struct em28xx *dev)
> * em28xx_i2c_unregister()
> * unregister i2c_bus
> */
> -int em28xx_i2c_unregister(struct em28xx *dev)
> +int em28xx_i2c_unregister(struct em28xx *dev, unsigned bus)
> {
> - i2c_del_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
> + if (bus >= NUM_I2C_BUSES)
> + return -ENODEV;
> +
> + i2c_del_adapter(&dev->i2c_adap[bus]);
> return 0;
> }
> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index 43eb1c6..6800992 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -375,7 +375,7 @@ struct em28xx_board {
> int vchannels;
> int tuner_type;
> int tuner_addr;
> - int def_i2c_bus; /* Default I2C bus */
> + unsigned def_i2c_bus; /* Default I2C bus */
>
> /* i2c flags */
> unsigned int tda9887_conf;
> @@ -460,6 +460,13 @@ struct em28xx_fh {
> enum v4l2_buf_type type;
> };
>
> +struct em28xx_i2c_bus {
> + struct em28xx *dev;
> +
> + unsigned bus;
> +};
> +
> +
> /* main device struct */
> struct em28xx {
> /* generic device properties */
> @@ -515,8 +522,10 @@ struct em28xx {
> /* i2c i/o */
> struct i2c_adapter i2c_adap[NUM_I2C_BUSES];
> struct i2c_client i2c_client[NUM_I2C_BUSES];
> + struct em28xx_i2c_bus i2c_bus[NUM_I2C_BUSES];
> +
> unsigned char eeprom_addrwidth_16bit:1;
> - int def_i2c_bus; /* Default I2C bus */
> + unsigned def_i2c_bus; /* Default I2C bus */
>
> /* video for linux */
> int users; /* user count for exclusive use */
> @@ -638,9 +647,9 @@ struct em28xx_ops {
> };
>
> /* Provided by em28xx-i2c.c */
> -void em28xx_do_i2c_scan(struct em28xx *dev);
> -int em28xx_i2c_register(struct em28xx *dev);
> -int em28xx_i2c_unregister(struct em28xx *dev);
> +void em28xx_do_i2c_scan(struct em28xx *dev, unsigned bus);
> +int em28xx_i2c_register(struct em28xx *dev, unsigned bus);
> +int em28xx_i2c_unregister(struct em28xx *dev, unsigned bus);
>
> /* Provided by em28xx-core.c */
> int em28xx_read_reg_req_len(struct em28xx *dev, u8 req, u16 reg,
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-03-05 15:43 ` [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Devin Heitmueller
@ 2013-03-06 17:44 ` Frank Schäfer
2013-03-18 21:22 ` Mauro Carvalho Chehab
2013-03-18 21:17 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 21+ messages in thread
From: Frank Schäfer @ 2013-03-06 17:44 UTC (permalink / raw)
To: Devin Heitmueller; +Cc: Mauro Carvalho Chehab, Linux Media Mailing List
Am 05.03.2013 16:43, schrieb Devin Heitmueller:
> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
>> currently used only by eeprom, and bus 1 for the rest. Add support to
>> register both buses.
> Did you add a mutex to ensure that both buses cannot be used at the
> same time? Because using the bus requires you to toggle a register
> (thus you cannot be using both busses at the same time), you cannot
> rely on the existing i2c adapter lock anymore.
>
> You don't want a situation where something is actively talking on bus
> 0, and then something else tries to talk on bus 1, flips the register
> bit and then the thread talking on bus 0 starts failing.
>
> Devin
Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
See hauppauge_hvr930c_init(), terratec_h5_init() and
terratec_htc_stick_init().
These functions are called from em28xx_dvb_init() at module init.
Module init is async, so yes, this is (or could at least become) a
problem...
I wonder if we can't simply remove all those writes to
EM28XX_R06_I2C_CLK from em28xx-dvb.
This is what the functions are doing:
hauppauge_hvr930c_init()
...
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
msleep(10);
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
msleep(10);
... [init sequence for slave at address 0x82]
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
msleep(30);
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
msleep(10);
terratec_h5_init():
...
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
msleep(10);
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
msleep(10);
...
terratec_htc_stick_init()
...
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
msleep(10);
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
msleep(10);
...
All three boards are using the following settings:
.i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
So what these functions are doing is
- switch to bus A and do nothing fo 10ms
- overwrite board settings for reg 0x06 with a local value (clears
EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
I can test the HVR-930C next week.
Regards,
Frank
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-03-05 15:43 ` [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Devin Heitmueller
2013-03-06 17:44 ` Frank Schäfer
@ 2013-03-18 21:17 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-18 21:17 UTC (permalink / raw)
To: Devin Heitmueller; +Cc: Linux Media Mailing List, Frank Schäfer
Em Tue, 5 Mar 2013 10:43:10 -0500
Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:
> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
> > The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
> > currently used only by eeprom, and bus 1 for the rest. Add support to
> > register both buses.
>
> Did you add a mutex to ensure that both buses cannot be used at the
> same time? Because using the bus requires you to toggle a register
> (thus you cannot be using both busses at the same time), you cannot
> rely on the existing i2c adapter lock anymore.
>
> You don't want a situation where something is actively talking on bus
> 0, and then something else tries to talk on bus 1, flips the register
> bit and then the thread talking on bus 0 starts failing.
Good point. The I2C mutex won't solve for this case. I'll add a mutex
there at the xfer function on a separate patch.
Cheers,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-03-06 17:44 ` Frank Schäfer
@ 2013-03-18 21:22 ` Mauro Carvalho Chehab
2013-04-01 17:14 ` Frank Schäfer
0 siblings, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-18 21:22 UTC (permalink / raw)
To: Frank Schäfer; +Cc: Devin Heitmueller, Linux Media Mailing List
Em Wed, 06 Mar 2013 18:44:07 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> Am 05.03.2013 16:43, schrieb Devin Heitmueller:
> > 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
> >> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
> >> currently used only by eeprom, and bus 1 for the rest. Add support to
> >> register both buses.
> > Did you add a mutex to ensure that both buses cannot be used at the
> > same time? Because using the bus requires you to toggle a register
> > (thus you cannot be using both busses at the same time), you cannot
> > rely on the existing i2c adapter lock anymore.
> >
> > You don't want a situation where something is actively talking on bus
> > 0, and then something else tries to talk on bus 1, flips the register
> > bit and then the thread talking on bus 0 starts failing.
> >
> > Devin
>
> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
> See hauppauge_hvr930c_init(), terratec_h5_init() and
> terratec_htc_stick_init().
> These functions are called from em28xx_dvb_init() at module init.
> Module init is async, so yes, this is (or could at least become) a
> problem...
>
> I wonder if we can't simply remove all those writes to
> EM28XX_R06_I2C_CLK from em28xx-dvb.
> This is what the functions are doing:
>
> hauppauge_hvr930c_init()
> ...
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> msleep(10);
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> msleep(10);
> ... [init sequence for slave at address 0x82]
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> msleep(30);
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> msleep(10);
>
> terratec_h5_init():
> ...
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> msleep(10);
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> msleep(10);
> ...
>
> terratec_htc_stick_init()
> ...
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> msleep(10);
> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> msleep(10);
> ...
>
> All three boards are using the following settings:
> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
>
> So what these functions are doing is
> - switch to bus A and do nothing fo 10ms
> - overwrite board settings for reg 0x06 with a local value (clears
> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
>
> I can test the HVR-930C next week.
There are some things there on the init sequence that can be
cleaned/removed. Those sequences generally comes from observing
what the original driver does. While it produces a working driver,
in general, it is not optimized and part of the init sequence can
be removed.
Regards,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] em28xx: Add a separate config dir for secondary bus
2013-03-06 16:40 ` Frank Schäfer
@ 2013-03-18 21:22 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-18 21:22 UTC (permalink / raw)
To: Frank Schäfer; +Cc: Linux Media Mailing List
Em Wed, 06 Mar 2013 17:40:44 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> Am 05.03.2013 11:55, schrieb Mauro Carvalho Chehab:
> > Prepare to register a separate bus for the second bus.
> >
> > For now, just add a new field. A latter patch will add the
> > bits to make it work.
> >
> > This patch was generated by this script:
> >
> > perl -e 'while (<>) { if (s/EM2874_I2C_SECONDARY_BUS_SELECT.*\n//) {
> > printf "\t\t.def_i2c_bus = 1,\n"; $found = 1; print $_ } else { if ($found) { s/^\s+// }; $found = 0; print $_; } }' \
> > drivers/media/usb/em28xx/em28xx-cards.c >a && mv a drivers/media/usb/em28xx/em28xx-cards.c
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> > ---
> > drivers/media/usb/em28xx/em28xx-cards.c | 43 ++++++++++++++++++---------------
> > drivers/media/usb/em28xx/em28xx.h | 1 +
> > 2 files changed, 24 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> > index d81f7ee..16ab4d7 100644
> > --- a/drivers/media/usb/em28xx/em28xx-cards.c
> > +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> > @@ -958,8 +958,8 @@ struct em28xx_board em28xx_boards[] = {
> > #else
> > .tuner_type = TUNER_ABSENT,
> > #endif
> > - .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
> > - EM28XX_I2C_CLK_WAIT_ENABLE |
> > + .def_i2c_bus = 1,
> > + .i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
> > EM28XX_I2C_FREQ_400_KHZ,
> ...
>
> Looks good, we need a separate field like this for em2765 and Co., too
> (which don't use reg 0x06 for bus switching).
Feel free to add it when sending the patches for em2765.
Regards,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] em28xx: add support for registering multiple i2c buses
2013-03-06 16:53 ` Frank Schäfer
@ 2013-03-18 22:36 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-03-18 22:36 UTC (permalink / raw)
To: Frank Schäfer; +Cc: Linux Media Mailing List, Devin Heitmueller
Em Wed, 06 Mar 2013 17:53:19 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> Hi Mauro,
>
> I'm basically fine this patch, just a few comments/thoughts:
>
> Am 05.03.2013 11:55, schrieb Mauro Carvalho Chehab:
> > Register both buses 0 and 1 via I2C API. For now, bus 0 is used
> > only by eeprom on all known devices. Later patches will be needed
> > if this changes in the future.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> > ---
> > drivers/media/usb/em28xx/em28xx-cards.c | 29 +++++++---
> > drivers/media/usb/em28xx/em28xx-i2c.c | 93 ++++++++++++++++++++++-----------
> > drivers/media/usb/em28xx/em28xx.h | 19 +++++--
> > 3 files changed, 97 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> > index 16ab4d7..496b938 100644
> > --- a/drivers/media/usb/em28xx/em28xx-cards.c
> > +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> > @@ -2235,8 +2235,8 @@ static inline void em28xx_set_model(struct em28xx *dev)
> > dev->board.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
> > EM28XX_I2C_FREQ_100_KHZ;
> >
> > - if (dev->board.def_i2c_bus == 1)
> > - dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
> > + /* Should be initialized early, for I2C to work */
> > + dev->def_i2c_bus = dev->board.def_i2c_bus;
> > }
> >
> >
> > @@ -2642,7 +2642,7 @@ static int em28xx_hint_board(struct em28xx *dev)
> >
> > /* user did not request i2c scanning => do it now */
> > if (!dev->i2c_hash)
> > - em28xx_do_i2c_scan(dev);
> > + em28xx_do_i2c_scan(dev, dev->def_i2c_bus);
> >
> > for (i = 0; i < ARRAY_SIZE(em28xx_i2c_hash); i++) {
> > if (dev->i2c_hash == em28xx_i2c_hash[i].hash) {
> > @@ -2953,7 +2953,9 @@ void em28xx_release_resources(struct em28xx *dev)
> >
> > em28xx_release_analog_resources(dev);
> >
> > - em28xx_i2c_unregister(dev);
> > + if (dev->def_i2c_bus)
> > + em28xx_i2c_unregister(dev, 1);
> > + em28xx_i2c_unregister(dev, 0);
> >
> > v4l2_ctrl_handler_free(&dev->ctrl_handler);
> >
> > @@ -3109,14 +3111,23 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
> > v4l2_ctrl_handler_init(hdl, 8);
> > dev->v4l2_dev.ctrl_handler = hdl;
> >
> > - /* register i2c bus */
> > - retval = em28xx_i2c_register(dev);
> > + /* register i2c bus 0 */
> > + retval = em28xx_i2c_register(dev, 0);
> > if (retval < 0) {
> > - em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
> > + em28xx_errdev("%s: em28xx_i2c_register bus 0 - error [%d]!\n",
> > __func__, retval);
> > goto unregister_dev;
> > }
> >
> > + if (dev->def_i2c_bus) {
> > + retval = em28xx_i2c_register(dev, 1);
>
> Hmm, ok, so you don't want to register unused busses ?
No, the reason here is to just not register it if the device has just
one bus. Ok, the driver could simply check for the em28xx chipset type,
but it sounded easier and cleaner to just check for this field.
> What about bus A when all subdevices are on bus B ?
Currently, there's no such case. If we ever find a device like that, then
we can change the logic. Doing it in advance sounds an overkill to me.
> You don't have to register the adapter for eeprom reading, the i2c
> transfer functions work with unregistered i2c_adapters, too.
Yes, I know it is possible to hack it. However, that makes the code
more complex to read and understand, as, on other drivers, each different
I2C bus is mapped as a different I2C adapter.
>
> > + if (retval < 0) {
> > + em28xx_errdev("%s: em28xx_i2c_register bus 1 - error [%d]!\n",
> > + __func__, retval);
> > + goto unregister_dev;
> > + }
> > + }
> > +
> > /*
> > * Default format, used for tvp5150 or saa711x output formats
> > */
> > @@ -3186,7 +3197,9 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
> > return 0;
> >
> > fail:
> > - em28xx_i2c_unregister(dev);
> > + if (dev->def_i2c_bus)
> > + em28xx_i2c_unregister(dev, 1);
> > + em28xx_i2c_unregister(dev, 0);
> > v4l2_ctrl_handler_free(&dev->ctrl_handler);
> >
> > unregister_dev:
> > diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
> > index 9086e57..ea63ac4 100644
> > --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> > +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> > @@ -280,9 +280,22 @@ static int em28xx_i2c_check_for_device(struct em28xx *dev, u16 addr)
> > static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
> > struct i2c_msg msgs[], int num)
> > {
> > - struct em28xx *dev = i2c_adap->algo_data;
> > + struct em28xx_i2c_bus *i2c_bus = i2c_adap->algo_data;
> > + struct em28xx *dev = i2c_bus->dev;
> > + unsigned bus = i2c_bus->bus, last_bus;
> > int addr, rc, i, byte;
> >
> > + /* Switch I2C bus if needed */
> > + last_bus = (dev->board.i2c_speed & EM2874_I2C_SECONDARY_BUS_SELECT) ?
> > + 1 : 0;
> > + if (bus != last_bus) {
> > + if (bus == 1)
> > + dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
> > + else
> > + dev->board.i2c_speed &= ~EM2874_I2C_SECONDARY_BUS_SELECT;
> > + em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
> > + }
> > +
>
> FYI: although it seems to be not necessary for the devices we've seen so
> far, the Windows driver clear/sets bit EM2874_I2C_SECONDARY_BUS_SELECT
> at the beginning of each i2c operation.
Yes, I'm aware of that, but we can do better than the alternative driver ;)
> Anyway, we shouldn't abuse the board data struct for saving the
> currently selected bus.
> This will also not work for briges which switch the bus by using a
> different algorithm instead of modifying register EM28XX_R06_I2C_CLK
> (like the em2765).
> So better save the currently used bus number to a variable in the device
> struct.
I actually started coding like that, but then I decided to just take the
simpler approach, as there's no big issue on modifying the value there.
Feel free to store it on a separate var at em28xx struct after adding support
for em2765.
>
> > if (num <= 0)
> > return 0;
> > for (i = 0; i < num; i++) {
> > @@ -384,7 +397,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
> > /* Select address */
> > buf[0] = addr >> 8;
> > buf[1] = addr & 0xff;
> > - ret = i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], buf + !addr_w16, 1 + addr_w16);
> > + ret = i2c_master_send(&dev->i2c_client[0], buf + !addr_w16, 1 + addr_w16);
>
> There is no need to restrict this function to bus 0.
> Just change dev->i2c_client[0] to dev->i2c_client[dev->current_i2c_bus]
> (or however you call this variable) or (even better) add a function
> parameter for bus selection (like you did it with em28xx_do_i2c_scan() ).
> It's currently used for eeprom reading only, but that may change in the
> future.
Again, I almost coded like that ;)
>
> > if (ret < 0)
> > return ret;
> > /* Read data */
> > @@ -398,7 +411,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
> > else
> > rsize = remain;
> >
> > - ret = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], data, rsize);
> > + ret = i2c_master_recv(&dev->i2c_client[0], data, rsize);
>
> The same here.
>
> The rest looks good and I think it's a step in the right direction. :)
>
> Regards,
> Frank
Ok, addressed the pointed issues and re-tested with HVR-930C.
[PATCH v2 3/3] em28xx: add support for registering multiple i2c buses
Register both buses 0 and 1 via I2C API. For now, bus 0 is used
only by eeprom on all known devices. Later patches will be needed
if this changes in the future.
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
-
v2:
- added a mutex to avoid using the two buses that depend on R06 at
the same time (Devin's request);
- pass bus as a parameter for eeprom routines;
- use a separate field to store the currently used bus
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 16ab4d7..6e62b72 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2235,8 +2235,8 @@ static inline void em28xx_set_model(struct em28xx *dev)
dev->board.i2c_speed = EM28XX_I2C_CLK_WAIT_ENABLE |
EM28XX_I2C_FREQ_100_KHZ;
- if (dev->board.def_i2c_bus == 1)
- dev->board.i2c_speed |= EM2874_I2C_SECONDARY_BUS_SELECT;
+ /* Should be initialized early, for I2C to work */
+ dev->def_i2c_bus = dev->board.def_i2c_bus;
}
@@ -2642,7 +2642,7 @@ static int em28xx_hint_board(struct em28xx *dev)
/* user did not request i2c scanning => do it now */
if (!dev->i2c_hash)
- em28xx_do_i2c_scan(dev);
+ em28xx_do_i2c_scan(dev, dev->def_i2c_bus);
for (i = 0; i < ARRAY_SIZE(em28xx_i2c_hash); i++) {
if (dev->i2c_hash == em28xx_i2c_hash[i].hash) {
@@ -2953,7 +2953,9 @@ void em28xx_release_resources(struct em28xx *dev)
em28xx_release_analog_resources(dev);
- em28xx_i2c_unregister(dev);
+ if (dev->def_i2c_bus)
+ em28xx_i2c_unregister(dev, 1);
+ em28xx_i2c_unregister(dev, 0);
v4l2_ctrl_handler_free(&dev->ctrl_handler);
@@ -3109,14 +3111,25 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
v4l2_ctrl_handler_init(hdl, 8);
dev->v4l2_dev.ctrl_handler = hdl;
- /* register i2c bus */
- retval = em28xx_i2c_register(dev);
+ rt_mutex_init(&dev->i2c_bus_lock);
+
+ /* register i2c bus 0 */
+ retval = em28xx_i2c_register(dev, 0);
if (retval < 0) {
- em28xx_errdev("%s: em28xx_i2c_register - error [%d]!\n",
+ em28xx_errdev("%s: em28xx_i2c_register bus 0 - error [%d]!\n",
__func__, retval);
goto unregister_dev;
}
+ if (dev->def_i2c_bus) {
+ retval = em28xx_i2c_register(dev, 1);
+ if (retval < 0) {
+ em28xx_errdev("%s: em28xx_i2c_register bus 1 - error [%d]!\n",
+ __func__, retval);
+ goto unregister_dev;
+ }
+ }
+
/*
* Default format, used for tvp5150 or saa711x output formats
*/
@@ -3186,7 +3199,9 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
return 0;
fail:
- em28xx_i2c_unregister(dev);
+ if (dev->def_i2c_bus)
+ em28xx_i2c_unregister(dev, 1);
+ em28xx_i2c_unregister(dev, 0);
v4l2_ctrl_handler_free(&dev->ctrl_handler);
unregister_dev:
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 9086e57..d4a48cb 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -280,11 +280,29 @@ static int em28xx_i2c_check_for_device(struct em28xx *dev, u16 addr)
static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
struct i2c_msg msgs[], int num)
{
- struct em28xx *dev = i2c_adap->algo_data;
+ struct em28xx_i2c_bus *i2c_bus = i2c_adap->algo_data;
+ struct em28xx *dev = i2c_bus->dev;
+ unsigned bus = i2c_bus->bus;
int addr, rc, i, byte;
- if (num <= 0)
+ rc = rt_mutex_trylock(&dev->i2c_bus_lock);
+ if (rc < 0)
+ return rc;
+
+ /* Switch I2C bus if needed */
+ if (bus != dev->cur_i2c_bus) {
+ if (bus == 1)
+ dev->cur_i2c_bus |= EM2874_I2C_SECONDARY_BUS_SELECT;
+ else
+ dev->cur_i2c_bus &= ~EM2874_I2C_SECONDARY_BUS_SELECT;
+ em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->cur_i2c_bus);
+ dev->cur_i2c_bus = bus;
+ }
+
+ if (num <= 0) {
+ rt_mutex_unlock(&dev->i2c_bus_lock);
return 0;
+ }
for (i = 0; i < num; i++) {
addr = msgs[i].addr << 1;
if (i2c_debug)
@@ -301,6 +319,7 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
if (rc == -ENODEV) {
if (i2c_debug)
printk(" no device\n");
+ rt_mutex_unlock(&dev->i2c_bus_lock);
return rc;
}
} else if (msgs[i].flags & I2C_M_RD) {
@@ -336,12 +355,14 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
if (rc < 0) {
if (i2c_debug)
printk(" ERROR: %i\n", rc);
+ rt_mutex_unlock(&dev->i2c_bus_lock);
return rc;
}
if (i2c_debug)
printk("\n");
}
+ rt_mutex_unlock(&dev->i2c_bus_lock);
return num;
}
@@ -372,8 +393,8 @@ static inline unsigned long em28xx_hash_mem(char *buf, int length, int bits)
/* Helper function to read data blocks from i2c clients with 8 or 16 bit
* address width, 8 bit register width and auto incrementation been activated */
-static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
- u16 len, u8 *data)
+static int em28xx_i2c_read_block(struct em28xx *dev, unsigned bus, u16 addr,
+ bool addr_w16, u16 len, u8 *data)
{
int remain = len, rsize, rsize_max, ret;
u8 buf[2];
@@ -384,7 +405,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
/* Select address */
buf[0] = addr >> 8;
buf[1] = addr & 0xff;
- ret = i2c_master_send(&dev->i2c_client[dev->def_i2c_bus], buf + !addr_w16, 1 + addr_w16);
+ ret = i2c_master_send(&dev->i2c_client[bus], buf + !addr_w16, 1 + addr_w16);
if (ret < 0)
return ret;
/* Read data */
@@ -398,7 +419,7 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
else
rsize = remain;
- ret = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], data, rsize);
+ ret = i2c_master_recv(&dev->i2c_client[bus], data, rsize);
if (ret < 0)
return ret;
@@ -409,7 +430,8 @@ static int em28xx_i2c_read_block(struct em28xx *dev, u16 addr, bool addr_w16,
return len;
}
-static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
+static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned bus,
+ u8 **eedata, u16 *eedata_len)
{
const u16 len = 256;
/* FIXME common length/size for bytes to read, to display, hash
@@ -422,10 +444,12 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
*eedata = NULL;
*eedata_len = 0;
- dev->i2c_client[dev->def_i2c_bus].addr = 0xa0 >> 1;
+ /* EEPROM is always on i2c bus 0 on all known devices. */
+
+ dev->i2c_client[bus].addr = 0xa0 >> 1;
/* Check if board has eeprom */
- err = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
+ err = i2c_master_recv(&dev->i2c_client[bus], &buf, 0);
if (err < 0) {
em28xx_info("board has no eeprom\n");
return -ENODEV;
@@ -436,7 +460,8 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
return -ENOMEM;
/* Read EEPROM content */
- err = em28xx_i2c_read_block(dev, 0x0000, dev->eeprom_addrwidth_16bit,
+ err = em28xx_i2c_read_block(dev, bus, 0x0000,
+ dev->eeprom_addrwidth_16bit,
len, data);
if (err != len) {
em28xx_errdev("failed to read eeprom (err=%d)\n", err);
@@ -485,7 +510,8 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
/* Read hardware config dataset offset from address
* (microcode start + 46) */
- err = em28xx_i2c_read_block(dev, mc_start + 46, 1, 2, data);
+ err = em28xx_i2c_read_block(dev, bus, mc_start + 46, 1, 2,
+ data);
if (err != 2) {
em28xx_errdev("failed to read hardware configuration data from eeprom (err=%d)\n",
err);
@@ -501,7 +527,8 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, u8 **eedata, u16 *eedata_len)
* the old eeprom and not longer than 256 bytes.
* tveeprom is currently also limited to 256 bytes.
*/
- err = em28xx_i2c_read_block(dev, hwconf_offset, 1, len, data);
+ err = em28xx_i2c_read_block(dev, bus, hwconf_offset, 1, len,
+ data);
if (err != len) {
em28xx_errdev("failed to read hardware configuration data from eeprom (err=%d)\n",
err);
@@ -590,9 +617,11 @@ error:
/*
* functionality()
*/
-static u32 functionality(struct i2c_adapter *adap)
+static u32 functionality(struct i2c_adapter *i2c_adap)
{
- struct em28xx *dev = adap->algo_data;
+ struct em28xx_i2c_bus *i2c_bus = i2c_adap->algo_data;
+ struct em28xx *dev = i2c_bus->dev;
+
u32 func_flags = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
if (dev->board.is_em2800)
func_flags &= ~I2C_FUNC_SMBUS_WRITE_BLOCK_DATA;
@@ -643,7 +672,7 @@ static char *i2c_devs[128] = {
* do_i2c_scan()
* check i2c address range for devices
*/
-void em28xx_do_i2c_scan(struct em28xx *dev)
+void em28xx_do_i2c_scan(struct em28xx *dev, unsigned bus)
{
u8 i2c_devicelist[128];
unsigned char buf;
@@ -652,55 +681,66 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
memset(i2c_devicelist, 0, ARRAY_SIZE(i2c_devicelist));
for (i = 0; i < ARRAY_SIZE(i2c_devs); i++) {
- dev->i2c_client[dev->def_i2c_bus].addr = i;
- rc = i2c_master_recv(&dev->i2c_client[dev->def_i2c_bus], &buf, 0);
+ dev->i2c_client[bus].addr = i;
+ rc = i2c_master_recv(&dev->i2c_client[bus], &buf, 0);
if (rc < 0)
continue;
i2c_devicelist[i] = i;
- em28xx_info("found i2c device @ 0x%x [%s]\n",
- i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
+ em28xx_info("found i2c device @ 0x%x on bus %d [%s]\n",
+ i << 1, bus, i2c_devs[i] ? i2c_devs[i] : "???");
}
- dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
- ARRAY_SIZE(i2c_devicelist), 32);
+ if (bus == dev->def_i2c_bus)
+ dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
+ ARRAY_SIZE(i2c_devicelist), 32);
}
/*
* em28xx_i2c_register()
* register i2c bus
*/
-int em28xx_i2c_register(struct em28xx *dev)
+int em28xx_i2c_register(struct em28xx *dev, unsigned bus)
{
int retval;
BUG_ON(!dev->em28xx_write_regs || !dev->em28xx_read_reg);
BUG_ON(!dev->em28xx_write_regs_req || !dev->em28xx_read_reg_req);
- dev->i2c_adap[dev->def_i2c_bus] = em28xx_adap_template;
- dev->i2c_adap[dev->def_i2c_bus].dev.parent = &dev->udev->dev;
- strcpy(dev->i2c_adap[dev->def_i2c_bus].name, dev->name);
- dev->i2c_adap[dev->def_i2c_bus].algo_data = dev;
- i2c_set_adapdata(&dev->i2c_adap[dev->def_i2c_bus], &dev->v4l2_dev);
- retval = i2c_add_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
+ if (bus >= NUM_I2C_BUSES)
+ return -ENODEV;
+
+ dev->i2c_adap[bus] = em28xx_adap_template;
+ dev->i2c_adap[bus].dev.parent = &dev->udev->dev;
+ strcpy(dev->i2c_adap[bus].name, dev->name);
+
+ dev->i2c_bus[bus].bus = bus;
+ dev->i2c_bus[bus].dev = dev;
+ dev->i2c_adap[bus].algo_data = &dev->i2c_bus[bus];
+ i2c_set_adapdata(&dev->i2c_adap[bus], &dev->v4l2_dev);
+
+ retval = i2c_add_adapter(&dev->i2c_adap[bus]);
if (retval < 0) {
em28xx_errdev("%s: i2c_add_adapter failed! retval [%d]\n",
__func__, retval);
return retval;
}
- dev->i2c_client[dev->def_i2c_bus] = em28xx_client_template;
- dev->i2c_client[dev->def_i2c_bus].adapter = &dev->i2c_adap[dev->def_i2c_bus];
+ dev->i2c_client[bus] = em28xx_client_template;
+ dev->i2c_client[bus].adapter = &dev->i2c_adap[bus];
- retval = em28xx_i2c_eeprom(dev, &dev->eedata, &dev->eedata_len);
- if ((retval < 0) && (retval != -ENODEV)) {
- em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
- __func__, retval);
+ /* Up to now, all eeproms are at bus 0 */
+ if (!bus) {
+ retval = em28xx_i2c_eeprom(dev, bus, &dev->eedata, &dev->eedata_len);
+ if ((retval < 0) && (retval != -ENODEV)) {
+ em28xx_errdev("%s: em28xx_i2_eeprom failed! retval [%d]\n",
+ __func__, retval);
- return retval;
+ return retval;
+ }
}
if (i2c_scan)
- em28xx_do_i2c_scan(dev);
+ em28xx_do_i2c_scan(dev, bus);
return 0;
}
@@ -709,8 +749,11 @@ int em28xx_i2c_register(struct em28xx *dev)
* em28xx_i2c_unregister()
* unregister i2c_bus
*/
-int em28xx_i2c_unregister(struct em28xx *dev)
+int em28xx_i2c_unregister(struct em28xx *dev, unsigned bus)
{
- i2c_del_adapter(&dev->i2c_adap[dev->def_i2c_bus]);
+ if (bus >= NUM_I2C_BUSES)
+ return -ENODEV;
+
+ i2c_del_adapter(&dev->i2c_adap[bus]);
return 0;
}
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 43eb1c6..f6ac1df 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -375,7 +375,7 @@ struct em28xx_board {
int vchannels;
int tuner_type;
int tuner_addr;
- int def_i2c_bus; /* Default I2C bus */
+ unsigned def_i2c_bus; /* Default I2C bus */
/* i2c flags */
unsigned int tda9887_conf;
@@ -460,6 +460,13 @@ struct em28xx_fh {
enum v4l2_buf_type type;
};
+struct em28xx_i2c_bus {
+ struct em28xx *dev;
+
+ unsigned bus;
+};
+
+
/* main device struct */
struct em28xx {
/* generic device properties */
@@ -515,8 +522,12 @@ struct em28xx {
/* i2c i/o */
struct i2c_adapter i2c_adap[NUM_I2C_BUSES];
struct i2c_client i2c_client[NUM_I2C_BUSES];
+ struct em28xx_i2c_bus i2c_bus[NUM_I2C_BUSES];
+
unsigned char eeprom_addrwidth_16bit:1;
- int def_i2c_bus; /* Default I2C bus */
+ unsigned def_i2c_bus; /* Default I2C bus */
+ unsigned cur_i2c_bus; /* Current I2C bus */
+ struct rt_mutex i2c_bus_lock;
/* video for linux */
int users; /* user count for exclusive use */
@@ -638,9 +649,9 @@ struct em28xx_ops {
};
/* Provided by em28xx-i2c.c */
-void em28xx_do_i2c_scan(struct em28xx *dev);
-int em28xx_i2c_register(struct em28xx *dev);
-int em28xx_i2c_unregister(struct em28xx *dev);
+void em28xx_do_i2c_scan(struct em28xx *dev, unsigned bus);
+int em28xx_i2c_register(struct em28xx *dev, unsigned bus);
+int em28xx_i2c_unregister(struct em28xx *dev, unsigned bus);
/* Provided by em28xx-core.c */
int em28xx_read_reg_req_len(struct em28xx *dev, u8 req, u16 reg,
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-03-18 21:22 ` Mauro Carvalho Chehab
@ 2013-04-01 17:14 ` Frank Schäfer
2013-04-01 19:22 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 21+ messages in thread
From: Frank Schäfer @ 2013-04-01 17:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Devin Heitmueller, Linux Media Mailing List
Am 18.03.2013 22:22, schrieb Mauro Carvalho Chehab:
> Em Wed, 06 Mar 2013 18:44:07 +0100
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> Am 05.03.2013 16:43, schrieb Devin Heitmueller:
>>> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
>>>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
>>>> currently used only by eeprom, and bus 1 for the rest. Add support to
>>>> register both buses.
>>> Did you add a mutex to ensure that both buses cannot be used at the
>>> same time? Because using the bus requires you to toggle a register
>>> (thus you cannot be using both busses at the same time), you cannot
>>> rely on the existing i2c adapter lock anymore.
>>>
>>> You don't want a situation where something is actively talking on bus
>>> 0, and then something else tries to talk on bus 1, flips the register
>>> bit and then the thread talking on bus 0 starts failing.
>>>
>>> Devin
>> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
>> See hauppauge_hvr930c_init(), terratec_h5_init() and
>> terratec_htc_stick_init().
>> These functions are called from em28xx_dvb_init() at module init.
>> Module init is async, so yes, this is (or could at least become) a
>> problem...
>>
>> I wonder if we can't simply remove all those writes to
>> EM28XX_R06_I2C_CLK from em28xx-dvb.
>> This is what the functions are doing:
>>
>> hauppauge_hvr930c_init()
>> ...
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>> msleep(10);
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>> msleep(10);
>> ... [init sequence for slave at address 0x82]
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>> msleep(30);
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
>> msleep(10);
>>
>> terratec_h5_init():
>> ...
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>> msleep(10);
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
>> msleep(10);
>> ...
>>
>> terratec_htc_stick_init()
>> ...
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>> msleep(10);
>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>> msleep(10);
>> ...
>>
>> All three boards are using the following settings:
>> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
>> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
>>
>> So what these functions are doing is
>> - switch to bus A and do nothing fo 10ms
>> - overwrite board settings for reg 0x06 with a local value (clears
>> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
>>
>> I can test the HVR-930C next week.
Ok, I finally had the chance to test with a HVR-930C, but it seems my
device () is different.
It has no slave device at i2c address 0x82, so I can't test this part.
Btw, which slave device is this ?
Removing the temporary switches to bus A makes no difference (as expected).
> There are some things there on the init sequence that can be
> cleaned/removed. Those sequences generally comes from observing
> what the original driver does. While it produces a working driver,
> in general, it is not optimized and part of the init sequence can
> be removed.
Do you want me to send patches to remove these writes ?
Which i2c speed settings do you suggest for the HVR-930C and the
HTC-Stick (board settings:
EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ?
Regards,
Frank
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-01 17:14 ` Frank Schäfer
@ 2013-04-01 19:22 ` Mauro Carvalho Chehab
2013-04-01 20:39 ` Frank Schäfer
0 siblings, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-01 19:22 UTC (permalink / raw)
To: Frank Schäfer; +Cc: Devin Heitmueller, Linux Media Mailing List
Em Mon, 01 Apr 2013 19:14:03 +0200
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> Am 18.03.2013 22:22, schrieb Mauro Carvalho Chehab:
> > Em Wed, 06 Mar 2013 18:44:07 +0100
> > Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> >
> >> Am 05.03.2013 16:43, schrieb Devin Heitmueller:
> >>> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
> >>>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
> >>>> currently used only by eeprom, and bus 1 for the rest. Add support to
> >>>> register both buses.
> >>> Did you add a mutex to ensure that both buses cannot be used at the
> >>> same time? Because using the bus requires you to toggle a register
> >>> (thus you cannot be using both busses at the same time), you cannot
> >>> rely on the existing i2c adapter lock anymore.
> >>>
> >>> You don't want a situation where something is actively talking on bus
> >>> 0, and then something else tries to talk on bus 1, flips the register
> >>> bit and then the thread talking on bus 0 starts failing.
> >>>
> >>> Devin
> >> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
> >> See hauppauge_hvr930c_init(), terratec_h5_init() and
> >> terratec_htc_stick_init().
> >> These functions are called from em28xx_dvb_init() at module init.
> >> Module init is async, so yes, this is (or could at least become) a
> >> problem...
> >>
> >> I wonder if we can't simply remove all those writes to
> >> EM28XX_R06_I2C_CLK from em28xx-dvb.
> >> This is what the functions are doing:
> >>
> >> hauppauge_hvr930c_init()
> >> ...
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> >> msleep(10);
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> >> msleep(10);
> >> ... [init sequence for slave at address 0x82]
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> >> msleep(30);
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> >> msleep(10);
> >>
> >> terratec_h5_init():
> >> ...
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> >> msleep(10);
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> >> msleep(10);
> >> ...
> >>
> >> terratec_htc_stick_init()
> >> ...
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> >> msleep(10);
> >> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> >> msleep(10);
> >> ...
> >>
> >> All three boards are using the following settings:
> >> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
> >> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
> >>
> >> So what these functions are doing is
> >> - switch to bus A and do nothing fo 10ms
> >> - overwrite board settings for reg 0x06 with a local value (clears
> >> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
> >>
> >> I can test the HVR-930C next week.
>
> Ok, I finally had the chance to test with a HVR-930C, but it seems my
> device () is different.
> It has no slave device at i2c address 0x82, so I can't test this part.
> Btw, which slave device is this ?
>
> Removing the temporary switches to bus A makes no difference (as expected).
>
> > There are some things there on the init sequence that can be
> > cleaned/removed. Those sequences generally comes from observing
> > what the original driver does. While it produces a working driver,
> > in general, it is not optimized and part of the init sequence can
> > be removed.
>
> Do you want me to send patches to remove these writes ?
> Which i2c speed settings do you suggest for the HVR-930C and the
> HTC-Stick (board settings:
> EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ?
Sure. The better would be to even remove the hauppauge_hvr930c_init()
function, as this is just a hack, and use the setup via the em28xx-cards
commented entries:
.tuner_type = TUNER_XC5000,
.tuner_addr = 0x41,
.dvb_gpio = hauppauge_930c_digital,
.tuner_gpio = hauppauge_930c_gpio,
Regards,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-01 19:22 ` Mauro Carvalho Chehab
@ 2013-04-01 20:39 ` Frank Schäfer
2013-04-01 22:12 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 21+ messages in thread
From: Frank Schäfer @ 2013-04-01 20:39 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List, Devin Heitmueller
Am 01.04.2013 21:22, schrieb Mauro Carvalho Chehab:
> Em Mon, 01 Apr 2013 19:14:03 +0200
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> Am 18.03.2013 22:22, schrieb Mauro Carvalho Chehab:
>>> Em Wed, 06 Mar 2013 18:44:07 +0100
>>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>>>
>>>> Am 05.03.2013 16:43, schrieb Devin Heitmueller:
>>>>> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
>>>>>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
>>>>>> currently used only by eeprom, and bus 1 for the rest. Add support to
>>>>>> register both buses.
>>>>> Did you add a mutex to ensure that both buses cannot be used at the
>>>>> same time? Because using the bus requires you to toggle a register
>>>>> (thus you cannot be using both busses at the same time), you cannot
>>>>> rely on the existing i2c adapter lock anymore.
>>>>>
>>>>> You don't want a situation where something is actively talking on bus
>>>>> 0, and then something else tries to talk on bus 1, flips the register
>>>>> bit and then the thread talking on bus 0 starts failing.
>>>>>
>>>>> Devin
>>>> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
>>>> See hauppauge_hvr930c_init(), terratec_h5_init() and
>>>> terratec_htc_stick_init().
>>>> These functions are called from em28xx_dvb_init() at module init.
>>>> Module init is async, so yes, this is (or could at least become) a
>>>> problem...
>>>>
>>>> I wonder if we can't simply remove all those writes to
>>>> EM28XX_R06_I2C_CLK from em28xx-dvb.
>>>> This is what the functions are doing:
>>>>
>>>> hauppauge_hvr930c_init()
>>>> ...
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>>>> msleep(10);
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>>>> msleep(10);
>>>> ... [init sequence for slave at address 0x82]
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>>>> msleep(30);
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
>>>> msleep(10);
>>>>
>>>> terratec_h5_init():
>>>> ...
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>>>> msleep(10);
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
>>>> msleep(10);
>>>> ...
>>>>
>>>> terratec_htc_stick_init()
>>>> ...
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>>>> msleep(10);
>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>>>> msleep(10);
>>>> ...
>>>>
>>>> All three boards are using the following settings:
>>>> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
>>>> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
>>>>
>>>> So what these functions are doing is
>>>> - switch to bus A and do nothing fo 10ms
>>>> - overwrite board settings for reg 0x06 with a local value (clears
>>>> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
>>>>
>>>> I can test the HVR-930C next week.
>> Ok, I finally had the chance to test with a HVR-930C, but it seems my
>> device () is different.
I forgot to insert the device/model of the device I tested: it's 16009, B1F0
>> It has no slave device at i2c address 0x82, so I can't test this part.
>> Btw, which slave device is this ?
>>
>> Removing the temporary switches to bus A makes no difference (as expected).
>>
>>> There are some things there on the init sequence that can be
>>> cleaned/removed. Those sequences generally comes from observing
>>> what the original driver does. While it produces a working driver,
>>> in general, it is not optimized and part of the init sequence can
>>> be removed.
>> Do you want me to send patches to remove these writes ?
>> Which i2c speed settings do you suggest for the HVR-930C and the
>> HTC-Stick (board settings:
>> EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ?
> Sure. The better would be to even remove the hauppauge_hvr930c_init()
> function, as this is just a hack, and use the setup via the em28xx-cards
> commented entries:
>
> .tuner_type = TUNER_XC5000,
> .tuner_addr = 0x41,
> .dvb_gpio = hauppauge_930c_digital,
> .tuner_gpio = hauppauge_930c_gpio,
Hmm... tuner address is 0x61 for the device I tested !
The register sequences in em28xx-cards.c also seem to be different to
the ones used in hauppauge_hvr930c_init() in em28xx-dvb.c...
Are you sure this will work for _all_ variants of the HVR-930C ?
I think it would be better if you would create those patches.
I really don't like writing patches without completely understanding the
code, not beeing able to test them and commit messages saying "Mauro
told me to do this"... ;)
You also didn't answer my question concerning the i2c speed settings. ;)
Regards,
Frank
>
> Regards,
> Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-01 20:39 ` Frank Schäfer
@ 2013-04-01 22:12 ` Mauro Carvalho Chehab
2013-04-01 22:14 ` Mauro Carvalho Chehab
2013-04-02 19:02 ` Frank Schäfer
0 siblings, 2 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-01 22:12 UTC (permalink / raw)
To: Frank Schäfer; +Cc: Linux Media Mailing List, Devin Heitmueller
Em Mon, 01 Apr 2013 22:39:28 +0200
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> Am 01.04.2013 21:22, schrieb Mauro Carvalho Chehab:
> > Em Mon, 01 Apr 2013 19:14:03 +0200
> > Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> >
> >> Am 18.03.2013 22:22, schrieb Mauro Carvalho Chehab:
> >>> Em Wed, 06 Mar 2013 18:44:07 +0100
> >>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> >>>
> >>>> Am 05.03.2013 16:43, schrieb Devin Heitmueller:
> >>>>> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
> >>>>>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
> >>>>>> currently used only by eeprom, and bus 1 for the rest. Add support to
> >>>>>> register both buses.
> >>>>> Did you add a mutex to ensure that both buses cannot be used at the
> >>>>> same time? Because using the bus requires you to toggle a register
> >>>>> (thus you cannot be using both busses at the same time), you cannot
> >>>>> rely on the existing i2c adapter lock anymore.
> >>>>>
> >>>>> You don't want a situation where something is actively talking on bus
> >>>>> 0, and then something else tries to talk on bus 1, flips the register
> >>>>> bit and then the thread talking on bus 0 starts failing.
> >>>>>
> >>>>> Devin
> >>>> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
> >>>> See hauppauge_hvr930c_init(), terratec_h5_init() and
> >>>> terratec_htc_stick_init().
> >>>> These functions are called from em28xx_dvb_init() at module init.
> >>>> Module init is async, so yes, this is (or could at least become) a
> >>>> problem...
> >>>>
> >>>> I wonder if we can't simply remove all those writes to
> >>>> EM28XX_R06_I2C_CLK from em28xx-dvb.
> >>>> This is what the functions are doing:
> >>>>
> >>>> hauppauge_hvr930c_init()
> >>>> ...
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> >>>> msleep(10);
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> >>>> msleep(10);
> >>>> ... [init sequence for slave at address 0x82]
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> >>>> msleep(30);
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> >>>> msleep(10);
> >>>>
> >>>> terratec_h5_init():
> >>>> ...
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> >>>> msleep(10);
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> >>>> msleep(10);
> >>>> ...
> >>>>
> >>>> terratec_htc_stick_init()
> >>>> ...
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> >>>> msleep(10);
> >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> >>>> msleep(10);
> >>>> ...
> >>>>
> >>>> All three boards are using the following settings:
> >>>> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
> >>>> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
> >>>>
> >>>> So what these functions are doing is
> >>>> - switch to bus A and do nothing fo 10ms
> >>>> - overwrite board settings for reg 0x06 with a local value (clears
> >>>> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
> >>>>
> >>>> I can test the HVR-930C next week.
> >> Ok, I finally had the chance to test with a HVR-930C, but it seems my
> >> device () is different.
>
> I forgot to insert the device/model of the device I tested: it's 16009, B1F0
It has the same model as the one I have here.
>
> >> It has no slave device at i2c address 0x82, so I can't test this part.
> >> Btw, which slave device is this ?
AFAIKT, at address 0x41, there is the analog demod (avf4910b). It is not
used by Digital TV (although I'm not sure if, for this device, it needs
to be initialized or not).
We don't have enough documentation to write a driver for avf4910b. Some
developers at the ML are trying to implement support for it for HVR-930C:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg59296.html
There is a code pointed there for avf4910a:
https://github.com/wurststulle/ngene_2400i/blob/2377b1fd99d91ff355a5e46881ef27ccc87cb376/avf4910a.c
Also, maybe to access the avf4910b some different GPIO setting may be needed,
as it might be powered off by the GPIO settings initialized at device init.
> >>
> >> Removing the temporary switches to bus A makes no difference (as expected).
> >>
> >>> There are some things there on the init sequence that can be
> >>> cleaned/removed. Those sequences generally comes from observing
> >>> what the original driver does. While it produces a working driver,
> >>> in general, it is not optimized and part of the init sequence can
> >>> be removed.
> >> Do you want me to send patches to remove these writes ?
> >> Which i2c speed settings do you suggest for the HVR-930C and the
> >> HTC-Stick (board settings:
> >> EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ?
> > Sure. The better would be to even remove the hauppauge_hvr930c_init()
> > function, as this is just a hack, and use the setup via the em28xx-cards
> > commented entries:
> >
> > .tuner_type = TUNER_XC5000,
> > .tuner_addr = 0x41,
> > .dvb_gpio = hauppauge_930c_digital,
> > .tuner_gpio = hauppauge_930c_gpio,
>
> Hmm... tuner address is 0x61 for the device I tested !
> The register sequences in em28xx-cards.c also seem to be different to
> the ones used in hauppauge_hvr930c_init() in em28xx-dvb.c...
I'm not telling that the entries there are right. They aren't. If they
where working, that data there weren't commented. This device entry
started with a clone from Terratec H5, which was the first em28xx
device with DRX-K.
On Terratec H5, the tuner is different (based on tda8290/tda8275).
The current device initialization started as a clone of the code
under terratec_h5_init().
As it worked like that, the patch author that added support for HVR-930
likely didn't touch on it.
The tuner for HVR930C is clearly at 0x61 address, as it can be seen at
em28xx-dvb:
/* Attach xc5000 */
memset(&cfg, 0, sizeof(cfg));
cfg.i2c_address = 0x61;
cfg.if_khz = 4000;
if (dvb->fe[0]->ops.i2c_gate_ctrl)
dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus],
&cfg)) {
result = -EINVAL;
goto out_free;
}
> Are you sure this will work for _all_ variants of the HVR-930C ?
Well, the current code will only work with a HVR-930C with a xc5000 tuner,
a drx-k demod and an em28xx (and an avf4910b analog TV demod).
Any other model with a different layout, if are there any, won't work
anyway.
While we can't discard that a different model might have a different GPIO
setting, Hauppauge tends to keep the GPIO settings equal for the same
device brand name.
So, it seems very unlikely that any change here will keep it working for
model 16009 while breaking it for other devices.
> I think it would be better if you would create those patches.
> I really don't like writing patches without completely understanding the
> code, not beeing able to test them and commit messages saying "Mauro
> told me to do this"... ;)
> You also didn't answer my question concerning the i2c speed settings. ;)
What question?
Each bus may have a different max I2C speed, but the speed should not
change on the same I2C bus over the time. If the driver is doing that,
this is a bug that needs to be fixed.
Regards,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-01 22:12 ` Mauro Carvalho Chehab
@ 2013-04-01 22:14 ` Mauro Carvalho Chehab
2013-04-02 0:48 ` Devin Heitmueller
2013-04-02 19:02 ` Frank Schäfer
1 sibling, 1 reply; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-01 22:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Frank Schäfer, Linux Media Mailing List, Devin Heitmueller
Em Mon, 1 Apr 2013 19:12:24 -0300
Mauro Carvalho Chehab <mchehab@redhat.com> escreveu:
> Em Mon, 01 Apr 2013 22:39:28 +0200
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
> > Am 01.04.2013 21:22, schrieb Mauro Carvalho Chehab:
> > > Em Mon, 01 Apr 2013 19:14:03 +0200
> > > Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> > >
> > >> Am 18.03.2013 22:22, schrieb Mauro Carvalho Chehab:
> > >>> Em Wed, 06 Mar 2013 18:44:07 +0100
> > >>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> > >>>
> > >>>> Am 05.03.2013 16:43, schrieb Devin Heitmueller:
> > >>>>> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
> > >>>>>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
> > >>>>>> currently used only by eeprom, and bus 1 for the rest. Add support to
> > >>>>>> register both buses.
> > >>>>> Did you add a mutex to ensure that both buses cannot be used at the
> > >>>>> same time? Because using the bus requires you to toggle a register
> > >>>>> (thus you cannot be using both busses at the same time), you cannot
> > >>>>> rely on the existing i2c adapter lock anymore.
> > >>>>>
> > >>>>> You don't want a situation where something is actively talking on bus
> > >>>>> 0, and then something else tries to talk on bus 1, flips the register
> > >>>>> bit and then the thread talking on bus 0 starts failing.
> > >>>>>
> > >>>>> Devin
> > >>>> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
> > >>>> See hauppauge_hvr930c_init(), terratec_h5_init() and
> > >>>> terratec_htc_stick_init().
> > >>>> These functions are called from em28xx_dvb_init() at module init.
> > >>>> Module init is async, so yes, this is (or could at least become) a
> > >>>> problem...
> > >>>>
> > >>>> I wonder if we can't simply remove all those writes to
> > >>>> EM28XX_R06_I2C_CLK from em28xx-dvb.
> > >>>> This is what the functions are doing:
> > >>>>
> > >>>> hauppauge_hvr930c_init()
> > >>>> ...
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> > >>>> msleep(10);
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> > >>>> msleep(10);
> > >>>> ... [init sequence for slave at address 0x82]
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> > >>>> msleep(30);
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> > >>>> msleep(10);
> > >>>>
> > >>>> terratec_h5_init():
> > >>>> ...
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> > >>>> msleep(10);
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
> > >>>> msleep(10);
> > >>>> ...
> > >>>>
> > >>>> terratec_htc_stick_init()
> > >>>> ...
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
> > >>>> msleep(10);
> > >>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
> > >>>> msleep(10);
> > >>>> ...
> > >>>>
> > >>>> All three boards are using the following settings:
> > >>>> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
> > >>>> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
> > >>>>
> > >>>> So what these functions are doing is
> > >>>> - switch to bus A and do nothing fo 10ms
> > >>>> - overwrite board settings for reg 0x06 with a local value (clears
> > >>>> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
> > >>>>
> > >>>> I can test the HVR-930C next week.
> > >> Ok, I finally had the chance to test with a HVR-930C, but it seems my
> > >> device () is different.
> >
> > I forgot to insert the device/model of the device I tested: it's 16009, B1F0
>
> It has the same model as the one I have here.
>
> >
> > >> It has no slave device at i2c address 0x82, so I can't test this part.
> > >> Btw, which slave device is this ?
>
> AFAIKT, at address 0x41, there is the analog demod (avf4910b). It is not
> used by Digital TV (although I'm not sure if, for this device, it needs
> to be initialized or not).
>
> We don't have enough documentation to write a driver for avf4910b. Some
> developers at the ML are trying to implement support for it for HVR-930C:
>
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg59296.html
>
> There is a code pointed there for avf4910a:
> https://github.com/wurststulle/ngene_2400i/blob/2377b1fd99d91ff355a5e46881ef27ccc87cb376/avf4910a.c
>
> Also, maybe to access the avf4910b some different GPIO setting may be needed,
> as it might be powered off by the GPIO settings initialized at device init.
>
> > >>
> > >> Removing the temporary switches to bus A makes no difference (as expected).
> > >>
> > >>> There are some things there on the init sequence that can be
> > >>> cleaned/removed. Those sequences generally comes from observing
> > >>> what the original driver does. While it produces a working driver,
> > >>> in general, it is not optimized and part of the init sequence can
> > >>> be removed.
> > >> Do you want me to send patches to remove these writes ?
> > >> Which i2c speed settings do you suggest for the HVR-930C and the
> > >> HTC-Stick (board settings:
> > >> EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ?
> > > Sure. The better would be to even remove the hauppauge_hvr930c_init()
> > > function, as this is just a hack, and use the setup via the em28xx-cards
> > > commented entries:
> > >
> > > .tuner_type = TUNER_XC5000,
> > > .tuner_addr = 0x41,
> > > .dvb_gpio = hauppauge_930c_digital,
> > > .tuner_gpio = hauppauge_930c_gpio,
> >
> > Hmm... tuner address is 0x61 for the device I tested !
> > The register sequences in em28xx-cards.c also seem to be different to
> > the ones used in hauppauge_hvr930c_init() in em28xx-dvb.c...
>
> I'm not telling that the entries there are right. They aren't. If they
> where working, that data there weren't commented. This device entry
> started with a clone from Terratec H5, which was the first em28xx
> device with DRX-K.
>
> On Terratec H5, the tuner is different (based on tda8290/tda8275).
> The current device initialization started as a clone of the code
> under terratec_h5_init().
>
> As it worked like that, the patch author that added support for HVR-930
> likely didn't touch on it.
>
> The tuner for HVR930C is clearly at 0x61 address, as it can be seen at
> em28xx-dvb:
>
> /* Attach xc5000 */
> memset(&cfg, 0, sizeof(cfg));
> cfg.i2c_address = 0x61;
> cfg.if_khz = 4000;
>
> if (dvb->fe[0]->ops.i2c_gate_ctrl)
> dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
> if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus],
> &cfg)) {
> result = -EINVAL;
> goto out_free;
> }
>
> > Are you sure this will work for _all_ variants of the HVR-930C ?
>
> Well, the current code will only work with a HVR-930C with a xc5000 tuner,
> a drx-k demod and an em28xx (and an avf4910b analog TV demod).
>
> Any other model with a different layout, if are there any, won't work
> anyway.
>
> While we can't discard that a different model might have a different GPIO
> setting, Hauppauge tends to keep the GPIO settings equal for the same
> device brand name.
>
> So, it seems very unlikely that any change here will keep it working for
> model 16009 while breaking it for other devices.
In time, I meant to say:
"So, it seems very unlikely that any change here will keep it working for
model 16009 while breaking it for other HVR-930 devices."
>
> > I think it would be better if you would create those patches.
> > I really don't like writing patches without completely understanding the
> > code, not beeing able to test them and commit messages saying "Mauro
> > told me to do this"... ;)
>
> > You also didn't answer my question concerning the i2c speed settings. ;)
>
> What question?
>
> Each bus may have a different max I2C speed, but the speed should not
> change on the same I2C bus over the time. If the driver is doing that,
> this is a bug that needs to be fixed.
>
> Regards,
> Mauro
--
Cheers,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-01 22:14 ` Mauro Carvalho Chehab
@ 2013-04-02 0:48 ` Devin Heitmueller
2013-04-02 19:06 ` Frank Schäfer
0 siblings, 1 reply; 21+ messages in thread
From: Devin Heitmueller @ 2013-04-02 0:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Frank Schäfer, Linux Media Mailing List
On Mon, Apr 1, 2013 at 6:14 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> In time, I meant to say:
> "So, it seems very unlikely that any change here will keep it working for
> model 16009 while breaking it for other HVR-930 devices."
As far as I know, there's only ever been one em28xx/drxk variant of
the 930c. And since it was obsoleted a while ago I don't see any
reason you will see any minor revisions of this board design in the
future (the current design uses the cx231xx).
Devin
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-01 22:12 ` Mauro Carvalho Chehab
2013-04-01 22:14 ` Mauro Carvalho Chehab
@ 2013-04-02 19:02 ` Frank Schäfer
2013-04-02 22:44 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 21+ messages in thread
From: Frank Schäfer @ 2013-04-02 19:02 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Devin Heitmueller, Linux Media Mailing List
Am 02.04.2013 00:12, schrieb Mauro Carvalho Chehab:
> Em Mon, 01 Apr 2013 22:39:28 +0200
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> Am 01.04.2013 21:22, schrieb Mauro Carvalho Chehab:
>>> Em Mon, 01 Apr 2013 19:14:03 +0200
>>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>>>
>>>> Am 18.03.2013 22:22, schrieb Mauro Carvalho Chehab:
>>>>> Em Wed, 06 Mar 2013 18:44:07 +0100
>>>>> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>>>>>
>>>>>> Am 05.03.2013 16:43, schrieb Devin Heitmueller:
>>>>>>> 2013/3/5 Mauro Carvalho Chehab <mchehab@redhat.com>:
>>>>>>>> The em2874 chips and upper have 2 buses. On all known devices, bus 0 is
>>>>>>>> currently used only by eeprom, and bus 1 for the rest. Add support to
>>>>>>>> register both buses.
>>>>>>> Did you add a mutex to ensure that both buses cannot be used at the
>>>>>>> same time? Because using the bus requires you to toggle a register
>>>>>>> (thus you cannot be using both busses at the same time), you cannot
>>>>>>> rely on the existing i2c adapter lock anymore.
>>>>>>>
>>>>>>> You don't want a situation where something is actively talking on bus
>>>>>>> 0, and then something else tries to talk on bus 1, flips the register
>>>>>>> bit and then the thread talking on bus 0 starts failing.
>>>>>>>
>>>>>>> Devin
>>>>>> Hmm... there are several writes to EM28XX_R06_I2C_CLK in em28xx-dvb...
>>>>>> See hauppauge_hvr930c_init(), terratec_h5_init() and
>>>>>> terratec_htc_stick_init().
>>>>>> These functions are called from em28xx_dvb_init() at module init.
>>>>>> Module init is async, so yes, this is (or could at least become) a
>>>>>> problem...
>>>>>>
>>>>>> I wonder if we can't simply remove all those writes to
>>>>>> EM28XX_R06_I2C_CLK from em28xx-dvb.
>>>>>> This is what the functions are doing:
>>>>>>
>>>>>> hauppauge_hvr930c_init()
>>>>>> ...
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>>>>>> msleep(10);
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>>>>>> msleep(10);
>>>>>> ... [init sequence for slave at address 0x82]
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>>>>>> msleep(30);
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
>>>>>> msleep(10);
>>>>>>
>>>>>> terratec_h5_init():
>>>>>> ...
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>>>>>> msleep(10);
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
>>>>>> msleep(10);
>>>>>> ...
>>>>>>
>>>>>> terratec_htc_stick_init()
>>>>>> ...
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
>>>>>> msleep(10);
>>>>>> em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
>>>>>> msleep(10);
>>>>>> ...
>>>>>>
>>>>>> All three boards are using the following settings:
>>>>>> .i2c_speed = EM2874_I2C_SECONDARY_BUS_SELECT |
>>>>>> EM28XX_I2C_CLK_WAIT_ENABLE | EM28XX_I2C_FREQ_400_KHZ = 0x45
>>>>>>
>>>>>> So what these functions are doing is
>>>>>> - switch to bus A and do nothing fo 10ms
>>>>>> - overwrite board settings for reg 0x06 with a local value (clears
>>>>>> EM28XX_I2C_FREQ_400_KHZ permanently for the HTC-Stick !).
>>>>>>
>>>>>> I can test the HVR-930C next week.
>>>> Ok, I finally had the chance to test with a HVR-930C, but it seems my
>>>> device () is different.
>> I forgot to insert the device/model of the device I tested: it's 16009, B1F0
> It has the same model as the one I have here.
Ok.
>
>>>> It has no slave device at i2c address 0x82, so I can't test this part.
>>>> Btw, which slave device is this ?
> AFAIKT, at address 0x41, there is the analog demod (avf4910b). It is not
> used by Digital TV (although I'm not sure if, for this device, it needs
> to be initialized or not).
>
> We don't have enough documentation to write a driver for avf4910b. Some
> developers at the ML are trying to implement support for it for HVR-930C:
>
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg59296.html
>
> There is a code pointed there for avf4910a:
> https://github.com/wurststulle/ngene_2400i/blob/2377b1fd99d91ff355a5e46881ef27ccc87cb376/avf4910a.c
>
> Also, maybe to access the avf4910b some different GPIO setting may be needed,
> as it might be powered off by the GPIO settings initialized at device i
>
Yeah, I remember that.
Anyway, I don't have time for this at the moment.
I also think this is really Hauppauges job. These devices are still
expensive (65-100€) but half working only.
That's why I'm going to do it with all my Hauppauge devices: wait 5
years and then buy them used for max. 10€.
Maybe I'll get back to this if it is still not working then and I find
some free time, but no guarantee... ;)
>
>>>> Removing the temporary switches to bus A makes no difference (as expected).
>>>>
>>>>> There are some things there on the init sequence that can be
>>>>> cleaned/removed. Those sequences generally comes from observing
>>>>> what the original driver does. While it produces a working driver,
>>>>> in general, it is not optimized and part of the init sequence can
>>>>> be removed.
>>>> Do you want me to send patches to remove these writes ?
>>>> Which i2c speed settings do you suggest for the HVR-930C and the
>>>> HTC-Stick (board settings:
>>>> EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ?
>>> Sure. The better would be to even remove the hauppauge_hvr930c_init()
>>> function, as this is just a hack, and use the setup via the em28xx-cards
>>> commented entries:
>>>
>>> .tuner_type = TUNER_XC5000,
>>> .tuner_addr = 0x41,
>>> .dvb_gpio = hauppauge_930c_digital,
>>> .tuner_gpio = hauppauge_930c_gpio,
>> Hmm... tuner address is 0x61 for the device I tested !
>> The register sequences in em28xx-cards.c also seem to be different to
>> the ones used in hauppauge_hvr930c_init() in em28xx-dvb.c...
> I'm not telling that the entries there are right. They aren't. If they
> where working, that data there weren't commented. This device entry
> started with a clone from Terratec H5, which was the first em28xx
> device with DRX-K.
>
> On Terratec H5, the tuner is different (based on tda8290/tda8275).
> The current device initialization started as a clone of the code
> under terratec_h5_init().
>
> As it worked like that, the patch author that added support for HVR-930
> likely didn't touch on it.
:-/
So the whole code for these devices is basically a quick and dirty hack.
I also checked the init sequences in em28xx-dvb.c and the GPIO sequences
and board parameters which are currently commented out...
No, thank you, I'm not going to touch this under the current circumstances !
What I'm still going to do is to remove at least these writes to reg
0x06 in em28xx-dvb.c.
>
> The tuner for HVR930C is clearly at 0x61 address, as it can be seen at
> em28xx-dvb:
>
> /* Attach xc5000 */
> memset(&cfg, 0, sizeof(cfg));
> cfg.i2c_address = 0x61;
> cfg.if_khz = 4000;
>
> if (dvb->fe[0]->ops.i2c_gate_ctrl)
> dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
> if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus],
> &cfg)) {
> result = -EINVAL;
> goto out_free;
> }
>
>> Are you sure this will work for _all_ variants of the HVR-930C ?
> Well, the current code will only work with a HVR-930C with a xc5000 tuner,
> a drx-k demod and an em28xx (and an avf4910b analog TV demod).
>
> Any other model with a different layout, if are there any, won't work
> anyway.
>
> While we can't discard that a different model might have a different GPIO
> setting, Hauppauge tends to keep the GPIO settings equal for the same
> device brand name.
>
> So, it seems very unlikely that any change here will keep it working for
> model 16009 while breaking it for other devices.
Ok, so if the changes work with my device, I can assume it works for the
others (if existing and working with the current code), too.
>
>> I think it would be better if you would create those patches.
>> I really don't like writing patches without completely understanding the
>> code, not beeing able to test them and commit messages saying "Mauro
>> told me to do this"... ;)
>> You also didn't answer my question concerning the i2c speed settings. ;)
> What question?
>
> Each bus may have a different max I2C speed, but the speed should not
> change on the same I2C bus over the time. If the driver is doing that,
> this is a bug that needs to be fixed.
For the HVR-930C and the HTC stick the board setting is 400KHz which we
overwrite in em28xx-dvb with 100KHz.
Which means that the current code (if it is really working) uses 100KHz.
So should I change the board setting to 100KHz when removing these
writes to be sure we don't break anything ?
I checked several datasheets of different i2c client devices, but none
of them says anything concerning the supported i2c speeds...
Can we assume that all devices are working with 100KHz ? And does 400KHz
really make things faster ? ;)
Regards,
Frank
>
> Regards,
> Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-02 0:48 ` Devin Heitmueller
@ 2013-04-02 19:06 ` Frank Schäfer
0 siblings, 0 replies; 21+ messages in thread
From: Frank Schäfer @ 2013-04-02 19:06 UTC (permalink / raw)
To: Devin Heitmueller; +Cc: Mauro Carvalho Chehab, Linux Media Mailing List
Am 02.04.2013 02:48, schrieb Devin Heitmueller:
> On Mon, Apr 1, 2013 at 6:14 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> In time, I meant to say:
>> "So, it seems very unlikely that any change here will keep it working for
>> model 16009 while breaking it for other HVR-930 devices."
> As far as I know, there's only ever been one em28xx/drxk variant of
> the 930c. And since it was obsoleted a while ago I don't see any
> reason you will see any minor revisions of this board design in the
> future (the current design uses the cx231xx).
Are the em28xx devices model 16xxx and the newer cx231xx devices model
111xxx ?
Frank
>
> Devin
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] em28xx: add support for two buses on em2874 and upper
2013-04-02 19:02 ` Frank Schäfer
@ 2013-04-02 22:44 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2013-04-02 22:44 UTC (permalink / raw)
To: Frank Schäfer; +Cc: Devin Heitmueller, Linux Media Mailing List
Em Tue, 02 Apr 2013 21:02:33 +0200
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> > We don't have enough documentation to write a driver for avf4910b. Some
> > developers at the ML are trying to implement support for it for HVR-930C:
> >
> > http://www.mail-archive.com/linux-media@vger.kernel.org/msg59296.html
> >
> > There is a code pointed there for avf4910a:
> > https://github.com/wurststulle/ngene_2400i/blob/2377b1fd99d91ff355a5e46881ef27ccc87cb376/avf4910a.c
> >
> > Also, maybe to access the avf4910b some different GPIO setting may be needed,
> > as it might be powered off by the GPIO settings initialized at device i
> >
>
> Yeah, I remember that.
> Anyway, I don't have time for this at the moment.
> I also think this is really Hauppauges job. These devices are still
> expensive (65-100€) but half working only.
> That's why I'm going to do it with all my Hauppauge devices: wait 5
> years and then buy them used for max. 10€.
> Maybe I'll get back to this if it is still not working then and I find
> some free time, but no guarantee... ;)
It's up to you. Even if Hauppauge would be interested on doing it, I
suspect they won't do anything anytime soon, as it may be hard to find
someone at Trident to ack with a source code release, as Trident bankrupted.
I probably won't have any spare time anytime soon for doing that, although
it could be fun. So, perhaps one day I might try to write a driver for
avf4910b, at least to make it work with PAL/M.
> >>>> Removing the temporary switches to bus A makes no difference (as expected).
> >>>>
> >>>>> There are some things there on the init sequence that can be
> >>>>> cleaned/removed. Those sequences generally comes from observing
> >>>>> what the original driver does. While it produces a working driver,
> >>>>> in general, it is not optimized and part of the init sequence can
> >>>>> be removed.
> >>>> Do you want me to send patches to remove these writes ?
> >>>> Which i2c speed settings do you suggest for the HVR-930C and the
> >>>> HTC-Stick (board settings:
> >>>> EM28XX_I2C_FREQ_400_KHZ, code overwrites it with EM28XX_I2C_FREQ_100_KHZ) ?
> >>> Sure. The better would be to even remove the hauppauge_hvr930c_init()
> >>> function, as this is just a hack, and use the setup via the em28xx-cards
> >>> commented entries:
> >>>
> >>> .tuner_type = TUNER_XC5000,
> >>> .tuner_addr = 0x41,
> >>> .dvb_gpio = hauppauge_930c_digital,
> >>> .tuner_gpio = hauppauge_930c_gpio,
> >> Hmm... tuner address is 0x61 for the device I tested !
> >> The register sequences in em28xx-cards.c also seem to be different to
> >> the ones used in hauppauge_hvr930c_init() in em28xx-dvb.c...
> > I'm not telling that the entries there are right. They aren't. If they
> > where working, that data there weren't commented. This device entry
> > started with a clone from Terratec H5, which was the first em28xx
> > device with DRX-K.
> >
> > On Terratec H5, the tuner is different (based on tda8290/tda8275).
> > The current device initialization started as a clone of the code
> > under terratec_h5_init().
> >
> > As it worked like that, the patch author that added support for HVR-930
> > likely didn't touch on it.
>
> :-/
> So the whole code for these devices is basically a quick and dirty hack.
Yes.
> I also checked the init sequences in em28xx-dvb.c and the GPIO sequences
> and board parameters which are currently commented out...
>
> No, thank you, I'm not going to touch this under the current circumstances !
Again, it is up to you to do whatever you want with your time ;)
The hack works, so there's no real need to fix it, although doing it
might save some power if the analog demod is turned on while in
digital mode.
> What I'm still going to do is to remove at least these writes to reg
> 0x06 in em28xx-dvb.c.
Ok.
> >
> > The tuner for HVR930C is clearly at 0x61 address, as it can be seen at
> > em28xx-dvb:
> >
> > /* Attach xc5000 */
> > memset(&cfg, 0, sizeof(cfg));
> > cfg.i2c_address = 0x61;
> > cfg.if_khz = 4000;
> >
> > if (dvb->fe[0]->ops.i2c_gate_ctrl)
> > dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
> > if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap[dev->def_i2c_bus],
> > &cfg)) {
> > result = -EINVAL;
> > goto out_free;
> > }
> >
> >> Are you sure this will work for _all_ variants of the HVR-930C ?
> > Well, the current code will only work with a HVR-930C with a xc5000 tuner,
> > a drx-k demod and an em28xx (and an avf4910b analog TV demod).
> >
> > Any other model with a different layout, if are there any, won't work
> > anyway.
> >
> > While we can't discard that a different model might have a different GPIO
> > setting, Hauppauge tends to keep the GPIO settings equal for the same
> > device brand name.
> >
> > So, it seems very unlikely that any change here will keep it working for
> > model 16009 while breaking it for other devices.
>
> Ok, so if the changes work with my device, I can assume it works for the
> others (if existing and working with the current code), too.
Yes.
> >
> >> I think it would be better if you would create those patches.
> >> I really don't like writing patches without completely understanding the
> >> code, not beeing able to test them and commit messages saying "Mauro
> >> told me to do this"... ;)
> >> You also didn't answer my question concerning the i2c speed settings. ;)
> > What question?
> >
> > Each bus may have a different max I2C speed, but the speed should not
> > change on the same I2C bus over the time. If the driver is doing that,
> > this is a bug that needs to be fixed.
>
> For the HVR-930C and the HTC stick the board setting is 400KHz which we
> overwrite in em28xx-dvb with 100KHz.
> Which means that the current code (if it is really working) uses 100KHz.
> So should I change the board setting to 100KHz when removing these
> writes to be sure we don't break anything ?
>
> I checked several datasheets of different i2c client devices, but none
> of them says anything concerning the supported i2c speeds...
> Can we assume that all devices are working with 100KHz ?
You can't really assume anything other than what is done by the original
driver ;)
AFAIKT, most of I2C chips work fine at 100kHz, and even at 400kHz, but
the board's layout might affect the bus speed. Most old analog tuners
only support 10kHz, but I know only a few em28xx devices with those tuners
(I have two such models here).
> And does 400KHz really make things faster ? ;)
Good question. 400kHz is 4 times 100kHz, so, it is obviously faster ;)
A faster firmware load means that the system would boot faster, if the
device is connected (and recognized) during boot time. So, a faster
speed is better for xc5000, for example.
On the other hand, firmware load at 10kHz is very frustrating, as it may
take ~30 seconds to load a firmware, and firmware needs to be loaded
when the tuner is activated.
Regards,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2013-04-02 22:44 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-05 10:55 [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 1/3] em28xx: Prepare to support 2 different I2C buses Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 2/3] em28xx: Add a separate config dir for secondary bus Mauro Carvalho Chehab
2013-03-06 16:40 ` Frank Schäfer
2013-03-18 21:22 ` Mauro Carvalho Chehab
2013-03-05 10:55 ` [PATCH 3/3] em28xx: add support for registering multiple i2c buses Mauro Carvalho Chehab
2013-03-06 16:53 ` Frank Schäfer
2013-03-18 22:36 ` Mauro Carvalho Chehab
2013-03-05 15:43 ` [PATCH 0/3] em28xx: add support for two buses on em2874 and upper Devin Heitmueller
2013-03-06 17:44 ` Frank Schäfer
2013-03-18 21:22 ` Mauro Carvalho Chehab
2013-04-01 17:14 ` Frank Schäfer
2013-04-01 19:22 ` Mauro Carvalho Chehab
2013-04-01 20:39 ` Frank Schäfer
2013-04-01 22:12 ` Mauro Carvalho Chehab
2013-04-01 22:14 ` Mauro Carvalho Chehab
2013-04-02 0:48 ` Devin Heitmueller
2013-04-02 19:06 ` Frank Schäfer
2013-04-02 19:02 ` Frank Schäfer
2013-04-02 22:44 ` Mauro Carvalho Chehab
2013-03-18 21:17 ` Mauro Carvalho Chehab
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.