Em 30-12-2010 10:36, Andy Walls escreveu: > On Thu, 2010-12-30 at 09:45 -0200, Mauro Carvalho Chehab wrote: >> lirc-i2c implements a get key logic for the Adaptec Remote >> Controller, at address 0x6b. The only driver that seems to have >> an Adaptec device is ivtv: >> >> $ git grep -i adaptec drivers/media >> drivers/media/video/cs53l32a.c: * cs53l32a (Adaptec AVC-2010 and AVC-2410) i2c ivtv driver. >> drivers/media/video/cs53l32a.c: * Audio source switching for Adaptec AVC-2410 added by Trev Jackson >> drivers/media/video/cs53l32a.c: /* Set cs53l32a internal register for Adaptec 2010/2410 setup */ >> drivers/media/video/ivtv/ivtv-cards.c:/* Adaptec VideOh! AVC-2410 card */ >> drivers/media/video/ivtv/ivtv-cards.c: { PCI_DEVICE_ID_IVTV16, IVTV_PCI_ID_ADAPTEC, 0x0093 }, >> drivers/media/video/ivtv/ivtv-cards.c: .name = "Adaptec VideOh! AVC-2410", >> drivers/media/video/ivtv/ivtv-cards.c:/* Adaptec VideOh! AVC-2010 card */ >> drivers/media/video/ivtv/ivtv-cards.c: { PCI_DEVICE_ID_IVTV16, IVTV_PCI_ID_ADAPTEC, 0x0092 }, >> drivers/media/video/ivtv/ivtv-cards.c: .name = "Adaptec VideOh! AVC-2010", >> drivers/media/video/ivtv/ivtv-cards.h:#define IVTV_CARD_AVC2410 7 /* Adaptec AVC-2410 */ >> drivers/media/video/ivtv/ivtv-cards.h:#define IVTV_CARD_AVC2010 8 /* Adaptec AVD-2010 (No Tuner) */ >> drivers/media/video/ivtv/ivtv-cards.h:#define IVTV_PCI_ID_ADAPTEC 0x9005 >> drivers/media/video/ivtv/ivtv-driver.c: "\t\t\t 8 = Adaptec AVC-2410\n" >> drivers/media/video/ivtv/ivtv-driver.c: "\t\t\t 9 = Adaptec AVC-2010\n" >> drivers/media/video/ivtv/ivtv-i2c.c: 0x6b, /* Adaptec IR */ >> >> There are two Adaptec cards defined there, but only one has tuner. >> I never found any device without tuners, but with a remote controllers, so >> the logic at lirc_i2c seems to be for Adaptec AVC-2410. >> >> As we'll remove lirc_i2c from kernel, move the getkey code to ivtv driver, and >> use it for AVC2410. >> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> > > Mauro, > > This patch is missing a few things, so it won't work. > The required - and some desired - changes are in the comments below. > > >> diff --git a/drivers/media/video/ivtv/ivtv-i2c.c b/drivers/media/video/ivtv/ivtv-i2c.c >> index 6817092..8d1b016 100644 >> --- a/drivers/media/video/ivtv/ivtv-i2c.c >> +++ b/drivers/media/video/ivtv/ivtv-i2c.c >> @@ -94,6 +94,7 @@ >> #define IVTV_HAUP_INT_IR_RX_I2C_ADDR 0x18 >> #define IVTV_Z8F0811_IR_TX_I2C_ADDR 0x70 >> #define IVTV_Z8F0811_IR_RX_I2C_ADDR 0x71 >> +#define IVTV_ADAPTEC_IR 0x6b > > For consistency with the other defines for I2C addresses, please rename > this to "IVTV_ADAPTEC_IR_I2C_ADDR" > > >> /* This array should match the IVTV_HW_ defines */ >> static const u8 hw_addrs[] = { >> @@ -118,6 +119,7 @@ static const u8 hw_addrs[] = { >> IVTV_HAUP_INT_IR_RX_I2C_ADDR, /* IVTV_HW_I2C_IR_RX_HAUP_INT */ >> IVTV_Z8F0811_IR_TX_I2C_ADDR, /* IVTV_HW_Z8F0811_IR_TX_HAUP */ >> IVTV_Z8F0811_IR_RX_I2C_ADDR, /* IVTV_HW_Z8F0811_IR_RX_HAUP */ >> + IVTV_ADAPTEC_IR, >> }; > > Please add this comment to the right of the new entry: /* IVTV_HW_I2C_IR_RX_ADAPTEC */ > > > You will need to add in ivtv-cards.h: > > #define IVTV_HW_I2C_IR_RX_ADAPTEC (1 << 21) > > and modify this define in ivtv-cards.h > > #define IVTV_HW_IR_RX_ANY (IVTV_HW_I2C_IR_RX_AVER | \ > IVTV_HW_I2C_IR_RX_HAUP_EXT | \ > IVTV_HW_I2C_IR_RX_HAUP_INT | \ > IVTV_HW_Z8F0811_IR_RX_HAUP | \ > IVTV_HW_I2C_IR_RX_ADAPTEC) > > In ivtv-cards.c, for the "ivtv_card_avc2410" entry, you must modify the > ".hw_all" to be: > > .hw_all = IVTV_HW_MSP34XX | IVTV_HW_CS53L32A | > IVTV_HW_SAA7115 | IVTV_HW_TUNER | > IVTV_HW_I2C_IR_RX_ADAPTEC, > > >> /* This array should match the IVTV_HW_ defines */ >> @@ -145,6 +147,31 @@ static const char * const hw_devicenames[] = { >> "ir_rx_z8f0811_haup", /* IVTV_HW_Z8F0811_IR_RX_HAUP */ >> }; > > To avoid walking off the end of "hw_devicenames[]", you will need to add > a new entry to the end of "hw_devicenames[]": > > "ir_video", /* IVTV_HW_I2C_IR_RX_ADAPTEC */ > >> +static int get_key_adaptec(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw) >> +{ >> + unsigned char keybuf[4]; >> + >> + keybuf[0] = 0x00; >> + i2c_master_send(ir->c, keybuf, 1); >> + /* poll IR chip */ >> + if (i2c_master_recv(ir->c, keybuf, sizeof(keybuf)) != sizeof(keybuf)) { >> + return 0; >> + } >> + >> + /* key pressed ? */ >> + if (keybuf[2] == 0xff) >> + return 0; >> + >> + /* remove repeat bit */ >> + keybuf[2] &= 0x7f; >> + keybuf[3] |= 0x80; >> + >> + *ir_key = (u32) keybuf; >> + *ir_raw = (u32) keybuf; >> + >> + return 1; >> +} >> + >> static int ivtv_i2c_new_ir(struct ivtv *itv, u32 hw, const char *type, u8 addr) >> { >> struct i2c_board_info info; >> @@ -190,6 +217,12 @@ static int ivtv_i2c_new_ir(struct ivtv *itv, u32 hw, const char *type, u8 addr) >> init_data->type = RC_TYPE_RC5; >> init_data->name = itv->card_name; >> break; >> + case IVTV_CARD_AVC2410: > > You are switching on a value from a different set. Instead use: > > case IVTV_HW_I2C_IR_RX_ADAPTEC: > > >> + init_data->ir_codes = RC_MAP_EMPTY; >> + init_data->get_key = get_key_adaptec; >> + init_data->type = RC_TYPE_UNKNOWN; >> + init_data->name = itv->card_name; >> + break; > > Please add a /*FIXME */ comment here, so that I will remember to figure > out the default keymap and RC protocol type. > > Regards, > Andy Thanks for the review. Version 3 of the patch enclosed. commit 8576bd14361ec75c91ddfb49cc2df389143cf06a Author: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> Date: Thu Dec 30 09:31:10 2010 -0200 [media] ivtv: Add Adaptec Remote Controller lirc-i2c implements a get key logic for the Adaptec Remote Controller, at address 0x6b. The only driver that seems to have an Adaptec device is ivtv: $ git grep -i adaptec drivers/media drivers/media/video/cs53l32a.c: * cs53l32a (Adaptec AVC-2010 and AVC-2410) i2c ivtv driver. drivers/media/video/cs53l32a.c: * Audio source switching for Adaptec AVC-2410 added by Trev Jackson drivers/media/video/cs53l32a.c: /* Set cs53l32a internal register for Adaptec 2010/2410 setup */ drivers/media/video/ivtv/ivtv-cards.c:/* Adaptec VideOh! AVC-2410 card */ drivers/media/video/ivtv/ivtv-cards.c: { PCI_DEVICE_ID_IVTV16, IVTV_PCI_ID_ADAPTEC, 0x0093 }, drivers/media/video/ivtv/ivtv-cards.c: .name = "Adaptec VideOh! AVC-2410", drivers/media/video/ivtv/ivtv-cards.c:/* Adaptec VideOh! AVC-2010 card */ drivers/media/video/ivtv/ivtv-cards.c: { PCI_DEVICE_ID_IVTV16, IVTV_PCI_ID_ADAPTEC, 0x0092 }, drivers/media/video/ivtv/ivtv-cards.c: .name = "Adaptec VideOh! AVC-2010", drivers/media/video/ivtv/ivtv-cards.h:#define IVTV_CARD_AVC2410 7 /* Adaptec AVC-2410 */ drivers/media/video/ivtv/ivtv-cards.h:#define IVTV_CARD_AVC2010 8 /* Adaptec AVD-2010 (No Tuner) */ drivers/media/video/ivtv/ivtv-cards.h:#define IVTV_PCI_ID_ADAPTEC 0x9005 drivers/media/video/ivtv/ivtv-driver.c: "\t\t\t 8 = Adaptec AVC-2410\n" drivers/media/video/ivtv/ivtv-driver.c: "\t\t\t 9 = Adaptec AVC-2010\n" drivers/media/video/ivtv/ivtv-i2c.c: 0x6b, /* Adaptec IR */ There are two Adaptec cards defined there, but AVC-2010 doesn't have a remote controller. So, the logic at lirc_i2c seems to be for Adaptec AVC-2410. As we'll remove lirc_i2c from kernel, move the getkey code to ivtv driver, and use it for AVC-2410. Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> diff --git a/drivers/media/video/ivtv/ivtv-cards.c b/drivers/media/video/ivtv/ivtv-cards.c index b6f2a2b..145e474 100644 --- a/drivers/media/video/ivtv/ivtv-cards.c +++ b/drivers/media/video/ivtv/ivtv-cards.c @@ -405,7 +405,8 @@ static const struct ivtv_card ivtv_card_avc2410 = { .hw_audio_ctrl = IVTV_HW_MSP34XX, .hw_muxer = IVTV_HW_CS53L32A, .hw_all = IVTV_HW_MSP34XX | IVTV_HW_CS53L32A | - IVTV_HW_SAA7115 | IVTV_HW_TUNER, + IVTV_HW_SAA7115 | IVTV_HW_TUNER | + IVTV_HW_I2C_IR_RX_ADAPTEC, .video_inputs = { { IVTV_CARD_INPUT_VID_TUNER, 0, IVTV_SAA71XX_COMPOSITE4 }, { IVTV_CARD_INPUT_SVIDEO1, 1, IVTV_SAA71XX_SVIDEO0 }, diff --git a/drivers/media/video/ivtv/ivtv-cards.h b/drivers/media/video/ivtv/ivtv-cards.h index 78eca99..e6f5c02 100644 --- a/drivers/media/video/ivtv/ivtv-cards.h +++ b/drivers/media/video/ivtv/ivtv-cards.h @@ -111,6 +111,7 @@ #define IVTV_HW_I2C_IR_RX_HAUP_INT (1 << 18) #define IVTV_HW_Z8F0811_IR_TX_HAUP (1 << 19) #define IVTV_HW_Z8F0811_IR_RX_HAUP (1 << 20) +#define IVTV_HW_I2C_IR_RX_ADAPTEC (1 << 21) #define IVTV_HW_Z8F0811_IR_HAUP (IVTV_HW_Z8F0811_IR_RX_HAUP | \ IVTV_HW_Z8F0811_IR_TX_HAUP) @@ -120,7 +121,8 @@ #define IVTV_HW_IR_RX_ANY (IVTV_HW_I2C_IR_RX_AVER | \ IVTV_HW_I2C_IR_RX_HAUP_EXT | \ IVTV_HW_I2C_IR_RX_HAUP_INT | \ - IVTV_HW_Z8F0811_IR_RX_HAUP) + IVTV_HW_Z8F0811_IR_RX_HAUP | \ + IVTV_HW_I2C_IR_RX_ADAPTEC) #define IVTV_HW_IR_TX_ANY (IVTV_HW_Z8F0811_IR_TX_HAUP) diff --git a/drivers/media/video/ivtv/ivtv-i2c.c b/drivers/media/video/ivtv/ivtv-i2c.c index 6817092..fb0ac68 100644 --- a/drivers/media/video/ivtv/ivtv-i2c.c +++ b/drivers/media/video/ivtv/ivtv-i2c.c @@ -94,6 +94,7 @@ #define IVTV_HAUP_INT_IR_RX_I2C_ADDR 0x18 #define IVTV_Z8F0811_IR_TX_I2C_ADDR 0x70 #define IVTV_Z8F0811_IR_RX_I2C_ADDR 0x71 +#define IVTV_ADAPTEC_IR_ADDR 0x6b /* This array should match the IVTV_HW_ defines */ static const u8 hw_addrs[] = { @@ -118,6 +119,7 @@ static const u8 hw_addrs[] = { IVTV_HAUP_INT_IR_RX_I2C_ADDR, /* IVTV_HW_I2C_IR_RX_HAUP_INT */ IVTV_Z8F0811_IR_TX_I2C_ADDR, /* IVTV_HW_Z8F0811_IR_TX_HAUP */ IVTV_Z8F0811_IR_RX_I2C_ADDR, /* IVTV_HW_Z8F0811_IR_RX_HAUP */ + IVTV_ADAPTEC_IR_ADDR, /* IVTV_HW_I2C_IR_RX_ADAPTEC */ }; /* This array should match the IVTV_HW_ defines */ @@ -143,8 +145,34 @@ static const char * const hw_devicenames[] = { "ir_video", /* IVTV_HW_I2C_IR_RX_HAUP_INT */ "ir_tx_z8f0811_haup", /* IVTV_HW_Z8F0811_IR_TX_HAUP */ "ir_rx_z8f0811_haup", /* IVTV_HW_Z8F0811_IR_RX_HAUP */ + "ir_adaptec", /* IVTV_HW_I2C_IR_RX_ADAPTEC */ }; +static int get_key_adaptec(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw) +{ + unsigned char keybuf[4]; + + keybuf[0] = 0x00; + i2c_master_send(ir->c, keybuf, 1); + /* poll IR chip */ + if (i2c_master_recv(ir->c, keybuf, sizeof(keybuf)) != sizeof(keybuf)) { + return 0; + } + + /* key pressed ? */ + if (keybuf[2] == 0xff) + return 0; + + /* remove repeat bit */ + keybuf[2] &= 0x7f; + keybuf[3] |= 0x80; + + *ir_key = (u32) keybuf; + *ir_raw = (u32) keybuf; + + return 1; +} + static int ivtv_i2c_new_ir(struct ivtv *itv, u32 hw, const char *type, u8 addr) { struct i2c_board_info info; @@ -190,6 +218,13 @@ static int ivtv_i2c_new_ir(struct ivtv *itv, u32 hw, const char *type, u8 addr) init_data->type = RC_TYPE_RC5; init_data->name = itv->card_name; break; + case IVTV_HW_I2C_IR_RX_ADAPTEC: + init_data->get_key = get_key_adaptec; + init_data->name = itv->card_name; + /* FIXME: The protocol and RC_MAP needs to be corrected */ + init_data->ir_codes = RC_MAP_EMPTY; + init_data->type = RC_TYPE_UNKNOWN; + break; } memset(&info, 0, sizeof(struct i2c_board_info)); @@ -219,7 +254,6 @@ struct i2c_client *ivtv_i2c_new_ir_legacy(struct ivtv *itv) 0x1a, /* Hauppauge IR external - collides with WM8739 */ 0x18, /* Hauppauge IR internal */ 0x71, /* Hauppauge IR (PVR150) */ - 0x6b, /* Adaptec IR */ I2C_CLIENT_END }; -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html