[PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

[PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

[Vojtech Bocek]

Initial input event has not yet arrived in joydev_connect()
where values are set, which means default values of input_absinfo
are used for init event, not the actual values from joystick.

Signed-off-by: Vojtech Bocek <vbocek@gmail.com>
---
 drivers/input/joydev.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 26043cc..11f24b4 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -318,9 +318,14 @@ static int joydev_generate_startup_event(struct joydev_client *client,
 			event->value = !!test_bit(joydev->keypam[event->number],
 						  input->key);
 		} else {
+			int evnum = client->startup - joydev->nkey;
+			int val = input_abs_get_val(input, joydev->abspam[evnum]);
+
+			joydev->abs[evnum] = joydev_correct(val, &joydev->corr[evnum]);
+
 			event->type = JS_EVENT_AXIS | JS_EVENT_INIT;
-			event->number = client->startup - joydev->nkey;
-			event->value = joydev->abs[event->number];
+			event->number = evnum;
+			event->value = joydev->abs[evnum];
 		}
 		client->startup++;
 	}
-- 
1.7.10.4

Re: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

[Dmitry Torokhov]

Hi Vojtech,

On Tue, Aug 14, 2012 at 12:11:54AM +0200, Vojtech Bocek wrote:
> Initial input event has not yet arrived in joydev_connect()
> where values are set, which means default values of input_absinfo
> are used for init event, not the actual values from joystick.

So what guarantees that joystick events will arrive in time, before
joydev_generate_startup_event() is called? It looks like your solution
is racy...

I wonder if we should not generate the startup event until we have seen
at least one EV_SYN, i.e. entire device state has been transmitted to
us.
> 
> Signed-off-by: Vojtech Bocek <vbocek@gmail.com>
> ---
>  drivers/input/joydev.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 26043cc..11f24b4 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -318,9 +318,14 @@ static int joydev_generate_startup_event(struct joydev_client *client,
>  			event->value = !!test_bit(joydev->keypam[event->number],
>  						  input->key);
>  		} else {
> +			int evnum = client->startup - joydev->nkey;
> +			int val = input_abs_get_val(input, joydev->abspam[evnum]);
> +
> +			joydev->abs[evnum] = joydev_correct(val, &joydev->corr[evnum]);
> +
>  			event->type = JS_EVENT_AXIS | JS_EVENT_INIT;
> -			event->number = client->startup - joydev->nkey;
> -			event->value = joydev->abs[event->number];
> +			event->number = evnum;
> +			event->value = joydev->abs[evnum];
>  		}
>  		client->startup++;
>  	}
> -- 
> 1.7.10.4
> 

Thanks.

-- 
Dmitry

Re: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

[Vojtěch Boček]

Hi,

2012/9/5 Dmitry Torokhov <dmitry.torokhov@gmail.com>
> So what guarantees that joystick events will arrive in time, before
> joydev_generate_startup_event() is called? It looks like your solution
> is racy...
>
> I wonder if we should not generate the startup event until we have seen
> at least one EV_SYN, i.e. entire device state has been transmitted to
> us.

I just tried to delay read() until first EV_SYN arrives as you suggested,
but that was not the problem, at least not the main one. First joystick
input events arrives immediately after it is plugged in and driver is loaded,
but when that happens, joydev may not be (and most likely is not)
opened yet, which means the events will never reach joydev because of
"if (!handle->open)" check in input_pass_event.

I wonder how to handle this. Is "if(!handle->open)" valid check? I think so,
my guess is that joydev is the only handler with internal buffer, and it
is useless to update that buffer when the device is not opened.
I think reloading abs values on joydev open should be okay.

I suppose that if we reload abs values on joydev open, waiting for first
sync is not needed, yes?

So the patch could look like this:

---
 drivers/input/joydev.c |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 26043cc..22a166c 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -158,6 +158,20 @@ static void joydev_event(struct input_handle *handle,
 	wake_up_interruptible(&joydev->wait);
 }

+/* joydev must be locked */
+static void joydev_reload_abs(struct joydev *joydev)
+{
+	int i, j, abs;
+	struct input_dev *input = joydev->handle.dev;
+
+	for (i = 0; i < joydev->nabs; ++i) {
+		j = joydev->abspam[i];
+		abs = input_abs_get_val(input, j);
+
+		joydev->abs[i] = joydev_correct(abs, &joydev->corr[i]);
+	}
+}
+
 static int joydev_fasync(int fd, struct file *file, int on)
 {
 	struct joydev_client *client = file->private_data;
@@ -202,8 +216,11 @@ static int joydev_open_device(struct joydev *joydev)
 		retval = -ENODEV;
 	else if (!joydev->open++) {
 		retval = input_open_device(&joydev->handle);
-		if (retval)
+		if (!retval)
+			joydev_reload_abs(joydev);
+		else
 			joydev->open--;
+		
 	}

 	mutex_unlock(&joydev->mutex);
-- 
1.7.10.4

Re: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

[Dmitry Torokhov]

On Wed, Sep 05, 2012 at 10:09:57PM +0200, Vojtěch Boček wrote:
> Hi,
> 
> 2012/9/5 Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > So what guarantees that joystick events will arrive in time, before
> > joydev_generate_startup_event() is called? It looks like your solution
> > is racy...
> >
> > I wonder if we should not generate the startup event until we have seen
> > at least one EV_SYN, i.e. entire device state has been transmitted to
> > us.
> 
> I just tried to delay read() until first EV_SYN arrives as you suggested,
> but that was not the problem, at least not the main one. First joystick
> input events arrives immediately after it is plugged in and driver is loaded,
> but when that happens, joydev may not be (and most likely is not)
> opened yet, which means the events will never reach joydev because of
> "if (!handle->open)" check in input_pass_event.
> 
> I wonder how to handle this. Is "if(!handle->open)" valid check? I think so,
> my guess is that joydev is the only handler with internal buffer, and it
> is useless to update that buffer when the device is not opened.
> I think reloading abs values on joydev open should be okay.
> 
> I suppose that if we reload abs values on joydev open, waiting for first
> sync is not needed, yes?
> 
> So the patch could look like this:

This makes sense. Just to confirm - have you tried the patch and
verified it works for you?

Thanks.

-- 
Dmitry

Re: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

[Vojtech Bocek]

2012/9/13, Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> On Wed, Sep 05, 2012 at 10:09:57PM +0200, Vojtěch Boček wrote:
>
> This makes sense. Just to confirm - have you tried the patch and
> verified it works for you?
>
> Thanks.
>
> --
> Dmitry
>

Yes, I did. Only one thing is incorrect(?). When I plug the joystick in,
then open my test program, everything is okay, startup event has
 correct values. Then, I unplug the joystick and plug it in again
 quickly, after that, startup event has wrong values, but another
 normal axis event with correct values is fired immediately after.

I suppose it does not matter, because in the end, values are
 correct.  Felt worth mentioning, though.

Re: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

[Vojtech Bocek]

Hi,
sorry for the impatience, but it's been nearly two months since
your last response, and I, trying to send patch to Linux for
the first time, simply don't know if should I just wait more or not.
Is the patch wrong/ugly, or are you simply too busy?

Patch for current linux-next attached.

=====================
>From be5bbc627e5c8ccfea240deef6a68b10f7f1ff40 Mon Sep 17 00:00:00 2001
From: Vojtech Bocek <vbocek@gmail.com>
Date: Thu, 1 Nov 2012 17:34:34 +0100
Subject: [PATCH 1/1] Input: joydev - fix axes values sent in initial js_event

Initial input ABS events can't reach joydev because it is not
opened yet.
This patch makes joydev reload ABS values on joydev_open_device.

Signed-off-by: Vojtech Bocek <vbocek@gmail.com>
---
 drivers/input/joydev.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index f362883..f53a7b1 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -156,6 +156,18 @@ static void joydev_event(struct input_handle *handle,
 	wake_up_interruptible(&joydev->wait);
 }

+/* joydev must be locked */
+static void joydev_reload_abs(struct joydev *joydev)
+{
+	int i, abs;
+	struct input_dev *input = joydev->handle.dev;
+
+	for (i = 0; i < joydev->nabs; ++i) {
+		abs = input_abs_get_val(input, joydev->abspam[i]);
+		joydev->abs[i] = joydev_correct(abs, &joydev->corr[i]);
+	}
+}
+
 static int joydev_fasync(int fd, struct file *file, int on)
 {
 	struct joydev_client *client = file->private_data;
@@ -200,7 +212,9 @@ static int joydev_open_device(struct joydev *joydev)
 		retval = -ENODEV;
 	else if (!joydev->open++) {
 		retval = input_open_device(&joydev->handle);
-		if (retval)
+		if (!retval)
+			joydev_reload_abs(joydev);
+		else
 			joydev->open--;
 	}

-- 
1.7.10.4