Re: [PATCH] Input/evdev: Add 64bit timestamp support

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Anshul,

On Tue, Jul 07, 2015 at 07:30:23PM +0530, Anshul Garg wrote:
> Hello Mr. Dmitry ,
> 
> On Tue, Jul 7, 2015 at 4:18 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Anshul,
> >
> > On Mon, Jul 06, 2015 at 12:33:44PM -0700, Anshul Garg wrote:
> >> From: Anshul Garg <aksgarg1989@gmail.com>
> >>
> >> As per current implementation input driver can only
> >> send ktime converted to timeval which user space again
> >> have to convert.
> >
> > Why? ktime is kernel construct and thus userspace has no business using
> > it directly.
> >
> As per current implementation , input subsystem fills the event timestamp
> from ktime_get_real,ktme_get_mono depending upon clock type and then
> converts it to timeval structure.
> 
> Then user space program uses timevaltoNano api to get the event time.

OK, so there is a [single?] program that uses some API that converts
timeval to nanoseconds. I do not think we should be changing kernel,
especially in an incompatible way, for the benefit of a single program.

> 
> >> In some cases input drivers need 64bit timestamp value
> >> from driver only so that same value can be used by upper
> >> layer without any manipulation.
> >
> > Why do they need 64 bit value? What exactly is special about 64 bits? Do
> > they need to know number of seconds since beginning of the universe?
> >
> > You need to specify the problem better instead of just saying we need 64
> > bits.
> >
> Since currently event time is of type timeval and event handlers send the
> time as per this format only.
> By 64bit timestamp i am suggesting support for sending timestamp
> directly obtained using api(ktime_get_real,ktime_get_boottime etc).
> 
> I am thinking of following reasons.
> 1. For every event sent from input subsystem user space has to convert
> time again.[from timeval to ns] which can be avoided if we add the
> functionality
> of sending ktime to user space.

Or you simply do not convert it to nanoseconds in userspace and use
timeval that you got.

> 
> 
> >> Proposed implementation is placed under CONFIG flag which
> >> will not break any existing code.
> >
> > Yes, it does. As soon as somebody enables this option their usersoace
> > will [potentially] break, because instead of timeval they will be
> > getting 64 bit values.
> >
> > If we want to do this users will have to explicitly request such
> > timestamps.
> >
> If someone enables this CONFIG option input subsystem will also send
> timestamp [nanosec]value along with time[timeval].
> So i think existing interface will not break.

How can it possibly not break (without recompiling userspace) if you
change the side of input_event structure? Do an experiment: install one
of the latest Linux distributions (Fedora, Ubuntu), recompile the kernel
with your change, and try booting it. See if your mouse works.

> 
> Yes we can achieve this by providing ioctl from userspace.
> So that user can decide which timestamp user needs from input subsystem

Yes, ioctl might be a better option, but I still haven't hear a good
reason for adding this feature.

Thanks.

-- 
Dmitry