Skip to contentSkip to conversation

Input: walkera0701 - set up input device's parent

PATCH New Dmitry Torokhov 1 participant 4 messages Jan 6, 2013

Subsystems (2)
lore
Share & tools
lore

Dmitry Torokhov

author maintainer
#0 lore raw

PATCH 1/4 Input: walkera0701 - set up input device's parent

This will place the joystick's input device into propoer place in sysfs hierarchy as long as th port has device assigned to it (i.e. it is not legacy port).

---
 drivers/input/joystick/walkera0701.c |    1 +
 1 file changed, 1 insertion(+)
drivers/input/joystick/walkera0701.c +1 -0
index f8f892b..38386ce 100644
@@ -241,6 +241,7 @@ static int walkera0701_connect(struct walkera_dev *w, int parport)
 	w->input_dev->id.vendor = 0x0001;
 	w->input_dev->id.product = 0x0001;
 	w->input_dev->id.version = 0x0100;
+	w->input_dev->dev.parent = w->parport->dev;
 	w->input_dev->open = walkera0701_open;
 	w->input_dev->close = walkera0701_close;
 
author maintainer
#1 lore raw

PATCH 2/4 Input: walkera0701 - switch to using pr_xxx() for messages

---
 drivers/input/joystick/walkera0701.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)
drivers/input/joystick/walkera0701.c +10 -13
index 38386ce..da657e6 100644
@@ -12,7 +12,7 @@
  * the Free Software Foundation.
 */
 
-/* #define WK0701_DEBUG */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #define RESERVE 20000
 #define SYNC_PULSE 1306000
@@ -67,6 +67,7 @@ static inline void walkera0701_parse_frame(struct walkera_dev *w)
 {
 	int i;
 	int val1, val2, val3, val4, val5, val6, val7, val8;
+	int magic, magic_bit;
 	int crc1, crc2;
 
 	for (crc1 = crc2 = i = 0; i < 10; i++) {
@@ -102,17 +103,12 @@ static inline void walkera0701_parse_frame(struct walkera_dev *w)
 	val8 = (w->buf[18] & 1) << 8 | (w->buf[19] << 4) | w->buf[20];
 	val8 *= (w->buf[18] & 2) - 1;	/*sign */
 
-#ifdef WK0701_DEBUG
-	{
-		int magic, magic_bit;
-		magic = (w->buf[21] << 4) | w->buf[22];
-		magic_bit = (w->buf[24] & 8) >> 3;
-		printk(KERN_DEBUG
-		       "walkera0701: %4d %4d %4d %4d  %4d %4d %4d %4d (magic %2x %d)\n",
-		       val1, val2, val3, val4, val5, val6, val7, val8, magic,
-		       magic_bit);
-	}
-#endif
+	magic = (w->buf[21] << 4) | w->buf[22];
+	magic_bit = (w->buf[24] & 8) >> 3;
+	pr_debug("%4d %4d %4d %4d  %4d %4d %4d %4d (magic %2x %d)\n",
+		 val1, val2, val3, val4, val5, val6, val7, val8,
+		 magic, magic_bit);
+
 	input_report_abs(w->input_dev, ABS_X, val2);
 	input_report_abs(w->input_dev, ABS_Y, val1);
 	input_report_abs(w->input_dev, ABS_Z, val6);
@@ -208,7 +204,8 @@ static int walkera0701_connect(struct walkera_dev *w, int parport)
 		return -ENODEV;
 
 	if (w->parport->irq == -1) {
-		printk(KERN_ERR "walkera0701: parport without interrupt\n");
+		pr_err("parport %d does not have interrupt assigned\n",
+			parport);
 		goto init_err;
 	}
 
author maintainer
#2 lore raw

PATCH 3/4 Input: walkera0701 - use proper error codes

In reply to Dmitry Torokhov

We have been using -EBUSY where we should have used -EIO or -ENOMEM, so let's fix that and also add some diagnostic messages.

---
 drivers/input/joystick/walkera0701.c |   56 ++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 20 deletions(-)
drivers/input/joystick/walkera0701.c +36 -20
index da657e6..b86134c 100644
@@ -197,37 +197,51 @@ static void walkera0701_close(struct input_dev *dev)
 
 static int walkera0701_connect(struct walkera_dev *w, int parport)
 {
-	int err = -ENODEV;
+	int error;
 
 	w->parport = parport_find_number(parport);
-	if (w->parport == NULL)
+	if (!w->parport) {
+		pr_err("parport %d does not exist\n", parport);
 		return -ENODEV;
+	}
 
 	if (w->parport->irq == -1) {
 		pr_err("parport %d does not have interrupt assigned\n",
 			parport);
-		goto init_err;
+		error = -EINVAL;
+		goto err_put_parport;
 	}
 
-	err = -EBUSY;
 	w->pardevice = parport_register_device(w->parport, "walkera0701",
 				    NULL, NULL, walkera0701_irq_handler,
 				    PARPORT_DEV_EXCL, w);
-	if (!w->pardevice)
-		goto init_err;
+	if (!w->pardevice) {
+		pr_err("failed to register parport device\n");
+		error = -EIO;
+		goto err_put_parport;
+	}
 
-	if (parport_negotiate(w->pardevice->port, IEEE1284_MODE_COMPAT))
-		goto init_err1;
+	if (parport_negotiate(w->pardevice->port, IEEE1284_MODE_COMPAT)) {
+		pr_err("failed to negotiate parport mode\n");
+		error = -EIO;
+		goto err_unregister_device;
+	}
 
-	if (parport_claim(w->pardevice))
-		goto init_err1;
+	if (parport_claim(w->pardevice)) {
+		pr_err("failed to claim parport\n");
+		error = -EBUSY;
+		goto err_unregister_device;
+	}
 
 	hrtimer_init(&w->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	w->timer.function = timer_handler;
 
 	w->input_dev = input_allocate_device();
-	if (!w->input_dev)
-		goto init_err2;
+	if (!w->input_dev) {
+		pr_err("failed to allocate input device\n");
+		error = -ENOMEM;
+		goto err_release_parport;
+	}
 
 	input_set_drvdata(w->input_dev, w);
 	w->input_dev->name = "Walkera WK-0701 TX";
@@ -252,21 +266,23 @@ static int walkera0701_connect(struct walkera_dev *w, int parport)
 	input_set_abs_params(w->input_dev, ABS_RUDDER, -512, 512, 0, 0);
 	input_set_abs_params(w->input_dev, ABS_MISC, -512, 512, 0, 0);
 
-	err = input_register_device(w->input_dev);
-	if (err)
-		goto init_err3;
+	error = input_register_device(w->input_dev);
+	if (error) {
+		pr_err("failed to register input device\n");
+		goto err_free_input_dev;
+	}
 
 	return 0;
 
- init_err3:
+err_free_input_dev:
 	input_free_device(w->input_dev);
- init_err2:
+err_release_parport:
 	parport_release(w->pardevice);
- init_err1:
+err_unregister_device:
 	parport_unregister_device(w->pardevice);
- init_err:
+err_put_parport:
 	parport_put_port(w->parport);
-	return err;
+	return error;
 }
 
 static void walkera0701_disconnect(struct walkera_dev *w)
author maintainer
#3 lore raw

PATCH 4/4 Input: walkera0701 - claim parport when opening the device

In reply to Dmitry Torokhov

Postpone claiming the port until the device is opened, instead of doing that when the driver is loaded.

---
 drivers/input/joystick/walkera0701.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/input/joystick/walkera0701.c +6 -10
index b86134c..b76ac58 100644
@@ -183,6 +183,9 @@ static int walkera0701_open(struct input_dev *dev)
 {
 	struct walkera_dev *w = input_get_drvdata(dev);
 
+	if (parport_claim(w->pardevice))
+		return -EBUSY;
+
 	parport_enable_irq(w->parport);
 	return 0;
 }
@@ -193,6 +196,8 @@ static void walkera0701_close(struct input_dev *dev)
 
 	parport_disable_irq(w->parport);
 	hrtimer_cancel(&w->timer);
+
+	parport_release(w->pardevice);
 }
 
 static int walkera0701_connect(struct walkera_dev *w, int parport)
@@ -227,12 +232,6 @@ static int walkera0701_connect(struct walkera_dev *w, int parport)
 		goto err_unregister_device;
 	}
 
-	if (parport_claim(w->pardevice)) {
-		pr_err("failed to claim parport\n");
-		error = -EBUSY;
-		goto err_unregister_device;
-	}
-
 	hrtimer_init(&w->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	w->timer.function = timer_handler;
 
@@ -240,7 +239,7 @@ static int walkera0701_connect(struct walkera_dev *w, int parport)
 	if (!w->input_dev) {
 		pr_err("failed to allocate input device\n");
 		error = -ENOMEM;
-		goto err_release_parport;
+		goto err_unregister_device;
 	}
 
 	input_set_drvdata(w->input_dev, w);
@@ -276,8 +275,6 @@ static int walkera0701_connect(struct walkera_dev *w, int parport)
 
 err_free_input_dev:
 	input_free_device(w->input_dev);
-err_release_parport:
-	parport_release(w->pardevice);
 err_unregister_device:
 	parport_unregister_device(w->pardevice);
 err_put_parport:
@@ -288,7 +285,6 @@ err_put_parport:
 static void walkera0701_disconnect(struct walkera_dev *w)
 {
 	input_unregister_device(w->input_dev);
-	parport_release(w->pardevice);
 	parport_unregister_device(w->pardevice);
 	parport_put_port(w->parport);
 }