Hope to have taken everying in consideration, and not messed up things while trying to account both for your and Peter's comments. Also: I'm going to send an independent v2 with a link to v1 after the "---". I'm not sure whether this is the right thing to do for a single patch like this one. >> +Many HID work out the box, even if their hardware is different. > > FWIW, I always refer to a device following the HID standard to a "HID > device". Done >> +For example, mouses can have a different number of buttons; they > > s/mouses/mice/ Ok >> +The HID subsystem is in charge of parsing the HID report descriptors, >> +and of converts HID events into normal input >> +device interfaces (see Documentation/hid/hiddev.rst). > > hiddev is deprecated I would say. There are still a few users, but I'm > not sure they are quite actively developping products. hidraw is what > you want when you want to talk to the HID devices at the raw level, and > evdev (/dev/input/event*) is what libinput and Xorg consume. > Hm... hope to have linked the correct document this time (hid-transport.rst). >> +The basic structure of a HID report descriptor is defined in the HIDUDC manual, while >> +HIDUT "defines constants that can be interpreted by an application to >> +identify the purpose and meaning of a data >> +field in a HID report". Each entry is defined by at least two bytes, >> +where the first one defines what type of value is following, >> +and is described in the HIDUDC manual, >> +and the second one carries the actual value, >> +and is described in the HIDUT manual. > > I think the next part up to the various tools that allow to decode the report descriptors > is interesting, but should probably be in a separate file, or in a footnote. > > IMO, what matters here is: > - it's a "simple" language defined in those 2 documents > - it's working as a stack: each elements adds to the previous, and > summing everything gives you the overview (there are subtleties of > course) > - noone should ever try to read it by hand, there are tools :) > - in case there is something wrong in the report descriptor, then yes > you'll need that explanation, but it's probably too early in the doc > to explain this IMO > > I'm not sure it's better to have is somewhere else. It's true that one should use tools. We both agree that you really need to get the meaning of those numbers in order to be able to fix a HID device. The problem is that if you are not going to scratch your head over those two manuals you are not going to understand neither the meaning of the hex numbers, nor the meaning of the parsing tool output. At least.... that's what happened to me. Anyway: I've tried to emphasize the points you made above, and to move the hand-parsing out of the way, into a different file. >> +.. Parsing the mouse HID report descriptor with `hidrdd <https://github.com/abend0c1/hidrdd>`_ one gets:: > > Maybe I'm too biased, but why adding the hidrdd output when you also have > the one from hid-tools? It was commented (".."): after writing it, I realized that it did not add that much; still, I did not want to throw away the text, just in case. I did non realize that you would have reviewed the raw patch, where the fact that it is commented is not at all obvious. Apologize for this. Deleted. I'm leaving, if you agree, the links to the online tool and to hiddrd. >> + >> +Outputs and Inputs >> +------------------ >> + >> +An HID devices can have inputs, like >> +in the mouse example, and outputs. >> +"Output" means that the information is fed >> +from the device to the human; for examples, > > The other way around: outputs are from the host (computer/human) to the > device, when input is from the device to the host. > I've tried to rephrase it, this time without mentioning the human (that perhaps is confusing). >> +a joystick with force feedback will have >> +some output. > > There are also Features, which is a side channel configuration of the > device from the host to the device. > > Most of the time Features have a state (are you using high reslution > wheel events?) and can be queried from the host. Most of the time :) > Added Feature items (hope to have got them right). >> +Report IDs and Evdev events >> +=========================== >> + >> +A single device can logically group >> +data into different, independent sets. >> +It is *as if* the HID presents >> +itself as different devices, each exchanging >> +its own data. The HID report descriptor is unique, >> +but the different reports are identified by means >> +of different ``Report ID`` fields. Whenever a ``Report ID`` >> +is needed it is transmitted as the first byte of any report. > > I wouldn't say this like that. > > The following is an attempt to explain to you the slight difference > between collections and report IDs, so it should not be taken verbatim > in the doc. > > You can group data by using "Collections". Each collection has a type > and purpose. You have "application" collections, "physical" collections > and "logical" collections. For each you can assign a purpose: for > example a touchpanel that exports a touchscreen and a stylus would > export 2 application collections of "Touchscreen" and "Pen". > > But then to be able to differentiate those collections, we have the > "Report ID", which is handled specifically in the HID standard as the > first byte (when defined) associated to a collection. > > But given that collections can be stacked, there is not a 1-to-1 > relation between Report IDs and all defined collection. > I've tried to rephrase the whole paragraph, also on the light of your explanation below that Linux builds a /dev/input/event* for each Application Collection, and not for each Report ID. >> +one can see that the device presents two mouses > > s/mouses/mice/ > Done >> +Events >> +====== >> + >> +One can expect that different ``/dev/input/event*`` are created for different >> +Report IDs. Going back to the mouse example, and repeating the sequence where >> +one clicks and holds button 1, then clicks and holds button 2, >> +releases button 1, and finally releases button 2, one gets:: >> + >> + marco@sun:~> sudo evtest /dev/input/event4 > > evtest has been deprecated for a while, and evemu too. Now, cool kids > are using "libinput record", but it's slightly more verbose. > > Using evemu is probably better at least. > Changed to libinput. Do you think I should trim the initial output? I'm mentioning evemu at the bottom of the paragraph. >> +When everything works well >> +========================== >> + >> +* The HID report descriptor makes sense; > > i.e. the current tools are capable of making some sense out of it :) > This is not what I meant; from the few emails I exchanged with you guys I got the impression that it's possible, for some HID report descriptors, to be almost completely bogus. Should this be the case, then the kernel could still able to make some sense out of it, but nevertheless it would not be bad to fix the report descriptor in the first place. Anyway, I've got rid of this Section, and merged it with the "When something does not work", as suggested by Peter. Hope it's better now. >> +* It is possible to verify, by reading the raw hid data, that >> + the HID report descriptor *does match* what is sent by the device; > > nitpick: extra space between "does" and "match" > Fixed. >> +* The HID report descriptor does not need any "quirk"s (see later on) >> +* For any Report ID a corresponding ``/dev/input/event*`` is created, >> + and the events there match what you would expect > > That last point is not stricly correct. Currently, each application > collection gets its own input node. You can have a collection with > several report IDs because they are all using the same device but a > different language. Changed. Thank you for the explanation! >> +When something does not work >> +============================ >> + >> +Sometimes not everything does work as it should. > > *not everything works Ok. >> + * ``HID_QUIRK_NO_IGNORE``, defined as ``BIT(30)``: >> + * ``HID_QUIRK_NO_INPUT_SYNC``, defined as ``BIT(31)``: > > We should definitely include the doc directly in hid.h and include it > there. I already explained why in the cover letter. > Good point. See whether the current solution (both in the doc and in hid.h) works for you . I was wondering whether it could be useful to add a :block: option to the kernel-doc directive, with the meaning that all the comments in between a :DOC and an :END_DOC should be included in the documentation. This could have allowed to have the documentation of the #defines line by line. After realizing that I should have modified scripts/kernel-doc (perl: I know nothing about it :( ) I quickly back-pedaled away from that overengineered solution. I have verified that It's possible to repeat the DOC: HID quirks comments, and they all get included, but it would still require at least a two-lines (three?) comment in between every #define in the source code, and I don't think you would want this. > But moreover, quirks are supposed to be the exception. If we are adding > too many quirks, and that the same devices work on Windows without a > special handling, that means that the quirk should be analyzed, and we > should probably rethink the processing of the HID devices to not have > this quirk. I have changed "common quirk." into "already known quirk." Ok for you? >> +Fixing the HID report descriptor >> +-------------------------------- >> + >> +Should you need to patch the HID report descriptor >> +the easiest way is to resort to eBPF, as described >> +in Documentation/output/hid/hid-bpf.rst. >> + >> +Basically, you can change any byte of the original report descriptor. >> +The examples in samples/hid should be relatively straightforward, >> +see e.g. samples/hid_mouse.bpf.c:: >> + >> + SEC("fmod_ret/hid_bpf_rdesc_fixup") >> + int BPF_PROG(hid_rdesc_fixup, struct hid_bpf_ctx *hctx) >> + { >> + .... >> + data[39] = 0x31; >> + data[41] = 0x30; >> + return 0; >> + } > > If you ever do that, please share your code on the LKML. The current > background work is to integrate those hid-bpf programs in the kernel > directly, so that we can share them with everyone without resorting to a > third party userspace program. Added three sentences about this. Too much? Btw: would you want me to add something like "If you are completely lost you could, as a last resort, try recording hidraw with hid-recorder (from the hid-tool utility set) and sending it to mailto://linux-input@xxxxxxxxxxxxxxx"; ??? :-/ (if yes: the mailing list filters too big messages; for example the recording I sent you did not reach the mailing list because the attachment was too big; how to deal with this?) >> +Writing a specialized driver >> +---------------------------- >> + >> +This should really be your last resort. > > YES, definitely YES :) > :)
