Re: [PATCH 2/3] 2/3: cx18: Add i2c initialization for Z8F0811/Hauppage IR transceivers
On Sunday 19 July 2009 09:38:54 Jean Delvare wrote: 3. When using the new i2c binding model, I opted not to use ir_video for the Z8F0811 loaded with microcode from Zilog/Hauppauge. Since I needed one name for Rx binding and one for Tx binding, I used these names: ir_tx_z8f0811_haup ir_rx_z8f0811_haup [Which is ir_(func)_(part number)_(firmware_oem)]. It made sense to me. I assume these are the names to which ir-kbd-i2c and lirc_* will have to bind. Is that correct? Yes, this is correct, and the approach is good. Ideally the ir_video type would not exist (or would go away over time) and we would have a separate type name for each IR chip, resulting in much cleaner code. The reason for the current implementation is solely historical. Cool. When fixing up lirc_i2c, I actually *did* have a question about that which I forgot about until reading this. The only name I could find in use anywhere at a glance was ir_video, so that's what lirc_i2c is set to hook up to for the moment, but yeah, device-specific names instead would be great. Hrm. Offhand, I don't have a clue what the actual IR chip is on the PVR-x50 series, let alone any of the other cards lirc_i2c claims to support... -- Jarod Wilson ja...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] 2/3: cx18: Add i2c initialization for Z8F0811/Hauppage IR transceivers
On Mon, 2009-07-20 at 14:51 -0400, Jarod Wilson wrote: On Sunday 19 July 2009 09:38:54 Jean Delvare wrote: 3. When using the new i2c binding model, I opted not to use ir_video for the Z8F0811 loaded with microcode from Zilog/Hauppauge. Since I needed one name for Rx binding and one for Tx binding, I used these names: ir_tx_z8f0811_haup ir_rx_z8f0811_haup [Which is ir_(func)_(part number)_(firmware_oem)]. It made sense to me. I assume these are the names to which ir-kbd-i2c and lirc_* will have to bind. Is that correct? Yes, this is correct, and the approach is good. Ideally the ir_video type would not exist (or would go away over time) and we would have a separate type name for each IR chip, resulting in much cleaner code. The reason for the current implementation is solely historical. Cool. When fixing up lirc_i2c, I actually *did* have a question about that which I forgot about until reading this. The only name I could find in use anywhere at a glance was ir_video, so that's what lirc_i2c is set to hook up to for the moment, but yeah, device-specific names instead would be great. Yes, I noted ir_video implied an IR receiver, but the IR blaster on the HVR-1600 and newer PVR-150's can't be called ir_video and have lirc_zilog do the right thing obviously. So in for a penny, in for a pound... Based on earlier posts from Jean, note that for microcontrollers the chip part number is not enough to uniquely identify the IR implementation since the firmware can be different. I used the name haup to distinguish a Z8F0811 loaded with firmware from Hauppauge/Zilog. Given reports from users using lirc_pvr150, the different versions of firmware loaded into th Z8F0811 chips on the Hauppaugue boards all seem to be compatable to some degree. Plus the IR program firmware can report its exact version if needed. Hrm. Offhand, I don't have a clue what the actual IR chip is on the PVR-x50 series, There are a few different IR chips on the PVR-x50 series AFAIK. I know that if you find one sitting at 0x71 on the PVR-x50's, then it's likely a Zilog Z8 Encore! family microcontroller loaded with firmware program that probably originates from Zilog. (A Zilog EULA comes with the Hauppauge Windows drivers.) Actually the Zilog Z8 chips respond to 0x70: blaster, 0x71 receiver, 0x72 ???, 0x73 ??? I was kind of hoping that addresses 0x72 and 0x73 might support some sort of raw mode or learning mode, so I could to avoid the whole lirc_zilog firmware image mess. But I haven't had time to expriment. Regards, Andy let alone any of the other cards lirc_i2c claims to support... -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] 2/3: cx18: Add i2c initialization for Z8F0811/Hauppage IR transceivers
On Sun, 2009-07-19 at 15:38 +0200, Jean Delvare wrote:
Hi Andy,
Jean,
Thanks for tking the time to answer my questions and review. My
responses are inline.
On Fri, 17 Jul 2009 16:46:55 -0400, Andy Walls wrote:
This patch add support to the cx18 driver for setting up the
Z8F0811/Hauppauge IR Tx/Rx chip with the i2c binding model in newer
kernels.
My concerns/questions:
1. When using the new i2c binding model, I'm not saving the returned
pointer from i2c_new_probed_device() and am hence taking no action on
trying to clean up IR i2c devices on module unload. Will the i2c
subsystem clean up this automatically when the adapter is unregistered
on module unload?
While this isn't currently documented, yes it will. If this ever
changes, I will first let i2c-core emit warnings and still clean up
orphan clients. But I suspect the current behavior is easier for
everyone and I couldn't see any reason to change it at this point.
OK. Sounds good. :)
2. When using the new i2c binding model, I'm calling
i2c_new_probed_device() twice: once for Rx (addr 0x71 of the Z8F0811)
and another time for Tx (addr 0x70 of the Z8F0811). Is it a problem to
have two Linux i2c devices for two distinct addresses of the same
underlying hardware device?
No, this is not a problem. The fact that this is the same device behind
both addresses is an implementation detail. As long as both functions
are independent, it should work just fine.
OK. :)
3. When using the new i2c binding model, I opted not to use ir_video for
the Z8F0811 loaded with microcode from Zilog/Hauppauge. Since I needed
one name for Rx binding and one for Tx binding, I used these names:
ir_tx_z8f0811_haup
ir_rx_z8f0811_haup
[Which is ir_(func)_(part number)_(firmware_oem)]. It made sense to me.
I assume these are the names to which ir-kbd-i2c and lirc_* will have to
bind. Is that correct?
Yes, this is correct, and the approach is good.
OK. :)
Ideally the ir_video
type would not exist (or would go away over time) and we would have a
separate type name for each IR chip, resulting in much cleaner code.
The reason for the current implementation is solely historical.
OK, I had suspected the reason was this.
Review:
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-cards.c
--- a/linux/drivers/media/video/cx18/cx18-cards.c Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-cards.c Fri Jul 17 16:05:28
2009 -0400
@@ -56,7 +56,8 @@
.hw_audio_ctrl = CX18_HW_418_AV,
.hw_muxer = CX18_HW_CS5345,
.hw_all = CX18_HW_TVEEPROM | CX18_HW_418_AV | CX18_HW_TUNER |
- CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL,
+ CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL |
+ CX18_HW_Z8F0811_IR_HAUP,
.video_inputs = {
{ CX18_CARD_INPUT_VID_TUNER, 0, CX18_AV_COMPOSITE7 },
{ CX18_CARD_INPUT_SVIDEO1,1, CX18_AV_SVIDEO1},
@@ -102,7 +103,8 @@
.hw_audio_ctrl = CX18_HW_418_AV,
.hw_muxer = CX18_HW_CS5345,
.hw_all = CX18_HW_TVEEPROM | CX18_HW_418_AV | CX18_HW_TUNER |
- CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL,
+ CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL |
+ CX18_HW_Z8F0811_IR_HAUP,
.video_inputs = {
{ CX18_CARD_INPUT_VID_TUNER, 0, CX18_AV_COMPOSITE7 },
{ CX18_CARD_INPUT_SVIDEO1,1, CX18_AV_SVIDEO1},
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-cards.h
--- a/linux/drivers/media/video/cx18/cx18-cards.h Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-cards.h Fri Jul 17 16:05:28
2009 -0400
@@ -22,13 +22,17 @@
*/
/* hardware flags */
-#define CX18_HW_TUNER (1 0)
-#define CX18_HW_TVEEPROM (1 1)
-#define CX18_HW_CS5345 (1 2)
-#define CX18_HW_DVB(1 3)
-#define CX18_HW_418_AV (1 4)
-#define CX18_HW_GPIO_MUX (1 5)
-#define CX18_HW_GPIO_RESET_CTRL(1 6)
+#define CX18_HW_TUNER (1 0)
+#define CX18_HW_TVEEPROM (1 1)
+#define CX18_HW_CS5345 (1 2)
+#define CX18_HW_DVB(1 3)
+#define CX18_HW_418_AV (1 4)
+#define CX18_HW_GPIO_MUX (1 5)
+#define CX18_HW_GPIO_RESET_CTRL(1 6)
+#define CX18_HW_Z8F0811_IR_TX_HAUP (1 7)
+#define CX18_HW_Z8F0811_IR_RX_HAUP (1 8)
+#define CX18_HW_Z8F0811_IR_HAUP(CX18_HW_Z8F0811_IR_RX_HAUP | \
+CX18_HW_Z8F0811_IR_TX_HAUP)
/* video inputs */
#defineCX18_CARD_INPUT_VID_TUNER 1
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-i2c.c
--- a/linux/drivers/media/video/cx18/cx18-i2c.c Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-i2c.c
Re: [PATCH 2/3] 2/3: cx18: Add i2c initialization for Z8F0811/Hauppage IR transceivers
Hi Andy,
On Fri, 17 Jul 2009 16:46:55 -0400, Andy Walls wrote:
This patch add support to the cx18 driver for setting up the
Z8F0811/Hauppauge IR Tx/Rx chip with the i2c binding model in newer
kernels.
My concerns/questions:
1. When using the new i2c binding model, I'm not saving the returned
pointer from i2c_new_probed_device() and am hence taking no action on
trying to clean up IR i2c devices on module unload. Will the i2c
subsystem clean up this automatically when the adapter is unregistered
on module unload?
While this isn't currently documented, yes it will. If this ever
changes, I will first let i2c-core emit warnings and still clean up
orphan clients. But I suspect the current behavior is easier for
everyone and I couldn't see any reason to change it at this point.
2. When using the new i2c binding model, I'm calling
i2c_new_probed_device() twice: once for Rx (addr 0x71 of the Z8F0811)
and another time for Tx (addr 0x70 of the Z8F0811). Is it a problem to
have two Linux i2c devices for two distinct addresses of the same
underlying hardware device?
No, this is not a problem. The fact that this is the same device behind
both addresses is an implementation detail. As long as both functions
are independent, it should work just fine.
3. When using the new i2c binding model, I opted not to use ir_video for
the Z8F0811 loaded with microcode from Zilog/Hauppauge. Since I needed
one name for Rx binding and one for Tx binding, I used these names:
ir_tx_z8f0811_haup
ir_rx_z8f0811_haup
[Which is ir_(func)_(part number)_(firmware_oem)]. It made sense to me.
I assume these are the names to which ir-kbd-i2c and lirc_* will have to
bind. Is that correct?
Yes, this is correct, and the approach is good. Ideally the ir_video
type would not exist (or would go away over time) and we would have a
separate type name for each IR chip, resulting in much cleaner code.
The reason for the current implementation is solely historical.
Review:
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-cards.c
--- a/linux/drivers/media/video/cx18/cx18-cards.c Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-cards.c Fri Jul 17 16:05:28
2009 -0400
@@ -56,7 +56,8 @@
.hw_audio_ctrl = CX18_HW_418_AV,
.hw_muxer = CX18_HW_CS5345,
.hw_all = CX18_HW_TVEEPROM | CX18_HW_418_AV | CX18_HW_TUNER |
- CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL,
+ CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL |
+ CX18_HW_Z8F0811_IR_HAUP,
.video_inputs = {
{ CX18_CARD_INPUT_VID_TUNER, 0, CX18_AV_COMPOSITE7 },
{ CX18_CARD_INPUT_SVIDEO1,1, CX18_AV_SVIDEO1},
@@ -102,7 +103,8 @@
.hw_audio_ctrl = CX18_HW_418_AV,
.hw_muxer = CX18_HW_CS5345,
.hw_all = CX18_HW_TVEEPROM | CX18_HW_418_AV | CX18_HW_TUNER |
- CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL,
+ CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL |
+ CX18_HW_Z8F0811_IR_HAUP,
.video_inputs = {
{ CX18_CARD_INPUT_VID_TUNER, 0, CX18_AV_COMPOSITE7 },
{ CX18_CARD_INPUT_SVIDEO1,1, CX18_AV_SVIDEO1},
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-cards.h
--- a/linux/drivers/media/video/cx18/cx18-cards.h Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-cards.h Fri Jul 17 16:05:28
2009 -0400
@@ -22,13 +22,17 @@
*/
/* hardware flags */
-#define CX18_HW_TUNER(1 0)
-#define CX18_HW_TVEEPROM (1 1)
-#define CX18_HW_CS5345 (1 2)
-#define CX18_HW_DVB (1 3)
-#define CX18_HW_418_AV (1 4)
-#define CX18_HW_GPIO_MUX (1 5)
-#define CX18_HW_GPIO_RESET_CTRL (1 6)
+#define CX18_HW_TUNER(1 0)
+#define CX18_HW_TVEEPROM (1 1)
+#define CX18_HW_CS5345 (1 2)
+#define CX18_HW_DVB (1 3)
+#define CX18_HW_418_AV (1 4)
+#define CX18_HW_GPIO_MUX (1 5)
+#define CX18_HW_GPIO_RESET_CTRL (1 6)
+#define CX18_HW_Z8F0811_IR_TX_HAUP (1 7)
+#define CX18_HW_Z8F0811_IR_RX_HAUP (1 8)
+#define CX18_HW_Z8F0811_IR_HAUP (CX18_HW_Z8F0811_IR_RX_HAUP | \
+ CX18_HW_Z8F0811_IR_TX_HAUP)
/* video inputs */
#define CX18_CARD_INPUT_VID_TUNER 1
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-i2c.c
--- a/linux/drivers/media/video/cx18/cx18-i2c.c Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-i2c.c Fri Jul 17 16:05:28
2009 -0400
@@ -40,16 +40,20 @@
#define GETSDL_BIT 0x0008
#define CX18_CS5345_I2C_ADDR 0x4c
+#define CX18_Z8F0811_IR_TX_I2C_ADDR 0x70
+#define CX18_Z8F0811_IR_RX_I2C_ADDR 0x71
/* This array should match the
[PATCH 2/3] 2/3: cx18: Add i2c initialization for Z8F0811/Hauppage IR transceivers
This patch add support to the cx18 driver for setting up the
Z8F0811/Hauppauge IR Tx/Rx chip with the i2c binding model in newer
kernels.
My concerns/questions:
1. When using the new i2c binding model, I'm not saving the returned
pointer from i2c_new_probed_device() and am hence taking no action on
trying to clean up IR i2c devices on module unload. Will the i2c
subsystem clean up this automatically when the adapter is unregistered
on module unload?
2. When using the new i2c binding model, I'm calling
i2c_new_probed_device() twice: once for Rx (addr 0x71 of the Z8F0811)
and another time for Tx (addr 0x70 of the Z8F0811). Is it a problem to
have two Linux i2c devices for two distinct addresses of the same
underlying hardware device?
3. When using the new i2c binding model, I opted not to use ir_video for
the Z8F0811 loaded with microcode from Zilog/Hauppauge. Since I needed
one name for Rx binding and one for Tx binding, I used these names:
ir_tx_z8f0811_haup
ir_rx_z8f0811_haup
[Which is ir_(func)_(part number)_(firmware_oem)]. It made sense to me.
I assume these are the names to which ir-kbd-i2c and lirc_* will have to
bind. Is that correct?
Regards,
Andy
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-cards.c
--- a/linux/drivers/media/video/cx18/cx18-cards.c Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-cards.c Fri Jul 17 16:05:28
2009 -0400
@@ -56,7 +56,8 @@
.hw_audio_ctrl = CX18_HW_418_AV,
.hw_muxer = CX18_HW_CS5345,
.hw_all = CX18_HW_TVEEPROM | CX18_HW_418_AV | CX18_HW_TUNER |
- CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL,
+ CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL |
+ CX18_HW_Z8F0811_IR_HAUP,
.video_inputs = {
{ CX18_CARD_INPUT_VID_TUNER, 0, CX18_AV_COMPOSITE7 },
{ CX18_CARD_INPUT_SVIDEO1,1, CX18_AV_SVIDEO1},
@@ -102,7 +103,8 @@
.hw_audio_ctrl = CX18_HW_418_AV,
.hw_muxer = CX18_HW_CS5345,
.hw_all = CX18_HW_TVEEPROM | CX18_HW_418_AV | CX18_HW_TUNER |
- CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL,
+ CX18_HW_CS5345 | CX18_HW_DVB | CX18_HW_GPIO_RESET_CTRL |
+ CX18_HW_Z8F0811_IR_HAUP,
.video_inputs = {
{ CX18_CARD_INPUT_VID_TUNER, 0, CX18_AV_COMPOSITE7 },
{ CX18_CARD_INPUT_SVIDEO1,1, CX18_AV_SVIDEO1},
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-cards.h
--- a/linux/drivers/media/video/cx18/cx18-cards.h Wed Jul 15 07:28:02
2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-cards.h Fri Jul 17 16:05:28
2009 -0400
@@ -22,13 +22,17 @@
*/
/* hardware flags */
-#define CX18_HW_TUNER (1 0)
-#define CX18_HW_TVEEPROM (1 1)
-#define CX18_HW_CS5345 (1 2)
-#define CX18_HW_DVB(1 3)
-#define CX18_HW_418_AV (1 4)
-#define CX18_HW_GPIO_MUX (1 5)
-#define CX18_HW_GPIO_RESET_CTRL(1 6)
+#define CX18_HW_TUNER (1 0)
+#define CX18_HW_TVEEPROM (1 1)
+#define CX18_HW_CS5345 (1 2)
+#define CX18_HW_DVB(1 3)
+#define CX18_HW_418_AV (1 4)
+#define CX18_HW_GPIO_MUX (1 5)
+#define CX18_HW_GPIO_RESET_CTRL(1 6)
+#define CX18_HW_Z8F0811_IR_TX_HAUP (1 7)
+#define CX18_HW_Z8F0811_IR_RX_HAUP (1 8)
+#define CX18_HW_Z8F0811_IR_HAUP(CX18_HW_Z8F0811_IR_RX_HAUP | \
+CX18_HW_Z8F0811_IR_TX_HAUP)
/* video inputs */
#defineCX18_CARD_INPUT_VID_TUNER 1
diff -r d754a2d5a376 linux/drivers/media/video/cx18/cx18-i2c.c
--- a/linux/drivers/media/video/cx18/cx18-i2c.c Wed Jul 15 07:28:02 2009 -0300
+++ b/linux/drivers/media/video/cx18/cx18-i2c.c Fri Jul 17 16:05:28 2009 -0400
@@ -40,16 +40,20 @@
#define GETSDL_BIT 0x0008
#define CX18_CS5345_I2C_ADDR 0x4c
+#define CX18_Z8F0811_IR_TX_I2C_ADDR0x70
+#define CX18_Z8F0811_IR_RX_I2C_ADDR0x71
/* This array should match the CX18_HW_ defines */
static const u8 hw_addrs[] = {
- 0, /* CX18_HW_TUNER */
- 0, /* CX18_HW_TVEEPROM */
- CX18_CS5345_I2C_ADDR, /* CX18_HW_CS5345 */
- 0, /* CX18_HW_DVB */
- 0, /* CX18_HW_418_AV */
- 0, /* CX18_HW_GPIO_MUX */
- 0, /* CX18_HW_GPIO_RESET_CTRL */
+ 0, /* CX18_HW_TUNER */
+ 0, /* CX18_HW_TVEEPROM */
+ CX18_CS5345_I2C_ADDR, /* CX18_HW_CS5345 */
+ 0, /* CX18_HW_DVB */
+ 0, /* CX18_HW_418_AV */
+ 0, /* CX18_HW_GPIO_MUX */
+ 0, /*
