The series is aimed at making input events y2038 safe. It extends the lifetime of the realtime timestamps in the events to year 2106. The plan is to deprecate realtime timestamps anyway as they are not appropriate for these timestamps as noted in the patch a80b83b7b8 by John Stultz.
The series is a result of many discussions with Arnd Bergmann.
Deepa Dinamani (4): uinput: Add ioctl for using monotonic/ boot times input: evdev: Replace timeval with timespec64 input: Deprecate real timestamps beyond year 2106 input: serio: Replace timeval by timespec64
drivers/input/evdev.c | 30 +++++++++++-------- drivers/input/input-compat.c | 25 ++++++++-------- drivers/input/input-compat.h | 19 +++++++----- drivers/input/misc/uinput.c | 64 +++++++++++++++++++++++++++++++++++++--- drivers/input/serio/hil_mlc.c | 37 +++++++++++------------ drivers/input/serio/hp_sdc.c | 17 ++++++----- drivers/input/serio/hp_sdc_mlc.c | 10 +++---- include/linux/hil_mlc.h | 6 ++-- include/linux/hp_sdc.h | 6 ++-- include/linux/uinput.h | 3 +- include/uapi/linux/input.h | 37 +++++++++++++++++++++++ include/uapi/linux/uinput.h | 3 ++ 12 files changed, 183 insertions(+), 74 deletions(-)
struct timeval which is part of struct input_event to maintain the event times is not y2038 safe.
Real time timestamps are also not ideal for input_event as this time can go backwards as noted in the patch a80b83b7b8 by John Stultz.
Arnd Bergmann suggested deprecating real time and using monotonic or other timers for all input_event times as a solution to both the above problems.
Add a new ioctl to let the user dictate the kind of time to be used for input events. This is similar to the evdev implementation of the feature. Realtime is still the default time. This is to maintain backward compatibility.
The structure to maintain input events will be changed in a different patch.
Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com --- drivers/input/misc/uinput.c | 58 ++++++++++++++++++++++++++++++++++++++++++++- include/linux/uinput.h | 1 + include/uapi/linux/uinput.h | 3 +++ 3 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index 92595b9..ae22927 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -46,11 +46,28 @@ static int uinput_dev_event(struct input_dev *dev, unsigned int type, unsigned int code, int value) { struct uinput_device *udev = input_get_drvdata(dev); + struct timespec64 ts; + ktime_t kt;
udev->buff[udev->head].type = type; udev->buff[udev->head].code = code; udev->buff[udev->head].value = value; - do_gettimeofday(&udev->buff[udev->head].time); + + kt = ktime_get(); + switch (udev->clk_type) { + case CLOCK_REALTIME: + ts = ktime_to_timespec64(ktime_mono_to_real(kt)); + break; + case CLOCK_MONOTONIC: + ts = ktime_to_timespec64(kt); + break; + case CLOCK_BOOTTIME: + ts = ktime_to_timespec64(ktime_mono_to_any(kt, TK_OFFS_BOOT)); + break; + } + + udev->buff[udev->head].time.tv_sec = ts.tv_sec; + udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC; udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE;
wake_up_interruptible(&udev->waitq); @@ -295,6 +312,7 @@ static int uinput_create_device(struct uinput_device *udev) if (error) goto fail2;
+ udev->clk_type = CLOCK_REALTIME; udev->state = UIST_CREATED;
return 0; @@ -304,6 +322,38 @@ static int uinput_create_device(struct uinput_device *udev) return error; }
+static int uinput_set_clk_type(struct uinput_device *udev, unsigned int clkid) +{ + unsigned int clk_type; + + if (udev->state != UIST_CREATED) + return -EINVAL; + + switch (clkid) { + /* Realtime clock is only valid until year 2038.*/ + case CLOCK_REALTIME: + clk_type = CLOCK_REALTIME; + break; + case CLOCK_MONOTONIC: + clk_type = CLOCK_MONOTONIC; + break; + case CLOCK_BOOTTIME: + clk_type = CLOCK_BOOTTIME; + break; + default: + return -EINVAL; + } + + if (udev->clk_type != clk_type) { + udev->clk_type = clk_type; + + /* Flush pending events */ + uinput_flush_requests(udev); + } + + return 0; +} + static int uinput_open(struct inode *inode, struct file *file) { struct uinput_device *newdev; @@ -787,6 +837,7 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, char *phys; const char *name; unsigned int size; + int clock_id;
retval = mutex_lock_interruptible(&udev->mutex); if (retval) @@ -817,6 +868,11 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, retval = uinput_dev_setup(udev, p); goto out;
+ case UI_SET_CLOCKID: + if (copy_from_user(&clock_id, p, sizeof(unsigned int))) + return -EFAULT; + return uinput_set_clk_type(udev, clock_id); + /* UI_ABS_SETUP is handled in the variable size ioctls */
case UI_SET_EVBIT: diff --git a/include/linux/uinput.h b/include/linux/uinput.h index 75de43d..6527fb7 100644 --- a/include/linux/uinput.h +++ b/include/linux/uinput.h @@ -72,6 +72,7 @@ struct uinput_device { unsigned char head; unsigned char tail; struct input_event buff[UINPUT_BUFFER_SIZE]; + int clk_type; unsigned int ff_effects_max;
struct uinput_request *requests[UINPUT_NUM_REQUESTS]; diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h index dc652e2..d9494ae 100644 --- a/include/uapi/linux/uinput.h +++ b/include/uapi/linux/uinput.h @@ -133,6 +133,9 @@ struct uinput_abs_setup { */ #define UI_ABS_SETUP _IOW(UINPUT_IOCTL_BASE, 4, struct uinput_abs_setup)
+/* Set clockid to be used for timestamps */ +#define UI_SET_CLOCKID _IOW(UINPUT_IOCTL_BASE, 5, int) + #define UI_SET_EVBIT _IOW(UINPUT_IOCTL_BASE, 100, int) #define UI_SET_KEYBIT _IOW(UINPUT_IOCTL_BASE, 101, int) #define UI_SET_RELBIT _IOW(UINPUT_IOCTL_BASE, 102, int)
On Tuesday, September 13, 2016 7:10:02 AM CEST Deepa Dinamani wrote:
--- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -46,11 +46,28 @@ static int uinput_dev_event(struct input_dev *dev, unsigned int type, unsigned int code, int value) { struct uinput_device *udev = input_get_drvdata(dev);
struct timespec64 ts;ktime_t kt;udev->buff[udev->head].type = type; udev->buff[udev->head].code = code; udev->buff[udev->head].value = value;
do_gettimeofday(&udev->buff[udev->head].time);
kt = ktime_get();switch (udev->clk_type) {case CLOCK_REALTIME:ts = ktime_to_timespec64(ktime_mono_to_real(kt));break;case CLOCK_MONOTONIC:ts = ktime_to_timespec64(kt);break;case CLOCK_BOOTTIME:ts = ktime_to_timespec64(ktime_mono_to_any(kt, TK_OFFS_BOOT));break;}udev->buff[udev->head].time.tv_sec = ts.tv_sec;udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
This is a bit inefficient. I think it's better to use ktime_get_real_ts64(), ktime_get_ts64() and get_monotonic_boottime64().
I see that we do the same thing in evdev_events(), so maybe that isn't overly critical though. Or we might want to change both.
The last one of these also uses the slow ktime_to_timespec64(), but that is something we can improve later in the common code.
Arnd
On Tue, Sep 13, 2016 at 8:07 AM, Arnd Bergmann arnd@arndb.de wrote:
On Tuesday, September 13, 2016 7:10:02 AM CEST Deepa Dinamani wrote:
--- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -46,11 +46,28 @@ static int uinput_dev_event(struct input_dev *dev, unsigned int type, unsigned int code, int value) { struct uinput_device *udev = input_get_drvdata(dev);
struct timespec64 ts;ktime_t kt; udev->buff[udev->head].type = type; udev->buff[udev->head].code = code; udev->buff[udev->head].value = value;
do_gettimeofday(&udev->buff[udev->head].time);
kt = ktime_get();switch (udev->clk_type) {case CLOCK_REALTIME:ts = ktime_to_timespec64(ktime_mono_to_real(kt));break;case CLOCK_MONOTONIC:ts = ktime_to_timespec64(kt);break;case CLOCK_BOOTTIME:ts = ktime_to_timespec64(ktime_mono_to_any(kt, TK_OFFS_BOOT));break;}udev->buff[udev->head].time.tv_sec = ts.tv_sec;udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;This is a bit inefficient. I think it's better to use ktime_get_real_ts64(), ktime_get_ts64() and get_monotonic_boottime64().
Right. The way I have is using sequence counters twice, once for ktime_get() and then again for offset.
I see that we do the same thing in evdev_events(), so maybe that isn't overly critical though. Or we might want to change both.
The last one of these also uses the slow ktime_to_timespec64(), but that is something we can improve later in the common code.
Yes, I tried to keep it same as evdev here. But, evdev itself has a couple of different ways of doing this:
For instance, __evdev_queue_syn_dropped() uses
time = client->clk_type == EV_CLK_REAL ? ktime_get_real() : client->clk_type == EV_CLK_MONO ? ktime_get() : ktime_get_boottime();
I can change all this to look the same.
I will wait until I hear if this approach is ok for input events. I will post an update to series after.
Thanks, Deepa
struct timeval is not y2038 safe.
All references to timeval in the kernel will be replaced by y2038 safe structures. Replace all references to timeval with y2038 safe struct timespec64 here.
struct input_event will be changed in a different patch.
Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com --- drivers/input/evdev.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index e9ae3d5..bda2b61 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -157,6 +157,7 @@ static void __evdev_queue_syn_dropped(struct evdev_client *client) { struct input_event ev; ktime_t time; + struct timespec64 ts;
time = client->clk_type == EV_CLK_REAL ? ktime_get_real() : @@ -164,7 +165,9 @@ static void __evdev_queue_syn_dropped(struct evdev_client *client) ktime_get() : ktime_get_boottime();
- ev.time = ktime_to_timeval(time); + ts = ktime_to_timespec64(time); + ev.time.tv_sec = ts.tv_sec; + ev.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC; ev.type = EV_SYN; ev.code = SYN_DROPPED; ev.value = 0; @@ -262,12 +265,15 @@ static void evdev_pass_values(struct evdev_client *client, struct evdev *evdev = client->evdev; const struct input_value *v; struct input_event event; + struct timespec64 ts; bool wakeup = false;
if (client->revoked) return;
- event.time = ktime_to_timeval(ev_time[client->clk_type]); + ts = ktime_to_timespec64(ev_time[client->clk_type]); + event.time.tv_sec = ts.tv_sec; + event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
/* Interrupts are disabled, just acquire the lock. */ spin_lock(&client->buffer_lock);
On Tuesday, September 13, 2016 7:10:03 AM CEST Deepa Dinamani wrote:
struct timeval is not y2038 safe.
All references to timeval in the kernel will be replaced by y2038 safe structures. Replace all references to timeval with y2038 safe struct timespec64 here.
struct input_event will be changed in a different patch.
Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com
Acked-by: Arnd Bergmann arnd@arndb.de
struct timeval is not y2038 safe. All usage of timeval in the kernel will be replaced by y2038 safe structures.
struct input_event maintains time for each input event. Real time timestamps are not ideal for input as this time can go backwards as noted in the patch a80b83b7b8 by John Stultz. Hence, having the input_event.time fields only big enough for monotonic and boot times are sufficient.
Leave the original input_event as is. This is to maintain backward compatibility with existing userspace interfaces that use input_event. Introduce a new replacement struct raw_input_event. This replaces timeval with struct input_timeval. This structure maintains time in __kernel_ulong_t or compat_ulong_t to allow for architectures to override types as in the case of x32.
The change requires any userspace utilities reading or writing from event nodes to update their reading format to match raw_input_event.The changes to the popular libraries will be posted along with the kernel changes. The driver version is also updated to reflect the change in event format.
Suggested-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com --- drivers/input/evdev.c | 20 ++++++++++---------- drivers/input/input-compat.c | 25 +++++++++++++------------ drivers/input/input-compat.h | 19 ++++++++++++------- drivers/input/misc/uinput.c | 6 +++--- include/linux/uinput.h | 2 +- include/uapi/linux/input.h | 37 +++++++++++++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 33 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index bda2b61..f56188d 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -60,7 +60,7 @@ struct evdev_client { bool revoked; unsigned long *evmasks[EV_CNT]; unsigned int bufsize; - struct input_event buffer[]; + struct raw_input_event buffer[]; };
static size_t evdev_get_mask_cnt(unsigned int type) @@ -113,7 +113,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type) unsigned int i, head, num; unsigned int mask = client->bufsize - 1; bool is_report; - struct input_event *ev; + struct raw_input_event *ev;
BUG_ON(type == EV_SYN);
@@ -155,7 +155,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
static void __evdev_queue_syn_dropped(struct evdev_client *client) { - struct input_event ev; + struct raw_input_event ev; ktime_t time; struct timespec64 ts;
@@ -232,7 +232,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid) }
static void __pass_event(struct evdev_client *client, - const struct input_event *event) + const struct raw_input_event *event) { client->buffer[client->head++] = *event; client->head &= client->bufsize - 1; @@ -264,7 +264,7 @@ static void evdev_pass_values(struct evdev_client *client, { struct evdev *evdev = client->evdev; const struct input_value *v; - struct input_event event; + struct raw_input_event event; struct timespec64 ts; bool wakeup = false;
@@ -504,7 +504,7 @@ static int evdev_open(struct inode *inode, struct file *file) struct evdev *evdev = container_of(inode->i_cdev, struct evdev, cdev); unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev); unsigned int size = sizeof(struct evdev_client) + - bufsize * sizeof(struct input_event); + bufsize * sizeof(struct raw_input_event); struct evdev_client *client; int error;
@@ -539,7 +539,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer, { struct evdev_client *client = file->private_data; struct evdev *evdev = client->evdev; - struct input_event event; + struct raw_input_event event; int retval = 0;
if (count != 0 && count < input_event_size()) @@ -572,7 +572,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer, }
static int evdev_fetch_next_event(struct evdev_client *client, - struct input_event *event) + struct raw_input_event *event) { int have_event;
@@ -594,7 +594,7 @@ static ssize_t evdev_read(struct file *file, char __user *buffer, { struct evdev_client *client = file->private_data; struct evdev *evdev = client->evdev; - struct input_event event; + struct raw_input_event event; size_t read = 0; int error;
@@ -1080,7 +1080,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, switch (cmd) {
case EVIOCGVERSION: - return put_user(EV_VERSION, ip); + return put_user(EV_VERSION_1_2, ip);
case EVIOCGID: if (copy_to_user(p, &dev->id, sizeof(struct input_id))) diff --git a/drivers/input/input-compat.c b/drivers/input/input-compat.c index d84d20b..c94746d 100644 --- a/drivers/input/input-compat.c +++ b/drivers/input/input-compat.c @@ -15,10 +15,10 @@ #ifdef CONFIG_COMPAT
int input_event_from_user(const char __user *buffer, - struct input_event *event) + struct raw_input_event *event) { - if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) { - struct input_event_compat compat_event; + if (in_compat_syscall()) { + struct raw_input_event_compat compat_event;
if (copy_from_user(&compat_event, buffer, sizeof(struct input_event_compat))) @@ -31,7 +31,8 @@ int input_event_from_user(const char __user *buffer, event->value = compat_event.value;
} else { - if (copy_from_user(event, buffer, sizeof(struct input_event))) + if (copy_from_user(event, buffer, + sizeof(struct raw_input_event))) return -EFAULT; }
@@ -39,10 +40,10 @@ int input_event_from_user(const char __user *buffer, }
int input_event_to_user(char __user *buffer, - const struct input_event *event) + const struct raw_input_event *event) { - if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) { - struct input_event_compat compat_event; + if (in_compat_syscall()) { + struct raw_input_event_compat compat_event;
compat_event.time.tv_sec = event->time.tv_sec; compat_event.time.tv_usec = event->time.tv_usec; @@ -55,7 +56,7 @@ int input_event_to_user(char __user *buffer, return -EFAULT;
} else { - if (copy_to_user(buffer, event, sizeof(struct input_event))) + if (copy_to_user(buffer, event, sizeof(struct raw_input_event))) return -EFAULT; }
@@ -100,18 +101,18 @@ int input_ff_effect_from_user(const char __user *buffer, size_t size, #else
int input_event_from_user(const char __user *buffer, - struct input_event *event) + struct raw_input_event *event) { - if (copy_from_user(event, buffer, sizeof(struct input_event))) + if (copy_from_user(event, buffer, sizeof(struct raw_input_event))) return -EFAULT;
return 0; }
int input_event_to_user(char __user *buffer, - const struct input_event *event) + const struct raw_input_event *event) { - if (copy_to_user(buffer, event, sizeof(struct input_event))) + if (copy_to_user(buffer, event, sizeof(struct raw_input_event))) return -EFAULT;
return 0; diff --git a/drivers/input/input-compat.h b/drivers/input/input-compat.h index 1563160..c18132d 100644 --- a/drivers/input/input-compat.h +++ b/drivers/input/input-compat.h @@ -17,8 +17,13 @@
#ifdef CONFIG_COMPAT
-struct input_event_compat { - struct compat_timeval time; +struct input_timeval_compat { + compat_ulong_t tv_sec; + compat_ulong_t tv_usec; +}; + +struct raw_input_event_compat { + struct input_timeval_compat time; __u16 type; __u16 code; __s32 value; @@ -55,24 +60,24 @@ struct ff_effect_compat {
static inline size_t input_event_size(void) { - return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ? - sizeof(struct input_event_compat) : sizeof(struct input_event); + return in_compat_syscall() ? sizeof(struct raw_input_event_compat) : + sizeof(struct raw_input_event); }
#else
static inline size_t input_event_size(void) { - return sizeof(struct input_event); + return sizeof(struct raw_input_event); }
#endif /* CONFIG_COMPAT */
int input_event_from_user(const char __user *buffer, - struct input_event *event); + struct raw_input_event *event);
int input_event_to_user(char __user *buffer, - const struct input_event *event); + const struct raw_input_event *event);
int input_ff_effect_from_user(const char __user *buffer, size_t size, struct ff_effect *effect); diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index ae22927..993bd24 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -564,7 +564,7 @@ static int uinput_setup_device_legacy(struct uinput_device *udev, static ssize_t uinput_inject_events(struct uinput_device *udev, const char __user *buffer, size_t count) { - struct input_event ev; + struct raw_input_event ev; size_t bytes = 0;
if (count != 0 && count < input_event_size()) @@ -610,7 +610,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer, }
static bool uinput_fetch_next_event(struct uinput_device *udev, - struct input_event *event) + struct raw_input_event *event) { bool have_event;
@@ -630,7 +630,7 @@ static bool uinput_fetch_next_event(struct uinput_device *udev, static ssize_t uinput_events_to_user(struct uinput_device *udev, char __user *buffer, size_t count) { - struct input_event event; + struct raw_input_event event; size_t read = 0;
while (read + input_event_size() <= count && diff --git a/include/linux/uinput.h b/include/linux/uinput.h index 6527fb7..d1accb3 100644 --- a/include/linux/uinput.h +++ b/include/linux/uinput.h @@ -71,7 +71,7 @@ struct uinput_device { unsigned char ready; unsigned char head; unsigned char tail; - struct input_event buff[UINPUT_BUFFER_SIZE]; + struct raw_input_event buff[UINPUT_BUFFER_SIZE]; int clk_type; unsigned int ff_effects_max;
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index e794f7b..1379899 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -21,7 +21,23 @@ /* * The event structure itself */ +struct input_timeval { + __kernel_ulong_t tv_sec; + __kernel_ulong_t tv_usec; +}; + +struct raw_input_event { + struct input_timeval time; + __u16 type; + __u16 code; + __s32 value; +};
+/* Userspace structure. + * Definition maintained here for userspace that is not yet updated to use + * struct raw_input_event. + * Not to be used anywhere within the kernel. + */ struct input_event { struct timeval time; __u16 type; @@ -29,11 +45,32 @@ struct input_event { __s32 value; };
+static inline void +raw_input_to_input_event(struct raw_input_event *raw, struct input_event *ev) +{ + ev->time.tv_sec = raw->time.tv_sec; + ev->time.tv_usec = raw->time.tv_usec; + ev->type = raw->type; + ev->code = raw->code; + ev->value = raw->value; +} + +static inline void +input_to_raw_event(struct input_event *ev, struct raw_input_event *raw) +{ + raw->time.tv_sec = ev->time.tv_sec; + raw->time.tv_usec = ev->time.tv_usec; + raw->type = ev->type; + raw->code = ev->code; + raw->value = ev->value; +} + /* * Protocol version. */
#define EV_VERSION 0x010001 +#define EV_VERSION_1_2 0x010002
/* * IOCTLs (0x00 - 0x7f)
On Tuesday, September 13, 2016 7:10:04 AM CEST Deepa Dinamani wrote:
struct timeval is not y2038 safe. All usage of timeval in the kernel will be replaced by y2038 safe structures.
struct input_event maintains time for each input event. Real time timestamps are not ideal for input as this time can go backwards as noted in the patch a80b83b7b8 by John Stultz. Hence, having the input_event.time fields only big enough for monotonic and boot times are sufficient.
Leave the original input_event as is. This is to maintain backward compatibility with existing userspace interfaces that use input_event. Introduce a new replacement struct raw_input_event. This replaces timeval with struct input_timeval. This structure maintains time in __kernel_ulong_t or compat_ulong_t to allow for architectures to override types as in the case of x32.
The change requires any userspace utilities reading or writing from event nodes to update their reading format to match raw_input_event.The changes to the popular libraries will be posted along with the kernel changes. The driver version is also updated to reflect the change in event format.
Suggested-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com
Looks very nice!
Two small suggestions from my side:
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index e794f7b..1379899 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -21,7 +21,23 @@ /*
- The event structure itself
*/ +struct input_timeval {
- __kernel_ulong_t tv_sec;
- __kernel_ulong_t tv_usec;
+};
+struct raw_input_event {
- struct input_timeval time;
- __u16 type;
- __u16 code;
- __s32 value;
+};
Maybe a comment here that this is intentionally "unsigned" so we can count until 2106 instead of 2038.
+/* Userspace structure.
- Definition maintained here for userspace that is not yet updated to use
- struct raw_input_event.
- Not to be used anywhere within the kernel.
- */
struct input_event { struct timeval time; __u16 type; @@ -29,11 +45,32 @@ struct input_event { __s32 value; }; +static inline void +raw_input_to_input_event(struct raw_input_event *raw, struct input_event *ev) +{
- ev->time.tv_sec = raw->time.tv_sec;
- ev->time.tv_usec = raw->time.tv_usec;
- ev->type = raw->type;
- ev->code = raw->code;
- ev->value = raw->value;
+}
+static inline void +input_to_raw_event(struct input_event *ev, struct raw_input_event *raw) +{
- raw->time.tv_sec = ev->time.tv_sec;
- raw->time.tv_usec = ev->time.tv_usec;
- raw->type = ev->type;
- raw->code = ev->code;
- raw->value = ev->value;
+}
Here, an "#ifndef __KERNEL__" guard might help to enforce what you write in the comment.
Arnd
Hi Deepa,
[auto build test ERROR on input/next] [also build test ERROR on v4.8-rc6 next-20160913] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] [Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on] [Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Deepa-Dinamani/Make-input-drivers-y... base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next config: x86_64-randconfig-s0-09132154 (attached as .config) compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/input/input-compat.c: In function 'input_event_from_user':
drivers/input/input-compat.c:24:15: error: invalid application of 'sizeof' to incomplete type 'struct input_event_compat'
sizeof(struct input_event_compat))) ^~~~~~ drivers/input/input-compat.c: In function 'input_event_to_user': drivers/input/input-compat.c:55:13: error: invalid application of 'sizeof' to incomplete type 'struct input_event_compat' sizeof(struct input_event_compat))) ^~~~~~
vim +24 drivers/input/input-compat.c
68d77942 Deepa Dinamani 2016-09-13 18 struct raw_input_event *event) 2d56f3a3 Philip Langdale 2008-10-16 19 { 68d77942 Deepa Dinamani 2016-09-13 20 if (in_compat_syscall()) { 68d77942 Deepa Dinamani 2016-09-13 21 struct raw_input_event_compat compat_event; 2d56f3a3 Philip Langdale 2008-10-16 22 2d56f3a3 Philip Langdale 2008-10-16 23 if (copy_from_user(&compat_event, buffer, 2d56f3a3 Philip Langdale 2008-10-16 @24 sizeof(struct input_event_compat))) 2d56f3a3 Philip Langdale 2008-10-16 25 return -EFAULT; 2d56f3a3 Philip Langdale 2008-10-16 26 2d56f3a3 Philip Langdale 2008-10-16 27 event->time.tv_sec = compat_event.time.tv_sec;
:::::: The code at line 24 was first introduced by commit :::::: 2d56f3a32c0e62f99c043d2579840f9731fe5855 Input: refactor evdev 32bit compat to be shareable with uinput
:::::: TO: Philip Langdale philipl@overt.org :::::: CC: Dmitry Torokhov dmitry.torokhov@gmail.com
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
All errors (new ones prefixed by >>):
drivers/input/input-compat.c: In function 'input_event_from_user':
drivers/input/input-compat.c:24:15: error: invalid application of 'sizeof' to incomplete type 'struct input_event_compat'
sizeof(struct input_event_compat))) ^~~~~~drivers/input/input-compat.c: In function 'input_event_to_user': drivers/input/input-compat.c:55:13: error: invalid application of 'sizeof' to incomplete type 'struct input_event_compat' sizeof(struct input_event_compat))) ^~~~~~
These 2 lines need to use raw_input_event_compat.
I will send an update to only this patch to help the discussion. I will wait for other comments before updating the series.
-Deepa
struct timeval is not y2038 safe. All usage of timeval in the kernel will be replaced by y2038 safe structures.
struct input_event maintains time for each input event. Real time timestamps are not ideal for input as this time can go backwards as noted in the patch a80b83b7b8 by John Stultz. Hence, having the input_event.time fields only big enough for monotonic and boot times are sufficient.
Leave the original input_event as is. This is to maintain backward compatibility with existing userspace interfaces that use input_event. Introduce a new replacement struct raw_input_event. This replaces timeval with struct input_timeval. This structure maintains time in __kernel_ulong_t or compat_ulong_t to allow for architectures to override types as in the case of x32.
The change requires any userspace utilities reading or writing from event nodes to update their reading format to match raw_input_event.The changes to the popular libraries will be posted along with the kernel changes. The driver version is also updated to reflect the change in event format.
Suggested-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com ---
Changes from v1 * Fixed the kbuild error * Used __KERNEL__ define as per Arnd's suggestion * Updated comment to indicate the movtivation for ulong types as per Arnd's suggestion.
drivers/input/evdev.c | 20 +++++++++---------- drivers/input/input-compat.c | 29 ++++++++++++++------------- drivers/input/input-compat.h | 19 +++++++++++------- drivers/input/misc/uinput.c | 6 +++--- include/linux/uinput.h | 2 +- include/uapi/linux/input.h | 47 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 35 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index bda2b61..f56188d 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -60,7 +60,7 @@ struct evdev_client { bool revoked; unsigned long *evmasks[EV_CNT]; unsigned int bufsize; - struct input_event buffer[]; + struct raw_input_event buffer[]; };
static size_t evdev_get_mask_cnt(unsigned int type) @@ -113,7 +113,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type) unsigned int i, head, num; unsigned int mask = client->bufsize - 1; bool is_report; - struct input_event *ev; + struct raw_input_event *ev;
BUG_ON(type == EV_SYN);
@@ -155,7 +155,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
static void __evdev_queue_syn_dropped(struct evdev_client *client) { - struct input_event ev; + struct raw_input_event ev; ktime_t time; struct timespec64 ts;
@@ -232,7 +232,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid) }
static void __pass_event(struct evdev_client *client, - const struct input_event *event) + const struct raw_input_event *event) { client->buffer[client->head++] = *event; client->head &= client->bufsize - 1; @@ -264,7 +264,7 @@ static void evdev_pass_values(struct evdev_client *client, { struct evdev *evdev = client->evdev; const struct input_value *v; - struct input_event event; + struct raw_input_event event; struct timespec64 ts; bool wakeup = false;
@@ -504,7 +504,7 @@ static int evdev_open(struct inode *inode, struct file *file) struct evdev *evdev = container_of(inode->i_cdev, struct evdev, cdev); unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev); unsigned int size = sizeof(struct evdev_client) + - bufsize * sizeof(struct input_event); + bufsize * sizeof(struct raw_input_event); struct evdev_client *client; int error;
@@ -539,7 +539,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer, { struct evdev_client *client = file->private_data; struct evdev *evdev = client->evdev; - struct input_event event; + struct raw_input_event event; int retval = 0;
if (count != 0 && count < input_event_size()) @@ -572,7 +572,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer, }
static int evdev_fetch_next_event(struct evdev_client *client, - struct input_event *event) + struct raw_input_event *event) { int have_event;
@@ -594,7 +594,7 @@ static ssize_t evdev_read(struct file *file, char __user *buffer, { struct evdev_client *client = file->private_data; struct evdev *evdev = client->evdev; - struct input_event event; + struct raw_input_event event; size_t read = 0; int error;
@@ -1080,7 +1080,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, switch (cmd) {
case EVIOCGVERSION: - return put_user(EV_VERSION, ip); + return put_user(EV_VERSION_1_2, ip);
case EVIOCGID: if (copy_to_user(p, &dev->id, sizeof(struct input_id))) diff --git a/drivers/input/input-compat.c b/drivers/input/input-compat.c index d84d20b..b58d35c 100644 --- a/drivers/input/input-compat.c +++ b/drivers/input/input-compat.c @@ -15,13 +15,13 @@ #ifdef CONFIG_COMPAT
int input_event_from_user(const char __user *buffer, - struct input_event *event) + struct raw_input_event *event) { - if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) { - struct input_event_compat compat_event; + if (in_compat_syscall()) { + struct raw_input_event_compat compat_event;
if (copy_from_user(&compat_event, buffer, - sizeof(struct input_event_compat))) + sizeof(struct raw_input_event_compat))) return -EFAULT;
event->time.tv_sec = compat_event.time.tv_sec; @@ -31,7 +31,8 @@ int input_event_from_user(const char __user *buffer, event->value = compat_event.value;
} else { - if (copy_from_user(event, buffer, sizeof(struct input_event))) + if (copy_from_user(event, buffer, + sizeof(struct raw_input_event))) return -EFAULT; }
@@ -39,10 +40,10 @@ int input_event_from_user(const char __user *buffer, }
int input_event_to_user(char __user *buffer, - const struct input_event *event) + const struct raw_input_event *event) { - if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) { - struct input_event_compat compat_event; + if (in_compat_syscall()) { + struct raw_input_event_compat compat_event;
compat_event.time.tv_sec = event->time.tv_sec; compat_event.time.tv_usec = event->time.tv_usec; @@ -51,11 +52,11 @@ int input_event_to_user(char __user *buffer, compat_event.value = event->value;
if (copy_to_user(buffer, &compat_event, - sizeof(struct input_event_compat))) + sizeof(struct raw_input_event_compat))) return -EFAULT;
} else { - if (copy_to_user(buffer, event, sizeof(struct input_event))) + if (copy_to_user(buffer, event, sizeof(struct raw_input_event))) return -EFAULT; }
@@ -100,18 +101,18 @@ int input_ff_effect_from_user(const char __user *buffer, size_t size, #else
int input_event_from_user(const char __user *buffer, - struct input_event *event) + struct raw_input_event *event) { - if (copy_from_user(event, buffer, sizeof(struct input_event))) + if (copy_from_user(event, buffer, sizeof(struct raw_input_event))) return -EFAULT;
return 0; }
int input_event_to_user(char __user *buffer, - const struct input_event *event) + const struct raw_input_event *event) { - if (copy_to_user(buffer, event, sizeof(struct input_event))) + if (copy_to_user(buffer, event, sizeof(struct raw_input_event))) return -EFAULT;
return 0; diff --git a/drivers/input/input-compat.h b/drivers/input/input-compat.h index 1563160..c18132d 100644 --- a/drivers/input/input-compat.h +++ b/drivers/input/input-compat.h @@ -17,8 +17,13 @@
#ifdef CONFIG_COMPAT
-struct input_event_compat { - struct compat_timeval time; +struct input_timeval_compat { + compat_ulong_t tv_sec; + compat_ulong_t tv_usec; +}; + +struct raw_input_event_compat { + struct input_timeval_compat time; __u16 type; __u16 code; __s32 value; @@ -55,24 +60,24 @@ struct ff_effect_compat {
static inline size_t input_event_size(void) { - return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ? - sizeof(struct input_event_compat) : sizeof(struct input_event); + return in_compat_syscall() ? sizeof(struct raw_input_event_compat) : + sizeof(struct raw_input_event); }
#else
static inline size_t input_event_size(void) { - return sizeof(struct input_event); + return sizeof(struct raw_input_event); }
#endif /* CONFIG_COMPAT */
int input_event_from_user(const char __user *buffer, - struct input_event *event); + struct raw_input_event *event);
int input_event_to_user(char __user *buffer, - const struct input_event *event); + const struct raw_input_event *event);
int input_ff_effect_from_user(const char __user *buffer, size_t size, struct ff_effect *effect); diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index ae22927..993bd24 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -564,7 +564,7 @@ static int uinput_setup_device_legacy(struct uinput_device *udev, static ssize_t uinput_inject_events(struct uinput_device *udev, const char __user *buffer, size_t count) { - struct input_event ev; + struct raw_input_event ev; size_t bytes = 0;
if (count != 0 && count < input_event_size()) @@ -610,7 +610,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer, }
static bool uinput_fetch_next_event(struct uinput_device *udev, - struct input_event *event) + struct raw_input_event *event) { bool have_event;
@@ -630,7 +630,7 @@ static bool uinput_fetch_next_event(struct uinput_device *udev, static ssize_t uinput_events_to_user(struct uinput_device *udev, char __user *buffer, size_t count) { - struct input_event event; + struct raw_input_event event; size_t read = 0;
while (read + input_event_size() <= count && diff --git a/include/linux/uinput.h b/include/linux/uinput.h index 6527fb7..d1accb3 100644 --- a/include/linux/uinput.h +++ b/include/linux/uinput.h @@ -71,7 +71,7 @@ struct uinput_device { unsigned char ready; unsigned char head; unsigned char tail; - struct input_event buff[UINPUT_BUFFER_SIZE]; + struct raw_input_event buff[UINPUT_BUFFER_SIZE]; int clk_type; unsigned int ff_effects_max;
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h index e794f7b..60d5869 100644 --- a/include/uapi/linux/input.h +++ b/include/uapi/linux/input.h @@ -22,6 +22,29 @@ * The event structure itself */
+/* The time structure for y2038 safe raw_input_event. + * The fields use unsigned types to extend times until + * year 2106 rather than 2038. + */ +struct input_timeval { + __kernel_ulong_t tv_sec; + __kernel_ulong_t tv_usec; +}; + +struct raw_input_event { + struct input_timeval time; + __u16 type; + __u16 code; + __s32 value; +}; + +#ifndef __KERNEL__ + +/* Userspace structure. + * Definition maintained here for userspace that is not yet updated to use + * struct raw_input_event. + * Not to be used anywhere within the kernel. + */ struct input_event { struct timeval time; __u16 type; @@ -29,11 +52,35 @@ struct input_event { __s32 value; };
+ +static inline void +raw_input_to_input_event(struct raw_input_event *raw, struct input_event *ev) +{ + ev->time.tv_sec = raw->time.tv_sec; + ev->time.tv_usec = raw->time.tv_usec; + ev->type = raw->type; + ev->code = raw->code; + ev->value = raw->value; +} + +static inline void +input_to_raw_event(struct input_event *ev, struct raw_input_event *raw) +{ + raw->time.tv_sec = ev->time.tv_sec; + raw->time.tv_usec = ev->time.tv_usec; + raw->type = ev->type; + raw->code = ev->code; + raw->value = ev->value; +} + +#endif + /* * Protocol version. */
#define EV_VERSION 0x010001 +#define EV_VERSION_1_2 0x010002
/* * IOCTLs (0x00 - 0x7f)
On Tuesday, September 13, 2016 1:10:06 PM CEST Deepa Dinamani wrote:
struct timeval is not y2038 safe. All usage of timeval in the kernel will be replaced by y2038 safe structures.
struct input_event maintains time for each input event. Real time timestamps are not ideal for input as this time can go backwards as noted in the patch a80b83b7b8 by John Stultz. Hence, having the input_event.time fields only big enough for monotonic and boot times are sufficient.
Leave the original input_event as is. This is to maintain backward compatibility with existing userspace interfaces that use input_event. Introduce a new replacement struct raw_input_event. This replaces timeval with struct input_timeval. This structure maintains time in __kernel_ulong_t or compat_ulong_t to allow for architectures to override types as in the case of x32.
The change requires any userspace utilities reading or writing from event nodes to update their reading format to match raw_input_event.The changes to the popular libraries will be posted along with the kernel changes. The driver version is also updated to reflect the change in event format.
Suggested-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com
Acked-by: Arnd Bergmann arnd@arndb.de
struct timeval is not y2038 safe. All references to timeval will be deleted from the kernel to make it y2038 safe. Replace its uses by y2038 safe struct timespec64.
The timestamps changed here only keep track of delta times. These timestamps are also internal to kernel. Hence, monotonic times are sufficient here. The unit of the delta times is also changed in certain cases to nanoseconds rather than microseconds. This is in line with timespec64 which keeps time in nanoseconds.
Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com --- drivers/input/serio/hil_mlc.c | 37 ++++++++++++++++++------------------- drivers/input/serio/hp_sdc.c | 17 +++++++++-------- drivers/input/serio/hp_sdc_mlc.c | 10 +++++----- include/linux/hil_mlc.h | 6 +++--- include/linux/hp_sdc.h | 6 +++--- 5 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/input/serio/hil_mlc.c b/drivers/input/serio/hil_mlc.c index 65605e4..7c59a79 100644 --- a/drivers/input/serio/hil_mlc.c +++ b/drivers/input/serio/hil_mlc.c @@ -274,14 +274,14 @@ static int hilse_match(hil_mlc *mlc, int unused) /* An LCV used to prevent runaway loops, forces 5 second sleep when reset. */ static int hilse_init_lcv(hil_mlc *mlc, int unused) { - struct timeval tv; + time64_t time;
- do_gettimeofday(&tv); + time = ktime_get_seconds();
- if (mlc->lcv && (tv.tv_sec - mlc->lcv_tv.tv_sec) < 5) + if (mlc->lcv && (time - mlc->lcv_tv.tv_sec) < 5) return -1;
- mlc->lcv_tv = tv; + mlc->lcv_tv.tv_sec = time; mlc->lcv = 0;
return 0; @@ -466,7 +466,7 @@ static const struct hilse_node hil_mlc_se[HILSEN_END] = { FUNC(hilse_init_lcv, 0, HILSEN_NEXT, HILSEN_SLEEP, 0)
/* 1 HILSEN_RESTART */ - FUNC(hilse_inc_lcv, 10, HILSEN_NEXT, HILSEN_START, 0) + FUNC(hilse_inc_lcv, 10000, HILSEN_NEXT, HILSEN_START, 0) OUT(HIL_CTRL_ONLY) /* Disable APE */ CTS
@@ -485,7 +485,7 @@ static const struct hilse_node hil_mlc_se[HILSEN_END] = { FUNC(hilse_init_lcv, 0, HILSEN_NEXT, HILSEN_SLEEP, 0)
/* 10 HILSEN_DHR2 */ - FUNC(hilse_inc_lcv, 10, HILSEN_NEXT, HILSEN_START, 0) + FUNC(hilse_inc_lcv, 10000, HILSEN_NEXT, HILSEN_START, 0) FUNC(hilse_set_ddi, -1, HILSEN_NEXT, 0, 0) OUT(HIL_PKT_CMD | HIL_CMD_DHR) IN(300000, HILSEN_DHR2, HILSEN_DHR2, HILSEN_NEXT) @@ -515,7 +515,7 @@ static const struct hilse_node hil_mlc_se[HILSEN_END] = { FUNC(hilse_init_lcv, 0, HILSEN_NEXT, HILSEN_DOZE, 0)
/* 22 HILSEN_ACF2 */ - FUNC(hilse_inc_lcv, 10, HILSEN_NEXT, HILSEN_START, 0) + FUNC(hilse_inc_lcv, 10000, HILSEN_NEXT, HILSEN_START, 0) OUT(HIL_PKT_CMD | HIL_CMD_ACF | 1) IN(20000, HILSEN_NEXT, HILSEN_DSR, HILSEN_NEXT)
@@ -572,7 +572,7 @@ static const struct hilse_node hil_mlc_se[HILSEN_END] = { OUT(HIL_PKT_CMD | HIL_CMD_RPL) EXPECT(HIL_PKT_CMD | HIL_CMD_RPL | HIL_ERR_INT, 20000, HILSEN_NEXT, HILSEN_DSR, HILSEN_NEXT) - FUNC(hilse_operate, 1, HILSEN_OPERATE, HILSEN_IFC, HILSEN_PROBE) + FUNC(hilse_operate, 1000, HILSEN_OPERATE, HILSEN_IFC, HILSEN_PROBE)
/* 58 HILSEN_IFCACF */ OUT(HIL_PKT_CMD | HIL_CMD_IFC) @@ -584,7 +584,6 @@ static const struct hilse_node hil_mlc_se[HILSEN_END] = {
static inline void hilse_setup_input(hil_mlc *mlc, const struct hilse_node *node) { - switch (node->act) { case HILSE_EXPECT_DISC: mlc->imatch = node->object.packet; @@ -605,7 +604,7 @@ static inline void hilse_setup_input(hil_mlc *mlc, const struct hilse_node *node } mlc->istarted = 1; mlc->intimeout = node->arg; - do_gettimeofday(&(mlc->instart)); + ktime_get_ts64(&(mlc->instart)); mlc->icount = 15; memset(mlc->ipacket, 0, 16 * sizeof(hil_packet)); BUG_ON(down_trylock(&mlc->isem)); @@ -710,7 +709,7 @@ static int hilse_donode(hil_mlc *mlc) break; } mlc->ostarted = 0; - do_gettimeofday(&(mlc->instart)); + ktime_get_ts64(&(mlc->instart)); write_unlock_irqrestore(&mlc->lock, flags); nextidx = HILSEN_NEXT; break; @@ -731,18 +730,18 @@ static int hilse_donode(hil_mlc *mlc) #endif
while (nextidx & HILSEN_SCHED) { - struct timeval tv; + struct timespec64 ts;
if (!sched_long) goto sched;
- do_gettimeofday(&tv); - tv.tv_usec += USEC_PER_SEC * (tv.tv_sec - mlc->instart.tv_sec); - tv.tv_usec -= mlc->instart.tv_usec; - if (tv.tv_usec >= mlc->intimeout) goto sched; - tv.tv_usec = (mlc->intimeout - tv.tv_usec) * HZ / USEC_PER_SEC; - if (!tv.tv_usec) goto sched; - mod_timer(&hil_mlcs_kicker, jiffies + tv.tv_usec); + ktime_get_ts64(&ts); + ts.tv_nsec += NSEC_PER_SEC * (ts.tv_sec - mlc->instart.tv_sec); + ts.tv_nsec -= mlc->instart.tv_nsec; + if (ts.tv_nsec >= mlc->intimeout) goto sched; + ts.tv_nsec = (mlc->intimeout - ts.tv_nsec) * HZ / NSEC_PER_SEC; + if (!ts.tv_nsec) goto sched; + mod_timer(&hil_mlcs_kicker, jiffies + ts.tv_nsec); break; sched: tasklet_schedule(&hil_mlcs_tasklet); diff --git a/drivers/input/serio/hp_sdc.c b/drivers/input/serio/hp_sdc.c index 852858e..ce8f21b 100644 --- a/drivers/input/serio/hp_sdc.c +++ b/drivers/input/serio/hp_sdc.c @@ -193,7 +193,7 @@ static void hp_sdc_take(int irq, void *dev_id, uint8_t status, uint8_t data) curr->seq[curr->idx++] = status; curr->seq[curr->idx++] = data; hp_sdc.rqty -= 2; - do_gettimeofday(&hp_sdc.rtv); + ktime_get_ts64(&hp_sdc.rtv);
if (hp_sdc.rqty <= 0) { /* All data has been gathered. */ @@ -306,13 +306,13 @@ static void hp_sdc_tasklet(unsigned long foo) write_lock_irq(&hp_sdc.rtq_lock);
if (hp_sdc.rcurr >= 0) { - struct timeval tv; + struct timespec64 ts;
- do_gettimeofday(&tv); - if (tv.tv_sec > hp_sdc.rtv.tv_sec) - tv.tv_usec += USEC_PER_SEC; + ktime_get_ts64(&ts); + if (ts.tv_sec > hp_sdc.rtv.tv_sec) + ts.tv_nsec += NSEC_PER_SEC;
- if (tv.tv_usec - hp_sdc.rtv.tv_usec > HP_SDC_MAX_REG_DELAY) { + if (ts.tv_nsec - hp_sdc.rtv.tv_nsec > HP_SDC_MAX_REG_DELAY) { hp_sdc_transaction *curr; uint8_t tmp;
@@ -322,7 +322,8 @@ static void hp_sdc_tasklet(unsigned long foo) * it back to the application. and be less verbose. */ printk(KERN_WARNING PREFIX "read timeout (%ius)!\n", - (int)(tv.tv_usec - hp_sdc.rtv.tv_usec)); + (int)(ts.tv_nsec - hp_sdc.rtv.tv_nsec) / + (int)NSEC_PER_USEC); curr->idx += hp_sdc.rqty; hp_sdc.rqty = 0; tmp = curr->seq[curr->actidx]; @@ -551,7 +552,7 @@ unsigned long hp_sdc_put(void)
/* Start a new read */ hp_sdc.rqty = curr->seq[curr->idx]; - do_gettimeofday(&hp_sdc.rtv); + ktime_get_ts64(&hp_sdc.rtv); curr->idx++; /* Still need to lock here in case of spurious irq. */ write_lock_irq(&hp_sdc.rtq_lock); diff --git a/drivers/input/serio/hp_sdc_mlc.c b/drivers/input/serio/hp_sdc_mlc.c index d50f067..66020cd 100644 --- a/drivers/input/serio/hp_sdc_mlc.c +++ b/drivers/input/serio/hp_sdc_mlc.c @@ -149,7 +149,7 @@ static int hp_sdc_mlc_in(hil_mlc *mlc, suseconds_t timeout)
/* Try to down the semaphore */ if (down_trylock(&mlc->isem)) { - struct timeval tv; + struct timespec64 ts; if (priv->emtestmode) { mlc->ipacket[0] = HIL_ERR_INT | (mlc->opacket & @@ -160,11 +160,11 @@ static int hp_sdc_mlc_in(hil_mlc *mlc, suseconds_t timeout) /* printk(KERN_DEBUG PREFIX ">[%x]\n", mlc->ipacket[0]); */ goto wasup; } - do_gettimeofday(&tv); - tv.tv_usec += USEC_PER_SEC * (tv.tv_sec - mlc->instart.tv_sec); - if (tv.tv_usec - mlc->instart.tv_usec > mlc->intimeout) { + ktime_get_ts64(&ts); + ts.tv_nsec += NSEC_PER_SEC * (ts.tv_sec - mlc->instart.tv_sec); + if (ts.tv_nsec - mlc->instart.tv_nsec > mlc->intimeout) { /* printk("!%i %i", - tv.tv_usec - mlc->instart.tv_usec, + tv.tv_nsec - mlc->instart.tv_nsec, mlc->intimeout); */ rc = 1; diff --git a/include/linux/hil_mlc.h b/include/linux/hil_mlc.h index 394a840..f846730 100644 --- a/include/linux/hil_mlc.h +++ b/include/linux/hil_mlc.h @@ -32,7 +32,7 @@ */
#include <linux/hil.h> -#include <linux/time.h> +#include <linux/time64.h> #include <linux/interrupt.h> #include <linux/semaphore.h> #include <linux/serio.h> @@ -144,12 +144,12 @@ struct hil_mlc { hil_packet ipacket[16]; hil_packet imatch; int icount; - struct timeval instart; + struct timespec64 instart; suseconds_t intimeout;
int ddi; /* Last operational device id */ int lcv; /* LCV to throttle loops */ - struct timeval lcv_tv; /* Time loop was started */ + struct timespec64 lcv_tv; /* Time loop was started */
int di_map[7]; /* Maps below items to live devs */ struct hil_mlc_devinfo di[HIL_MLC_DEVMEM]; diff --git a/include/linux/hp_sdc.h b/include/linux/hp_sdc.h index d392975..d863944 100644 --- a/include/linux/hp_sdc.h +++ b/include/linux/hp_sdc.h @@ -47,9 +47,9 @@ #endif
-/* No 4X status reads take longer than this (in usec). +/* No 4X status reads take longer than this (in nsec). */ -#define HP_SDC_MAX_REG_DELAY 20000 +#define HP_SDC_MAX_REG_DELAY 20000000
typedef void (hp_sdc_irqhook) (int irq, void *dev_id, uint8_t status, uint8_t data); @@ -281,7 +281,7 @@ typedef struct { hp_sdc_transaction *tq[HP_SDC_QUEUE_LEN]; /* All pending read/writes */
int rcurr, rqty; /* Current read transact in process */ - struct timeval rtv; /* Time when current read started */ + struct timespec64 rtv; /* Monotonic time when current read started */ int wcurr; /* Current write transact in process */
int dev_err; /* carries status from registration */
On Tuesday, September 13, 2016 7:10:05 AM CEST Deepa Dinamani wrote:
struct timeval is not y2038 safe. All references to timeval will be deleted from the kernel to make it y2038 safe. Replace its uses by y2038 safe struct timespec64.
The timestamps changed here only keep track of delta times. These timestamps are also internal to kernel. Hence, monotonic times are sufficient here. The unit of the delta times is also changed in certain cases to nanoseconds rather than microseconds. This is in line with timespec64 which keeps time in nanoseconds.
Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com
Looks all correct to me,
Acked-by: Arnd Bergmann arnd@arndb.de
I think it could be done a little nicer using ktime_get() instead of ktime_get_ts64(), but probably nobody cares with this driver.
Arnd
On Tue, Sep 13, 2016 at 8:13 AM, Arnd Bergmann arnd@arndb.de wrote:
On Tuesday, September 13, 2016 7:10:05 AM CEST Deepa Dinamani wrote:
struct timeval is not y2038 safe. All references to timeval will be deleted from the kernel to make it y2038 safe. Replace its uses by y2038 safe struct timespec64.
The timestamps changed here only keep track of delta times. These timestamps are also internal to kernel. Hence, monotonic times are sufficient here. The unit of the delta times is also changed in certain cases to nanoseconds rather than microseconds. This is in line with timespec64 which keeps time in nanoseconds.
Signed-off-by: Deepa Dinamani deepa.kernel@gmail.com
Looks all correct to me,
Acked-by: Arnd Bergmann arnd@arndb.de
I think it could be done a little nicer using ktime_get() instead of ktime_get_ts64(), but probably nobody cares with this driver.
I tried to keep the processing same as timeval. This could be changed to ktime_get() if that is preferred.
-Deepa
-
Arnd Bergmann -
Deepa Dinamani -
kbuild test robot