Re: [PATCH 1/1 FROM FIXED] Revert "HID: fix unit exponent parsing"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Benjamin,

On 10/09/2013 12:02 PM, Benjamin Tissoires wrote:
first, I wouldn't revert the patch as this, because I think the patch
also fixes the nibbles parsing in unit:
- the part regarding the unit exponent seems blurry
- the part regarding the unit exponent seems correct to me.

This statement is sure confusing :)

On Wed, Oct 9, 2013 at 9:37 AM, Nikolai Kondrashov<spbnick@xxxxxxxxx>  wrote:
On 10/08/2013 10:05 PM, Nikolai Kondrashov wrote:
 From what data I have on various non-Wacom graphics tablets, most of the
 older ones provide incorrect "unit" item value, so "unit exponent" doesn't
 apply.

Yes. The "correct" specification of unit and unit exponent has only be
a requirements since Windows 8 [1]. (Note, there used to be a pdf [2],
which I found more convenient to read, but still...).

I've stumbled on the very same document yesterday.

So definitely, Microsoft considers that the unit exponent is a 4 bits
two's complement, otherwise, the firmware will not get a Windows 8
certification.

I think this is due to their use of the "HID Descriptor Tool". Maybe
assuming it is a reference implementation, which it is not, but more likely
just not noticing the discrepancy.

I'll try to reach Microsoft on this and maybe make them correct their
specification.

Meanwhile, can I suggest a hybrid approach? As positive unit exponent
beyond 7 is unlikely to appear for axes which have their resolution
calculated currently, can we assume anything higher than 7 to be
nibble-stored negative exponents and anything else normal signed integers?

I would say that the current approach (without the revert) is exactly this:
- if the data is stored on only 1 byte ( if (!(raw_value&
0xfffffff0))), do the two's complement ->  any value less than 7 will
be the same, above are considered as negative.
- if not, then use the raw value.

It is not exactly what I suggested. It also considers anything above 15 to be
a normal integer. However, it might be a cleaner way.

All-in-all, I'd say that the relevant hid-core.c code should have its comment
fixed, and the hid-input.c (hidinput_calc_abs_res) change needs to be reverted
as it (incorrectly) takes the component unit power into account for resolution
calculation and makes the "unit" item value handling harder to comprehend.

I'll prepare and test a patch and we can carry on from there.

Could you please share some of the report descriptors which you found
problematic, so that I can have a better understanding of the problem?
If you want to have a look at some multitouch descriptors (and few
other devices), I started to build a database of hid devices[3].

I have my repository of tablet diagnostics published at
https://github.com/DIGImend/devices

The original report descriptors are in */rd/original*.txt files and their XML
representation is in */rd/original*.xml files. Most (if not all) of them have
the "unit exponent" as nibble.

The XML representation was created by my "hidrd-convert" tool [1], which can
also output binary, specification example format and code from either binary
or XML input.

I use it to generate fixed report descriptors for graphics tablets. Maybe you
can find it useful as well. However, it interprets the "unit exponent" per
specification, i.e. just as an integer.

[1] http://msdn.microsoft.com/en-us/library/windows/hardware/dn383621.aspx
[2] http://feishare.com/attachments/article/299/windows-pointer-device-protocol.pdf
[3] https://github.com/bentiss/hid-devices

PS: I'll be mostly offline until the 1st of November when I'll finish
my transfer to the RH Westford Office.
PPS: my nick on the RH internal IRC is bentiss :)

My nick there is "nkondrashov". However, I would prefer to keep this
discussion on the mailing list.

Good luck in the US!

Sincerely,
Nick

[1] https://sf.net/apps/mediawiki/digimend?title=Hidrd
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux