Re: [PATCH 6/6] saa7134: Simplify handling of IR on AVerMedia Cardbus

Re: [PATCH 6/6] saa7134: Simplify handling of IR on AVerMedia Cardbus

[Oldrich Jedlicka]

Hi Jean,

On Saturday 04 of April 2009 at 14:31:37, Jean Delvare wrote:
> Now that we instantiate I2C IR devices explicitly, we can skip probing
> altogether on boards where the I2C IR device address is known. The
> AVerMedia Cardbus are two of these boards.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
>  linux/drivers/media/video/saa7134/saa7134-input.c |   35
> +++------------------ 1 file changed, 5 insertions(+), 30 deletions(-)
>
> ---
> v4l-dvb.orig/linux/drivers/media/video/saa7134/saa7134-input.c	2009-04-04
> 10:41:44.000000000 +0200 +++
> v4l-dvb/linux/drivers/media/video/saa7134/saa7134-input.c	2009-04-04
> 10:47:10.000000000 +0200 @@ -691,22 +691,6 @@ void
> saa7134_probe_i2c_ir(struct saa7134
>  		I2C_CLIENT_END
>  	};
>
> -	unsigned char subaddr, data;
> -	struct i2c_msg msg_avermedia[] = { {
> -		.addr = 0x40,
> -		.flags = 0,
> -		.len = 1,
> -		.buf = &subaddr,
> -	}, {
> -		.addr = 0x40,
> -		.flags = I2C_M_RD,
> -		.len = 1,
> -		.buf = &data,
> -	} };
> -
> -	struct i2c_client *client;
> -	int rc;
> -
>  	if (disable_ir) {
>  		dprintk("IR has been disabled, not probing for i2c remote\n");
>  		return;
> @@ -753,6 +737,10 @@ void saa7134_probe_i2c_ir(struct saa7134
>  		init_data.get_key = get_key_beholdm6xx;
>  		init_data.ir_codes = ir_codes_behold;
>  		break;
> +	case SAA7134_BOARD_AVERMEDIA_CARDBUS:
> +	case SAA7134_BOARD_AVERMEDIA_CARDBUS_506:
> +		info.addr = 0x40;
> +		break;
>  	}

The Avermedia Cardbus (E500 - SAA7134_BOARD_AVERMEDIA_CARDBUS) doesn't have 
remote control as far as I know. The first model was Cardbus Plus (E501R) 
which is not supported (yet), but Grigory Milev reported that it works with 
small patching. I plan to send patches after some more testing.

Cheers,
Oldrich.

(Jean, sorry for double-sending, I replied wrongly, so the LMML was not on the 
CC list)

>
>  	if (init_data.name)
> @@ -764,20 +752,7 @@ void saa7134_probe_i2c_ir(struct saa7134
>  	}
>
>  	/* Address not known, fallback to probing */
> -	client = i2c_new_probed_device(&dev->i2c_adap, &info, addr_list);
> -	if (client)
> -		return;
> -
> -	/* Special case for AVerMedia Cardbus remote */
> -	subaddr = 0x0d;
> -	rc = i2c_transfer(&dev->i2c_adap, msg_avermedia, 2);
> -	dprintk(KERN_DEBUG "probe 0x%02x/0x%02x @ %s: %s\n",
> -		msg_avermedia[0].addr, subaddr, dev->i2c_adap.name,
> -		(2 == rc) ? "yes" : "no");
> -	if (2 == rc) {
> -		info.addr = msg_avermedia[0].addr;
> -		i2c_new_device(&dev->i2c_adap, &info);
> -	}
> +	i2c_new_probed_device(&dev->i2c_adap, &info, addr_list);
>  }
>
>  static int saa7134_rc5_irq(struct saa7134_dev *dev)

Re: [PATCH 6/6] saa7134: Simplify handling of IR on AVerMedia Cardbus

[Jean Delvare]

Hi Oldrich,

On Thu, 9 Apr 2009 23:12:51 +0200, Oldrich Jedlicka wrote:
> On Saturday 04 of April 2009 at 14:31:37, Jean Delvare wrote:
> > Now that we instantiate I2C IR devices explicitly, we can skip probing
> > altogether on boards where the I2C IR device address is known. The
> > AVerMedia Cardbus are two of these boards.
> >
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > ---
> >  linux/drivers/media/video/saa7134/saa7134-input.c |   35
> > +++------------------ 1 file changed, 5 insertions(+), 30 deletions(-)
> >
> > ---
> > v4l-dvb.orig/linux/drivers/media/video/saa7134/saa7134-input.c	2009-04-04
> > 10:41:44.000000000 +0200 +++
> > v4l-dvb/linux/drivers/media/video/saa7134/saa7134-input.c	2009-04-04
> > 10:47:10.000000000 +0200 @@ -691,22 +691,6 @@ void
> > saa7134_probe_i2c_ir(struct saa7134
> >  		I2C_CLIENT_END
> >  	};
> >
> > -	unsigned char subaddr, data;
> > -	struct i2c_msg msg_avermedia[] = { {
> > -		.addr = 0x40,
> > -		.flags = 0,
> > -		.len = 1,
> > -		.buf = &subaddr,
> > -	}, {
> > -		.addr = 0x40,
> > -		.flags = I2C_M_RD,
> > -		.len = 1,
> > -		.buf = &data,
> > -	} };
> > -
> > -	struct i2c_client *client;
> > -	int rc;
> > -
> >  	if (disable_ir) {
> >  		dprintk("IR has been disabled, not probing for i2c remote\n");
> >  		return;
> > @@ -753,6 +737,10 @@ void saa7134_probe_i2c_ir(struct saa7134
> >  		init_data.get_key = get_key_beholdm6xx;
> >  		init_data.ir_codes = ir_codes_behold;
> >  		break;
> > +	case SAA7134_BOARD_AVERMEDIA_CARDBUS:
> > +	case SAA7134_BOARD_AVERMEDIA_CARDBUS_506:
> > +		info.addr = 0x40;
> > +		break;
> >  	}
> 
> The Avermedia Cardbus (E500 - SAA7134_BOARD_AVERMEDIA_CARDBUS) doesn't have 
> remote control as far as I know. The first model was Cardbus Plus (E501R) 
> which is not supported (yet), but Grigory Milev reported that it works with 
> small patching. I plan to send patches after some more testing.

OK, I've removed case SAA7134_BOARD_AVERMEDIA_CARDBUS from my patch,
thanks for letting me know.

-- 
Jean Delvare

Re: [PATCH 6/6] saa7134: Simplify handling of IR on AVerMedia Cardbus

[Oldrich Jedlicka]

Hi Jean,

On Friday 17 of April 2009 at 15:45:20, Jean Delvare wrote:
> Hi Oldrich,
>
> On Thu, 9 Apr 2009 23:12:51 +0200, Oldrich Jedlicka wrote:
> > On Saturday 04 of April 2009 at 14:31:37, Jean Delvare wrote:
[sniff]
> > > @@ -753,6 +737,10 @@ void saa7134_probe_i2c_ir(struct saa7134
> > >  		init_data.get_key = get_key_beholdm6xx;
> > >  		init_data.ir_codes = ir_codes_behold;
> > >  		break;
> > > +	case SAA7134_BOARD_AVERMEDIA_CARDBUS:
> > > +	case SAA7134_BOARD_AVERMEDIA_CARDBUS_506:
> > > +		info.addr = 0x40;
> > > +		break;
> > >  	}
> >
> > The Avermedia Cardbus (E500 - SAA7134_BOARD_AVERMEDIA_CARDBUS) doesn't
> > have remote control as far as I know. The first model was Cardbus Plus
> > (E501R) which is not supported (yet), but Grigory Milev reported that it
> > works with small patching. I plan to send patches after some more
> > testing.
>
> OK, I've removed case SAA7134_BOARD_AVERMEDIA_CARDBUS from my patch,
> thanks for letting me know.

You can add SAA7134_BOARD_AVERMEDIA_501 there - if my patch for it get 
accepted :-)

Cheers,
Oldrich.

Re: [PATCH 6/6] saa7134: Simplify handling of IR on AVerMedia Cardbus

[Jean Delvare]

On Fri, 17 Apr 2009 18:16:06 +0200, Oldrich Jedlicka wrote:
> Hi Jean,
> 
> On Friday 17 of April 2009 at 15:45:20, Jean Delvare wrote:
> > Hi Oldrich,
> >
> > On Thu, 9 Apr 2009 23:12:51 +0200, Oldrich Jedlicka wrote:
> > > On Saturday 04 of April 2009 at 14:31:37, Jean Delvare wrote:
> [sniff]
> > > > @@ -753,6 +737,10 @@ void saa7134_probe_i2c_ir(struct saa7134
> > > >  		init_data.get_key = get_key_beholdm6xx;
> > > >  		init_data.ir_codes = ir_codes_behold;
> > > >  		break;
> > > > +	case SAA7134_BOARD_AVERMEDIA_CARDBUS:
> > > > +	case SAA7134_BOARD_AVERMEDIA_CARDBUS_506:
> > > > +		info.addr = 0x40;
> > > > +		break;
> > > >  	}
> > >
> > > The Avermedia Cardbus (E500 - SAA7134_BOARD_AVERMEDIA_CARDBUS) doesn't
> > > have remote control as far as I know. The first model was Cardbus Plus
> > > (E501R) which is not supported (yet), but Grigory Milev reported that it
> > > works with small patching. I plan to send patches after some more
> > > testing.
> >
> > OK, I've removed case SAA7134_BOARD_AVERMEDIA_CARDBUS from my patch,
> > thanks for letting me know.
> 
> You can add SAA7134_BOARD_AVERMEDIA_501 there - if my patch for it get 
> accepted :-)

Obviously I can't add it before your own patch is in ;)

-- 
Jean Delvare

[PATCH 0/6] ir-kbd-i2c conversion to the new i2c binding model

[Jean Delvare]

Hi all,

Here finally comes my conversion of ir-kbd-i2c to the new i2c binding
model. I've split it into 6 pieces for easier review. Firstly there are
2 preliminary patches:

media-video-01-cx18-fix-i2c-error-handling.patch
media-video-02-ir-kbd-i2c-dont-abuse-client-name.patch

Then 2 patches doing the actual conversion:

media-video-03-ir-kbd-i2c-convert-to-new-style.patch
media-video-04-configure-ir-receiver.patch

And lastly 2 patches cleaning up saa7134-input thanks to the new
possibilities offered by the conversion:

media-video-05-saa7134-input-cleanup-msi-ir.patch
media-video-06-saa7134-input-cleanup-avermedia-cardbus.patch

This patch set is against the v4l-dvb repository, but I didn't pay
attention to the compatibility issues. I simply build-tested it on
2.6.27 and 2.6.29.

This patch set touches many different drivers and I can't test any of
them. My only TV card with an IR receiver doesn't make use of
ir-kbd-i2c. So I would warmly welcome testers. The more testing my
changes can get, the better.

And of course I welcome reviews and comments as well. I had to touch
many drivers I don't know anything about so it is possible that I
missed something.

I'll post all 6 patches as replies to this post. They can also be
temporarily downloaded from:
  http://jdelvare.pck.nerim.net/linux/ir-kbd-i2c/
Additionally I've put a combined patch there, to make testing easier:
  http://jdelvare.pck.nerim.net/linux/ir-kbd-i2c/ir-kbd-i2c-conversion-ALL-IN-ONE.patch
But for review the individual patches are much better.

Thanks,
-- 
Jean Delvare

[PATCH 6/6] saa7134: Simplify handling of IR on AVerMedia Cardbus

[Jean Delvare]

Now that we instantiate I2C IR devices explicitly, we can skip probing
altogether on boards where the I2C IR device address is known. The
AVerMedia Cardbus are two of these boards.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
 linux/drivers/media/video/saa7134/saa7134-input.c |   35 +++------------------
 1 file changed, 5 insertions(+), 30 deletions(-)

--- v4l-dvb.orig/linux/drivers/media/video/saa7134/saa7134-input.c	2009-04-04 10:41:44.000000000 +0200
+++ v4l-dvb/linux/drivers/media/video/saa7134/saa7134-input.c	2009-04-04 10:47:10.000000000 +0200
@@ -691,22 +691,6 @@ void saa7134_probe_i2c_ir(struct saa7134
 		I2C_CLIENT_END
 	};
 
-	unsigned char subaddr, data;
-	struct i2c_msg msg_avermedia[] = { {
-		.addr = 0x40,
-		.flags = 0,
-		.len = 1,
-		.buf = &subaddr,
-	}, {
-		.addr = 0x40,
-		.flags = I2C_M_RD,
-		.len = 1,
-		.buf = &data,
-	} };
-
-	struct i2c_client *client;
-	int rc;
-
 	if (disable_ir) {
 		dprintk("IR has been disabled, not probing for i2c remote\n");
 		return;
@@ -753,6 +737,10 @@ void saa7134_probe_i2c_ir(struct saa7134
 		init_data.get_key = get_key_beholdm6xx;
 		init_data.ir_codes = ir_codes_behold;
 		break;
+	case SAA7134_BOARD_AVERMEDIA_CARDBUS:
+	case SAA7134_BOARD_AVERMEDIA_CARDBUS_506:
+		info.addr = 0x40;
+		break;
 	}
 
 	if (init_data.name)
@@ -764,20 +752,7 @@ void saa7134_probe_i2c_ir(struct saa7134
 	}
 
 	/* Address not known, fallback to probing */
-	client = i2c_new_probed_device(&dev->i2c_adap, &info, addr_list);
-	if (client)
-		return;
-
-	/* Special case for AVerMedia Cardbus remote */
-	subaddr = 0x0d;
-	rc = i2c_transfer(&dev->i2c_adap, msg_avermedia, 2);
-	dprintk(KERN_DEBUG "probe 0x%02x/0x%02x @ %s: %s\n",
-		msg_avermedia[0].addr, subaddr, dev->i2c_adap.name,
-		(2 == rc) ? "yes" : "no");
-	if (2 == rc) {
-		info.addr = msg_avermedia[0].addr;
-		i2c_new_device(&dev->i2c_adap, &info);
-	}
+	i2c_new_probed_device(&dev->i2c_adap, &info, addr_list);
 }
 
 static int saa7134_rc5_irq(struct saa7134_dev *dev)

-- 
Jean Delvare