diff mbox

Allow the user to restrict the RC5 address

Message ID 20090326033453.7d90236d@laptop.hypervisor.org (mailing list archive)
State Changes Requested
Headers show

Commit Message

Udo Steinberg March 26, 2009, 2:34 a.m. UTC 
Mauro,

This patch allows users with multiple remotes to specify an RC5 address for a
remote from which key codes will be accepted. If no address is specified, the
default value of 0 accepts key codes from any remote. This replaces the current
hard-coded address checks, which are too restrictive.


Signed-off-by: Udo Steinberg <udo@hypervisor.org>

Comments

Hans Verkuil March 26, 2009, 7:24 a.m. UTC | #1 
On Thursday 26 March 2009 03:34:53 Udo A. Steinberg wrote:
> Mauro,
>
> This patch allows users with multiple remotes to specify an RC5 address
> for a remote from which key codes will be accepted. If no address is
> specified, the default value of 0 accepts key codes from any remote. This
> replaces the current hard-coded address checks, which are too
> restrictive.

I think this should be reviewed by Steve Toth first (CC-ed him).

One thing that this patch breaks is if you have multiple Hauppauge remotes, 
some sending 0x1e, some 0x1f. With this patch I can't use both, only one.

It might be better to have an option to explicitly allow old Hauppauge 
remotes that send 0x00.

Or an heuristic: if you see a remote with device 0x1e or 0x1f, then filter 
remotes with device 0x00 afterwards.

A third option might be to extend the hauppauge option, allowing you to 
specify '2=Old WinTV remote' or something like that, and if set device 0x00 
will be allowed.

Regards,

	Hans

>
>
> Signed-off-by: Udo Steinberg <udo@hypervisor.org>
>
>
> --- linux-2.6.29/drivers/media/video/ir-kbd-i2c.c	2009-03-24
> 00:12:14.000000000 +0100 +++
> linux-2.6.29/drivers/media/video/ir-kbd-i2c.new	2009-03-26
> 03:12:11.000000000 +0100 @@ -58,6 +58,9 @@
>  module_param(hauppauge, int, 0644);    /* Choose Hauppauge remote */
>  MODULE_PARM_DESC(hauppauge, "Specify Hauppauge remote: 0=black, 1=grey
> (defaults to 0)");
>
> +static unsigned int device;
> +module_param(device, uint, 0644);    /* RC5 device address */
> +MODULE_PARM_DESC(device, "Specify device address: 0=any (defaults to
> 0)");
>
>  #define DEVNAME "ir-kbd-i2c"
>  #define dprintk(level, fmt, arg...)	if (debug >= level) \
> @@ -104,8 +107,8 @@
>  		/* invalid key press */
>  		return 0;
>
> -	if (dev!=0x1e && dev!=0x1f)
> -		/* not a hauppauge remote */
> +	if (device && device != dev)
> +		/* not an acceptable remote */
>  		return 0;
>
>  	if (!range)
Steven Toth March 26, 2009, 4:45 p.m. UTC | #2 
Hans Verkuil wrote:
> On Thursday 26 March 2009 03:34:53 Udo A. Steinberg wrote:
>   
>> Mauro,
>>
>> This patch allows users with multiple remotes to specify an RC5 address
>> for a remote from which key codes will be accepted. If no address is
>> specified, the default value of 0 accepts key codes from any remote. This
>> replaces the current hard-coded address checks, which are too
>> restrictive.
>>     
>
> I think this should be reviewed by Steve Toth first (CC-ed him).
>
> One thing that this patch breaks is if you have multiple Hauppauge remotes, 
> some sending 0x1e, some 0x1f. With this patch I can't use both, only one.
>
>   
Hans, thanks for bringing this to my attention.

Mauro, This patch is a regression, although a small one. it probably 
needs a little more work.

I too tend to have multiple remotes, I don't think it's that unusual for 
long standing Hauppauge customers to have many boards with many types of 
remotes.

> It might be better to have an option to explicitly allow old Hauppauge 
> remotes that send 0x00.
>
>   
I could live with this. It relegates older remotes but those remotes are 
no longer made. This feels like a good compromise.

- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Udo Steinberg March 26, 2009, 6:45 p.m. UTC | #3 
On Thu, 26 Mar 2009 12:45:18 -0400 Steven Toth (ST) wrote:

ST> I too tend to have multiple remotes, I don't think it's that unusual for
ST> long standing Hauppauge customers to have many boards with many types of
ST> remotes.
ST> 
ST> > It might be better to have an option to explicitly allow old Hauppauge
ST> > remotes that send 0x00.
ST> >
ST> I could live with this. It relegates older remotes but those remotes
ST> are no longer made. This feels like a good compromise.

How about changing the parameter such that it is a filter mask? The default
value of 0x0 would accept all remotes. For non-zero values, each bit set in
the parameter would filter the device address corresponding to that bit, e.g.
0x1 would filter address 0x0, 0x80000000 would filter address 0x1f, etc.

Cheers,

	- Udo
Hans Verkuil March 27, 2009, 7:28 a.m. UTC | #4 
On Thursday 26 March 2009 19:45:53 Udo A. Steinberg wrote:
> On Thu, 26 Mar 2009 12:45:18 -0400 Steven Toth (ST) wrote:
>
> ST> I too tend to have multiple remotes, I don't think it's that unusual
> for ST> long standing Hauppauge customers to have many boards with many
> types of ST> remotes.
> ST>
> ST> > It might be better to have an option to explicitly allow old
> Hauppauge ST> > remotes that send 0x00.
> ST> >
> ST> I could live with this. It relegates older remotes but those remotes
> ST> are no longer made. This feels like a good compromise.
>
> How about changing the parameter such that it is a filter mask? The
> default value of 0x0 would accept all remotes. For non-zero values, each
> bit set in the parameter would filter the device address corresponding to
> that bit, e.g. 0x1 would filter address 0x0, 0x80000000 would filter
> address 0x1f, etc.

Seems way to complicated to me. Remember that end users have no idea about 
the device addresses, so it should be a simple to understand module option. 
So I'd just go with Steve's preference.

Regards,

	Hans

>
> Cheers,
>
> 	- Udo
diff mbox

Patch

--- linux-2.6.29/drivers/media/video/ir-kbd-i2c.c	2009-03-24 00:12:14.000000000 +0100
+++ linux-2.6.29/drivers/media/video/ir-kbd-i2c.new	2009-03-26 03:12:11.000000000 +0100
@@ -58,6 +58,9 @@ 
 module_param(hauppauge, int, 0644);    /* Choose Hauppauge remote */
 MODULE_PARM_DESC(hauppauge, "Specify Hauppauge remote: 0=black, 1=grey (defaults to 0)");
 
+static unsigned int device;
+module_param(device, uint, 0644);    /* RC5 device address */
+MODULE_PARM_DESC(device, "Specify device address: 0=any (defaults to 0)");
 
 #define DEVNAME "ir-kbd-i2c"
 #define dprintk(level, fmt, arg...)	if (debug >= level) \
@@ -104,8 +107,8 @@ 
 		/* invalid key press */
 		return 0;
 
-	if (dev!=0x1e && dev!=0x1f)
-		/* not a hauppauge remote */
+	if (device && device != dev)
+		/* not an acceptable remote */
 		return 0;
 
 	if (!range)