Skip to contentSkip to conversation

sh: maple: add Maple controller as a joystick device

PATCH In review Adrian McMenamin 3 participants 3 messages Dec 20, 2008 – Dec 24, 2008

Subsystems (3)
HID SuperH Input drivers
lore
Share & tools
lore

Adrian McMenamin

author
#0 lore raw

PATCH 1/1 sh: maple: add Maple controller as a joystick device

Now with proper checks for NULL pointers.

sh: maple: add Maple controller as joystick device

Signed-off-by Adrian McMenamin <[email protected]>

---

drivers/input/joystick/Kconfig +13 -0
index be5c14a..ca7ce42 100644
@@ -294,4 +294,17 @@ config JOYSTICK_XPAD_LEDS
 	  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 as a joystick.
+
+	  Most Dreamcast users will say Y.
+
+	  To compile this as a module choose M here: the module will be called
+	  maplecontrol.
+
 endif
drivers/input/joystick/Makefile +1 -0
index fdbf8c4..bfd848d 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_JOYSTICK_AMIGA)		+= amijoy.o
 obj-$(CONFIG_JOYSTICK_ANALOG)		+= analog.o
 obj-$(CONFIG_JOYSTICK_COBRA)		+= cobra.o
 obj-$(CONFIG_JOYSTICK_DB9)		+= db9.o
+obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
 obj-$(CONFIG_JOYSTICK_GAMECON)		+= gamecon.o
 obj-$(CONFIG_JOYSTICK_GF2K)		+= gf2k.o
 obj-$(CONFIG_JOYSTICK_GRIP)		+= grip.o
drivers/input/joystick/maplecontrol.c new file +211 -0

211 lines not on this page · raw message

maintainer
#1 lore raw
Quoted 11 lines · On Saturday 20 December 2008 10:26:54 Adrian McMenamin wrote:

+       mdev = to_maple_dev(dev);
+       if (!mdev) {
+               error = EINVAL;
+               goto fail;
+       }
+
+       mdrv = to_maple_driver(dev->driver);
+       if (!mdrv) {
+               error = EINVAL;
+               goto fail;
+       }

This will never happen - to_maple_* are simple wrappers around container_of and so the result will never be NULL but some small offset from it if dev is NULL. I'm staying on the old version of the patch with small fixups.

Signature · 2 lines
Dmitry
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±şÈÿŠ{ayºʇڙë,j­¢f£¢·hš�ï�êÿ‘êçz_è®(­éš�Šİ¢j"�ú¶m§ÿÿ¾«şG«�éÿ¢¸?™¨è­Ú&£ø§~�á
#2 lore raw
In reply to Adrian McMenamin
Quoted 4 lines · On Saturday 20 December 2008 13:26:54 Adrian McMenamin wrote:
drivers/input/joystick/maplecontrol.c new file +3 -0
+static int probe_maple_controller(struct device *dev)
+{
...
+	mdev = to_maple_dev(dev);

drivers/input/joystick/maplecontrol.c line 113

Quoted 10 lines
drivers/input/joystick/maplecontrol.c new file +10 -0
+	if (!mdev) {
+		error = EINVAL;
+		goto fail;
+	}
+
+	mdrv = to_maple_driver(dev->driver);
+	if (!mdrv) {
+		error = EINVAL;
+		goto fail;
+	}

like Dmitry said, these NULL checks make no sense

Quoted 13 lines
drivers/input/joystick/maplecontrol.c new file +12 -0
+	pad = kzalloc(sizeof(struct dc_pad), GFP_KERNEL);
+	if (!pad) {
+		error = ENOMEM;
+		goto fail;
+	}
+	idev = input_allocate_device();
+	if (!idev){
+		error = ENOMEM;
+		goto fail_idev;
+	}
...
+	error = input_register_device(idev);
+	if (error)

drivers/input/joystick/maplecontrol.c line 177

Quoted 10 lines
drivers/input/joystick/maplecontrol.c new file +9 -0
+		goto fail_register;
...
+fail_register:
+	maple_set_drvdata(mdev, NULL);
+	input_free_device(pad->dev);
+fail_idev:
+	kfree(pad);
+fail:
+	return -error;
+}

this -error stuff is weird and i think wrong. for constants, you're adding overhead, and for the input_register_device(), i think you're negating an already negative value thus breaking it.
-mike