Re: Joystick Autofire Support for linux

Re: Joystick Autofire Support for linux

[Juan Antonio Martinez]

[-- Attachment #1: Type: text/plain, Size: 10194 bytes --]

El vie, 02-01-2004 a las 17:09, Emard escribió:
> HI!
> 
> For linux I made the kernel patch that adds autofire support
> for old mechanic joysticks that don't have it. The linux personna
> in charge doesn't want this functionality in kernel because he
> thinks it belongs to the application but I don't think so 
> because I know there exist some joysticks with autofire hardware
> TTL circuits built in.
> 
> Please forward this patch to some xmame related site so people can
> use it.. it is for 2.4 and 2.6 kernels.

I'm not the mantainer of XMame since 1999; anyway, i'll try to forward
your mail to xmame and l-kernel lists

A few comments on your code an a humble opinion. Note that I'm
a kernel "observer" not "programmer" :-)

- Joystick api is too simple: just a syscall to read events and 
an ioctl's to set device. It's responsability of the user to 
make polling ant take note on extra features. There are lot of
devices in Linux kernel that follow this approach

- DB9 joysticks are really old. If you really want to implement
 autofire, perhaps a better approach is to make it a generic
option, not exclusive to db9 joysticks... A doubt: are there
similar features in HID devices ? (keypads, mouse buttons...)

- Your code stats autofire on module load. I think it would 
be better make it a runtime option, via ioctl or sysfs. Not
sure that making changes in API would be a good idea...

- I need further study, but a simple on/off swicht as 
used in your code could result on missfunction: think on 
hot plug/unplug issues...

- Some middleware already implements autofire features. No real
need of doing it at kernel level. In fact, i remember some
games that takes care on it... my very (very) early versions 
on Xmame, for instance... :-)

IM(very)HO, a generic autofire input runtime option is a 
"could be" at kernel level, but not with this approach

Regards

> Best regards, and Happy New Year
> Emard
> 
> --- linux-2.4.23/drivers/char/joystick/db9.c.orig	Thu Sep 13 00:34:06 2001
> +++ linux-2.4.23/drivers/char/joystick/db9.c	Thu Jan  1 14:26:33 2004
> @@ -42,9 +42,9 @@
>  
>  MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
>  MODULE_LICENSE("GPL");
> -MODULE_PARM(db9, "2i");
> -MODULE_PARM(db9_2, "2i");
> -MODULE_PARM(db9_3, "2i");
> +MODULE_PARM(db9, "2-3i");
> +MODULE_PARM(db9_2, "2-3i");
> +MODULE_PARM(db9_3, "2-3i");
>  
>  #define DB9_MULTI_STICK		0x01
>  #define DB9_MULTI2_STICK	0x02
> @@ -77,15 +77,16 @@ MODULE_PARM(db9_3, "2i");
>  #define DB9_GENESIS6_DELAY	14
>  #define DB9_REFRESH_TIME	HZ/100
>  
> -static int db9[] __initdata = { -1, 0 };
> -static int db9_2[] __initdata = { -1, 0 };
> -static int db9_3[] __initdata = { -1, 0 };
> +static int db9[] __initdata = { -1, 0, 0 };
> +static int db9_2[] __initdata = { -1, 0, 0 };
> +static int db9_3[] __initdata = { -1, 0, 0 };
>  
>  struct db9 {
>  	struct input_dev dev[2];
>  	struct timer_list timer;
>  	struct pardevice *pd;	
>  	int mode;
> +	unsigned int autofire, autofire_hammer;
>  	int used;
>  };
>  
> @@ -108,6 +109,7 @@ static void db9_timer(unsigned long priv
>  	struct parport *port = db9->pd->port;
>  	struct input_dev *dev = db9->dev;
>  	int data, i;
> +	unsigned int autofire = (db9->autofire_hammer ^= db9->autofire);
>  
>  	switch(db9->mode) {
>  		case DB9_MULTI_0802_2:
> @@ -130,7 +132,7 @@ static void db9_timer(unsigned long priv
>  		case DB9_MULTI_STICK:
>  
>  			data = parport_read_data(port);
> -
> +			data |= autofire;
>  			input_report_abs(dev, ABS_X, (data & DB9_RIGHT ? 0 : 1) - (data & DB9_LEFT ? 0 : 1));
>  			input_report_abs(dev, ABS_Y, (data & DB9_DOWN  ? 0 : 1) - (data & DB9_UP   ? 0 : 1));
>  			input_report_key(dev, BTN_TRIGGER, ~data & DB9_FIRE1);
> @@ -139,7 +141,7 @@ static void db9_timer(unsigned long priv
>  		case DB9_MULTI2_STICK:
>  
>  			data = parport_read_data(port);
> -
> +			data |= autofire;
>  			input_report_abs(dev, ABS_X, (data & DB9_RIGHT ? 0 : 1) - (data & DB9_LEFT ? 0 : 1));
>  			input_report_abs(dev, ABS_Y, (data & DB9_DOWN  ? 0 : 1) - (data & DB9_UP   ? 0 : 1));
>  			input_report_key(dev, BTN_TRIGGER, ~data & DB9_FIRE1);
> @@ -306,7 +308,7 @@ static struct db9 __init *db9_probe(int 
>  	if (config[0] < 0)
>  		return NULL;
>  	if (config[1] < 1 || config[1] >= DB9_MAX_PAD || !db9_buttons[config[1]]) {
> -		printk(KERN_ERR "db9.c: bad config\n");
> +		printk(KERN_ERR "db9.c: bad config %d,%d,%d\n", config[0], config[1], config[2]);
>  		return NULL;
>  	}
>  
> @@ -328,6 +330,8 @@ static struct db9 __init *db9_probe(int 
>  	memset(db9, 0, sizeof(struct db9));
>  
>  	db9->mode = config[1];
> +	db9->autofire = config[2];
> +	db9->autofire_hammer = 0;
>  	init_timer(&db9->timer);
>  	db9->timer.data = (long) db9;
>  	db9->timer.function = db9_timer;
> 
> --- linux-2.6.0/drivers/input/joystick/db9.c.orig	2004-01-01 14:59:30.000000000 +0100
> +++ linux-2.6.0/drivers/input/joystick/db9.c	2004-01-01 14:59:35.000000000 +0100
> @@ -42,9 +42,9 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@u
>  MODULE_DESCRIPTION("Atari, Amstrad, Commodore, Amiga, Sega, etc. joystick driver");
>  MODULE_LICENSE("GPL");
>  
> -MODULE_PARM(db9, "2i");
> -MODULE_PARM(db9_2, "2i");
> -MODULE_PARM(db9_3, "2i");
> +MODULE_PARM(db9, "2-3i");
> +MODULE_PARM(db9_2, "2-3i");
> +MODULE_PARM(db9_3, "2-3i");
>  
>  #define DB9_MULTI_STICK		0x01
>  #define DB9_MULTI2_STICK	0x02
> @@ -76,15 +76,16 @@ MODULE_PARM(db9_3, "2i");
>  #define DB9_GENESIS6_DELAY	14
>  #define DB9_REFRESH_TIME	HZ/100
>  
> -static int db9[] __initdata = { -1, 0 };
> -static int db9_2[] __initdata = { -1, 0 };
> -static int db9_3[] __initdata = { -1, 0 };
> +static int db9[] __initdata = { -1, 0, 0 };
> +static int db9_2[] __initdata = { -1, 0, 0 };
> +static int db9_3[] __initdata = { -1, 0, 0 };
>  
>  struct db9 {
>  	struct input_dev dev[DB9_MAX_DEVICES];
>  	struct timer_list timer;
>  	struct pardevice *pd;	
>  	int mode;
> +	unsigned int autofire, autofire_hammer;
>  	int used;
>  	char phys[2][32];
>  };
> @@ -343,6 +344,7 @@ static void db9_timer(unsigned long priv
>  	struct parport *port = db9->pd->port;
>  	struct input_dev *dev = db9->dev;
>  	int data, i;
> +	unsigned int autofire = (db9->autofire_hammer ^= db9->autofire);
>  
>  	switch(db9->mode) {
>  		case DB9_MULTI_0802_2:
> @@ -365,7 +367,7 @@ static void db9_timer(unsigned long priv
>  		case DB9_MULTI_STICK:
>  
>  			data = parport_read_data(port);
> -
> +			data |= autofire;
>  			input_report_abs(dev, ABS_X, (data & DB9_RIGHT ? 0 : 1) - (data & DB9_LEFT ? 0 : 1));
>  			input_report_abs(dev, ABS_Y, (data & DB9_DOWN  ? 0 : 1) - (data & DB9_UP   ? 0 : 1));
>  			input_report_key(dev, BTN_TRIGGER, ~data & DB9_FIRE1);
> @@ -374,7 +376,7 @@ static void db9_timer(unsigned long priv
>  		case DB9_MULTI2_STICK:
>  
>  			data = parport_read_data(port);
> -
> +			data |= autofire;
>  			input_report_abs(dev, ABS_X, (data & DB9_RIGHT ? 0 : 1) - (data & DB9_LEFT ? 0 : 1));
>  			input_report_abs(dev, ABS_Y, (data & DB9_DOWN  ? 0 : 1) - (data & DB9_UP   ? 0 : 1));
>  			input_report_key(dev, BTN_TRIGGER, ~data & DB9_FIRE1);
> @@ -527,7 +529,7 @@ static struct db9 __init *db9_probe(int 
>  	if (config[0] < 0)
>  		return NULL;
>  	if (config[1] < 1 || config[1] >= DB9_MAX_PAD || !db9_buttons[config[1]]) {
> -		printk(KERN_ERR "db9.c: bad config\n");
> +		printk(KERN_ERR "db9.c: bad config %d,%d,%d\n", config[0], config[1], config[2]);
>  		return NULL;
>  	}
>  
> @@ -551,6 +553,8 @@ static struct db9 __init *db9_probe(int 
>  	memset(db9, 0, sizeof(struct db9));
>  
>  	db9->mode = config[1];
> +	db9->autofire = config[2];
> +	db9->autofire_hammer = 0;
>  	init_timer(&db9->timer);
>  	db9->timer.data = (long) db9;
>  	db9->timer.function = db9_timer;
> 
> --- joystick-parport.txt.orig	Thu Jan  1 14:03:27 2004
> +++ joystick-parport.txt	Thu Jan  1 14:50:35 2004
> @@ -465,7 +465,7 @@ ports.
>    Apart from making an interface, there is nothing difficult on using the
>  db9.c driver. It uses the following kernel/module command line:
>  
> -	db9=port,type
> +	db9=port,type[,autofire]
>  
>    Where 'port' is the number of the parport interface (eg. 0 for parport0).
>  
> @@ -473,7 +473,7 @@ db9.c driver. It uses the following kern
>  your parallel port is recent enough, you should have no trouble with this.
>  Old parallel ports may not have this feature.
>  
> -  'Type' is the type of joystick or pad attached:
> +  'type' is the type of joystick or pad attached:
>  
>  	Type | Joystick/Pad
>  	--------------------
> @@ -491,6 +491,37 @@ Old parallel ports may not have this fea
>    Should you want to use more than one of these joysticks/pads at once, you
>  can use db9_2 and db9_3 as additional command line parameters for two
>  more joysticks/pads.
> +
> +  'autofire' is bitmask of the controls for which you want to enable
> +autofire. This means when e.g. Button 1 is pressed and the autofire 
> +control bit is set (nonzero) for this button, then it will keep sending 
> +rapid button press and release events until the Button 1 is released.
> +
> +	Autofire | Control
> +	------------------
> +	   0     | None
> +	   1     | Up
> +	   2     | Down
> +	   4     | Left
> +	   8     | Right
> +	  16     | Button 1
> +	  32	 | Button 2
> +	  64	 | Button 3
> +	 128     | Button 4
> +
> +Caveat: autofire option takes effect only for multisystem joyticks 
> +of types 1 and 2. However it is easy to add support for other types 
> +as well. If you do own such joysticks, support it, test it and send 
> +the patch.
> +
> +Usage example:
> +
> +On parport0 (port=0) is connected Commodore 64 joystick 
> +(type=1, Multisystem 1-button joystick). 
> +Let autofire be enabled for the 'Button 1' (Control=16) 
> +and 'Up' (Control=1), (autofire = 16+1 = 17)
> +
> +modprobe db9 db9=0,1,17
>  
>  3.3 turbografx.c
>  ~~~~~~~~~~~~~~~~
> 
> 
> ----- End forwarded message -----
-- 
Juan Antonio Martinez <jantonio@dit.upm.es>
Dpto Ingenieria Telematica

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Joystick Autofire Support for linux

[Vojtech Pavlik]

On Wed, Jan 07, 2004 at 11:46:48AM +0100, Juan Antonio Martinez wrote:

> I'm not the mantainer of XMame since 1999; anyway, i'll try to forward
> your mail to xmame and l-kernel lists
> 
> A few comments on your code an a humble opinion. Note that I'm
> a kernel "observer" not "programmer" :-)
> 
> - Joystick api is too simple: just a syscall to read events and 
> an ioctl's to set device. It's responsability of the user to 
> make polling ant take note on extra features. There are lot of
> devices in Linux kernel that follow this approach

It used to be that way a couple years back. Now it supports
read/select/nonblocking read, etc. You can also query the joystick
configuration (number of buttons and of axes), etc. And if you want
more, you can use the input event interface, which is generic for all
devices in the system, and get even more information from that.

But there is one (and I think a good one) rule for the kernel input
drivers: Never mangle the input data from the device. Interpret it, put
it into a simple event format and pass it to applications. Don't
generate synthetic events, don't invent new buttons which don't exist on
the device, etc, etc. So far this rule helped to keep the drivers sane.

> - DB9 joysticks are really old. If you really want to implement
>  autofire, perhaps a better approach is to make it a generic
> option, not exclusive to db9 joysticks... A doubt: are there
> similar features in HID devices ? (keypads, mouse buttons...)

Autofire? I haven't seen it in any at least remotely recent device.

> - Some middleware already implements autofire features. No real
> need of doing it at kernel level. In fact, i remember some
> games that takes care on it... my very (very) early versions 
> on Xmame, for instance... :-)

That's what I (the person in charge of care for the joystick drivers in
the kernel), think too - the autofire feature is better in some lib
(thinking SDL and the likes), or in apps directly.

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR