Input: Add support for CLOCK_BOOTTIME

PATCH In review Aniroop Mathur 2 participants 5 messages Dec 16, 2014 – Dec 17, 2014

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Aniroop Mathur

author

2014-12-16 16:56 UTC #0 lore raw

PATCH 1/1 Input: Add support for CLOCK_BOOTTIME

This patch adds support for CLOCK_BOOTTIME for input event timestamp. CLOCK_BOOTTIME includes suspend time, so it would allow aplications to get correct time difference between two events even when system resumes from suspend state.

Signed-off-by Aniroop Mathur <[email protected]>

---
 drivers/input/evdev.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

drivers/input/evdev.c +21 ~~-12~~

index de05545..7825794 100644
@@ -28,6 +28,13 @@
 #include <linux/cdev.h>
 #include "input-compat.h"
 
+enum clock_type {
+	REAL = 0,
+	MONO,
+	BOOT,
+	CLK_MAX
+};
+
 struct evdev {
 	int open;
 	struct input_handle handle;

@@ -108,8 +115,9 @@ static void evdev_queue_syn_dropped(struct evdev_client *client)
 	struct input_event ev;
 	ktime_t time;
 
-	time = (client->clkid == CLOCK_MONOTONIC) ?
-		ktime_get() : ktime_get_real();
+	time = (client->clkid == CLOCK_REALTIME) ?
+		ktime_get_real() : (client->clkid == CLOCK_MONOTONIC) ?
+		ktime_get() : ktime_get_boottime();
 
 	ev.time = ktime_to_timeval(time);
 	ev.type = EV_SYN;

@@ -159,7 +167,7 @@ static void __pass_event(struct evdev_client *client,
 
 static void evdev_pass_values(struct evdev_client *client,
 			const struct input_value *vals, unsigned int count,
-			ktime_t mono, ktime_t real)
+			ktime_t *ev_time)
 {
 	struct evdev *evdev = client->evdev;
 	const struct input_value *v;

@@ -169,8 +177,9 @@ static void evdev_pass_values(struct evdev_client *client,
 	if (client->revoked)
 		return;
 
-	event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
-				      mono : real);
+	event.time = ktime_to_timeval(client->clkid == CLOCK_REALTIME ?
+			ev_time[REAL] : client->clkid == CLOCK_MONOTONIC ?
+			ev_time[MONO] : ev_time[BOOT]);
 
 	/* Interrupts are disabled, just acquire the lock. */
 	spin_lock(&client->buffer_lock);

@@ -198,21 +207,21 @@ static void evdev_events(struct input_handle *handle,
 {
 	struct evdev *evdev = handle->private;
 	struct evdev_client *client;
-	ktime_t time_mono, time_real;
+	ktime_t ev_time[CLK_MAX];
 
-	time_mono = ktime_get();
-	time_real = ktime_mono_to_real(time_mono);
+	ev_time[MONO] = ktime_get();
+	ev_time[REAL] = ktime_mono_to_real(ev_time[MONO]);
+	ev_time[BOOT] = ktime_get_boottime();
 
 	rcu_read_lock();
 
 	client = rcu_dereference(evdev->grab);
 
 	if (client)
-		evdev_pass_values(client, vals, count, time_mono, time_real);
+		evdev_pass_values(client, vals, count, ev_time);
 	else
 		list_for_each_entry_rcu(client, &evdev->client_list, node)
-			evdev_pass_values(client, vals, count,
-					  time_mono, time_real);
+			evdev_pass_values(client, vals, count, ev_time);
 
 	rcu_read_unlock();
 }

@@ -874,7 +883,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 	case EVIOCSCLOCKID:
 		if (copy_from_user(&i, p, sizeof(unsigned int)))
 			return -EFAULT;
-		if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME)
+		if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME && i != CLOCK_BOOTTIME)
 			return -EINVAL;
 		client->clkid = i;
 		return 0;

Signature · 1 line

1.9.1

Dmitry Torokhov

maintainer

2014-12-17 18:20 UTC #1 lore raw

Hi Aniroop,

Quoted 50 lines · Aniroop Mathur

Signed-off-by Aniroop Mathur <[email protected]>

---
 drivers/input/evdev.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

drivers/input/evdev.c +11 -3

index de05545..7825794 100644
@@ -28,6 +28,13 @@
 #include <linux/cdev.h>
 #include "input-compat.h"
 
+enum clock_type {
+	REAL = 0,
+	MONO,
+	BOOT,
+	CLK_MAX
+};
+
 struct evdev {
 	int open;
 	struct input_handle handle;

@@ -108,8 +115,9 @@ static void evdev_queue_syn_dropped(struct evdev_client *client)
 	struct input_event ev;
 	ktime_t time;
 
-	time = (client->clkid == CLOCK_MONOTONIC) ?
-		ktime_get() : ktime_get_real();
+	time = (client->clkid == CLOCK_REALTIME) ?
+		ktime_get_real() : (client->clkid == CLOCK_MONOTONIC) ?
+		ktime_get() : ktime_get_boottime();
 
 	ev.time = ktime_to_timeval(time);
 	ev.type = EV_SYN;

@@ -159,7 +167,7 @@ static void __pass_event(struct evdev_client *client,
 
 static void evdev_pass_values(struct evdev_client *client,
 			const struct input_value *vals, unsigned int count,
-			ktime_t mono, ktime_t real)
+			ktime_t *ev_time)
 {
 	struct evdev *evdev = client->evdev;
 	const struct input_value *v;

drivers/input/evdev.c static void evdev_pass_values(struct evdev_client *client, · line 182

Quoted 8 lines

drivers/input/evdev.c +3 -2

@@ -169,8 +177,9 @@ static void evdev_pass_values(struct evdev_client *client,
 	if (client->revoked)
 		return;
 
-	event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
-				      mono : real);
+	event.time = ktime_to_timeval(client->clkid == CLOCK_REALTIME ?
+			ev_time[REAL] : client->clkid == CLOCK_MONOTONIC ?
+			ev_time[MONO] : ev_time[BOOT]);

This becomes unwieldy. Should we have client->clk_type instead of client->clkid and convert CLOCK_* into your EVDEV_CLK_{REAL|MONO|BOOT} when setting it in ioctl and then do

	event.time = ktime_to_timeval(ev_time[client->clk_offset]);

Quoted 5 lines

drivers/input/evdev.c +0 -0

 	/* Interrupts are disabled, just acquire the lock. */
 	spin_lock(&client->buffer_lock);

@@ -198,21 +207,21 @@ static void evdev_events(struct input_handle *handle,
 {

drivers/input/evdev.c static void evdev_events(struct input_handle *handle, · line 214

Quoted 10 lines

drivers/input/evdev.c +4 -3

 	struct evdev *evdev = handle->private;
 	struct evdev_client *client;
-	ktime_t time_mono, time_real;
+	ktime_t ev_time[CLK_MAX];
 
-	time_mono = ktime_get();
-	time_real = ktime_mono_to_real(time_mono);
+	ev_time[MONO] = ktime_get();
+	ev_time[REAL] = ktime_mono_to_real(ev_time[MONO]);
+	ev_time[BOOT] = ktime_get_boottime();

I do not think we want to use ktime_get_boottime() here; I'd rather you use ktime_mono_to_anY(ev_time[MONO], TK_OFFS_BOOT) so that all 3 times are consistent.

Quoted 28 lines

drivers/input/evdev.c +3 -4

 	rcu_read_lock();
 
 	client = rcu_dereference(evdev->grab);
 
 	if (client)
-		evdev_pass_values(client, vals, count, time_mono, time_real);
+		evdev_pass_values(client, vals, count, ev_time);
 	else
 		list_for_each_entry_rcu(client, &evdev->client_list, node)
-			evdev_pass_values(client, vals, count,
-					  time_mono, time_real);
+			evdev_pass_values(client, vals, count, ev_time);
 
 	rcu_read_unlock();
 }

@@ -874,7 +883,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 	case EVIOCSCLOCKID:
 		if (copy_from_user(&i, p, sizeof(unsigned int)))
 			return -EFAULT;
-		if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME)
+		if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME && i != CLOCK_BOOTTIME)
 			return -EINVAL;
 		client->clkid = i;
 		return 0;

Signature · 1 line

1.9.1

Thanks.

Signature · 1 line

Dmitry

Aniroop Mathur

author

2014-12-17 21:33 UTC #2 lore raw

Hello Mr. Torokhov,

Quoted 35 lines · Dmitry Torokhov

Hi Aniroop,
Quoted 32 lines · Aniroop Mathur
This patch adds support for CLOCK_BOOTTIME for input event timestamp. CLOCK_BOOTTIME includes suspend time, so it would allow aplications to get correct time difference between two events even when system resumes from suspend state.
Signed-off-by Aniroop Mathur <[email protected]>
---
 drivers/input/evdev.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)
drivers/input/evdev.c +7 -0
index de05545..7825794 100644
@@ -28,6 +28,13 @@
 #include <linux/cdev.h>
 #include "input-compat.h"
 
+enum clock_type {
+     REAL = 0,
+     MONO,
+     BOOT,
+     CLK_MAX
+};
+
 struct evdev {
      int open;
      struct input_handle handle;
@@ -108,8 +115,9 @@ static void evdev_queue_syn_dropped(struct evdev_client *client)
      struct input_event ev;
      ktime_t time;
 

Quoted 6 lines

drivers/input/evdev.c static void evdev_queue_syn_dropped(struct evdev_client *client) · line 121

Quoted 6 lines

drivers/input/evdev.c +3 -2

-     time = (client->clkid == CLOCK_MONOTONIC) ?
-             ktime_get() : ktime_get_real();
+     time = (client->clkid == CLOCK_REALTIME) ?
+             ktime_get_real() : (client->clkid == CLOCK_MONOTONIC) ?
+             ktime_get() : ktime_get_boottime();

Is this okay ?
To improve readabilty,
how about adding a new funtion to get time depending upon clkid like below ? time = get_clk_time(client->clkid);

static ktime_t get_clk_time(int id)

{
        switch (id) {

        case CLOCK_REALTIME:     return ktime_get_real();
        case CLOCK_MONOTONIC: return ktime_get();
        case CLOCK_BOOTTIME:    return ktime_get_bootime();
        }
}

Quoted 25 lines

Quoted 12 lines

drivers/input/evdev.c +1 -1

      ev.time = ktime_to_timeval(time);
      ev.type = EV_SYN;

@@ -159,7 +167,7 @@ static void __pass_event(struct evdev_client *client,
 
 static void evdev_pass_values(struct evdev_client *client,
                      const struct input_value *vals, unsigned int count,
-                     ktime_t mono, ktime_t real)
+                     ktime_t *ev_time)
 {
      struct evdev *evdev = client->evdev;
      const struct input_value *v;

drivers/input/evdev.c static void evdev_pass_values(struct evdev_client *client, · line 182

Quoted 8 lines

drivers/input/evdev.c +3 -2

@@ -169,8 +177,9 @@ static void evdev_pass_values(struct evdev_client *client,
      if (client->revoked)
              return;
 
-     event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
-                                   mono : real);
+     event.time = ktime_to_timeval(client->clkid == CLOCK_REALTIME ?
+                     ev_time[REAL] : client->clkid == CLOCK_MONOTONIC ?
+                     ev_time[MONO] : ev_time[BOOT]);

This becomes unwieldy. Should we have client->clk_type instead of client->clkid and convert CLOCK_* into your EVDEV_CLK_{REAL|MONO|BOOT} when setting it in ioctl and then do

Quoted 2 lines

        event.time = ktime_to_timeval(ev_time[client->clk_offset]);

Sure, using clk_type would be better and improve code readability.

I am afraid how converting CLOCK_* into your EVDEV_CLK_{REAL|MONO|BOOT} will help
because we cannot have ev_time array size more than CLK_MAX (3).

Did you mean to have following changes:

1. Add clk_type variable
struct evdev_client {
int clkid:
+ int clk_type;
}

2. Add new funtion for setting clk_type to improve readability static int set_clk_type(struct evdev_client *client, int id)

{
        switch (id) {

        case CLOCK_REALTIME:     client->clk_type = EV_REAL;  break;
        case CLOCK_MONOTONIC: client->clk_type = EV_MONO; break;
        case CLOCK_BOOTTIME:    client->clk_type = EV_BOOT;  break;
        default: return -1
        }

        return 0;
}

3. Addition in ioctl call

    case EVIOCSCLOCKID:
    ...
    if (!set_clk_type())
           return -EINVAL;

     client->clkid = i;
     return 0;

4. Change in evdev_pass_values function event.time = ktime_to_timeval(ev_time[client->clk_type]);

Quoted 10 lines

drivers/input/evdev.c +1 -1

      /* Interrupts are disabled, just acquire the lock. */
      spin_lock(&client->buffer_lock);

@@ -198,21 +207,21 @@ static void evdev_events(struct input_handle *handle,
 {
      struct evdev *evdev = handle->private;
      struct evdev_client *client;
-     ktime_t time_mono, time_real;
+     ktime_t ev_time[CLK_MAX];

Quoted 10 lines

drivers/input/evdev.c static void evdev_events(struct input_handle *handle, · line 214
Quoted 5 lines
drivers/input/evdev.c +3 -2
-     time_mono = ktime_get();
-     time_real = ktime_mono_to_real(time_mono);
+     ev_time[MONO] = ktime_get();
+     ev_time[REAL] = ktime_mono_to_real(ev_time[MONO]);
+     ev_time[BOOT] = ktime_get_boottime();
I do not think we want to use ktime_get_boottime() here; I'd rather you use ktime_mono_to_anY(ev_time[MONO], TK_OFFS_BOOT) so that all 3 times are consistent.

Sure. I will send the updated patch with this change.

Quoted 31 lines

Quoted 26 lines

drivers/input/evdev.c +3 -5

      rcu_read_lock();
 
      client = rcu_dereference(evdev->grab);
 
      if (client)
-             evdev_pass_values(client, vals, count, time_mono, time_real);
+             evdev_pass_values(client, vals, count, ev_time);
      else
              list_for_each_entry_rcu(client, &evdev->client_list, node)
-                     evdev_pass_values(client, vals, count,
-                                       time_mono, time_real);
+                     evdev_pass_values(client, vals, count, ev_time);
 
      rcu_read_unlock();
 }

@@ -874,7 +883,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
      case EVIOCSCLOCKID:
              if (copy_from_user(&i, p, sizeof(unsigned int)))
                      return -EFAULT;
-             if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME)
+             if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME && i != CLOCK_BOOTTIME)
                      return -EINVAL;
              client->clkid = i;
              return 0;
--

Quoted 2 lines

1.9.1

Thanks.

Quoted 2 lines

--
Dmitry

Thanks,
Aniroop Mathur

Dmitry Torokhov

maintainer

2014-12-17 22:03 UTC #3 lore raw

Quoted 52 lines · Aniroop Mathur

Hello Mr. Torokhov,
Quoted 35 lines · Dmitry Torokhov
Hi Aniroop,
Quoted 32 lines · Aniroop Mathur
This patch adds support for CLOCK_BOOTTIME for input event timestamp. CLOCK_BOOTTIME includes suspend time, so it would allow aplications to get correct time difference between two events even when system resumes from suspend state.
Signed-off-by Aniroop Mathur <[email protected]>
---
 drivers/input/evdev.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)
drivers/input/evdev.c +7 -0
index de05545..7825794 100644
@@ -28,6 +28,13 @@
 #include <linux/cdev.h>
 #include "input-compat.h"
 
+enum clock_type {
+     REAL = 0,
+     MONO,
+     BOOT,
+     CLK_MAX
+};
+
 struct evdev {
      int open;
      struct input_handle handle;
@@ -108,8 +115,9 @@ static void evdev_queue_syn_dropped(struct evdev_client *client)
      struct input_event ev;
      ktime_t time;
 
Quoted 6 lines
drivers/input/evdev.c static void evdev_queue_syn_dropped(struct evdev_client *client) · line 121
Quoted 6 lines
drivers/input/evdev.c +3 -2
-     time = (client->clkid == CLOCK_MONOTONIC) ?
-             ktime_get() : ktime_get_real();
+     time = (client->clkid == CLOCK_REALTIME) ?
+             ktime_get_real() : (client->clkid == CLOCK_MONOTONIC) ?
+             ktime_get() : ktime_get_boottime();
 
Is this okay ?
To improve readabilty,
how about adding a new funtion to get time depending upon clkid like below ? time = get_clk_time(client->clkid);
static ktime_t get_clk_time(int id)

Quoted 8 lines

{
        switch (id) {

        case CLOCK_REALTIME:     return ktime_get_real();
        case CLOCK_MONOTONIC: return ktime_get();
        case CLOCK_BOOTTIME:    return ktime_get_bootime();
        }
}

Just open-code it in evdev_queue_syn_dropped() since there is only place where we call it. And use clk_type instead of clock id-.

Quoted 35 lines

Quoted 26 lines

Quoted 13 lines

drivers/input/evdev.c +1 -1

      ev.time = ktime_to_timeval(time);
      ev.type = EV_SYN;

@@ -159,7 +167,7 @@ static void __pass_event(struct evdev_client *client,
 
 static void evdev_pass_values(struct evdev_client *client,
                      const struct input_value *vals, unsigned int count,
-                     ktime_t mono, ktime_t real)
+                     ktime_t *ev_time)
 {
      struct evdev *evdev = client->evdev;
      const struct input_value *v;

drivers/input/evdev.c static void evdev_pass_values(struct evdev_client *client, · line 182

Quoted 8 lines

drivers/input/evdev.c +3 -2

@@ -169,8 +177,9 @@ static void evdev_pass_values(struct evdev_client *client,
      if (client->revoked)
              return;
 
-     event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
-                                   mono : real);
+     event.time = ktime_to_timeval(client->clkid == CLOCK_REALTIME ?
+                     ev_time[REAL] : client->clkid == CLOCK_MONOTONIC ?
+                     ev_time[MONO] : ev_time[BOOT]);

This becomes unwieldy. Should we have client->clk_type instead of client->clkid and convert CLOCK_* into your EVDEV_CLK_{REAL|MONO|BOOT} when setting it in ioctl and then do

Quoted 2 lines

        event.time = ktime_to_timeval(ev_time[client->clk_offset]);

Sure, using clk_type would be better and improve code readability.

I am afraid how converting CLOCK_* into your EVDEV_CLK_{REAL|MONO|BOOT} will help
because we cannot have ev_time array size more than CLK_MAX (3).

Quoted 7 lines

Did you mean to have following changes:
1. Add clk_type variable
struct evdev_client {
int clkid:
+ int clk_type;
}

I meant replacing clkid with clk_type in evdev_client.

Quoted 3 lines

2. Add new funtion for setting clk_type to improve readability static int set_clk_type(struct evdev_client *client, int id)

evdev_set_clk_type() to keep with the style.

Quoted 7 lines

{
        switch (id) {

        case CLOCK_REALTIME:     client->clk_type = EV_REAL;  break;
        case CLOCK_MONOTONIC: client->clk_type = EV_MONO; break;
        case CLOCK_BOOTTIME:    client->clk_type = EV_BOOT;  break;
        default: return -1

-EINVAL here, otherwise yes.

Quoted 11 lines

        }

        return 0;
}

3. Addition in ioctl call

    case EVIOCSCLOCKID:
    ...
    if (!set_clk_type())
           return -EINVAL;

	error = evdev_set_clktype(i);
	if (error)
		return error;

Quoted 1 line

     client->clkid = i;

Drop this.

Quoted 4 lines

     return 0;
4. Change in evdev_pass_values function event.time = ktime_to_timeval(ev_time[client->clk_type]);

Yes.

Quoted 60 lines

Quoted 11 lines

drivers/input/evdev.c +1 -1

      /* Interrupts are disabled, just acquire the lock. */
      spin_lock(&client->buffer_lock);

@@ -198,21 +207,21 @@ static void evdev_events(struct input_handle *handle,
 {
      struct evdev *evdev = handle->private;
      struct evdev_client *client;
-     ktime_t time_mono, time_real;
+     ktime_t ev_time[CLK_MAX];

Quoted 10 lines

drivers/input/evdev.c static void evdev_events(struct input_handle *handle, · line 214
Quoted 5 lines
drivers/input/evdev.c +3 -2
-     time_mono = ktime_get();
-     time_real = ktime_mono_to_real(time_mono);
+     ev_time[MONO] = ktime_get();
+     ev_time[REAL] = ktime_mono_to_real(ev_time[MONO]);
+     ev_time[BOOT] = ktime_get_boottime();
I do not think we want to use ktime_get_boottime() here; I'd rather you use ktime_mono_to_anY(ev_time[MONO], TK_OFFS_BOOT) so that all 3 times are consistent.

Sure. I will send the updated patch with this change.

Quoted 31 lines

Quoted 26 lines

drivers/input/evdev.c +3 -5

      rcu_read_lock();
 
      client = rcu_dereference(evdev->grab);
 
      if (client)
-             evdev_pass_values(client, vals, count, time_mono, time_real);
+             evdev_pass_values(client, vals, count, ev_time);
      else
              list_for_each_entry_rcu(client, &evdev->client_list, node)
-                     evdev_pass_values(client, vals, count,
-                                       time_mono, time_real);
+                     evdev_pass_values(client, vals, count, ev_time);
 
      rcu_read_unlock();
 }

@@ -874,7 +883,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
      case EVIOCSCLOCKID:
              if (copy_from_user(&i, p, sizeof(unsigned int)))
                      return -EFAULT;
-             if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME)
+             if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME && i != CLOCK_BOOTTIME)
                      return -EINVAL;
              client->clkid = i;
              return 0;
--

Quoted 2 lines

1.9.1

Thanks.

Quoted 2 lines

--
Dmitry

Thanks,
Aniroop Mathur

Signature · 1 line

Dmitry

Aniroop Mathur

author

2014-12-17 23:11 UTC #4 lore raw

Hello Mr. Torokhov,

Quoted 209 lines · Dmitry Torokhov

Quoted 52 lines · Aniroop Mathur
Hello Mr. Torokhov,
Quoted 35 lines · Dmitry Torokhov
Hi Aniroop,
Quoted 32 lines · Aniroop Mathur
This patch adds support for CLOCK_BOOTTIME for input event timestamp. CLOCK_BOOTTIME includes suspend time, so it would allow aplications to get correct time difference between two events even when system resumes from suspend state.
Signed-off-by Aniroop Mathur <[email protected]>
---
 drivers/input/evdev.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)
drivers/input/evdev.c +7 -0
index de05545..7825794 100644
@@ -28,6 +28,13 @@
 #include <linux/cdev.h>
 #include "input-compat.h"
 
+enum clock_type {
+     REAL = 0,
+     MONO,
+     BOOT,
+     CLK_MAX
+};
+
 struct evdev {
      int open;
      struct input_handle handle;
@@ -108,8 +115,9 @@ static void evdev_queue_syn_dropped(struct evdev_client *client)
      struct input_event ev;
      ktime_t time;
 
Quoted 6 lines
drivers/input/evdev.c static void evdev_queue_syn_dropped(struct evdev_client *client) · line 121
Quoted 6 lines
drivers/input/evdev.c +3 -2
-     time = (client->clkid == CLOCK_MONOTONIC) ?
-             ktime_get() : ktime_get_real();
+     time = (client->clkid == CLOCK_REALTIME) ?
+             ktime_get_real() : (client->clkid == CLOCK_MONOTONIC) ?
+             ktime_get() : ktime_get_boottime();
 
Is this okay ?
To improve readabilty,
how about adding a new funtion to get time depending upon clkid like below ? time = get_clk_time(client->clkid);
static ktime_t get_clk_time(int id)
Quoted 8 lines
{
        switch (id) {

        case CLOCK_REALTIME:     return ktime_get_real();
        case CLOCK_MONOTONIC: return ktime_get();
        case CLOCK_BOOTTIME:    return ktime_get_bootime();
        }
}
Just open-code it in evdev_queue_syn_dropped() since there is only place where we call it. And use clk_type instead of clock id-.
Quoted 35 lines
Quoted 26 lines
Quoted 13 lines
drivers/input/evdev.c +1 -1
      ev.time = ktime_to_timeval(time);
      ev.type = EV_SYN;
@@ -159,7 +167,7 @@ static void __pass_event(struct evdev_client *client,
 
 static void evdev_pass_values(struct evdev_client *client,
                      const struct input_value *vals, unsigned int count,
-                     ktime_t mono, ktime_t real)
+                     ktime_t *ev_time)
 {
      struct evdev *evdev = client->evdev;
      const struct input_value *v;
drivers/input/evdev.c static void evdev_pass_values(struct evdev_client *client, · line 182
Quoted 8 lines
drivers/input/evdev.c +3 -2
@@ -169,8 +177,9 @@ static void evdev_pass_values(struct evdev_client *client,
      if (client->revoked)
              return;
 
-     event.time = ktime_to_timeval(client->clkid == CLOCK_MONOTONIC ?
-                                   mono : real);
+     event.time = ktime_to_timeval(client->clkid == CLOCK_REALTIME ?
+                     ev_time[REAL] : client->clkid == CLOCK_MONOTONIC ?
+                     ev_time[MONO] : ev_time[BOOT]);
This becomes unwieldy. Should we have client->clk_type instead of client->clkid and convert CLOCK_* into your EVDEV_CLK_{REAL|MONO|BOOT} when setting it in ioctl and then do
Quoted 2 lines
        event.time = ktime_to_timeval(ev_time[client->clk_offset]);
Sure, using clk_type would be better and improve code readability.
I am afraid how converting CLOCK_* into your EVDEV_CLK_{REAL|MONO|BOOT} will help
because we cannot have ev_time array size more than CLK_MAX (3).
Quoted 7 lines
Did you mean to have following changes:
1. Add clk_type variable
struct evdev_client {
int clkid:
+ int clk_type;
}
I meant replacing clkid with clk_type in evdev_client.
Quoted 3 lines
2. Add new funtion for setting clk_type to improve readability static int set_clk_type(struct evdev_client *client, int id)
evdev_set_clk_type() to keep with the style.
Quoted 7 lines
{
        switch (id) {

        case CLOCK_REALTIME:     client->clk_type = EV_REAL;  break;
        case CLOCK_MONOTONIC: client->clk_type = EV_MONO; break;
        case CLOCK_BOOTTIME:    client->clk_type = EV_BOOT;  break;
        default: return -1
-EINVAL here, otherwise yes.
Quoted 11 lines
        }

        return 0;
}
3. Addition in ioctl call
    case EVIOCSCLOCKID:
    ...
    if (!set_clk_type())
           return -EINVAL;
        error = evdev_set_clktype(i);
        if (error)
                return error;
Quoted 1 line
     client->clkid = i;
Drop this.
Quoted 4 lines
     return 0;
4. Change in evdev_pass_values function event.time = ktime_to_timeval(ev_time[client->clk_type]);
Yes.
Quoted 55 lines
Quoted 11 lines
Quoted 11 lines
drivers/input/evdev.c +1 -1
      /* Interrupts are disabled, just acquire the lock. */
      spin_lock(&client->buffer_lock);
@@ -198,21 +207,21 @@ static void evdev_events(struct input_handle *handle,
 {
      struct evdev *evdev = handle->private;
      struct evdev_client *client;
-     ktime_t time_mono, time_real;
+     ktime_t ev_time[CLK_MAX];
 
Quoted 10 lines
drivers/input/evdev.c static void evdev_events(struct input_handle *handle, · line 214
Quoted 5 lines
drivers/input/evdev.c +3 -2
-     time_mono = ktime_get();
-     time_real = ktime_mono_to_real(time_mono);
+     ev_time[MONO] = ktime_get();
+     ev_time[REAL] = ktime_mono_to_real(ev_time[MONO]);
+     ev_time[BOOT] = ktime_get_boottime();
I do not think we want to use ktime_get_boottime() here; I'd rather you use ktime_mono_to_anY(ev_time[MONO], TK_OFFS_BOOT) so that all 3 times are consistent.
Sure. I will send the updated patch with this change.
Quoted 31 lines
Quoted 26 lines
drivers/input/evdev.c +3 -5
      rcu_read_lock();
 
      client = rcu_dereference(evdev->grab);
 
      if (client)
-             evdev_pass_values(client, vals, count, time_mono, time_real);
+             evdev_pass_values(client, vals, count, ev_time);
      else
              list_for_each_entry_rcu(client, &evdev->client_list, node)
-                     evdev_pass_values(client, vals, count,
-                                       time_mono, time_real);
+                     evdev_pass_values(client, vals, count, ev_time);
 
      rcu_read_unlock();
 }
@@ -874,7 +883,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
      case EVIOCSCLOCKID:
              if (copy_from_user(&i, p, sizeof(unsigned int)))
                      return -EFAULT;
-             if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME)
+             if (i != CLOCK_MONOTONIC && i != CLOCK_REALTIME && i != CLOCK_BOOTTIME)
                      return -EINVAL;
              client->clkid = i;
              return 0;
--
Quoted 2 lines
1.9.1
Thanks.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
--
Dmitry
Thanks,
Aniroop Mathur
--
Dmitry

Thanks for the suggestions !!
I have sent you the updated patch with new changes done.

Thanks,
Aniroop