2004-01-15 12:20:49

by Gerd Knorr

[permalink] [raw]
Subject: [patch] v4l-05 add infrared remote support

Hi,

This patch adds a module with some helper functions to handle
infrared remotes using the linux input layer. It doesn't do
any useful stuff alone, but the saa7134 and bttv drivers will
use that to support the remotes shipped with some TV cards.

Gerd

diff -u linux-2.6.1/drivers/media/Kconfig linux/drivers/media/Kconfig
--- linux-2.6.1/drivers/media/Kconfig 2004-01-14 15:05:10.000000000 +0100
+++ linux/drivers/media/Kconfig 2004-01-14 15:09:35.000000000 +0100
@@ -49,5 +49,11 @@
default VIDEO_BT848
depends on VIDEO_DEV

+config VIDEO_IR
+ tristate
+ default y if VIDEO_BT848=y || VIDEO_SAA7134=y
+ default m if VIDEO_BT848=m || VIDEO_SAA7134=m
+ depends on VIDEO_DEV
+
endmenu

diff -u linux-2.6.1/drivers/media/common/Makefile linux/drivers/media/common/Makefile
--- linux-2.6.1/drivers/media/common/Makefile 2004-01-14 15:06:26.000000000 +0100
+++ linux/drivers/media/common/Makefile 2004-01-14 15:09:35.000000000 +0100
@@ -3,4 +3,4 @@

obj-$(CONFIG_VIDEO_SAA7146) += saa7146.o
obj-$(CONFIG_VIDEO_SAA7146_VV) += saa7146_vv.o
-
+obj-$(CONFIG_VIDEO_IR) += ir-common.o
diff -u linux-2.6.1/drivers/media/common/ir-common.c linux/drivers/media/common/ir-common.c
--- linux-2.6.1/drivers/media/common/ir-common.c 2004-01-14 15:09:35.000000000 +0100
+++ linux/drivers/media/common/ir-common.c 2004-01-14 15:09:35.000000000 +0100
@@ -0,0 +1,218 @@
+/*
+ * some common structs and functions to handle infrared remotes via
+ * input layer ...
+ *
+ * (c) 2003 Gerd Knorr <[email protected]> [SuSE Labs]
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/module.h>
+
+#include <media/ir-common.h>
+
+/* -------------------------------------------------------------------------- */
+
+MODULE_AUTHOR("Gerd Knorr <[email protected]> [SuSE Labs]");
+MODULE_LICENSE("GPL");
+
+static int repeat = 1;
+MODULE_PARM(repeat,"i");
+MODULE_PARM_DESC(repeat,"auto-repeat for IR keys (default: on)");
+
+static int debug = 0; /* debug level (0,1,2) */
+MODULE_PARM(debug,"i");
+
+#define dprintk(level, fmt, arg...) if (debug >= level) \
+ printk(KERN_DEBUG fmt , ## arg)
+
+/* -------------------------------------------------------------------------- */
+
+/* generic RC5 keytable */
+/* see http://users.pandora.be/nenya/electronics/rc5/codes00.htm */
+IR_KEYTAB_TYPE ir_codes_rc5_tv[IR_KEYTAB_SIZE] = {
+ [ 0x00 ] = KEY_KP0, // 0
+ [ 0x01 ] = KEY_KP1, // 1
+ [ 0x02 ] = KEY_KP2, // 2
+ [ 0x03 ] = KEY_KP3, // 3
+ [ 0x04 ] = KEY_KP4, // 4
+ [ 0x05 ] = KEY_KP5, // 5
+ [ 0x06 ] = KEY_KP6, // 6
+ [ 0x07 ] = KEY_KP7, // 7
+ [ 0x08 ] = KEY_KP8, // 8
+ [ 0x09 ] = KEY_KP9, // 9
+
+ [ 0x0b ] = KEY_CHANNEL, // channel / program (japan: 11)
+ [ 0x0c ] = KEY_POWER, // standby
+ [ 0x0d ] = KEY_MUTE, // mute / demute
+ [ 0x0f ] = KEY_TV, // display
+ [ 0x10 ] = KEY_VOLUMEUP, // volume +
+ [ 0x11 ] = KEY_VOLUMEDOWN, // volume -
+ [ 0x12 ] = KEY_BRIGHTNESSUP, // brightness +
+ [ 0x13 ] = KEY_BRIGHTNESSDOWN, // brightness -
+ [ 0x1e ] = KEY_SEARCH, // search +
+ [ 0x20 ] = KEY_CHANNELUP, // channel / program +
+ [ 0x21 ] = KEY_CHANNELDOWN, // channel / program -
+ [ 0x22 ] = KEY_CHANNEL, // alt / channel
+ [ 0x23 ] = KEY_LANGUAGE, // 1st / 2nd language
+ [ 0x26 ] = KEY_SLEEP, // sleeptimer
+ [ 0x2e ] = KEY_MENU, // 2nd controls (USA: menu)
+ [ 0x30 ] = KEY_PAUSE, // pause
+ [ 0x32 ] = KEY_REWIND, // rewind
+ [ 0x33 ] = KEY_GOTO, // go to
+ [ 0x35 ] = KEY_PLAY, // play
+ [ 0x36 ] = KEY_STOP, // stop
+ [ 0x37 ] = KEY_RECORD, // recording
+
+#if 0 /* FIXME */
+ [ 0x0a ] = KEY_RESERVED, // 1/2/3 digits (japan: 10)
+ [ 0x0e ] = KEY_RESERVED, // P.P. (personal preference)
+ [ 0x14 ] = KEY_RESERVED, // colour saturation +
+ [ 0x15 ] = KEY_RESERVED, // colour saturation -
+ [ 0x16 ] = KEY_RESERVED, // bass +
+ [ 0x17 ] = KEY_RESERVED, // bass -
+ [ 0x18 ] = KEY_RESERVED, // treble +
+ [ 0x19 ] = KEY_RESERVED, // treble -
+ [ 0x1a ] = KEY_RESERVED, // balance right
+ [ 0x1b ] = KEY_RESERVED, // balance left
+ [ 0x1c ] = KEY_RESERVED, // contrast +
+ [ 0x1d ] = KEY_RESERVED, // contrast -
+ [ 0x1f ] = KEY_RESERVED, // tint/hue +
+ [ 0x24 ] = KEY_RESERVED, // spacial stereo on/off
+ [ 0x25 ] = KEY_RESERVED, // mono / stereo (USA)
+ [ 0x27 ] = KEY_RESERVED, // tint / hue -
+ [ 0x28 ] = KEY_RESERVED, // RF switch/PIP select
+ [ 0x29 ] = KEY_RESERVED, // vote
+ [ 0x2a ] = KEY_RESERVED, // timed page/channel clck
+ [ 0x2b ] = KEY_RESERVED, // increment (USA)
+ [ 0x2c ] = KEY_RESERVED, // decrement (USA)
+ [ 0x2d ] = KEY_RESERVED, //
+ [ 0x2f ] = KEY_RESERVED, // PIP shift
+ [ 0x31 ] = KEY_RESERVED, // erase
+ [ 0x34 ] = KEY_RESERVED, // wind
+ [ 0x38 ] = KEY_RESERVED, // external 1
+ [ 0x39 ] = KEY_RESERVED, // external 2
+ [ 0x3a ] = KEY_RESERVED, // PIP display mode
+ [ 0x3b ] = KEY_RESERVED, // view data mode / advance
+ [ 0x3c ] = KEY_RESERVED, // teletext submode (Japan: 12)
+ [ 0x3d ] = KEY_RESERVED, // system standby
+ [ 0x3e ] = KEY_RESERVED, // crispener on/off
+ [ 0x3f ] = KEY_RESERVED, // system select
+#endif
+};
+EXPORT_SYMBOL_GPL(ir_codes_rc5_tv);
+
+/* empty keytable, can be used as placeholder for not-yet created keytables */
+IR_KEYTAB_TYPE ir_codes_empty[IR_KEYTAB_SIZE] = {
+ [ 42 ] = KEY_COFFEE,
+};
+EXPORT_SYMBOL_GPL(ir_codes_empty);
+
+/* -------------------------------------------------------------------------- */
+
+static void ir_input_key_event(struct input_dev *dev, struct ir_input_state *ir)
+{
+ if (KEY_RESERVED == ir->keycode) {
+ printk(KERN_INFO "%s: unknown key: key=0x%02x raw=0x%02x down=%d\n",
+ dev->name,ir->ir_key,ir->ir_raw,ir->keypressed);
+ return;
+ }
+ dprintk(1,"%s: key event code=%d down=%d\n",
+ dev->name,ir->keycode,ir->keypressed);
+ input_report_key(dev,ir->keycode,ir->keypressed);
+ input_sync(dev);
+}
+
+/* -------------------------------------------------------------------------- */
+
+void ir_input_init(struct input_dev *dev, struct ir_input_state *ir,
+ int ir_type, IR_KEYTAB_TYPE *ir_codes)
+{
+ int i;
+
+ ir->ir_type = ir_type;
+ if (ir_codes)
+ memcpy(ir->ir_codes, ir_codes, sizeof(ir->ir_codes));
+
+ init_input_dev(dev);
+ dev->keycode = ir->ir_codes;
+ dev->keycodesize = sizeof(IR_KEYTAB_TYPE);
+ dev->keycodemax = IR_KEYTAB_SIZE;
+ for (i = 0; i < IR_KEYTAB_SIZE; i++)
+ set_bit(ir->ir_codes[i], dev->keybit);
+ clear_bit(0, dev->keybit);
+
+ set_bit(EV_KEY, dev->evbit);
+ if (repeat)
+ set_bit(EV_REP, dev->evbit);
+}
+
+void ir_input_nokey(struct input_dev *dev, struct ir_input_state *ir)
+{
+ if (ir->keypressed) {
+ ir->keypressed = 0;
+ ir_input_key_event(dev,ir);
+ }
+}
+
+void ir_input_keydown(struct input_dev *dev, struct ir_input_state *ir,
+ u32 ir_key, u32 ir_raw)
+{
+ u32 keycode = IR_KEYCODE(ir->ir_codes, ir_key);
+
+ if (ir->keypressed && ir->keycode != keycode) {
+ ir->keypressed = 0;
+ ir_input_key_event(dev,ir);
+ }
+ if (!ir->keypressed) {
+ ir->ir_key = ir_key;
+ ir->ir_raw = ir_raw;
+ ir->keycode = keycode;
+ ir->keypressed = 1;
+ ir_input_key_event(dev,ir);
+ }
+#if 0
+ /* maybe do something like this ??? */
+ input_event(a, EV_IR, ir->ir_type, ir->ir_raw);
+#endif
+}
+
+u32 ir_extract_bits(u32 data, u32 mask)
+{
+ int mbit, vbit;
+ u32 value;
+
+ value = 0;
+ vbit = 0;
+ for (mbit = 0; mbit < 32; mbit++) {
+ if (!(mask & ((u32)1 << mbit)))
+ continue;
+ if (data & ((u32)1 << mbit))
+ value |= (1 << vbit);
+ vbit++;
+ }
+ return value;
+}
+
+EXPORT_SYMBOL_GPL(ir_input_init);
+EXPORT_SYMBOL_GPL(ir_input_nokey);
+EXPORT_SYMBOL_GPL(ir_input_keydown);
+EXPORT_SYMBOL_GPL(ir_extract_bits);
+
+/*
+ * Local variables:
+ * c-basic-offset: 8
+ * End:
+ */
diff -u linux-2.6.1/include/media/ir-common.h linux/include/media/ir-common.h
--- linux-2.6.1/include/media/ir-common.h 2004-01-14 15:09:35.000000000 +0100
+++ linux/include/media/ir-common.h 2004-01-14 15:09:35.000000000 +0100
@@ -0,0 +1,61 @@
+/*
+ * some common structs and functions to handle infrared remotes via
+ * input layer ...
+ *
+ * (c) 2003 Gerd Knorr <[email protected]> [SuSE Labs]
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/version.h>
+#include <linux/input.h>
+
+
+#define IR_TYPE_RC5 1
+#define IR_TYPE_OTHER 99
+
+#define IR_KEYTAB_TYPE u32
+#define IR_KEYTAB_SIZE 64 // enougth for rc5, probably need more some day ...
+
+#define IR_KEYCODE(tab,code) (((unsigned)code < IR_KEYTAB_SIZE) \
+ ? tab[code] : KEY_RESERVED)
+
+struct ir_input_state {
+ /* configuration */
+ int ir_type;
+ IR_KEYTAB_TYPE ir_codes[IR_KEYTAB_SIZE];
+
+ /* key info */
+ u32 ir_raw; /* raw data */
+ u32 ir_key; /* ir key code */
+ u32 keycode; /* linux key code */
+ int keypressed; /* current state */
+};
+
+extern IR_KEYTAB_TYPE ir_codes_rc5_tv[IR_KEYTAB_SIZE];
+extern IR_KEYTAB_TYPE ir_codes_empty[IR_KEYTAB_SIZE];
+
+void ir_input_init(struct input_dev *dev, struct ir_input_state *ir,
+ int ir_type, IR_KEYTAB_TYPE *ir_codes);
+void ir_input_nokey(struct input_dev *dev, struct ir_input_state *ir);
+void ir_input_keydown(struct input_dev *dev, struct ir_input_state *ir,
+ u32 ir_key, u32 ir_raw);
+u32 ir_extract_bits(u32 data, u32 mask);
+
+/*
+ * Local variables:
+ * c-basic-offset: 8
+ * End:
+ */

--
You have a new virus in /var/mail/kraxel


2004-01-15 14:26:53

by Roman Zippel

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

Hi,

On Thu, 15 Jan 2004, Gerd Knorr wrote:

> diff -u linux-2.6.1/drivers/media/Kconfig linux/drivers/media/Kconfig
> --- linux-2.6.1/drivers/media/Kconfig 2004-01-14 15:05:10.000000000 +0100
> +++ linux/drivers/media/Kconfig 2004-01-14 15:09:35.000000000 +0100
> @@ -49,5 +49,11 @@
> default VIDEO_BT848
> depends on VIDEO_DEV
>
> +config VIDEO_IR
> + tristate
> + default y if VIDEO_BT848=y || VIDEO_SAA7134=y
> + default m if VIDEO_BT848=m || VIDEO_SAA7134=m
> + depends on VIDEO_DEV
> +
> endmenu
>

This can also be written as:

config VIDEO_IR
def_tristate VIDEO_BT848 || VIDEO_SAA7134

def_tristate is the same as tristate & default ...
The dependency is redundant as the other options already depend on it.

Another possibility is to use select:

# selected as needed
config VIDEO_IR
tristate

config VIDEO_BT848
...
select VIDEO_IR

This has the advantage that adding/removing drivers only requires changes
at a single place.

bye, Roman

2004-01-15 15:40:13

by Gerd Knorr

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

> Another possibility is to use select:
>
> # selected as needed
> config VIDEO_IR
> tristate
>
> config VIDEO_BT848
> ...
> select VIDEO_IR

I like that one more, but last time I tried it didn't work.
Is support select in Linus tree now?

Gerd

--
You have a new virus in /var/mail/kraxel

2004-01-15 16:32:13

by Roman Zippel

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

Hi,

On Thu, 15 Jan 2004, Gerd Knorr wrote:

> > Another possibility is to use select:
> >
> > # selected as needed
> > config VIDEO_IR
> > tristate
> >
> > config VIDEO_BT848
> > ...
> > select VIDEO_IR
>
> I like that one more, but last time I tried it didn't work.
> Is support select in Linus tree now?

Yes, it should work, it was merged quite some time ago.

bye, Roman

2004-01-20 02:12:01

by Rusty Russell

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

In message <[email protected]> you write:
> +static int repeat = 1;
> +MODULE_PARM(repeat,"i");
> +MODULE_PARM_DESC(repeat,"auto-repeat for IR keys (default: on)");
> +
> +static int debug = 0; /* debug level (0,1,2) */
> +MODULE_PARM(debug,"i");

Please replace the MODULE_PARM lines with the modern form:

module_param(repeat, bool, 0644);
module_param(debug, int, 0644);

(I'm assuming that it's safe to change repeat and debug on the fly
without any locking. If not, change to 0444).

For more information, see include/linux/moduleparam.h.

Thanks!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

2004-01-20 09:20:14

by Gerd Knorr

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

On Tue, Jan 20, 2004 at 12:55:39PM +1100, Rusty Russell wrote:
> In message <[email protected]> you write:
> > +static int repeat = 1;
> > +MODULE_PARM(repeat,"i");
> > +MODULE_PARM_DESC(repeat,"auto-repeat for IR keys (default: on)");
> > +
> > +static int debug = 0; /* debug level (0,1,2) */
> > +MODULE_PARM(debug,"i");
>
> Please replace the MODULE_PARM lines with the modern form:
>
> module_param(repeat, bool, 0644);
> module_param(debug, int, 0644);

No. The code in question must also build on 2.4 kernels which don't
have module_param(). And I don't want to clutter up the code with
#ifdefs unless I absolutely have to.

I'll do for the bttv ir support, that is 2.6 only anyway due to the
usage of tasklets.

Gerd

--
"... und auch das ganze Wochenende oll" -- Wetterbericht auf RadioEins

2004-01-20 12:40:21

by Gerd Knorr

[permalink] [raw]
Subject: [patch] new module args for ir-kbd-*.c

> On Tue, Jan 20, 2004 at 12:55:39PM +1100, Rusty Russell wrote:
> > Please replace the MODULE_PARM lines with the modern form:
> >
> > module_param(repeat, bool, 0644);
> > module_param(debug, int, 0644);
>
> I'll do for the bttv ir support, that is 2.6 only anyway due to the
> usage of tasklets.

Here we go.

Gerd

==============================[ cut here ]==============================
diff -u linux-2.6.1-mm5/drivers/media/video/ir-kbd-i2c.c linux/drivers/media/video/ir-kbd-i2c.c
--- linux-2.6.1-mm5/drivers/media/video/ir-kbd-i2c.c 2004-01-20 12:59:51.491270352 +0100
+++ linux/drivers/media/video/ir-kbd-i2c.c 2004-01-20 13:03:04.982855160 +0100
@@ -25,6 +25,7 @@
*/

#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/sched.h>
@@ -55,8 +56,7 @@
/* ----------------------------------------------------------------------- */
/* insmod parameters */

-static int debug = 0; /* debug level (0,1,2) */
-MODULE_PARM(debug,"i");
+module_param(debug, int, 0644); /* debug level (0,1,2) */

#define DEVNAME "ir-kbd-i2c"
#define dprintk(level, fmt, arg...) if (debug >= level) \
diff -u linux-2.6.1-mm5/drivers/media/video/ir-kbd-gpio.c linux/drivers/media/video/ir-kbd-gpio.c
--- linux-2.6.1-mm5/drivers/media/video/ir-kbd-gpio.c 2004-01-20 12:59:51.490270504 +0100
+++ linux/drivers/media/video/ir-kbd-gpio.c 2004-01-20 13:03:05.080840264 +0100
@@ -18,6 +18,7 @@
*/

#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/input.h>
#include <linux/delay.h>
@@ -170,8 +171,7 @@
struct timer_list timer;
};

-static int debug = 0; /* debug level (0,1,2) */
-MODULE_PARM(debug,"i");
+module_param(debug, int, 0644); /* debug level (0,1,2) */

#define DEVNAME "ir-kbd-gpio"
#define dprintk(fmt, arg...) if (debug) \

2004-01-21 04:35:58

by Rusty Russell

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

In message <[email protected]> you write:
> No. The code in question must also build on 2.4 kernels which don't
> have module_param(). And I don't want to clutter up the code with
> #ifdefs unless I absolutely have to.

Marcelo, please read and apply.

This provides simple forwards compat for 2.4. It doesn't do arrays or
strings, but they can be added if required (this will cover the easy
90%).

Tested on 2.5.24-pre6.
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: 2.4 module_param Forward Compatibility Macros
Author: Rusty Russell
Status: Tested on 2.5.24-pre6
Version: 2.4

D: Simple uses of module_param() (implemented in 2.6) can be mapped
D: onto the old MODULE_PARM macros.
D:
D: New code should use module_param() because:
D: 1) Types are checked,
D: 2) Existence of parameters are checked,
D: 3) Customized types are possible [1]
D: 4) Customized set/get routines are possible [1]
D: 5) Parameters appear as boot params with prefix "<modname>." [1]
D: 6) Optional viewing and control through sysfs [2]
D:
D: [1] Not for 2.4 compatibility macros
D: [2] Not in 2.6.1 or 2.4, and only if third arg non-zero.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .25425-linux-2.4.25-pre6/include/linux/moduleparam.h .25425-linux-2.4.25-pre6.updated/include/linux/moduleparam.h
--- .25425-linux-2.4.25-pre6/include/linux/moduleparam.h 1970-01-01 10:00:00.000000000 +1000
+++ .25425-linux-2.4.25-pre6.updated/include/linux/moduleparam.h 2004-01-21 14:24:41.000000000 +1100
@@ -0,0 +1,25 @@
+#ifndef _LINUX_MODULE_PARAMS_H
+#define _LINUX_MODULE_PARAMS_H
+/* Macros for (very simple) module parameter compatibility with 2.6. */
+#include <linux/module.h>
+
+/* type is byte, short, ushort, int, uint, long, ulong, bool. (2.6
+ has more, but they are not supported). perm is permissions when
+ it appears in sysfs: 0 means doens't appear, 0444 means read-only
+ by everyone, 0644 means changable dynamically by root, etc. name
+ must be in scope (unlike MODULE_PARM).
+*/
+#define module_param(name, type, perm) \
+ static inline void *__check_existence_##name(void) { return &name; } \
+ MODULE_PARM(name, _MODULE_PARM_STRING_ ## type)
+
+#define _MODULE_PARM_STRING_byte "b"
+#define _MODULE_PARM_STRING_short "h"
+#define _MODULE_PARM_STRING_ushort "h"
+#define _MODULE_PARM_STRING_int "i"
+#define _MODULE_PARM_STRING_uint "i"
+#define _MODULE_PARM_STRING_long "l"
+#define _MODULE_PARM_STRING_ulong "l"
+#define _MODULE_PARM_STRING_bool "i"
+
+#endif /* _LINUX_MODULE_PARAM_TYPES_H */

2004-01-21 04:38:46

by Rusty Russell

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

In message <[email protected]> you write:
> On Tue, Jan 20, 2004 at 12:55:39PM +1100, Rusty Russell wrote:
> > In message <[email protected]> you write:
> > > +static int repeat = 1;
> > > +MODULE_PARM(repeat,"i");
> > > +MODULE_PARM_DESC(repeat,"auto-repeat for IR keys (default: on)");
> > > +
> > > +static int debug = 0; /* debug level (0,1,2) */
> > > +MODULE_PARM(debug,"i");
> >
> > Please replace the MODULE_PARM lines with the modern form:
> >
> > module_param(repeat, bool, 0644);
> > module_param(debug, int, 0644);
>
> No. The code in question must also build on 2.4 kernels which don't
> have module_param(). And I don't want to clutter up the code with
> #ifdefs unless I absolutely have to.

Sure! I'll write and test the forward compat macros for 2.4, submit
them to Marcelo, and then bother you again 8)

Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

2004-01-21 10:20:08

by Gerd Knorr

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

> This provides simple forwards compat for 2.4. It doesn't do arrays or
> strings, but they can be added if required (this will cover the easy
> 90%).

At least the array stuff I'm using in my drivers, to handle the
"multiple tv cards in one box" case, like this:

static unsigned int card[] = {[0 ... (SAA7134_MAXBOARDS - 1)] = UNSET };
MODULE_PARM(card,"1-" __stringify(SAA7134_MAXBOARDS) "i");
MODULE_PARM_DESC(card,"card type");

So having that in 2.4 too would be nice.

I have also two more questions: How can I specify default values != 0
for insmod options using the new macros? How specify help/description
texts? Using the MODULE_PARM_DESC() macro or is there something new too?

Gerd

2004-01-22 03:29:29

by Rusty Russell

[permalink] [raw]
Subject: Re: [patch] v4l-05 add infrared remote support

In message <[email protected]> you write:
> > This provides simple forwards compat for 2.4. It doesn't do arrays or
> > strings, but they can be added if required (this will cover the easy
> > 90%).
>
> At least the array stuff I'm using in my drivers, to handle the
> "multiple tv cards in one box" case, like this:
>
> static unsigned int card[] = {[0 ... (SAA7134_MAXBOARDS - 1)] = UNSET };
> MODULE_PARM(card,"1-" __stringify(SAA7134_MAXBOARDS) "i");
> MODULE_PARM_DESC(card,"card type");
>
> So having that in 2.4 too would be nice.

Unfortunately the module_param_array() macro is not really fungible
into the old MODULE_PARM() macro: in particular the array size is now
implicit.

I could introduce another macro which could be #defined to both, but
for the moment I thought I'd see we get without one.

> I have also two more questions: How can I specify default values != 0
> for insmod options using the new macros?

Not sure I understand? You'd presumably do it like so:

int foo = 7;
MODULE_PARM(foo, "i"); / module_param(foo, int, 0600)

> How specify help/description
> texts? Using the MODULE_PARM_DESC() macro or is there something new too?

I didn't change it. I thought about it, but I think that would just
be neatening for the sake of neatening, which I dislike.

Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.