[PATCH] input: add support for SEGA Dreamcast controller as joystick

[PATCH] input: add support for SEGA Dreamcast controller as joystick

[Adrian McMenamin]

Use a Maple controller as a joystick device.

Signed-off-by: Adrian McMenamin <adrian@mcmen.demon.co.uk>
---
diff -ruN a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
--- a/drivers/input/joystick/Kconfig	2008-05-18 20:04:42.000000000 +0100
+++ b/drivers/input/joystick/Kconfig	2008-05-18 19:53:42.000000000 +0100
@@ -294,4 +294,17 @@
 	  This option enables support for the LED which surrounds the Big X on
 	  XBox 360 controller.
 
+config JOYSTICK_MAPLE
+	tristate "Dreamcast control pad"
+	depends on SH_DREAMCAST
+	select MAPLE
+	help
+	  Say Y here if you have a SEGA Dreamcast and want to use your
+	  controller.
+
+	  Most Dreamcast users will say Y.
+
+	  To compile this as a module choose M here: the
+	  module will be called maplecontrol.
+
 endif
diff -ruN a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
--- a/drivers/input/joystick/Makefile	2008-05-18 20:04:42.000000000 +0100
+++ b/drivers/input/joystick/Makefile	2008-05-18 19:53:42.000000000 +0100
@@ -19,6 +19,7 @@
 obj-$(CONFIG_JOYSTICK_INTERACT)		+= interact.o
 obj-$(CONFIG_JOYSTICK_JOYDUMP)		+= joydump.o
 obj-$(CONFIG_JOYSTICK_MAGELLAN)		+= magellan.o
+obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
 obj-$(CONFIG_JOYSTICK_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
 obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
diff -ruN a/drivers/input/joystick/maplecontrol.c b/drivers/input/joystick/maplecontrol.c
--- a/drivers/input/joystick/maplecontrol.c	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/input/joystick/maplecontrol.c	2008-05-18 19:53:42.000000000 +0100
@@ -0,0 +1,217 @@
+/*
+ * 	SEGA Dreamcast controller driver
+ *	Based on drivers/usb/iforce.c
+ *
+ * 	Copyright Yaegashi Takeshi, 2001
+ *	Porting to 2.6 by Adrian McMenamin, copyright 2008
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/timer.h>
+#include <linux/maple.h>
+#include <asm/mach/maple.h>
+
+MODULE_AUTHOR("YAEGASHI Takeshi <t@keshi.org>");
+MODULE_DESCRIPTION("SEGA Dreamcast controller driver");
+MODULE_LICENSE("GPL");
+
+struct dc_pad {
+	struct input_dev *dev;
+	struct maple_device *mdev;
+};
+
+static void dc_pad_callback(struct mapleq *mq)
+{
+	unsigned short buttons;
+	struct maple_device *mapledev = mq->dev;
+	struct dc_pad *pad = mapledev->private_data;
+	struct input_dev *dev = pad->dev;
+	unsigned char *res = mq->recvbuf;
+
+	buttons = ~cpu_to_le16(*(unsigned short *)(res + 8));
+
+	input_report_abs(dev, ABS_HAT0Y,
+			 (buttons & 0x0010 ? -1:0) + (buttons & 0x0020 ? 1:0));
+	input_report_abs(dev, ABS_HAT0X,
+			 (buttons & 0x0040 ? -1:0) + (buttons & 0x0080 ? 1:0));
+	input_report_abs(dev, ABS_HAT1Y,
+			 (buttons & 0x1000 ? -1:0) + (buttons & 0x2000 ? 1:0));
+	input_report_abs(dev, ABS_HAT1X,
+			 (buttons & 0x4000 ? -1:0) + (buttons & 0x8000 ? 1:0));
+
+	input_report_key(dev, BTN_C,      buttons & 0x0001);
+	input_report_key(dev, BTN_B,      buttons & 0x0002);
+	input_report_key(dev, BTN_A,      buttons & 0x0004);
+	input_report_key(dev, BTN_START,  buttons & 0x0008);
+	input_report_key(dev, BTN_Z,      buttons & 0x0100);
+	input_report_key(dev, BTN_Y,      buttons & 0x0200);
+	input_report_key(dev, BTN_X,      buttons & 0x0400);
+	input_report_key(dev, BTN_SELECT, buttons & 0x0800);
+
+	input_report_abs(dev, ABS_GAS,   res[10]);
+	input_report_abs(dev, ABS_BRAKE, res[11]);
+	input_report_abs(dev, ABS_X,     res[12]);
+	input_report_abs(dev, ABS_Y,     res[13]);
+	input_report_abs(dev, ABS_RX,    res[14]);
+	input_report_abs(dev, ABS_RY,    res[15]);
+}
+
+static void dc_pad_disconnect(struct maple_device *mdev)
+{
+	struct dc_pad *pad = mdev->private_data;
+
+	mdev->callback = NULL;
+	input_unregister_device(pad->dev);
+	kfree(pad);
+}
+
+static int dc_pad_open(struct input_dev *dev)
+{
+	struct dc_pad *pad;
+
+	pad = dev->dev.platform_data;
+	maple_getcond_callback(pad->mdev, dc_pad_callback, HZ/20,
+		MAPLE_FUNC_CONTROLLER);
+	return 0;
+}
+
+static void dc_pad_close(struct input_dev *dev)
+{
+	struct dc_pad *pad;
+	
+	pad = dev->dev.platform_data;
+	maple_getcond_callback(pad->mdev, dc_pad_callback, 0,
+		MAPLE_FUNC_CONTROLLER);
+}
+
+static int dc_pad_connect(struct maple_device *mdev)
+{
+	int i, error;
+	unsigned long data = be32_to_cpu(mdev->devinfo.function_data[0]);
+	struct dc_pad *pad;
+	struct input_dev *dev;
+
+	const short btn_bit[32] = {
+		BTN_C, BTN_B, BTN_A, BTN_START, -1, -1, -1, -1,
+		BTN_Z, BTN_Y, BTN_X, BTN_SELECT, -1, -1, -1, -1,
+		-1, -1, -1, -1, -1, -1, -1, -1,
+		-1, -1, -1, -1, -1, -1, -1, -1,
+	};
+
+	const short abs_bit[32] = {
+		-1, -1, -1, -1, ABS_HAT0Y, ABS_HAT0Y, ABS_HAT0X, ABS_HAT0X,
+		-1, -1, -1, -1, ABS_HAT1Y, ABS_HAT1Y, ABS_HAT1X, ABS_HAT1X,
+		ABS_GAS, ABS_BRAKE, ABS_X, ABS_Y, ABS_RX, ABS_RY, -1, -1,
+		-1, -1, -1, -1, -1, -1, -1, -1,
+	};
+
+	pad = kzalloc(sizeof(struct dc_pad), GFP_KERNEL);
+	if (!pad){
+		error = ENOMEM;
+		goto fail_nomem_1;
+	}
+
+	dev = input_allocate_device();
+	if (!dev) {
+		error = ENOMEM;
+		goto fail_nomem_2;
+	}
+
+	pad->dev = dev;
+	pad->mdev = mdev;
+	mdev->private_data = pad;
+	dev->open = dc_pad_open;
+	dev->close = dc_pad_close;
+
+	for (i = 0; i < 32; i++)
+		if (data & (1<<i) && btn_bit[i] >= 0)
+			dev->keybit[BIT_WORD(BTN_JOYSTICK)] |= BIT_MASK(btn_bit[i]);
+
+	if (dev->keybit[BTN_JOYSTICK/32])
+		dev->evbit[0] |= BIT_MASK(EV_KEY);
+
+	for (i = 0; i < 32; i++)
+		if (data & (1<<i) && abs_bit[i] >= 0)
+			dev->absbit[0] |= BIT_MASK(abs_bit[i]);
+
+	if (dev->absbit[0])
+		dev->evbit[0] |= BIT_MASK(EV_ABS);
+
+	for (i = ABS_X; i <= ABS_BRAKE; i++)
+		input_set_abs_params(dev, i, 0, 255, 0, 0);
+
+	for (i = ABS_HAT0X; i <= ABS_HAT3Y; i++)
+		input_set_abs_params(dev, i, 1, -1, 0, 0);
+
+	dev->dev.platform_data = pad;
+	dev->event = NULL;
+	dev->dev.parent = &mdev->dev;
+	dev->name = mdev->product_name;
+	dev->id.bustype = BUS_HOST;
+	input_set_drvdata(dev, pad);
+
+	error = input_register_device(dev);
+	if (error)
+		goto fail_register;
+
+	return error;
+
+fail_register:
+	input_free_device(pad->dev);
+fail_nomem_2:
+	kfree(pad);
+fail_nomem_1:
+	return -error;
+}
+
+/* allow the controller to be used */
+static int probe_maple_controller(struct device *dev)
+{
+	struct maple_device *mdev = to_maple_dev(dev);
+	struct maple_driver *mdrv = to_maple_driver(dev->driver);
+	int error;
+
+	error = dc_pad_connect(mdev);
+	if (error)
+		return error;
+
+	mdev->driver = mdrv;
+
+	return 0;
+}
+
+static int remove_maple_controller(struct device *dev)
+{
+	struct maple_device *mdev = to_maple_dev(dev);
+
+	dc_pad_disconnect(mdev);
+	return 0;
+}
+
+static struct maple_driver dc_pad_driver = {
+	.function =	MAPLE_FUNC_CONTROLLER,
+	.connect =	dc_pad_connect,
+	.disconnect =	dc_pad_disconnect,
+	.drv = {
+		.name = "Dreamcast_controller",
+		.probe = probe_maple_controller,
+		.remove = remove_maple_controller,
+	},
+};
+
+static int __init dc_pad_init(void)
+{
+	return 	maple_driver_register(&dc_pad_driver.drv);
+}
+
+static void __exit dc_pad_exit(void)
+{
+	driver_unregister(&dc_pad_driver.drv);
+}
+
+module_init(dc_pad_init);
+module_exit(dc_pad_exit);

[PATCH] input: add support for SEGA Dreamcast controller as

[Adrian McMenamin]

Use a Maple controller as a joystick device.

Signed-off-by: Adrian McMenamin <adrian@mcmen.demon.co.uk>
---
diff -ruN a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
--- a/drivers/input/joystick/Kconfig	2008-05-18 20:04:42.000000000 +0100
+++ b/drivers/input/joystick/Kconfig	2008-05-18 19:53:42.000000000 +0100
@@ -294,4 +294,17 @@
 	  This option enables support for the LED which surrounds the Big X on
 	  XBox 360 controller.
 
+config JOYSTICK_MAPLE
+	tristate "Dreamcast control pad"
+	depends on SH_DREAMCAST
+	select MAPLE
+	help
+	  Say Y here if you have a SEGA Dreamcast and want to use your
+	  controller.
+
+	  Most Dreamcast users will say Y.
+
+	  To compile this as a module choose M here: the
+	  module will be called maplecontrol.
+
 endif
diff -ruN a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
--- a/drivers/input/joystick/Makefile	2008-05-18 20:04:42.000000000 +0100
+++ b/drivers/input/joystick/Makefile	2008-05-18 19:53:42.000000000 +0100
@@ -19,6 +19,7 @@
 obj-$(CONFIG_JOYSTICK_INTERACT)		+= interact.o
 obj-$(CONFIG_JOYSTICK_JOYDUMP)		+= joydump.o
 obj-$(CONFIG_JOYSTICK_MAGELLAN)		+= magellan.o
+obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
 obj-$(CONFIG_JOYSTICK_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
 obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
diff -ruN a/drivers/input/joystick/maplecontrol.c b/drivers/input/joystick/maplecontrol.c
--- a/drivers/input/joystick/maplecontrol.c	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/input/joystick/maplecontrol.c	2008-05-18 19:53:42.000000000 +0100
@@ -0,0 +1,217 @@
+/*
+ * 	SEGA Dreamcast controller driver
+ *	Based on drivers/usb/iforce.c
+ *
+ * 	Copyright Yaegashi Takeshi, 2001
+ *	Porting to 2.6 by Adrian McMenamin, copyright 2008
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/timer.h>
+#include <linux/maple.h>
+#include <asm/mach/maple.h>
+
+MODULE_AUTHOR("YAEGASHI Takeshi <t@keshi.org>");
+MODULE_DESCRIPTION("SEGA Dreamcast controller driver");
+MODULE_LICENSE("GPL");
+
+struct dc_pad {
+	struct input_dev *dev;
+	struct maple_device *mdev;
+};
+
+static void dc_pad_callback(struct mapleq *mq)
+{
+	unsigned short buttons;
+	struct maple_device *mapledev = mq->dev;
+	struct dc_pad *pad = mapledev->private_data;
+	struct input_dev *dev = pad->dev;
+	unsigned char *res = mq->recvbuf;
+
+	buttons = ~cpu_to_le16(*(unsigned short *)(res + 8));
+
+	input_report_abs(dev, ABS_HAT0Y,
+			 (buttons & 0x0010 ? -1:0) + (buttons & 0x0020 ? 1:0));
+	input_report_abs(dev, ABS_HAT0X,
+			 (buttons & 0x0040 ? -1:0) + (buttons & 0x0080 ? 1:0));
+	input_report_abs(dev, ABS_HAT1Y,
+			 (buttons & 0x1000 ? -1:0) + (buttons & 0x2000 ? 1:0));
+	input_report_abs(dev, ABS_HAT1X,
+			 (buttons & 0x4000 ? -1:0) + (buttons & 0x8000 ? 1:0));
+
+	input_report_key(dev, BTN_C,      buttons & 0x0001);
+	input_report_key(dev, BTN_B,      buttons & 0x0002);
+	input_report_key(dev, BTN_A,      buttons & 0x0004);
+	input_report_key(dev, BTN_START,  buttons & 0x0008);
+	input_report_key(dev, BTN_Z,      buttons & 0x0100);
+	input_report_key(dev, BTN_Y,      buttons & 0x0200);
+	input_report_key(dev, BTN_X,      buttons & 0x0400);
+	input_report_key(dev, BTN_SELECT, buttons & 0x0800);
+
+	input_report_abs(dev, ABS_GAS,   res[10]);
+	input_report_abs(dev, ABS_BRAKE, res[11]);
+	input_report_abs(dev, ABS_X,     res[12]);
+	input_report_abs(dev, ABS_Y,     res[13]);
+	input_report_abs(dev, ABS_RX,    res[14]);
+	input_report_abs(dev, ABS_RY,    res[15]);
+}
+
+static void dc_pad_disconnect(struct maple_device *mdev)
+{
+	struct dc_pad *pad = mdev->private_data;
+
+	mdev->callback = NULL;
+	input_unregister_device(pad->dev);
+	kfree(pad);
+}
+
+static int dc_pad_open(struct input_dev *dev)
+{
+	struct dc_pad *pad;
+
+	pad = dev->dev.platform_data;
+	maple_getcond_callback(pad->mdev, dc_pad_callback, HZ/20,
+		MAPLE_FUNC_CONTROLLER);
+	return 0;
+}
+
+static void dc_pad_close(struct input_dev *dev)
+{
+	struct dc_pad *pad;
+	
+	pad = dev->dev.platform_data;
+	maple_getcond_callback(pad->mdev, dc_pad_callback, 0,
+		MAPLE_FUNC_CONTROLLER);
+}
+
+static int dc_pad_connect(struct maple_device *mdev)
+{
+	int i, error;
+	unsigned long data = be32_to_cpu(mdev->devinfo.function_data[0]);
+	struct dc_pad *pad;
+	struct input_dev *dev;
+
+	const short btn_bit[32] = {
+		BTN_C, BTN_B, BTN_A, BTN_START, -1, -1, -1, -1,
+		BTN_Z, BTN_Y, BTN_X, BTN_SELECT, -1, -1, -1, -1,
+		-1, -1, -1, -1, -1, -1, -1, -1,
+		-1, -1, -1, -1, -1, -1, -1, -1,
+	};
+
+	const short abs_bit[32] = {
+		-1, -1, -1, -1, ABS_HAT0Y, ABS_HAT0Y, ABS_HAT0X, ABS_HAT0X,
+		-1, -1, -1, -1, ABS_HAT1Y, ABS_HAT1Y, ABS_HAT1X, ABS_HAT1X,
+		ABS_GAS, ABS_BRAKE, ABS_X, ABS_Y, ABS_RX, ABS_RY, -1, -1,
+		-1, -1, -1, -1, -1, -1, -1, -1,
+	};
+
+	pad = kzalloc(sizeof(struct dc_pad), GFP_KERNEL);
+	if (!pad){
+		error = ENOMEM;
+		goto fail_nomem_1;
+	}
+
+	dev = input_allocate_device();
+	if (!dev) {
+		error = ENOMEM;
+		goto fail_nomem_2;
+	}
+
+	pad->dev = dev;
+	pad->mdev = mdev;
+	mdev->private_data = pad;
+	dev->open = dc_pad_open;
+	dev->close = dc_pad_close;
+
+	for (i = 0; i < 32; i++)
+		if (data & (1<<i) && btn_bit[i] >= 0)
+			dev->keybit[BIT_WORD(BTN_JOYSTICK)] |= BIT_MASK(btn_bit[i]);
+
+	if (dev->keybit[BTN_JOYSTICK/32])
+		dev->evbit[0] |= BIT_MASK(EV_KEY);
+
+	for (i = 0; i < 32; i++)
+		if (data & (1<<i) && abs_bit[i] >= 0)
+			dev->absbit[0] |= BIT_MASK(abs_bit[i]);
+
+	if (dev->absbit[0])
+		dev->evbit[0] |= BIT_MASK(EV_ABS);
+
+	for (i = ABS_X; i <= ABS_BRAKE; i++)
+		input_set_abs_params(dev, i, 0, 255, 0, 0);
+
+	for (i = ABS_HAT0X; i <= ABS_HAT3Y; i++)
+		input_set_abs_params(dev, i, 1, -1, 0, 0);
+
+	dev->dev.platform_data = pad;
+	dev->event = NULL;
+	dev->dev.parent = &mdev->dev;
+	dev->name = mdev->product_name;
+	dev->id.bustype = BUS_HOST;
+	input_set_drvdata(dev, pad);
+
+	error = input_register_device(dev);
+	if (error)
+		goto fail_register;
+
+	return error;
+
+fail_register:
+	input_free_device(pad->dev);
+fail_nomem_2:
+	kfree(pad);
+fail_nomem_1:
+	return -error;
+}
+
+/* allow the controller to be used */
+static int probe_maple_controller(struct device *dev)
+{
+	struct maple_device *mdev = to_maple_dev(dev);
+	struct maple_driver *mdrv = to_maple_driver(dev->driver);
+	int error;
+
+	error = dc_pad_connect(mdev);
+	if (error)
+		return error;
+
+	mdev->driver = mdrv;
+
+	return 0;
+}
+
+static int remove_maple_controller(struct device *dev)
+{
+	struct maple_device *mdev = to_maple_dev(dev);
+
+	dc_pad_disconnect(mdev);
+	return 0;
+}
+
+static struct maple_driver dc_pad_driver = {
+	.function =	MAPLE_FUNC_CONTROLLER,
+	.connect =	dc_pad_connect,
+	.disconnect =	dc_pad_disconnect,
+	.drv = {
+		.name = "Dreamcast_controller",
+		.probe = probe_maple_controller,
+		.remove = remove_maple_controller,
+	},
+};
+
+static int __init dc_pad_init(void)
+{
+	return 	maple_driver_register(&dc_pad_driver.drv);
+}
+
+static void __exit dc_pad_exit(void)
+{
+	driver_unregister(&dc_pad_driver.drv);
+}
+
+module_init(dc_pad_init);
+module_exit(dc_pad_exit);

Re: [PATCH] input: add support for SEGA Dreamcast controller as joystick

[Paul Mundt]

On Sun, May 18, 2008 at 08:18:42PM +0100, Adrian McMenamin wrote:
> +MODULE_AUTHOR("YAEGASHI Takeshi <t@keshi.org>");
> +MODULE_DESCRIPTION("SEGA Dreamcast controller driver");
> +MODULE_LICENSE("GPL");
> +
MODULE_AUTHOR() is a bit ambiguous here. As this is user-visible by way
of modinfo, it really doesn't make any sense to list someone who has
nothing to do with the driver in its present form here. Especially since
it's been 7 years since their last contribution or even basic list
activity, I think it's safe to say that you are going to be the only one
that users wish to contact about this.

In this case, the copyright should suffice. You may wish to add a bit of
a note there indicating where the driver came from, but there's not much
point in going beyond that.

Note that if you aren't comfortable with being in MODULE_AUTHOR(), you
can also make this more generic and attribute it to the LinuxDC or
Linux/SH teams respectively, as that's a bit more true to form as far as
all copyright holders are concerned, and gives people a pointer to a list
to go to with problems.

It's probably more reasonable to treat MODULE_AUTHOR() as
MODULE_MAINTAINER() rather than the literal original author, as the
latter is of zero interest or relevance to users that are looking at
this information on the other side of the fence.

> +struct dc_pad {
> +	struct input_dev *dev;
> +	struct maple_device *mdev;
> +};
> +
This is pretty ugly, you should really fix up your private device
pointer layering to get around having to use this structure at all.

> +static struct maple_driver dc_pad_driver = {
> +	.function =	MAPLE_FUNC_CONTROLLER,
> +	.connect =	dc_pad_connect,
> +	.disconnect =	dc_pad_disconnect,
> +	.drv = {
> +		.name = "Dreamcast_controller",

Is there some particular reason why the driver name can't reflect the
module name? There's no reason to have such sickeningly verbose driver
names.

> +		.probe = probe_maple_controller,
> +		.remove = remove_maple_controller,
> +	},
> +};
> +
These should be __devinit/__devexit at least, with the latter being
__devexit_p() wrapped..

> +static int __init dc_pad_init(void)
> +{
> +	return 	maple_driver_register(&dc_pad_driver.drv);
> +}
> +
> +static void __exit dc_pad_exit(void)
> +{
> +	driver_unregister(&dc_pad_driver.drv);
> +}
> +
If you don't have a maple_driver_unregister(), stub one in, even if it's
just a wrapper.

Re: [PATCH] input: add support for SEGA Dreamcast controller as joystick

[Paul Mundt]

On Sun, May 18, 2008 at 08:18:42PM +0100, Adrian McMenamin wrote:
> +MODULE_AUTHOR("YAEGASHI Takeshi <t@keshi.org>");
> +MODULE_DESCRIPTION("SEGA Dreamcast controller driver");
> +MODULE_LICENSE("GPL");
> +
MODULE_AUTHOR() is a bit ambiguous here. As this is user-visible by way
of modinfo, it really doesn't make any sense to list someone who has
nothing to do with the driver in its present form here. Especially since
it's been 7 years since their last contribution or even basic list
activity, I think it's safe to say that you are going to be the only one
that users wish to contact about this.

In this case, the copyright should suffice. You may wish to add a bit of
a note there indicating where the driver came from, but there's not much
point in going beyond that.

Note that if you aren't comfortable with being in MODULE_AUTHOR(), you
can also make this more generic and attribute it to the LinuxDC or
Linux/SH teams respectively, as that's a bit more true to form as far as
all copyright holders are concerned, and gives people a pointer to a list
to go to with problems.

It's probably more reasonable to treat MODULE_AUTHOR() as
MODULE_MAINTAINER() rather than the literal original author, as the
latter is of zero interest or relevance to users that are looking at
this information on the other side of the fence.

> +struct dc_pad {
> +	struct input_dev *dev;
> +	struct maple_device *mdev;
> +};
> +
This is pretty ugly, you should really fix up your private device
pointer layering to get around having to use this structure at all.

> +static struct maple_driver dc_pad_driver = {
> +	.function =	MAPLE_FUNC_CONTROLLER,
> +	.connect =	dc_pad_connect,
> +	.disconnect =	dc_pad_disconnect,
> +	.drv = {
> +		.name = "Dreamcast_controller",

Is there some particular reason why the driver name can't reflect the
module name? There's no reason to have such sickeningly verbose driver
names.

> +		.probe = probe_maple_controller,
> +		.remove = remove_maple_controller,
> +	},
> +};
> +
These should be __devinit/__devexit at least, with the latter being
__devexit_p() wrapped..

> +static int __init dc_pad_init(void)
> +{
> +	return 	maple_driver_register(&dc_pad_driver.drv);
> +}
> +
> +static void __exit dc_pad_exit(void)
> +{
> +	driver_unregister(&dc_pad_driver.drv);
> +}
> +
If you don't have a maple_driver_unregister(), stub one in, even if it's
just a wrapper.

Re: [PATCH] input: add support for SEGA Dreamcast controller as joystick

[Dmitry Torokhov]

On Mon, May 19, 2008 at 10:09:28AM +0900, Paul Mundt wrote:
> On Sun, May 18, 2008 at 08:18:42PM +0100, Adrian McMenamin wrote:
> > +MODULE_AUTHOR("YAEGASHI Takeshi <t@keshi.org>");
> > +MODULE_DESCRIPTION("SEGA Dreamcast controller driver");
> > +MODULE_LICENSE("GPL");
> > +
> MODULE_AUTHOR() is a bit ambiguous here. As this is user-visible by way
> of modinfo, it really doesn't make any sense to list someone who has
> nothing to do with the driver in its present form here. Especially since
> it's been 7 years since their last contribution or even basic list
> activity, I think it's safe to say that you are going to be the only one
> that users wish to contact about this.
> 
> In this case, the copyright should suffice. You may wish to add a bit of
> a note there indicating where the driver came from, but there's not much
> point in going beyond that.
> 
> Note that if you aren't comfortable with being in MODULE_AUTHOR(), you
> can also make this more generic and attribute it to the LinuxDC or
> Linux/SH teams respectively, as that's a bit more true to form as far as
> all copyright holders are concerned, and gives people a pointer to a list
> to go to with problems.
> 
> It's probably more reasonable to treat MODULE_AUTHOR() as
> MODULE_MAINTAINER() rather than the literal original author, as the
> latter is of zero interest or relevance to users that are looking at
> this information on the other side of the fence.
> 
> > +struct dc_pad {
> > +	struct input_dev *dev;
> > +	struct maple_device *mdev;
> > +};
> > +
> This is pretty ugly, you should really fix up your private device
> pointer layering to get around having to use this structure at all.
> 
> > +static struct maple_driver dc_pad_driver = {
> > +	.function =	MAPLE_FUNC_CONTROLLER,
> > +	.connect =	dc_pad_connect,
> > +	.disconnect =	dc_pad_disconnect,
> > +	.drv = {
> > +		.name = "Dreamcast_controller",
> 
> Is there some particular reason why the driver name can't reflect the
> module name? There's no reason to have such sickeningly verbose driver
> names.
> 
> > +		.probe = probe_maple_controller,
> > +		.remove = remove_maple_controller,
> > +	},
> > +};
> > +
> These should be __devinit/__devexit at least, with the latter being
> __devexit_p() wrapped..
> 

In addition to Pauls comments - is there a reson why maple drivers
need to brovide both connect/disconnect and probe/remove functions?
Normally the layer has generic probe/remove defined on bus level and
individual drivers provide connect/disconnect (or their equivalent)
which is called from the bus's probe/remove.

-- 
Dmitry

Re: [PATCH] input: add support for SEGA Dreamcast controller as joystick

[Dmitry Torokhov]

On Mon, May 19, 2008 at 10:09:28AM +0900, Paul Mundt wrote:
> On Sun, May 18, 2008 at 08:18:42PM +0100, Adrian McMenamin wrote:
> > +MODULE_AUTHOR("YAEGASHI Takeshi <t@keshi.org>");
> > +MODULE_DESCRIPTION("SEGA Dreamcast controller driver");
> > +MODULE_LICENSE("GPL");
> > +
> MODULE_AUTHOR() is a bit ambiguous here. As this is user-visible by way
> of modinfo, it really doesn't make any sense to list someone who has
> nothing to do with the driver in its present form here. Especially since
> it's been 7 years since their last contribution or even basic list
> activity, I think it's safe to say that you are going to be the only one
> that users wish to contact about this.
> 
> In this case, the copyright should suffice. You may wish to add a bit of
> a note there indicating where the driver came from, but there's not much
> point in going beyond that.
> 
> Note that if you aren't comfortable with being in MODULE_AUTHOR(), you
> can also make this more generic and attribute it to the LinuxDC or
> Linux/SH teams respectively, as that's a bit more true to form as far as
> all copyright holders are concerned, and gives people a pointer to a list
> to go to with problems.
> 
> It's probably more reasonable to treat MODULE_AUTHOR() as
> MODULE_MAINTAINER() rather than the literal original author, as the
> latter is of zero interest or relevance to users that are looking at
> this information on the other side of the fence.
> 
> > +struct dc_pad {
> > +	struct input_dev *dev;
> > +	struct maple_device *mdev;
> > +};
> > +
> This is pretty ugly, you should really fix up your private device
> pointer layering to get around having to use this structure at all.
> 
> > +static struct maple_driver dc_pad_driver = {
> > +	.function =	MAPLE_FUNC_CONTROLLER,
> > +	.connect =	dc_pad_connect,
> > +	.disconnect =	dc_pad_disconnect,
> > +	.drv = {
> > +		.name = "Dreamcast_controller",
> 
> Is there some particular reason why the driver name can't reflect the
> module name? There's no reason to have such sickeningly verbose driver
> names.
> 
> > +		.probe = probe_maple_controller,
> > +		.remove = remove_maple_controller,
> > +	},
> > +};
> > +
> These should be __devinit/__devexit at least, with the latter being
> __devexit_p() wrapped..
> 

In addition to Pauls comments - is there a reson why maple drivers
need to brovide both connect/disconnect and probe/remove functions?
Normally the layer has generic probe/remove defined on bus level and
individual drivers provide connect/disconnect (or their equivalent)
which is called from the bus's probe/remove.

-- 
Dmitry

Re: [PATCH] input: add support for SEGA Dreamcast controller as

[Dmitry Torokhov]

On Mon, May 19, 2008 at 10:09:28AM +0900, Paul Mundt wrote:
> On Sun, May 18, 2008 at 08:18:42PM +0100, Adrian McMenamin wrote:
> > +MODULE_AUTHOR("YAEGASHI Takeshi <t@keshi.org>");
> > +MODULE_DESCRIPTION("SEGA Dreamcast controller driver");
> > +MODULE_LICENSE("GPL");
> > +
> MODULE_AUTHOR() is a bit ambiguous here. As this is user-visible by way
> of modinfo, it really doesn't make any sense to list someone who has
> nothing to do with the driver in its present form here. Especially since
> it's been 7 years since their last contribution or even basic list
> activity, I think it's safe to say that you are going to be the only one
> that users wish to contact about this.
> 
> In this case, the copyright should suffice. You may wish to add a bit of
> a note there indicating where the driver came from, but there's not much
> point in going beyond that.
> 
> Note that if you aren't comfortable with being in MODULE_AUTHOR(), you
> can also make this more generic and attribute it to the LinuxDC or
> Linux/SH teams respectively, as that's a bit more true to form as far as
> all copyright holders are concerned, and gives people a pointer to a list
> to go to with problems.
> 
> It's probably more reasonable to treat MODULE_AUTHOR() as
> MODULE_MAINTAINER() rather than the literal original author, as the
> latter is of zero interest or relevance to users that are looking at
> this information on the other side of the fence.
> 
> > +struct dc_pad {
> > +	struct input_dev *dev;
> > +	struct maple_device *mdev;
> > +};
> > +
> This is pretty ugly, you should really fix up your private device
> pointer layering to get around having to use this structure at all.
> 
> > +static struct maple_driver dc_pad_driver = {
> > +	.function =	MAPLE_FUNC_CONTROLLER,
> > +	.connect =	dc_pad_connect,
> > +	.disconnect =	dc_pad_disconnect,
> > +	.drv = {
> > +		.name = "Dreamcast_controller",
> 
> Is there some particular reason why the driver name can't reflect the
> module name? There's no reason to have such sickeningly verbose driver
> names.
> 
> > +		.probe = probe_maple_controller,
> > +		.remove = remove_maple_controller,
> > +	},
> > +};
> > +
> These should be __devinit/__devexit at least, with the latter being
> __devexit_p() wrapped..
> 

In addition to Pauls comments - is there a reson why maple drivers
need to brovide both connect/disconnect and probe/remove functions?
Normally the layer has generic probe/remove defined on bus level and
individual drivers provide connect/disconnect (or their equivalent)
which is called from the bus's probe/remove.

-- 
Dmitry

Re: [PATCH] input: add support for SEGA Dreamcast controller as joystick

[Adrian McMenamin]

On Mon, 2008-05-19 at 11:47 -0400, Dmitry Torokhov wrote:

> 
> In addition to Pauls comments - is there a reson why maple drivers
> need to brovide both connect/disconnect and probe/remove functions?
> Normally the layer has generic probe/remove defined on bus level and
> individual drivers provide connect/disconnect (or their equivalent)
> which is called from the bus's probe/remove.
> 

Cruft from another era. I will try to clean all that up.

Re: [PATCH] input: add support for SEGA Dreamcast controller as

[Adrian McMenamin]

On Mon, 2008-05-19 at 11:47 -0400, Dmitry Torokhov wrote:

> 
> In addition to Pauls comments - is there a reson why maple drivers
> need to brovide both connect/disconnect and probe/remove functions?
> Normally the layer has generic probe/remove defined on bus level and
> individual drivers provide connect/disconnect (or their equivalent)
> which is called from the bus's probe/remove.
> 

Cruft from another era. I will try to clean all that up.