Re: [PATCH v4 14/14] HID: hid-multitouch: forwards MSC_TIMESTAMP

Hi Henrik,

On Fri, Nov 16, 2012 at 9:09 PM, Henrik Rydberg [off-list ref] wrote:

Hi Benjamin,

quoted

quoted

This was not what I envisioned from the discussion yesterday, I was
probably too vague. Firstly, the absolute time interval checked seems
to be 500 ms, which is arbitrary. Second, the wrap should probably use

Not exactly. The time interval was 5s, near enough the minimum requirement for this field (6.5535 s).

Sorry about that, but the argument remains; it should depend on logical_maximum.

I must be tired when I read your mail, I only understand now what you
meant by logical_maximum in your first mail... sorry.

So, totally agree, we could make this kind of thing depending on logical_max.

quoted

quoted

(logical_maximum + 1). Third, we are still not sure whether we should
take the 'time = 0 means reset' logic literally, I think it needs to
be tested.

Again, the device I have never does any reset at the first touch, it just wraps the counter.
The problem is that when there is no event, we now that no events occurred since the previous timestamp, modulus 6.5535 seconds.

So the very first win8 device we test already diverges from the win8
specification, how nice.

hehe... well, the scan time is not as critical as can be the other
fields. Anyway, it's fun :)

Ok, you have conviced me that comparing to a proper time makes
sense. However, I am not happy about using a separate time for this,
given that we will fill in the event time later in the
chain.

that may explains the delta I observed from the kernel time and the device time.

In addition, it would make perfect sense to extend the validity of the
hardware time with the event time for longer intervals. The relative
error only makes a difference on milisecond intervals.

A patch that seamlessly extends the validity of the hardware time,
ideally using the event time, seems like a viable solution.

Just to be sure:
When I receive scan_time, I increment timestamp with the device value.
When not, I find out the number of counter wrap I missed with the
kernel timer (jiffies) to get the actual device time.

Is that ok for you?

quoted

quoted

+static void mt_compute_timestamp(struct mt_device *td, struct hid_field *field,
+                            unsigned value)
+{
+   unsigned dt;
+
+   if (value) {
+           dt = (value - td->dev_time) % (field->logical_maximum + 1);

The curious thing is that this is not working on my kernel:

Oups, I suspect (value - td->dev_time) needs to be explicitly converted to unsigned.

I can't remember if I tried this one, but I'll try to make it one line
in the next version.

quoted

I don't understand the meaning of adding !td->timestamp. :/

With the definition that timestamp == 0 means an unknown interval, we
do not want to send that value by accident.

ok, makes sense.

Cheers,
Benjamin

Thanks,
Henrik