* Re: [PATCH v3 1/4] Input: ts-overlay - Add touchscreen overlay object handling
From: Javier Carrasco @ 2023-06-29 7:53 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <ZJz6jnt9hDjuU3du@nixie71>
Hi Jeff,
On 29.06.23 05:29, Jeff LaBundy wrote:
> Hi Javier,
>
> On Wed, Jun 28, 2023 at 08:44:51AM +0200, Javier Carrasco wrote:
>
> [...]
>
>>>>>> +static const char *const ts_overlay_names[] = {
>>>>>> + [TOUCHSCREEN] = "overlay-touchscreen",
>>>>>
>>>>> I'm a little confused why we need new code for this particular function; it's
>>>>> what touchscreen-min-x/y and touchscreen-size-x/y were meant to define. Why
>>>>> can't we keep using those?
>>>>>
>>>> According to the bindings, touchscreen-min-x/y define the minimum
>>>> reported values, but the overlay-touchscreen is actually setting a new
>>>> origin. Therefore I might be misusing those properties. On the other
>>>> hand touchscreen-size-x/y would make more sense, but I also considered
>>>> the case where someone would like to describe the real size of the
>>>> touchscreen outside of the overlay node as well as the clipped size
>>>> inside the node. In that case using the same property twice would be
>>>> confusing.
>>>> So in the end I thought that the origin/size properties are more precise
>>>> and applicable for all objects and not only the overlay touchscreen.
>>>> These properties are needed for the buttons anyways and in the future
>>>> more overlay would use the same properties.
>>>
>>> Ah, I understand now. touchscreen-min-x/y define the lower limits of the axes
>>> reported to input but they don't move the origin. I'm aligned with the reason
>>> to introduce this function.
>>>
>>> This does beg the question as to whether we need two separate types of children
>>> and related parsing code. Can we not simply have one overlay definition, and
>>> make the decision as to whether we are dealing with a border or virtual button
>>> based on whether 'linux,code' is present?
>>>
>> A single overlay definition would be possible, but in case more objects
>> are added in the future, looking for single properties and then deciding
>> what object it is might get messy pretty fast. You could end up needing
>> a decision tree and the definition in the DT would get more complex.
>>
>> Now the decision tree is straightforward (linux,code -> button), but
>> that might not always be the case. In the current implementation there
>> are well-defined objects and adding a new one will never affect the
>> parsing of the rest.
>> Therefore I would like to keep it more readable and easily extendable.
>
> As a potential customer of this feature, I'm ultimately looking to describe
> the hardware as succinctly as possible. Currently we have two overlay types,
> a border and button(s). The former will never have linux,code defined, while
> the latter will. From my naive perspective, it seems redundant to define the
> overlay types differently when their properties imply the difference already.
>
> Ultimately it seems we are simply dealing with generic "segments" scattered
> throughout a larger touch surface. These segments start to look something
> like the following:
>
> struct touch_segment {
> unsigned int x_origin;
> unsigned int y_origin;
> unsigned int x_size;
> unsigned int y_size;
> unsigned int code;
> };
>
> You then have one exported function akin to touchscreen_parse_properties() that
> simply walks the parent device looking for children named "touch-segment-0",
> "touch-segment-1", etc. and parses the five properties, with the fifth (keycode)
> being optional.
>
> And then, you have one last exported function akin to touchscreen_report_pos()
> that processes the touch coordinates. If the coordinates are in a given segment
> and segment->code == KEY_RESERVED (i.e. linux,code was never given), then this
> function simply passes the shifted coordinates to touchscreen_report_pos().
>
> If however segment->code != KEY_RESERVED, it calls input_report_key() based on
> whether the coordinates are within the segment. If this simplified solution
> shrinks the code enough, it may even make sense to keep it in touchscreen.c
> which this new feature is so tightly coupled to anyway.
>
> I'm sure the devil is in the details however, and I understand the value in
> future-proofing. Can you help me understand a potential future case where this
> simplified view would break, and the existing definitions would be better?
>
> Kind regards,
> Jeff LaBundy
I agree that your approach would reduce the code and then moving this
feature to touchscreen.c would be reasonable. So if in the end that is
the desired solution, I will go for it. But there are some points where
I think the bit of extra code would be worth it.
From a DT perspective, I can imagine some scenarios where a bunch of
segments scattered around would be messy. An example would be a keypad
with let's say N=9 buttons. It could be described easily with a buttons
node and the keys inside. Understanding what the node describes would be
straightforward as well, let alone N being much bigger.
You could argue that the buttons node could have segments inside instead
of buttons, but in the case where a cropped touchscreen is also
described, you would end up with a segment outside the buttons node and
the rest inside. That would reduce the parsing savings. Some labeling
would help in that case, but that would be not as clear as the current
implementation.
There is another point that I will just touch upon because I have no
experience in the matter. I have seen that some keys use the
'linux,input-type' property to define themselves as keys, switches, etc.
If that property or any other that I do not know is necessary for other
implementations, the button object will cover them better than a generic
segment where half of the properties would be meaningless in some
scenarios. Buttons/keys are so ubiquitous that a dedicated object for
them does not look that bad imho.
But as I said, I do not want to make a strong statement here because I
have seen that you maintain several bindings where this properties are
present and I am not the right person to explain that to you... or
actually anyone else out there :)
Talking about the code itself, having a structure for buttons is handy
because you can keep track of the button status (e.g. pressed) and in
the end it is just a child of the base shape that is used for the
overlay touchscreen. The same applies to any function that handles
buttons: they just wrap around the shape functions and add the
button-specific management. So if the parsing is taken aside, the code
does not get much savings from that side and it is again much more
readable and comprehensible.
Thank you for your efforts to improve these patches and the constructive
discussion.
Best regards,
Javier Carrasco
^ permalink raw reply
* Re: [PATCH v3 1/4] Input: ts-overlay - Add touchscreen overlay object handling
From: Jeff LaBundy @ 2023-06-29 3:29 UTC (permalink / raw)
To: Javier Carrasco
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <543b2d74-eac4-3856-17c6-0654459a91b4@wolfvision.net>
Hi Javier,
On Wed, Jun 28, 2023 at 08:44:51AM +0200, Javier Carrasco wrote:
[...]
> >>>> +static const char *const ts_overlay_names[] = {
> >>>> + [TOUCHSCREEN] = "overlay-touchscreen",
> >>>
> >>> I'm a little confused why we need new code for this particular function; it's
> >>> what touchscreen-min-x/y and touchscreen-size-x/y were meant to define. Why
> >>> can't we keep using those?
> >>>
> >> According to the bindings, touchscreen-min-x/y define the minimum
> >> reported values, but the overlay-touchscreen is actually setting a new
> >> origin. Therefore I might be misusing those properties. On the other
> >> hand touchscreen-size-x/y would make more sense, but I also considered
> >> the case where someone would like to describe the real size of the
> >> touchscreen outside of the overlay node as well as the clipped size
> >> inside the node. In that case using the same property twice would be
> >> confusing.
> >> So in the end I thought that the origin/size properties are more precise
> >> and applicable for all objects and not only the overlay touchscreen.
> >> These properties are needed for the buttons anyways and in the future
> >> more overlay would use the same properties.
> >
> > Ah, I understand now. touchscreen-min-x/y define the lower limits of the axes
> > reported to input but they don't move the origin. I'm aligned with the reason
> > to introduce this function.
> >
> > This does beg the question as to whether we need two separate types of children
> > and related parsing code. Can we not simply have one overlay definition, and
> > make the decision as to whether we are dealing with a border or virtual button
> > based on whether 'linux,code' is present?
> >
> A single overlay definition would be possible, but in case more objects
> are added in the future, looking for single properties and then deciding
> what object it is might get messy pretty fast. You could end up needing
> a decision tree and the definition in the DT would get more complex.
>
> Now the decision tree is straightforward (linux,code -> button), but
> that might not always be the case. In the current implementation there
> are well-defined objects and adding a new one will never affect the
> parsing of the rest.
> Therefore I would like to keep it more readable and easily extendable.
As a potential customer of this feature, I'm ultimately looking to describe
the hardware as succinctly as possible. Currently we have two overlay types,
a border and button(s). The former will never have linux,code defined, while
the latter will. From my naive perspective, it seems redundant to define the
overlay types differently when their properties imply the difference already.
Ultimately it seems we are simply dealing with generic "segments" scattered
throughout a larger touch surface. These segments start to look something
like the following:
struct touch_segment {
unsigned int x_origin;
unsigned int y_origin;
unsigned int x_size;
unsigned int y_size;
unsigned int code;
};
You then have one exported function akin to touchscreen_parse_properties() that
simply walks the parent device looking for children named "touch-segment-0",
"touch-segment-1", etc. and parses the five properties, with the fifth (keycode)
being optional.
And then, you have one last exported function akin to touchscreen_report_pos()
that processes the touch coordinates. If the coordinates are in a given segment
and segment->code == KEY_RESERVED (i.e. linux,code was never given), then this
function simply passes the shifted coordinates to touchscreen_report_pos().
If however segment->code != KEY_RESERVED, it calls input_report_key() based on
whether the coordinates are within the segment. If this simplified solution
shrinks the code enough, it may even make sense to keep it in touchscreen.c
which this new feature is so tightly coupled to anyway.
I'm sure the devil is in the details however, and I understand the value in
future-proofing. Can you help me understand a potential future case where this
simplified view would break, and the existing definitions would be better?
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Touchpad lost sync at byte 6
From: Jonathan Denose @ 2023-06-28 21:00 UTC (permalink / raw)
To: linux-input; +Cc: gregkh
I am on a Lenovo N24 laptop.
When this device resumes after closing the lid, the mouse freezes and
is unusable. After right-clicking multiple times the touchpad resumes
working. I am seeing the following in the logs:
[ 1188.690138] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1188.699636] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1188.709124] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1188.718557] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1188.728110] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1188.975964] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1188.985456] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1188.994954] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1189.004443] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
[ 1189.013930] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
sync at byte 6
This device has an ETPS/2 Elantech Touchpad. I am on kernel version:
5.15.117-19629-gf1eb3139d206
Currently the device suspends to ram when the lid closes. When the
device idles without the lid closing, the touchpad works fine on
resume.
I have tried adding various i8042 and psmouse kernel parameters from
searching different forums related to these error messages, but none
of them fix the issue. I have tried
- psmouse.resetafter=0
- psmouse.resetafter=1
- i8042.nomux=1
- i8042.resetafter=1/0
- i8042.reset=1 i8042.nomux=1
- i8042.noloop=1 i8042.nopnp=1 i8042.nomux=1 i8042.reset=1
I was able to resolve this issue by doing `modprobe -r psmouse &&
modprobe.psmouse proto=imps`. proto=bare and proto=exps also resolve
the issue. However, with those options enabled I can no longer use
gestures like two-finger scroll.
This issue reproduces on ChromeOS Flex and Ubuntu 22.04 but not on
Windows. On Windows the touchpad resuming after closing the lid works
fine.
What can I do to resolve this issue while also keeping touchpad gestures?
Any help is greatly appreciated!
^ permalink raw reply
* Re: [PATCH v4 2/2] Input: cap11xx - add advanced sensitivity settings
From: Dmitry Torokhov @ 2023-06-28 19:06 UTC (permalink / raw)
To: Jiri Valek - 2N
Cc: krzysztof.kozlowski+dt, devicetree, linux-input, linux-kernel,
robh+dt, u.kleine-koenig
In-Reply-To: <20230627065316.1065911-3-jiriv@axis.com>
Hi Jiri,
On Tue, Jun 27, 2023 at 08:53:16AM +0200, Jiri Valek - 2N wrote:
> @@ -439,7 +589,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
> priv->idev->id.bustype = BUS_I2C;
> priv->idev->evbit[0] = BIT_MASK(EV_KEY);
>
> - if (of_property_read_bool(node, "autorepeat"))
> + if (of_property_read_bool(dev->of_node, "autorepeat"))
It would be good to have this driver switched from of_property_*() to
device_property_() API.
> __set_bit(EV_REP, priv->idev->evbit);
>
> for (i = 0; i < cap->num_channels; i++)
> @@ -474,14 +624,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
> if (error)
> return error;
>
> - irq = irq_of_parse_and_map(node, 0);
> - if (!irq) {
> - dev_err(dev, "Unable to parse or map IRQ\n");
> - return -ENXIO;
> - }
> -
> - error = devm_request_threaded_irq(dev, irq, NULL, cap11xx_thread_func,
> - IRQF_ONESHOT, dev_name(dev), priv);
> + error = devm_request_threaded_irq(dev, i2c_client->irq, NULL,
> + cap11xx_thread_func, IRQF_ONESHOT, dev_name(dev), priv);
I would prefer this change be a separate patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: Add introduction about HID for non-kernel programmers
From: Marco Morandini @ 2023-06-28 13:37 UTC (permalink / raw)
To: Peter Hutterer
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, Jonathan Corbet,
linux-doc
In-Reply-To: <20230627060437.GA726439@quokka>
On 6/27/23 08:04, Peter Hutterer wrote:
> On Thu, Jun 22, 2023 at 10:39:15PM +0200, Marco Morandini wrote:
>> Add an introduction about HID
>> meant for the casual programmers that is trying
>> either to fix his device or to understand what
>> is going wrong.
>>
>> Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
>> ---
>> v1: https://lore.kernel.org/linux-input/3mbw67akm2xzd2kgzb6sdfh4dly6im5jrz5umuvczjvrgxtf46@q5ooib3zkmfq/T/#m00b625a4d2c605dd7f62a866df7bf97ef2921d63
>
> Thanks for all the fixes, much easier to read and follow already. A few
> nitpicks like last time but this is coming together!
:)
Unfortunately while reading hidreport-parsing.rst I found it not very clear, at least for me.
I tried to rearrange things there, but I fear we'll need v4 for that file :/
It would be great if you could double-check it, especially the reference to the
spec; for example, I think one should look into HID spec 6.2.2.2 (and not HID spec 6.2.2.7 as written in v2)
for inferring the Type of the item (Global in this case). From there you jump to HID spec 6.2.2.7
that defines the different Global items; without looking into HID spec 6.2.2.2 you
all you can do is to browse the spec until you find the bit sequence "0000 01 nn" in HID spec 6.2.2.7.
I've also finished "parsing" the fourth byte at the end of the document.
I'm going to wait a few more days for Banjamin's comments, if any,
then will send v3. At any rate, I'm attaching the current diff at the bottom of this email
(what I would send, right now, as v3).
On a side note: after sending v2 I've realized that _lot_ of sentences
now come from you. Thus, for v3, I think it would be better to send something like
Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
Co-developed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Ok for you?
If yes: I don't have clear whether I should send
Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
Co-developed-by: Peter Hutterer <peter.hutterer@who-t.net>
and you should reply with
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
or I should simply write the Signed-off-by on your behalf.
> A general request - can you reformat the paragraphs please to be roughly
> the max (or just the same-ish) line lengths. Some of these paragraphs
> read more like a poem
> with superfluous
> linebreaks
> sprinkled all over it.
>
Should be done
>> +may specify that "in a report with ID 3 the second byte is the delta x
>> +coordinate of a mouse".
>
> staring at this and if you're doing a v3 anyway, I'd say "bits 8 to 15"
> instead of "second byte" to emphasise that HID reports are usually more
> bit-y than byte-y. But that's a minor nitpick that doesn't matter too
> much.
Ok.
>> +This does not mean that the HID subsystem can deal with USB devices only;
>> +rather, different transport drivers, such as I2C or Bluetooth, can be dealt
>> +with, see Documentation/hid/hid-transport.rst.
>
> Simpler wording: "The HID subsystem can deal with different transport drivers
> (USB, I2C, Bluetooth, etc.). See Documentation/hid/hid-transport.rst."
>
Ok
>> +where the first one defines what type of value is following,
>> +and is described in the HID spec,
>
> no comma before "and" in this case
>
Ok
>> +while the second one carries the actual value,
>
> superfluous comma-man to the rescue! :)
:)
>> +A short introduction
>> +on how to do this is sketched in Documentation/hid/hidreport-parsing.rst;
>> +you need to understand it only if you need to patch HID report descriptors.
>
> I think this should be "you only need to ..." (but I'm ESL too)
>
I've double-checked with an ESL girl in my Department that should be good with
English, and she thinks that you are right.
>> + marco@sun:~/Programmi/linux/hid-tools (master =)> ./hid-decode /sys/bus/hid/devices/0003\:093A\:2510.0002/report_descriptor
>
> shorten the prompt to just "$ hid-decode ..." and it'll fit better on a
> single line
Ok, everywhere.
>> + and take Constant (Cnst) values
>
> "i.e. they can be ignored"
Ok
>> + The mouse has also two physical positions (``Usage (X)``, ``Usage (Y)``)
>
> too many spaces here
>
Fixed
>> + # 0x81, 0x06, // Input (Data,Var,Rel) 48
>> + This time the data is Relative (Rel), i.e. it represent
>> + the change from the previous configuration
>
> "the previously sent report (event)"
>
Ok
>> + marco@sun:~/> sudo ./hid-recorder /dev/hidraw1
>
> for consistency please change the prompt here to `$` as well
>
Done
>> +An HID devices can have Input Reports, like
>> +in the mouse example, Output Reports, and Features Reports.
>
> s/Features/Feature/
>
Ok
>> +"Output" means that the information is fed
>
> s/fed/sent/
>
Ok
>> +"Input" means that data
>> +come from the device.
>
> s/come/comes/
> Not sure, never got the hang of singular vs plural data, whatever
> is used in the rest of the documentation.
>
I tried to digest
https://grammarist.com/usage/data/
and I'm more confused than before reading it.
I searched the document for "data is" and ended up changing it into "data values are" in
This time the data values are Relative (Rel), i.e. they represent
and
remember that the data values are *absolute*
Not 100% sure with "data value is" vs. "data values are" in the last one
(in the first one we are explicitly writing about bits).
Besides that, we have
"Input" means that data come from the device.
The data sent will begin
For example, the data transmitted for the last consumer control::
All the Input data sent
to modify the data exchanged with the device.
where I think we are fine.
>> +A single device can logically group
>> +data into different, independent sets,
>> +called *Collection*.
>
> "called a *Collection*"
Ok
>
>> +Collections can be nested, and there
>> +are different types of collections
>> +(see the HID spec 6.2.2.6
>> +"Collection, End Collection Items" for details).
>> +
>> +Different reports are identified by means
>> +of different *Report ID* fields, i.e. a number identifying
>> +the structure of the immediately following report.
>> +Whenever a Report ID
>> +is needed it is transmitted as the first byte of any report.
>
> "A device with only one supported HID report (like the mouse example above)
> may omit the report ID."
>
Added.
>> +All the Input data sent by the device should be translated into corresponding
>> +Evdev events, so that the remaining part of the stack can know what is going on,
>> +e.g. that a mouse button was pressed, rather
>> +than a mouse has been moved in the X direction.
>
> This example is so obvious that it's actually confusing, a better
> example may be: "e.g. the bit for the first button translates into the
> EV_KEY/BTN_LEFT evdev event and relative x movement translates into the
> EV_REL/REL_X evdev event".
>
Ok
>> + marco@sun:~> sudo libinput record /dev/input/event1
>
> $ prompt reminder
>
:)
>> + * it is possible to verify, by reading the raw HID data, that
>> + the HID report descriptor *does not match* what is actually
>> + sent by the device;
>
> * the HID report descriptor *does not match* what is actually
> sent by the device (this can be verified by reading the raw HID
> data),
>
Ok
>> +
>> +or
>> +
>
> skip the or, we're already in an enumeration anyway so this can be a
> single list
>
Ok
>
> s/suitable//
> s/will/may/
>
Ok
>> +for each Application Collection, and/or the events
>> +there will match what you would expect.
>
> s/will/may not/
>
Ok
>> +This can be done in file `drivers/hid/hid-quirks.c`.
>
> s/file/the file/
>
Ok
>> +See, again, Documentation/process/submitting-patches.rst
>
> s/, again,// (sounds too patronising otherwise)
>
Ok
>> + marco@sun:~> sudo ./hid-example
>
> $ prompt reminder
>
Ok
>> + +----------+
>> + | 00000101 |
>> + +----------+
>> + ^^
>> + ---- Length of data (see HID spec 6.2.2.2)
>> + ^^
>> + ------ Type of the item (see HID spec 6.2.2.7)
>> + ^^^^
>> + --------- Function of the item (see HUT Sec 3)
>
> this is misaligned
>
Thank you for noticing! And I've replaced tabs with spaces there.
Marco
=================================
From: Marco Morandini <marco.morandini@polimi.it>
Add an introduction about HID
meant for the casual programmers that is trying
either to fix his device or to understand what
is going wrong.
Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
Co-developed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
v2: https://lore.kernel.org/linux-input/20230627060437.GA726439@quokka/T/#t
changes:
- corrections suggested in https://lore.kernel.org/linux-input/20230627060437.GA726439@quokka/T/#m21b0a280e2ce2b21332ca31cfafaddf44a1b5fb1
- corrections suggested in https://lore.kernel.org/linux-input/20230627060437.GA726439@quokka/T/#mafb91d69de2d56e6a1eeff5ba4ed88a764cf559e
- some rewording of Documentation/hid/hidreport-parsing.rst
--- /dev/null 2023-06-28 09:46:06.188159532 +0200
+++ Documentation/hid/hidintro.rst 2023-06-28 15:26:31.543904010 +0200
@@ -0,0 +1,510 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======================================
+Introduction to HID report descriptors
+======================================
+
+This chapter is meant to give a broad overview of what HID report
+descriptors are, and of how a casual (non-kernel) programmer can deal
+with HID devices that are not working well with Linux.
+
+.. contents::
+ :local:
+ :depth: 2
+
+.. toctree::
+ :maxdepth: 2
+
+ hidreport-parsing
+
+
+Introduction
+============
+
+HID stands for Human Interface Device, and can be whatever device you
+are using to interact with a computer, be it a mouse, a touchpad, a
+tablet, a microphone.
+
+Many HID devices work out the box, even if their hardware is different.
+For example, mice can have any number of buttons; they may have a
+wheel; movement sensitivity differs between different models, and so
+on. Nonetheless, most of the time everything just works, without the
+need to have specialized code in the kernel for every mouse model
+developed since 1970.
+
+This is because modern HID devices do advertise their capabilities
+through the *HID report descriptor*, a fixed set of bytes describing
+exactly what *HID reports* may be sent between the device and the host
+and the meaning of each individual bit in those reports. For example,
+a HID Report Descriptor may specify that "in a report with ID 3 the
+bits from 8 to 15 is the delta x coordinate of a mouse".
+
+The HID report itself then merely carries the actual data values
+without any extra meta information. Note that HID reports may be sent
+from the device ("Input Reports", i.e. input events), to the device
+("Output Reports" to e.g. change LEDs) or used for device configuration
+("Feature reports"). A device may support one or more HID reports.
+
+The HID subsystem is in charge of parsing the HID report descriptors,
+and converts HID events into normal input device interfaces (see
+Documentation/hid/hid-transport.rst). Devices may misbehave because the
+HID report descriptor provided by the device is wrong, or because it
+needs to be dealt with in a special way, or because some special
+device or interaction mode is not handled by the default code.
+
+The format of HID report descriptors is described by two documents,
+available from the `USB Implementers Forum <https://www.usb.org/>`_
+`HID web page <https://www.usb.org/hid>`_ address:
+
+ * the `HID USB Device Class Definition
+ <https://www.usb.org/document-library/device-class-definition-hid-111>`_ (HID Spec from now on)
+ * the `HID Usage Tables <https://usb.org/document-library/hid-usage-tables-14>`_ (HUT from now on)
+
+The HID subsystem can deal with different transport drivers
+(USB, I2C, Bluetooth, etc.). See Documentation/hid/hid-transport.rst.
+
+Parsing HID report descriptors
+==============================
+
+The current list of HID devices can be found at ``/sys/bus/hid/devices/``.
+For each device, say ``/sys/bus/hid/devices/0003\:093A\:2510.0002/``,
+one can read the corresponding report descriptor::
+
+ $ hexdump -C /sys/bus/hid/devices/0003\:093A\:2510.0002/report_descriptor
+ 00000000 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 03 |..............).|
+ 00000010 15 00 25 01 75 01 95 03 81 02 75 05 95 01 81 01 |..%.u.....u.....|
+ 00000020 05 01 09 30 09 31 09 38 15 81 25 7f 75 08 95 03 |...0.1.8..%.u...|
+ 00000030 81 06 c0 c0 |....|
+ 00000034
+
+Optional stuff: the HID report descriptor can be read also by
+directly accessing the hidraw driver [#hidraw]_.
+
+The basic structure of HID report descriptors is defined in the HID
+spec, while HUT "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 HID spec, while the second one carries the actual value and is
+described in the HUT.
+
+HID report descriptors can, in principle, be painstakingly parsed by
+hand, byte by byte.
+
+A short introduction on how to do this is sketched in
+Documentation/hid/hidreport-parsing.rst; you only need to understand it
+if you need to patch HID report descriptors.
+
+In practice you should not parse HID report descriptors by hand; rather,
+you should use an existing parser. Among all the available ones
+
+ * the online `USB Descriptor and Request Parser
+ <http://eleccelerator.com/usbdescreqparser/>`_;
+ * `hidrdd <https://github.com/abend0c1/hidrdd>`_,
+ that provides very detailed and somewhat verbose descriptions
+ (verbosity can be useful if you are not familiar with HID report
+ descriptors);
+ * `hid-tools <https://gitlab.freedesktop.org/libevdev/hid-tools>`_,
+ a complete utility set that allows, among other things,
+ to record and replay the raw HID reports and to debug
+ and replay HID devices.
+ It is being actively developed by the Linux HID subsystem maintainers.
+
+Parsing the mouse HID report descriptor with `hid-tools
+<https://gitlab.freedesktop.org/libevdev/hid-tools>`_ leads to
+(explanations interposed)::
+
+ $ ./hid-decode /sys/bus/hid/devices/0003\:093A\:2510.0002/report_descriptor
+ # device 0:0
+ # 0x05, 0x01, // Usage Page (Generic Desktop) 0
+ # 0x09, 0x02, // Usage (Mouse) 2
+ # 0xa1, 0x01, // Collection (Application) 4
+ # 0x09, 0x01, // Usage (Pointer) 6
+ # 0xa1, 0x00, // Collection (Physical) 8
+ # 0x05, 0x09, // Usage Page (Button) 10
+ what follows is a button
+
+ # 0x19, 0x01, // Usage Minimum (1) 12
+ # 0x29, 0x03, // Usage Maximum (3) 14
+ first button is button number 1, last button is button number 3
+
+ # 0x15, 0x00, // Logical Minimum (0) 16
+ # 0x25, 0x01, // Logical Maximum (1) 18
+ each button can send values from 0 up to including 1
+ (i.e. they are binary buttons)
+
+ # 0x75, 0x01, // Report Size (1) 20
+ each button is sent as exactly one bit
+
+ # 0x95, 0x03, // Report Count (3) 22
+ and there are three of those bits (matching the three buttons)
+
+ # 0x81, 0x02, // Input (Data,Var,Abs) 24
+ it's actual Data (not constant padding), they represent
+ a single variable (Var) and their values are Absolute (not relative);
+ See HID spec Sec. 6.2.2.5 "Input, Output, and Feature Items
+
+ # 0x75, 0x05, // Report Size (5) 26
+ five additional padding bits, needed to reach a byte
+
+ # 0x95, 0x01, // Report Count (1) 28
+ those five bits are repeated only once
+
+ # 0x81, 0x01, // Input (Cnst,Arr,Abs) 30
+ and take Constant (Cnst) values i.e. they can be ignored
+
+ # 0x05, 0x01, // Usage Page (Generic Desktop) 32
+ # 0x09, 0x30, // Usage (X) 34
+ # 0x09, 0x31, // Usage (Y) 36
+ # 0x09, 0x38, // Usage (Wheel) 38
+ The mouse has also two physical positions (Usage (X), Usage (Y))
+ and a wheel (Usage (Wheel))
+
+ # 0x15, 0x81, // Logical Minimum (-127) 40
+ # 0x25, 0x7f, // Logical Maximum (127) 42
+ each of them can send values ranging from -127 up to including 127
+
+ # 0x75, 0x08, // Report Size (8) 44
+ is represented by eight bits
+
+ # 0x95, 0x03, // Report Count (3) 46
+ and there are three of those eight bits, matching X, Y and Wheel
+
+ # 0x81, 0x06, // Input (Data,Var,Rel) 48
+ This time the data values are Relative (Rel), i.e. they represent
+ the change from the previously sent report (event)
+
+ # 0xc0, // End Collection 50
+ # 0xc0, // End Collection 51
+ #
+ R: 52 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 03 15 00 25 01 75 01 95 03 81 02 75 05 95 01 81 01 05 01 09 30 09 31 09 38 15 81 25 7f 75 08 95 03 81 06 c0 c0
+ N: device 0:0
+ I: 3 0001 0001
+
+
+This Report Descriptor tells us that the mouse input will be
+transmitted using four bytes: the first one for the buttons (three
+bits used, five for padding), the last three for the mouse X, Y and
+wheel changes, respectively.
+
+Indeed, for any event, the mouse will send a *report* of four bytes.
+We can check the values sent by resorting e.g. to the `hid-recorder`
+tool, from `hid-tools <https://gitlab.freedesktop.org/libevdev/hid-tools>`_:
+The sequence of bytes sent by clicking and releasing button 1, then button 2, then button 3 is::
+
+ $ sudo ./hid-recorder /dev/hidraw1
+
+ ....
+ output of hid-decode
+ ....
+
+ # Button: 1 0 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000000.000000 4 01 00 00 00
+ # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000000.183949 4 00 00 00 00
+ # Button: 0 1 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000001.959698 4 02 00 00 00
+ # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000002.103899 4 00 00 00 00
+ # Button: 0 0 1 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000004.855799 4 04 00 00 00
+ # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000005.103864 4 00 00 00 00
+
+This example shows that when button 2 is clicked,
+the bytes ``02 00 00 00`` are sent, and the immediately subsequent
+event (``00 00 00 00``) is the release of button 2 (no buttons are
+pressed, remember that the data values are *absolute*).
+
+If instead one clicks and holds button 1, then clicks and holds button
+2, releases button 1, and finally releases button 2, the reports are::
+
+ # Button: 1 0 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000044.175830 4 01 00 00 00
+ # Button: 1 1 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000045.975997 4 03 00 00 00
+ # Button: 0 1 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000047.407930 4 02 00 00 00
+ # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
+ E: 000049.199919 4 00 00 00 00
+
+where with ``03 00 00 00`` both buttons are pressed, and with the
+subsequent ``02 00 00 00`` button 1 is released while button 2 is still
+active.
+
+Output, Input and Feature Reports
+---------------------------------
+
+HID devices can have Input Reports, like in the mouse example, Output
+Reports, and Feature Reports. "Output" means that the information is
+sent to the device. For example, a joystick with force feedback will
+have some output; the led of a keyboard would need an output as well.
+"Input" means that data come from the device.
+
+"Feature"s are not meant to be consumed by the end user and define
+configuration options for the device. They can be queried from the host;
+when declared as *Volatile* they should be changed by the host.
+
+
+Collections, Report IDs and Evdev events
+========================================
+
+A single device can logically group data into different, independent
+sets, called a *Collection*. Collections can be nested, and there are
+different types of collections (see the HID spec 6.2.2.6
+"Collection, End Collection Items" for details).
+
+Different reports are identified by means of different *Report ID*
+fields, i.e. a number identifying the structure of the immediately
+following report.
+Whenever a Report ID is needed it is transmitted as the first byte of
+any report. A device with only one supported HID report (like the mouse
+example above) may omit the report ID.
+
+Consider the following HID report descriptor::
+
+ 05 01 09 02 A1 01 85 01 05 09 19 01 29 05 15 00
+ 25 01 95 05 75 01 81 02 95 01 75 03 81 01 05 01
+ 09 30 09 31 16 00 F8 26 FF 07 75 0C 95 02 81 06
+ 09 38 15 80 25 7F 75 08 95 01 81 06 05 0C 0A 38
+ 02 15 80 25 7F 75 08 95 01 81 06 C0 05 01 09 02
+ A1 01 85 02 05 09 19 01 29 05 15 00 25 01 95 05
+ 75 01 81 02 95 01 75 03 81 01 05 01 09 30 09 31
+ 16 00 F8 26 FF 07 75 0C 95 02 81 06 09 38 15 80
+ 25 7F 75 08 95 01 81 06 05 0C 0A 38 02 15 80 25
+ 7F 75 08 95 01 81 06 C0 05 01 09 07 A1 01 85 05
+ 05 07 15 00 25 01 09 29 09 3E 09 4B 09 4E 09 E3
+ 09 E8 09 E8 09 E8 75 01 95 08 81 02 95 00 81 01
+ C0 05 0C 09 01 A1 01 85 06 15 00 25 01 75 01 95
+ 01 09 3F 81 06 09 3F 81 06 09 3F 81 06 09 3F 81
+ 06 09 3F 81 06 09 3F 81 06 09 3F 81 06 09 3F 81
+ 06 C0 05 0C 09 01 A1 01 85 03 09 05 15 00 26 FF
+ 00 75 08 95 02 B1 02 C0
+
+After parsing it (try to parse it on your own using the suggested
+tools!) one can see that the device presents two ``Mouse`` Application
+Collections (with reports identified by Reports IDs 1 and 2,
+respectively), a ``Keypad`` Application Collection (whose report is
+identified by the Report ID 5) and two ``Consumer Controls`` Application
+Collections, (with Report IDs 6 and 3, respectively). Note, however,
+that you can have different Report IDs for the same Application
+Collection.
+
+The data sent will begin with the Report ID byte, and will be followed
+by the corresponding information. For example, the data transmitted for
+the last consumer control::
+
+ 0x05, 0x0C, // Usage Page (Consumer)
+ 0x09, 0x01, // Usage (Consumer Control)
+ 0xA1, 0x01, // Collection (Application)
+ 0x85, 0x03, // Report ID (3)
+ 0x09, 0x05, // Usage (Headphone)
+ 0x15, 0x00, // Logical Minimum (0)
+ 0x26, 0xFF, 0x00, // Logical Maximum (255)
+ 0x75, 0x08, // Report Size (8)
+ 0x95, 0x02, // Report Count (2)
+ 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
+ 0xC0, // End Collection
+
+will be of three bytes: the first for the Report ID (3), the next two
+for the headphone, with two (``Report Count (2)``) bytes
+(``Report Size (8)``), each ranging from 0 (``Logical Minimum (0)``)
+to 255 (``Logical Maximum (255)``).
+
+All the Input data sent by the device should be translated into
+corresponding Evdev events, so that the remaining part of the stack can
+know what is going on, e.g. the bit for the first button translates into
+the ``EV_KEY/BTN_LEFT`` evdev event and relative X movement translates
+into the ``EV_REL/REL_X`` evdev event".
+
+Events
+======
+
+In Linux, one ``/dev/input/event*`` is created for each ``Application
+Collection``. 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::
+
+ $ sudo libinput record /dev/input/event1
+ # libinput record
+ version: 1
+ ndevices: 1
+ libinput:
+ version: "1.23.0"
+ git: "unknown"
+ system:
+ os: "opensuse-tumbleweed:20230619"
+ kernel: "6.3.7-1-default"
+ dmi: "dmi:bvnHP:bvrU77Ver.01.05.00:bd03/24/2022:br5.0:efr20.29:svnHP:pnHPEliteBook64514inchG9NotebookPC:pvr:rvnHP:rn89D2:rvrKBCVersion14.1D.00:cvnHP:ct10:cvr:sku5Y3J1EA#ABZ:"
+ devices:
+ - node: /dev/input/event1
+ evdev:
+ # Name: PixArt HP USB Optical Mouse
+ # ID: bus 0x3 vendor 0x3f0 product 0x94a version 0x111
+ # Supported Events:
+ # Event type 0 (EV_SYN)
+ # Event type 1 (EV_KEY)
+ # Event code 272 (BTN_LEFT)
+ # Event code 273 (BTN_RIGHT)
+ # Event code 274 (BTN_MIDDLE)
+ # Event type 2 (EV_REL)
+ # Event code 0 (REL_X)
+ # Event code 1 (REL_Y)
+ # Event code 8 (REL_WHEEL)
+ # Event code 11 (REL_WHEEL_HI_RES)
+ # Event type 4 (EV_MSC)
+ # Event code 4 (MSC_SCAN)
+ # Properties:
+ name: "PixArt HP USB Optical Mouse"
+ id: [3, 1008, 2378, 273]
+ codes:
+ 0: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15] # EV_SYN
+ 1: [272, 273, 274] # EV_KEY
+ 2: [0, 1, 8, 11] # EV_REL
+ 4: [4] # EV_MSC
+ properties: []
+ hid: [
+ 0x05, 0x01, 0x09, 0x02, 0xa1, 0x01, 0x09, 0x01, 0xa1, 0x00, 0x05, 0x09, 0x19, 0x01, 0x29, 0x03,
+ 0x15, 0x00, 0x25, 0x01, 0x95, 0x08, 0x75, 0x01, 0x81, 0x02, 0x05, 0x01, 0x09, 0x30, 0x09, 0x31,
+ 0x09, 0x38, 0x15, 0x81, 0x25, 0x7f, 0x75, 0x08, 0x95, 0x03, 0x81, 0x06, 0xc0, 0xc0
+ ]
+ udev:
+ properties:
+ - ID_INPUT=1
+ - ID_INPUT_MOUSE=1
+ - LIBINPUT_DEVICE_GROUP=3/3f0/94a:usb-0000:05:00.3-2
+ quirks:
+ events:
+ # Current time is 12:31:56
+ - evdev:
+ - [ 0, 0, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
+ - [ 0, 0, 1, 272, 1] # EV_KEY / BTN_LEFT 1
+ - [ 0, 0, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +0ms
+ - evdev:
+ - [ 1, 207892, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
+ - [ 1, 207892, 1, 273, 1] # EV_KEY / BTN_RIGHT 1
+ - [ 1, 207892, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +1207ms
+ - evdev:
+ - [ 2, 367823, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
+ - [ 2, 367823, 1, 272, 0] # EV_KEY / BTN_LEFT 0
+ - [ 2, 367823, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +1160ms
+ # Current time is 12:32:00
+ - evdev:
+ - [ 3, 247617, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
+ - [ 3, 247617, 1, 273, 0] # EV_KEY / BTN_RIGHT 0
+ - [ 3, 247617, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +880ms
+
+Note: if ``libinput`` is not available on your system try using
+``evemu-record``.
+
+When something does not work
+============================
+
+There can be a number of reasons why a device does not behave
+correctly. For example
+
+* The HID report descriptor provided by the HID device may be wrong
+ because e.g.
+
+ * it does not follow the standard, so that the kernel
+ will not able to make sense of the HID report descriptor;
+ * the HID report descriptor *does not match* what is actually
+ sent by the device (this can be verified by reading the raw HID
+ data);
+* the HID report descriptor may need some "quirks" (see later on).
+
+As a consequence, a ``/dev/input/event*`` may not be created
+for each Application Collection, and/or the events
+there may not match what you would expect.
+
+
+Quirks
+------
+
+There are some known peculiarities of HID devices that the kernel
+knows how to fix - these are called the HID quirks and a list of those
+is available in `include/linux/hid.h`.
+
+Should this be the case, it should be enough to add the required quirk
+in the kernel, for the HID device at hand. This can be done in the file
+`drivers/hid/hid-quirks.c`. How to do it should be relatively
+straightforward after looking into the file.
+
+The list of currently defined quirks, from `include/linux/hid.h`, is
+
+.. kernel-doc:: include/linux/hid.h
+ :doc: HID quirks
+
+Quirks for USB devices can be specified while loading the usbhid module,
+see ``modinfo usbhid``, although the proper fix should go into
+hid-quirks.c and **be submitted upstream**.
+See Documentation/process/submitting-patches.rst for guidelines on how
+to submit a patch. Quirks for other busses need to go into hid-quirks.c.
+
+Fixing HID report descriptors
+-----------------------------
+
+Should you need to patch HID report descriptors the easiest way is to
+resort to eBPF, as described in Documentation/hid/hid-bpf.rst.
+
+Basically, you can change any byte of the original HID report
+descriptor. The examples in samples/hid should be a good starting point
+for your code, see e.g. `samples/hid/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;
+ }
+
+Of course this can be also done within the kernel source code, see e.g.
+`drivers/hid/hid-aureal.c` or `drivers/hid/hid-samsung.c` for a slightly
+more complex file.
+
+Check Documentation/hid/hidreport-parsing.rst if you need any help
+navigating the HID manuals and understanding the exact meaning of
+the HID report descriptor hex numbers.
+
+Whatever solution you come up with, please remember to **submit the
+fix to the HID maintainers**, so that it can be directly integrated in
+the kernel and that particular HID device will start working for
+everyone else. See Documentation/process/submitting-patches.rst for
+guidelines on how to do this.
+
+
+Modifying the transmitted data on the fly
+-----------------------------------------
+
+Using eBPF it is also possible to modify the data exchanged with the
+device. See again the examples in `samples/hid`.
+
+Again, **please post your fix**, so that it can be integrated in the
+kernel!
+
+Writing a specialized driver
+----------------------------
+
+This should really be your last resort.
+
+
+.. rubric:: Footnotes
+
+.. [#hidraw] read hidraw: see Documentation/hid/hidraw.rst and
+ file `samples/hidraw/hid-example.c` for an example.
+ The output of ``hid-example`` would be, for the same mouse::
+
+ $ sudo ./hid-example
+ Report Descriptor Size: 52
+ Report Descriptor:
+ 5 1 9 2 a1 1 9 1 a1 0 5 9 19 1 29 3 15 0 25 1 75 1 95 3 81 2 75 5 95 1 81 1 5 1 9 30 9 31 9 38 15 81 25 7f 75 8 95 3 81 6 c0 c0
+
+ Raw Name: PixArt USB Optical Mouse
+ Raw Phys: usb-0000:05:00.4-2.3/input0
+ Raw Info:
+ bustype: 3 (USB)
+ vendor: 0x093a
+ product: 0x2510
+ ...
--- /dev/null 2023-06-28 09:46:06.188159532 +0200
+++ Documentation/hid/hidreport-parsing.rst 2023-06-27 22:56:18.886315875 +0200
@@ -0,0 +1,49 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+========================================
+Manual parsing of HID report descriptors
+========================================
+
+Consider again the mouse HID report descriptor
+introduced in Documentation/hid/hidintro.rst::
+
+ $ hexdump -C /sys/bus/hid/devices/0003\:093A\:2510.0002/report_descriptor
+ 00000000 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 03 |..............).|
+ 00000010 15 00 25 01 75 01 95 03 81 02 75 05 95 01 81 01 |..%.u.....u.....|
+ 00000020 05 01 09 30 09 31 09 38 15 81 25 7f 75 08 95 03 |...0.1.8..%.u...|
+ 00000030 81 06 c0 c0 |....|
+ 00000034
+
+and try to parse it by hand.
+
+Start with the first number, 0x05: it carries 2 bits for the
+length of the item, 2 bits for the type of the item and 4 bits for the
+function::
+
+ +----------+
+ | 00000101 |
+ +----------+
+ ^^
+ ---- Length of data (see HID spec 6.2.2.2)
+ ^^
+ ------ Type of the item (see HID spec 6.2.2.2, then jump to 6.2.2.7)
+ ^^^^
+ --------- Function of the item (see HID spec 6.2.2.7, then HUT Sec 3)
+
+In our case, the length is 1 byte, the type is ``Global`` and the
+function is ``Usage Page``, thus for parsing the value 0x01 in the second byte
+we need to refer to HUT Sec 3.
+
+The second number is the actual data, and its meaning can be found in
+the HUT. We have a ``Usage Page``, thus we need to refer to HUT
+Sec. 3, "Usage Pages"; from there, one sees that ``0x01`` stands for
+``Generic Desktop Page``.
+
+Moving now to the second two bytes, and following the same scheme,
+``0x09`` (i.e. ``00001001``) will be followed by one byte (``01``)
+and is a ``Local`` item (``10``). Thus, the meaning of the remaining four bits
+(``0000``) is given in the HID spec Sec. 6.2.2.8 "Local Items", so that
+we have a ``Usage``. From HUT, Sec. 4, "Generic Desktop Page", we see that
+0x02 stands for ``Mouse``.
+
+The following numbers can be parsed in the same way.
diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst
index b2028f382f11..af02cf7cfa82 100644
--- a/Documentation/hid/index.rst
+++ b/Documentation/hid/index.rst
@@ -7,6 +7,7 @@ Human Interface Devices (HID)
.. toctree::
:maxdepth: 1
+ hidintro
hiddev
hidraw
hid-sensor
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7f2e8ba7d783..8d1699f355ec 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -341,6 +341,29 @@ struct hid_item {
*/
#define MAX_USBHID_BOOT_QUIRKS 4
+/**
+ * DOC: HID quirks
+ * | @HID_QUIRK_NOTOUCH:
+ * | @HID_QUIRK_IGNORE: ignore this device
+ * | @HID_QUIRK_NOGET:
+ * | @HID_QUIRK_HIDDEV_FORCE:
+ * | @HID_QUIRK_BADPAD:
+ * | @HID_QUIRK_MULTI_INPUT:
+ * | @HID_QUIRK_HIDINPUT_FORCE:
+ * | @HID_QUIRK_ALWAYS_POLL:
+ * | @HID_QUIRK_INPUT_PER_APP:
+ * | @HID_QUIRK_X_INVERT:
+ * | @HID_QUIRK_Y_INVERT:
+ * | @HID_QUIRK_SKIP_OUTPUT_REPORTS:
+ * | @HID_QUIRK_SKIP_OUTPUT_REPORT_ID:
+ * | @HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP:
+ * | @HID_QUIRK_HAVE_SPECIAL_DRIVER:
+ * | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE:
+ * | @HID_QUIRK_FULLSPEED_INTERVAL:
+ * | @HID_QUIRK_NO_INIT_REPORTS:
+ * | @HID_QUIRK_NO_IGNORE:
+ * | @HID_QUIRK_NO_INPUT_SYNC:
+ */
/* BIT(0) reserved for backward compatibility, was HID_QUIRK_INVERT */
#define HID_QUIRK_NOTOUCH BIT(1)
#define HID_QUIRK_IGNORE BIT(2)
^ permalink raw reply related
* Re: [RESEND PATCH 2/2] HID: logitech-hidpp: Add support for the Pro X Superlight
From: Bastien Nocera @ 2023-06-28 10:50 UTC (permalink / raw)
To: Mavroudis Chatzilazaridis, jikos; +Cc: linux-input, benjamin.tissoires, lains
In-Reply-To: <20230625162131.17921-2-mavchatz@protonmail.com>
On Sun, 2023-06-25 at 16:21 +0000, Mavroudis Chatzilazaridis wrote:
> This patch adds support for the Pro X Superlight. Tested over USB.
Could you please list which functionality this gains the device?
Battery reporting? Hi-res scrolling? Something else?
>
> Co-developed-by: Filipe Laíns <lains@riseup.net>
> Signed-off-by: Filipe Laíns <lains@riseup.net>
> Signed-off-by: Mavroudis Chatzilazaridis <mavchatz@protonmail.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 5e1a412fd28f..a718535fb87d 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4616,6 +4616,8 @@ static const struct hid_device_id
> hidpp_devices[] = {
> .driver_data = HIDPP_QUIRK_CLASS_G920 |
> HIDPP_QUIRK_FORCE_OUTPUT_REPORTS },
> { /* Logitech G Pro Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
> + { /* Logitech G Pro X Superlight Gaming Mouse over USB */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC094) },
>
> { /* G935 Gaming Headset */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87),
> --
> 2.34.1
>
>
^ permalink raw reply
* Re: [PATCH v3 1/4] Input: ts-overlay - Add touchscreen overlay object handling
From: Javier Carrasco @ 2023-06-28 6:44 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <ZJmW/KOCJF25Z79t@nixie71>
Hi Jeff,
On 26.06.23 15:47, Jeff LaBundy wrote:
> Hi Javier,
>
> On Mon, Jun 26, 2023 at 12:31:21PM +0200, Javier Carrasco wrote:
>> Hi Jeff,
>>
>> On 26.06.23 04:35, Jeff LaBundy wrote:
>>> Hi Javier,
>>>
>>> On Fri, Jun 16, 2023 at 09:28:51AM +0200, Javier Carrasco wrote:
>>>> Some touchscreens provide mechanical overlays with different objects
>>>> like buttons or clipped touchscreen surfaces.
>>>>
>>>> In order to support these objects, add a series of helper functions
>>>> to the input subsystem to transform them into overlay objects via
>>>> device tree nodes.
>>>>
>>>> These overlay objects consume the raw touch events and report the
>>>> expected input events depending on the object properties.
>>>>
>>>> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
>>>> ---
>>>
>>> Excellent work; it's great to see this series move along.
>>>
>>>> MAINTAINERS | 7 +
>>>> drivers/input/touchscreen/Kconfig | 9 +
>>>> drivers/input/touchscreen/Makefile | 1 +
>>>> drivers/input/touchscreen/ts-overlay.c | 356 +++++++++++++++++++++++++++++++++
>>>> include/linux/input/ts-overlay.h | 43 ++++
>>>> 5 files changed, 416 insertions(+)
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 7e0b87d5aa2e..db9427926a4c 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -21434,6 +21434,13 @@ W: https://github.com/srcres258/linux-doc
>>>> T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
>>>> F: Documentation/translations/zh_TW/
>>>>
>>>> +TOUCHSCREEN OVERLAY OBJECTS
>>>> +M: Javier Carrasco <javier.carrasco@wolfvision.net>
>>>> +L: linux-input@vger.kernel.org
>>>> +S: Maintained
>>>> +F: drivers/input/touchscreen/ts-overlay.c
>>>> +F: include/linux/input/ts-overlay.h
>>>> +
>>>> TTY LAYER
>>>> M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> M: Jiri Slaby <jirislaby@kernel.org>
>>>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>>>> index 143ff43c67ae..8382a4d68326 100644
>>>> --- a/drivers/input/touchscreen/Kconfig
>>>> +++ b/drivers/input/touchscreen/Kconfig
>>>> @@ -1388,4 +1388,13 @@ config TOUCHSCREEN_HIMAX_HX83112B
>>>> To compile this driver as a module, choose M here: the
>>>> module will be called himax_hx83112b.
>>>>
>>>> +config TOUCHSCREEN_TS_OVERLAY
>>>> + bool "Touchscreen Overlay Objects"
>>>> + help
>>>> + Say Y here if you are using a touchscreen driver that supports
>>>> + printed overlays with keys or a clipped touchscreen area.
>>>> +
>>>> + Should be selected by the touchscren drivers that support
>>>> + this feature.
>>>> +
>>>> endif
>>>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>>>> index 159cd5136fdb..f554826706ff 100644
>>>> --- a/drivers/input/touchscreen/Makefile
>>>> +++ b/drivers/input/touchscreen/Makefile
>>>> @@ -117,3 +117,4 @@ obj-$(CONFIG_TOUCHSCREEN_RASPBERRYPI_FW) += raspberrypi-ts.o
>>>> obj-$(CONFIG_TOUCHSCREEN_IQS5XX) += iqs5xx.o
>>>> obj-$(CONFIG_TOUCHSCREEN_ZINITIX) += zinitix.o
>>>> obj-$(CONFIG_TOUCHSCREEN_HIMAX_HX83112B) += himax_hx83112b.o
>>>> +obj-$(CONFIG_TOUCHSCREEN_TS_OVERLAY) += ts-overlay.o
>>>
>>> It seems like this feature is useful for any two-dimensional touch surface
>>> (e.g. trackpads) and not just touchscreens. For that reason, the touchscreen
>>> helpers in touchscreen.c were moved out of input/touchscreen and into input/
>>> such that they are not guarded by CONFIG_INPUT_TOUCHSCREEN. A growing number
>>> of devices in input/misc are taking advantage of these.
>>>
>>> Therefore, I think this feature should follow suit and be available to any
>>> input device as is the case for touchscreen.c. As written, the newly updated
>>> binding is misleading because one may believe that any device that includes
>>> touchscreen.yaml can define an overlay child, but the code does not currently
>>> support this.
>>>
>>> To that end, it seems like touch-overlay would be a more descriptive name as
>>> well. I understand that the name has changed once already, so hopefully this
>>> feedback is not too annoying :)
>>>
>> changing names is no problem at all as long as it makes the feature more
>> comprehensible and/or takes more suitable devices into account, which
>> would be the case. So I will move the code from touchscreen to input and
>> I will update the names and descriptions to make them more generic.
>>
>> I guess then I will need to move the properties to a separate binding
>> for this feature because it will not be an addition to the touchscreen
>> bindings anymore, right?
>
> Technically this feature was never bound to touchscreen.yaml in the first place.
> touchscreen.yaml defines scalar properties under a parent input device; your new
> binding defines a child node and subsequent properties under the same, or another
> parent input device.
>
> That said, it is highly unlikely that one would use your feature without also
> using the properties from touchscreen.yaml. It is perfectly fine in my opinion,
> and perhaps more convenient, to define the overlay child in touchscreen.yaml as
> you have done so that binding authors need not reference two files.
>
> I do agree that it seems more natural for code living in input to be bound by
> bindings in dt-bindings/input and not dt-bindings/input/touchscreen/, but there
> was push back when the same question came up for touchscreen.yaml [1] some time
> ago.
>
> [1] https://patchwork.kernel.org/patch/12042037/
>
I will move this feature from input/touchscreen to input and add the
object to the input-core as it is done with the touchscreen object,
making it available for the rest of input devices. On the other hand the
bindings will stay where they are inside of touchscreen.yaml as you
suggested.
>>
>>>> diff --git a/drivers/input/touchscreen/ts-overlay.c b/drivers/input/touchscreen/ts-overlay.c
>>>> new file mode 100644
>>>> index 000000000000..7afa77d86c1f
>>>> --- /dev/null
>>>> +++ b/drivers/input/touchscreen/ts-overlay.c
>>>> @@ -0,0 +1,356 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Helper functions for overlay objects on touchscreens
>>>> + *
>>>> + * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
>>>> + */
>>>> +
>>>> +#include <linux/property.h>
>>>> +#include <linux/input.h>
>>>> +#include <linux/input/mt.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/input/ts-overlay.h>
>>>> +
>>>> +enum ts_overlay_valid_objects {
>>>> + TOUCHSCREEN,
>>>> + BUTTON,
>>>
>>> Please namespace these, i.e. TOUCH_OVERLAY_*.
>>>
>>>> +};
>>>> +
>>>> +static const char *const ts_overlay_names[] = {
>>>> + [TOUCHSCREEN] = "overlay-touchscreen",
>>>
>>> I'm a little confused why we need new code for this particular function; it's
>>> what touchscreen-min-x/y and touchscreen-size-x/y were meant to define. Why
>>> can't we keep using those?
>>>
>> According to the bindings, touchscreen-min-x/y define the minimum
>> reported values, but the overlay-touchscreen is actually setting a new
>> origin. Therefore I might be misusing those properties. On the other
>> hand touchscreen-size-x/y would make more sense, but I also considered
>> the case where someone would like to describe the real size of the
>> touchscreen outside of the overlay node as well as the clipped size
>> inside the node. In that case using the same property twice would be
>> confusing.
>> So in the end I thought that the origin/size properties are more precise
>> and applicable for all objects and not only the overlay touchscreen.
>> These properties are needed for the buttons anyways and in the future
>> more overlay would use the same properties.
>
> Ah, I understand now. touchscreen-min-x/y define the lower limits of the axes
> reported to input but they don't move the origin. I'm aligned with the reason
> to introduce this function.
>
> This does beg the question as to whether we need two separate types of children
> and related parsing code. Can we not simply have one overlay definition, and
> make the decision as to whether we are dealing with a border or virtual button
> based on whether 'linux,code' is present?
>
A single overlay definition would be possible, but in case more objects
are added in the future, looking for single properties and then deciding
what object it is might get messy pretty fast. You could end up needing
a decision tree and the definition in the DT would get more complex.
Now the decision tree is straightforward (linux,code -> button), but
that might not always be the case. In the current implementation there
are well-defined objects and adding a new one will never affect the
parsing of the rest.
Therefore I would like to keep it more readable and easily extendable.
>>
>>>> + [BUTTON] = "overlay-buttons",
>>>> +};
>>>> +
>>>> +struct ts_overlay_shape {
>>>> + u32 x_origin;
>>>> + u32 y_origin;
>>>> + u32 x_size;
>>>> + u32 y_size;
>>>> +};
>>>> +
>>>> +struct ts_overlay_button {
>>>> + struct ts_overlay_shape shape;
>>>> + u32 key;
>>>> + bool pressed;
>>>> + int slot;
>>>> +};
>>>> +
>>>> +static int ts_overlay_get_shape_properties(struct fwnode_handle *child_node,
>>>> + struct ts_overlay_shape *shape)
>>>> +{
>>>> + int rc;
>>>
>>> In input, the common practice is to use 'error' for return values that are either
>>> zero or negative. The reasoning is because the variable quite literally represents
>>> an error, or lack thereof. And then:
>>>
>>> error = ...
>>> if (error)
>>> return error;
>>>
>>>> +
>>>> + rc = fwnode_property_read_u32(child_node, "x-origin", &shape->x_origin);
>>>> + if (rc < 0)
>>>> + return rc;
>>>
>>> It seems like all of these properties are required; if so, please update the
>>> binding to make this clear.
>>>
>>> If the binding is correct and these properties are in fact optional, then you
>>> must evaluate fwnode_property_read_u32() against -EINVAL.
>>>
>> If I end up writing new bindings for this feature, it will be more clear
>> what is required and what not because I will not be part only of the
>> touchscreen anymore. These properties will be required.
>
> The question of whether to split the overlay child definition from touchscreen.yaml
> is orthogonal to this point. If the code fails in the absence of these properties,
> then you must add a "required:" heading within the overlay child node definition to
> call out these properties.
>
>>>> +
>>>> + rc = fwnode_property_read_u32(child_node, "y-origin", &shape->y_origin);
>>>> + if (rc < 0)
>>>> + return rc;
>>>> +
>>>> + rc = fwnode_property_read_u32(child_node, "x-size", &shape->x_size);
>>>> + if (rc < 0)
>>>> + return rc;
>>>> +
>>>> + rc = fwnode_property_read_u32(child_node, "y-size", &shape->y_size);
>>>> + if (rc < 0)
>>>> + return rc;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int ts_overlay_get_button_properties(struct device *dev,
>>>> + struct fwnode_handle *child_node,
>>>> + struct ts_overlay_button *btn)
>>>> +{
>>>> + struct fwnode_handle *child_btn;
>>>> + int rc;
>>>> + int j = 0;
>>>> +
>>>> + fwnode_for_each_child_node(child_node, child_btn) {
>>>> + rc = ts_overlay_get_shape_properties(child_btn, &btn[j].shape);
>>>> + if (rc < 0)
>>>> + goto button_prop_cleanup;
>>>> +
>>>> + rc = fwnode_property_read_u32(child_btn, "linux,code",
>>>> + &btn[j].key);
>>>> + if (rc < 0)
>>>> + goto button_prop_cleanup;
>>>
>>> The binding needs to list this property as required, too.
>>>
>> Do you mean "linux,code"? It is already listed with the same pattern
>> that some other bindings use:
>>
>> linux,code: true
>>
>> Is that not right? I did not want to redefine an existing property that
>> other bindings already make use of.
>
> These are separate things. You must additionally list 'linux,code' under a
> 'required:' heading if the code fails without the property defined. If you
> adopt the idea above to decide whether we are dealing with a border or button
> based on the presence of this property, then it goes back to being optional
> of course.
>
As I mentioned above, I will keep the feature documentation in the
touchscreen.yaml as a node and add the 'required' to the properties
needed for the code not to fail.
>>>> +
>>>> + dev_info(dev, "Added button at (%u, %u), size %ux%u, code=%u\n",
>>>> + btn[j].shape.x_origin, btn[j].shape.y_origin,
>>>> + btn[j].shape.x_size, btn[j].shape.y_size, btn[j].key);
>>>
>>> This seems like a dev_dbg() to me.
>>>
>>>> + j++;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +
>>>> +button_prop_cleanup:
>>>> + fwnode_handle_put(child_btn);
>>>> + return rc;
>>>> +}
>>>> +
>>>> +void ts_overlay_set_button_caps(struct ts_overlay_map *map,
>>>> + struct input_dev *dev)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < map->button_count; i++)
>>>> + input_set_capability(dev, EV_KEY, map->buttons[i].key);
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_set_button_caps);
>>>
>>> I don't see a need to expose this function and require participating drivers
>>> to call it; we should just have one over-arching function for processing the
>>> overlay(s), akin to touchscreen_parse_properties but for the button input
>>> device in case the driver separates the button and touchscreen input devices.
>>>
>>> That one function would then be responsible for parsing the overlay(s) and
>>> calling input_set_capability() on each button.
>>>
>>>> +
>>>> +static int ts_overlay_count_buttons(struct device *dev)
>>>> +{
>>>> + struct fwnode_handle *child_node;
>>>> + struct fwnode_handle *child_button;
>>>
>>> These names confused me; they're both children, but only the second is aptly
>>> named. How about child_overlay and child_button, or perhaps overlay_node and
>>> button_node?
>>>
>>>> + int count = 0;
>>>> +
>>>> + child_node = device_get_named_child_node(dev, ts_overlay_names[BUTTON]);
>>>> + if (!child_node)
>>>> + return 0;
>>>> +
>>>> + fwnode_for_each_child_node(child_node, child_button)
>>>> + count++;
>>>> + fwnode_handle_put(child_node);
>>>> +
>>>> + return count;
>>>> +}
>>>> +
>>>> +static int ts_overlay_map_touchscreen(struct device *dev,
>>>> + struct ts_overlay_map *map)
>>>> +{
>>>> + struct fwnode_handle *child;
>>>
>>> Same here; there are two layers of children, so please use more descriptive
>>> variable names.
>>>
>>>> + int rc = 0;
>>>> +
>>>> + child = device_get_named_child_node(dev, ts_overlay_names[TOUCHSCREEN]);
>>>> + if (!child)
>>>> + goto touchscreen_ret;
>>>
>>> I don't think we need a label here; just return 0 directly.
>>>
>>>> +
>>>> + map->touchscreen =
>>>> + devm_kzalloc(dev, sizeof(*map->touchscreen), GFP_KERNEL);
>>>> + if (!map->touchscreen) {
>>>> + rc = -ENOMEM;
>>>> + goto touchscreen_handle;
>>>> + }
>>>> + rc = ts_overlay_get_shape_properties(child, map->touchscreen);
>>>> + if (rc < 0)
>>>> + goto touchscreen_free;
>>>> +
>>>> + map->overlay_touchscreen = true;
>>>> + dev_info(dev, "Added overlay touchscreen at (%u, %u), size %u x %u\n",
>>>> + map->touchscreen->x_origin, map->touchscreen->y_origin,
>>>> + map->touchscreen->x_size, map->touchscreen->y_size);
>>>
>>> dev_dbg()
>>>
>>>> +
>>>> + rc = 0;
>>>
>>> rc (error) can only be zero if we have gotten this far; I don't see a need
>>> to assign it here.
>>>
>>>> + goto touchscreen_handle;
>>>
>>> Please think about whether this can be reorganized to prevent jumping over
>>> small bits of code; I found it hard to follow. Maybe one or more tasks can
>>> be offloaded to a helper function?
>>>
>>>> +
>>>> +touchscreen_free:
>>>> + devm_kfree(dev, map->touchscreen);
>>>
>>> This set off a red flag; it's unclear that it's necessary. Regardless of
>>> whether the participating driver is smart enough to bail during probe()
>>> if the overlay parsing fails, or it happily continues, this memory will
>>> get freed when the driver tied to 'dev' is torn down.
>>>
>>> Calling devm_kfree() is generally limited to special cases where you are
>>> dynamically reallocating memory at runtime. In case I have misunderstood
>>> the intent, please let me know.
>>>
>> Would devm_kfree() not free the memory immediately if the parsing fails,
>> making it available for any process instead of waiting until the driver
>> is torn down, which might never happen? Otherwise that chunk of memory
>> will not be accessible even though it is useless because the operation
>> failed, right? Or am I missing something?
>
> If the participating driver does not immediately fail to probe (and hence
> free all of its device-managed resources) upon failure to parse this or any
> other required properties, that is a bug in that driver.
>
In that case I will remove the call to devm_kfree and trust the
participating drivers.
>>>> +touchscreen_handle:
>>>> + fwnode_handle_put(child);
>>>> +touchscreen_ret:
>>>> + return rc;
>>>> +}
>>>> +
>>>> +static int ts_overlay_map_buttons(struct device *dev,
>>>> + struct ts_overlay_map *map,
>>>> + struct input_dev *input)
>>>> +{
>>>> + struct fwnode_handle *child;
>>>> + u32 button_count;
>>>> + int rc = 0;
>>>> +
>>>> + button_count = ts_overlay_count_buttons(dev);
>>>> + if (button_count) {
>>>> + map->buttons = devm_kcalloc(dev, button_count,
>>>> + sizeof(*map->buttons), GFP_KERNEL);
>>>> + if (!map->buttons) {
>>>> + rc = -ENOMEM;
>>>> + goto map_buttons_ret;
>>>> + }
>>>> + child = device_get_named_child_node(dev,
>>>> + ts_overlay_names[BUTTON]);
>>>> + if (unlikely(!child))
>>>> + goto map_buttons_free;
>>>> +
>>>> + rc = ts_overlay_get_button_properties(dev, child, map->buttons);
>>>> + if (rc < 0)
>>>> + goto map_buttons_free;
>>>> +
>>>> + map->button_count = button_count;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +
>>>> +map_buttons_free:
>>>> + devm_kfree(dev, map->buttons);
>>>> +map_buttons_ret:
>>>> + return rc;
>>>> +}
>>>> +
>>>> +static bool ts_overlay_defined_objects(struct device *dev)
>>>> +{
>>>> + struct fwnode_handle *child;
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < ARRAY_SIZE(ts_overlay_names); i++) {
>>>> + child = device_get_named_child_node(dev, ts_overlay_names[i]);
>>>> + if (child) {
>>>> + fwnode_handle_put(child);
>>>> + return true;
>>>> + }
>>>> + fwnode_handle_put(child);
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>>> +
>>>> +struct ts_overlay_map *ts_overlay_map_objects(struct device *dev,
>>>> + struct input_dev *input)
>>>> +{
>>>> + struct ts_overlay_map *map = NULL;
>>>> + int rc;
>>>> +
>>>> + if (!ts_overlay_defined_objects(dev))
>>>> + return NULL;
>>>> +
>>>> + map = devm_kzalloc(dev, sizeof(*map), GFP_KERNEL);
>>>> + if (!map) {
>>>> + rc = -ENOMEM;
>>>> + goto objects_err;
>>>> + }
>>>> + rc = ts_overlay_map_touchscreen(dev, map);
>>>> + if (rc < 0)
>>>> + goto objects_free;
>>>> +
>>>> + rc = ts_overlay_map_buttons(dev, map, input);
>>>> + if (rc < 0)
>>>> + goto objects_free;
>>>> +
>>>> + return map;
>>>> +
>>>> +objects_free:
>>>> + devm_kfree(dev, map);
>>>> +objects_err:
>>>> + return ERR_PTR(rc);
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_map_objects);
>>>> +
>>>> +void ts_overlay_get_touchscreen_abs(struct ts_overlay_map *map, u16 *x, u16 *y)
>>>> +{
>>>> + *x = map->touchscreen->x_size - 1;
>>>> + *y = map->touchscreen->y_size - 1;
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_get_touchscreen_abs);
>>>> +
>>>> +static bool ts_overlay_shape_event(struct ts_overlay_shape *shape, u32 x, u32 y)
>>>> +{
>>>> + if (!shape)
>>>> + return false;
>>>> +
>>>> + if (x >= shape->x_origin && x < (shape->x_origin + shape->x_size) &&
>>>> + y >= shape->y_origin && y < (shape->y_origin + shape->y_size))
>>>> + return true;
>>>> +
>>>> + return false;
>>>> +}
>>>> +
>>>> +static bool ts_overlay_touchscreen_event(struct ts_overlay_shape *touchscreen,
>>>> + u32 *x, u32 *y)
>>>> +{
>>>> + if (ts_overlay_shape_event(touchscreen, *x, *y)) {
>>>> + *x -= touchscreen->x_origin;
>>>> + *y -= touchscreen->y_origin;
>>>> + return true;
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>>> +
>>>> +bool ts_overlay_mapped_touchscreen(struct ts_overlay_map *map)
>>>> +{
>>>> + if (!map || !map->overlay_touchscreen)
>>>> + return false;
>>>> +
>>>> + return true;
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_mapped_touchscreen);
>>>> +
>>>> +bool ts_overlay_mapped_buttons(struct ts_overlay_map *map)
>>>> +{
>>>> + if (!map || !map->button_count)
>>>> + return false;
>>>> +
>>>> + return true;
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_mapped_buttons);
>>>> +
>>>> +bool ts_overlay_mt_on_touchscreen(struct ts_overlay_map *map, u32 *x, u32 *y)
>>>> +{
>>>> + if (!ts_overlay_mapped_touchscreen(map))
>>>> + return true;
>>>> +
>>>> + if (!ts_overlay_touchscreen_event(map->touchscreen, x, y))
>>>> + return false;
>>>> +
>>>> + return true;
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_mt_on_touchscreen);
>>>> +
>>>> +bool ts_overlay_button_press(struct ts_overlay_map *map,
>>>> + struct input_dev *input, u32 x, u32 y, u32 slot)
>>>> +{
>>>> + int i;
>>>> +
>>>> + if (!ts_overlay_mapped_buttons(map))
>>>> + return false;
>>>> +
>>>> + for (i = 0; i < map->button_count; i++) {
>>>> + if (ts_overlay_shape_event(&map->buttons[i].shape, x, y)) {
>>>> + input_report_key(input, map->buttons[i].key, 1);
>>>> + map->buttons[i].pressed = true;
>>>> + map->buttons[i].slot = slot;
>>>> + return true;
>>>> + }
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_button_press);
>>>
>>> The level of abstraction here does not seem quite right. Rather than force
>>> each participating driver to call a press and release function, I think it
>>> is better to expose something like touch_overlay_process_buttons() which
>>> then handles the press and release events internally.
>>>
>>> You're also relying on each individual driver to decide whether a touch
>>> coordinate is inside or outside the overlay, and selectively call the press
>>> and release functions OR report coordinates which is non-optimal.
>>>
>>> To me, this says we actually need one wrapper function that accepts handles
>>> to both the touchscreen and button input devices (which may be the same at
>>> the driver's discretion) as well as the coordinates. If the coordinate is
>>> within an overlay area, handle press/release; if not, call touchscreen_report_pos().
>>>
>>>> +
>>>> +bool ts_overlay_is_button_slot(struct ts_overlay_map *map, int slot)
>>>> +{
>>>> + int i;
>>>> +
>>>> + if (!map || !map->button_count)
>>>> + return false;
>>>> +
>>>> + for (i = 0; i < map->button_count; i++) {
>>>> + if (map->buttons[i].pressed && map->buttons[i].slot == slot)
>>>> + return true;
>>>> + }
>>>> +
>>>> + return false;
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_is_button_slot);
>>>> +
>>>> +void ts_overlay_button_release(struct ts_overlay_map *map,
>>>> + struct input_dev *input, u32 slot)
>>>> +{
>>>> + int i;
>>>> +
>>>> + if (!map || !map->button_count)
>>>> + return;
>>>> +
>>>> + for (i = 0; i < map->button_count; i++) {
>>>> + if (map->buttons[i].pressed && map->buttons[i].slot == slot) {
>>>> + input_report_key(input, map->buttons[i].key, 0);
>>>> + map->buttons[i].pressed = false;
>>>> + }
>>>> + }
>>>> +}
>>>> +EXPORT_SYMBOL(ts_overlay_button_release);
>>>> +
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_DESCRIPTION("Helper functions for overlay objects on touchscreens");
>>>> diff --git a/include/linux/input/ts-overlay.h b/include/linux/input/ts-overlay.h
>>>> new file mode 100644
>>>> index 000000000000..b75df0dec3ab
>>>> --- /dev/null
>>>> +++ b/include/linux/input/ts-overlay.h
>>>> @@ -0,0 +1,43 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +/*
>>>> + * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
>>>> + */
>>>> +
>>>> +#ifndef _TS_OVERLAY
>>>> +#define _TS_OVERLAY
>>>> +
>>>> +#include <linux/types.h>
>>>> +
>>>> +struct input_dev;
>>>> +struct device;
>>>> +
>>>> +struct ts_overlay_map {
>>>> + struct ts_overlay_shape *touchscreen;
>>>> + bool overlay_touchscreen;
>>>> + struct ts_overlay_button *buttons;
>>>> + u32 button_count;
>>>> +};
>>>> +
>>>> +struct ts_overlay_map *ts_overlay_map_objects(struct device *dev,
>>>> + struct input_dev *input);
>>>> +
>>>> +void ts_overlay_get_touchscreen_abs(struct ts_overlay_map *map, u16 *x, u16 *y);
>>>> +
>>>> +bool ts_overlay_mapped_touchscreen(struct ts_overlay_map *map);
>>>> +
>>>> +bool ts_overlay_mapped_buttons(struct ts_overlay_map *map);
>>>> +
>>>> +bool ts_overlay_mt_on_touchscreen(struct ts_overlay_map *map, u32 *x, u32 *y);
>>>> +
>>>> +bool ts_overlay_button_press(struct ts_overlay_map *map,
>>>> + struct input_dev *input, u32 x, u32 y, u32 slot);
>>>> +
>>>> +bool ts_overlay_is_button_slot(struct ts_overlay_map *map, int slot);
>>>> +
>>>> +void ts_overlay_button_release(struct ts_overlay_map *map,
>>>> + struct input_dev *input, u32 slot);
>>>> +
>>>> +void ts_overlay_set_button_caps(struct ts_overlay_map *map,
>>>> + struct input_dev *dev);
>>>> +
>>>> +#endif
>>>>
>>>> --
>>>> 2.39.2
>>>>
>>>
>>> Kind regards,
>>> Jeff LaBundy
>>
>> Thanks again for your feedback, I really appreciate it. All the comments
>> without a reply can be taken as acknowledged and accepted as they are
>> without further discussion. I will work on them for the next version.
>
> Sure thing! Thank you for your efforts.
>
>>
>> Thank you for your time and best regards,
>> Javier Carrasco
>
> Kind regards,
> Jeff LaBundy
Thanks again, I will start working on the next version asap.
Best regards,
Javier Carrasco
^ permalink raw reply
* [git pull] Input updates for v6.5-rc0
From: Dmitry Torokhov @ 2023-06-27 22:48 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.5-rc0
to receive updates for the input subsystem. You will get:
- improvements to PS/2 handling for case when EC has already latched a
scancode in the data register, but the kernel expects to receive an ACK
to a command it sent to a device (such as keyboard LED toggle)
- input drivers for devices connected over I2C bus have been switched back
to using [new] .probe()
- uinput allows userspace to inject timestamps for input events
- support for capacitive keys in Atmel touch controller driver
- assorted fixes to drv260x, pwm-vibra, ili210x, adxl34x, and other
drivers.
Changelog:
---------
André Apitzsch (2):
dt-bindings: input: atmel,maxtouch: add linux,keycodes
Input: atmel_mxt_ts - support capacitive keys
Biswarup Pal (1):
Input: uinput - allow injecting event times
Caleb Connolly (2):
MAINTAINERS: Adjust Qualcomm driver globbing
Input: pm8941-powerkey - fix debounce on gen2+ PMICs
Dana Elfassy (1):
Input: tests - add test to cover all input_grab_device() function
Dmitry Torokhov (10):
Input: avoid calling input_set_abs_val() in the event handling core
Input: tests - fix input_test_match_device_id test
Input: libps2 - attach ps2dev instances as serio port's drvdata
Input: libps2 - remove special handling of ACK for command byte
Input: libps2 - rework handling of command response
Input: libps2 - fix NAK handling
Input: libps2 - fix aborting PS/2 commands
Input: libps2 - introduce common interrupt handler
Input: libps2 - do not discard non-ack bytes when controlling LEDs
Input: gameport - provide default trigger() and read()
Duoming Zhou (1):
Input: cyttsp4_core - change del_timer_sync() to timer_shutdown_sync()
Geert Uytterhoeven (3):
Input: tests - fix use-after-free and refcount underflow in input_test_exit()
Input: tests - modular KUnit tests should not depend on KUNIT=y
Input: gpio-keys - use input_report_key()
Ismael Ferreras Morezuelas (1):
Input: xpad - spelling fixes for "Xbox"
Krzysztof Kozlowski (1):
dt-bindings: input: cypress,cyapa: convert to dtschema
Luca Weiss (7):
Input: drv260x - fix typo in register value define
Input: drv260x - sleep between polling GO bit
Input: drv260x - remove unused .reg_defaults
Input: drv260x - fix magnitude handling
dt-bindings: input: pwm-vibrator: Add enable-gpio
Input: pwm-vibra - add newline to dev_err prints
Input: pwm-vibra - add support for enable GPIO
Marek Vasut (2):
Input: ili210x - probe even if no resolution information
Input: adxl34x - do not hardcode interrupt trigger type
Maximilian Weigand (3):
Input: cyttsp5 - remove unused code
dt-bindings: input: cypress,tt21000 - add wakeup-source entry to documentation
Input: cyttsp5 - implement proper sleep and wakeup procedures
Michał Mirosław (1):
Input: i8042 - add missing include
Niklas Schnelle (1):
Input: add HAS_IOPORT dependencies
Philipp Puschmann (1):
Input: edt-ft5x06 - add delay after waking up
Uwe Kleine-König (3):
Input: Switch i2c drivers back to use .probe()
Input: tps65219-pwrbutton - convert to .remove_new()
Input: tps65219-pwrbutton - use regmap_set_bits()
Diffstat:
--------
.../devicetree/bindings/input/atmel,maxtouch.yaml | 7 +
.../devicetree/bindings/input/cypress,cyapa.txt | 42 ---
.../devicetree/bindings/input/cypress,cyapa.yaml | 49 ++++
.../devicetree/bindings/input/pwm-vibrator.yaml | 2 +
.../input/touchscreen/cypress,tt21000.yaml | 2 +
Documentation/input/devices/xpad.rst | 10 +-
Documentation/input/gamepad.rst | 2 +-
MAINTAINERS | 2 +-
drivers/input/Kconfig | 2 +-
drivers/input/gameport/gameport.c | 18 +-
drivers/input/input.c | 6 +-
drivers/input/joystick/Kconfig | 14 +-
drivers/input/joystick/as5011.c | 2 +-
drivers/input/joystick/qwiic-joystick.c | 2 +-
drivers/input/joystick/xpad.c | 64 ++---
drivers/input/keyboard/adp5588-keys.c | 2 +-
drivers/input/keyboard/adp5589-keys.c | 2 +-
drivers/input/keyboard/atkbd.c | 102 ++++---
drivers/input/keyboard/cap11xx.c | 2 +-
drivers/input/keyboard/cypress-sf.c | 2 +-
drivers/input/keyboard/dlink-dir685-touchkeys.c | 2 +-
drivers/input/keyboard/gpio_keys.c | 6 +-
drivers/input/keyboard/lm8323.c | 2 +-
drivers/input/keyboard/lm8333.c | 2 +-
drivers/input/keyboard/max7359_keypad.c | 2 +-
drivers/input/keyboard/mcs_touchkey.c | 2 +-
drivers/input/keyboard/mpr121_touchkey.c | 2 +-
drivers/input/keyboard/pinephone-keyboard.c | 2 +-
drivers/input/keyboard/qt1050.c | 2 +-
drivers/input/keyboard/qt1070.c | 2 +-
drivers/input/keyboard/qt2160.c | 2 +-
drivers/input/keyboard/tca6416-keypad.c | 2 +-
drivers/input/keyboard/tca8418_keypad.c | 2 +-
drivers/input/keyboard/tm2-touchkey.c | 2 +-
drivers/input/misc/ad714x-i2c.c | 2 +-
drivers/input/misc/adxl34x-i2c.c | 2 +-
drivers/input/misc/adxl34x.c | 3 +-
drivers/input/misc/apanel.c | 2 +-
drivers/input/misc/atmel_captouch.c | 2 +-
drivers/input/misc/bma150.c | 2 +-
drivers/input/misc/cma3000_d0x_i2c.c | 2 +-
drivers/input/misc/da7280.c | 2 +-
drivers/input/misc/drv260x.c | 58 +---
drivers/input/misc/drv2665.c | 2 +-
drivers/input/misc/drv2667.c | 2 +-
drivers/input/misc/ibm-panel.c | 2 +-
drivers/input/misc/iqs269a.c | 2 +-
drivers/input/misc/iqs626a.c | 2 +-
drivers/input/misc/iqs7222.c | 2 +-
drivers/input/misc/kxtj9.c | 2 +-
drivers/input/misc/mma8450.c | 2 +-
drivers/input/misc/pcf8574_keypad.c | 2 +-
drivers/input/misc/pm8941-pwrkey.c | 19 +-
drivers/input/misc/pwm-vibra.c | 36 ++-
drivers/input/misc/tps65219-pwrbutton.c | 12 +-
drivers/input/misc/uinput.c | 34 +++
drivers/input/mouse/cyapa.c | 2 +-
drivers/input/mouse/elan_i2c_core.c | 2 +-
drivers/input/mouse/psmouse-base.c | 86 +++---
drivers/input/mouse/psmouse.h | 2 +
drivers/input/mouse/synaptics.c | 10 +-
drivers/input/mouse/synaptics_i2c.c | 2 +-
drivers/input/mouse/trackpoint.c | 2 +-
drivers/input/rmi4/rmi_i2c.c | 2 +-
drivers/input/rmi4/rmi_smbus.c | 2 +-
drivers/input/serio/Kconfig | 1 +
drivers/input/serio/libps2.c | 293 +++++++++++++++------
drivers/input/tests/input_test.c | 38 ++-
drivers/input/touchscreen/Kconfig | 1 +
drivers/input/touchscreen/ad7879-i2c.c | 2 +-
drivers/input/touchscreen/ar1021_i2c.c | 2 +-
drivers/input/touchscreen/atmel_mxt_ts.c | 87 +++++-
drivers/input/touchscreen/auo-pixcir-ts.c | 2 +-
drivers/input/touchscreen/bu21013_ts.c | 2 +-
drivers/input/touchscreen/bu21029_ts.c | 2 +-
drivers/input/touchscreen/chipone_icn8318.c | 2 +-
drivers/input/touchscreen/chipone_icn8505.c | 2 +-
drivers/input/touchscreen/cy8ctma140.c | 2 +-
drivers/input/touchscreen/cy8ctmg110_ts.c | 2 +-
drivers/input/touchscreen/cyttsp4_core.c | 3 +-
drivers/input/touchscreen/cyttsp4_i2c.c | 2 +-
drivers/input/touchscreen/cyttsp5.c | 84 +++++-
drivers/input/touchscreen/cyttsp_i2c.c | 2 +-
drivers/input/touchscreen/edt-ft5x06.c | 3 +-
drivers/input/touchscreen/eeti_ts.c | 2 +-
drivers/input/touchscreen/egalax_ts.c | 2 +-
drivers/input/touchscreen/ektf2127.c | 2 +-
drivers/input/touchscreen/elants_i2c.c | 2 +-
drivers/input/touchscreen/exc3000.c | 2 +-
drivers/input/touchscreen/goodix.c | 2 +-
drivers/input/touchscreen/hideep.c | 2 +-
drivers/input/touchscreen/himax_hx83112b.c | 2 +-
drivers/input/touchscreen/hycon-hy46xx.c | 2 +-
drivers/input/touchscreen/hynitron_cstxxx.c | 2 +-
drivers/input/touchscreen/ili210x.c | 38 ++-
drivers/input/touchscreen/ilitek_ts_i2c.c | 2 +-
drivers/input/touchscreen/imagis.c | 2 +-
drivers/input/touchscreen/iqs5xx.c | 2 +-
drivers/input/touchscreen/max11801_ts.c | 2 +-
drivers/input/touchscreen/mcs5000_ts.c | 2 +-
drivers/input/touchscreen/melfas_mip4.c | 2 +-
drivers/input/touchscreen/migor_ts.c | 2 +-
drivers/input/touchscreen/mms114.c | 2 +-
drivers/input/touchscreen/msg2638.c | 2 +-
drivers/input/touchscreen/novatek-nvt-ts.c | 2 +-
drivers/input/touchscreen/pixcir_i2c_ts.c | 2 +-
drivers/input/touchscreen/raydium_i2c_ts.c | 2 +-
drivers/input/touchscreen/rohm_bu21023.c | 2 +-
drivers/input/touchscreen/s6sy761.c | 2 +-
drivers/input/touchscreen/silead.c | 2 +-
drivers/input/touchscreen/sis_i2c.c | 2 +-
drivers/input/touchscreen/st1232.c | 2 +-
drivers/input/touchscreen/stmfts.c | 2 +-
drivers/input/touchscreen/sx8654.c | 2 +-
drivers/input/touchscreen/tsc2004.c | 2 +-
drivers/input/touchscreen/tsc2007_core.c | 2 +-
drivers/input/touchscreen/wacom_i2c.c | 2 +-
drivers/input/touchscreen/wdt87xx_i2c.c | 2 +-
drivers/input/touchscreen/zet6223.c | 2 +-
drivers/input/touchscreen/zforce_ts.c | 2 +-
drivers/input/touchscreen/zinitix.c | 2 +-
include/linux/gameport.h | 11 +-
include/linux/i8042.h | 1 +
include/linux/libps2.h | 62 +++--
124 files changed, 901 insertions(+), 485 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/cypress,cyapa.txt
create mode 100644 Documentation/devicetree/bindings/input/cypress,cyapa.yaml
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: input: fix mapping for camera access keys
From: Dmitry Torokhov @ 2023-06-27 22:09 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Eray Orçunus, linux-input, linux-kernel
Commit 9f4211bf7f81 ("HID: add mapping for camera access keys") added
mapping for the camera access keys, but unfortunately used wrong usage
codes for them. HUTRR72[1] specifies that camera access controls use 0x76,
0x077 and 0x78 usages in the consumer control page. Previously mapped 0xd5,
0xd6 and 0xd7 usages are actually defined in HUTRR64[2] as game recording
controls.
[1] https://www.usb.org/sites/default/files/hutrr72_-_usages_to_control_camera_access_0.pdf
[2] https://www.usb.org/sites/default/files/hutrr64b_-_game_recording_controllers_0.pdf
Fixes: 9f4211bf7f81 ("HID: add mapping for camera access keys")
Cc: stable@vger.kernel.org
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-input.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 7fc967964dd8..d7b5ace90bd8 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1085,6 +1085,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x074: map_key_clear(KEY_BRIGHTNESS_MAX); break;
case 0x075: map_key_clear(KEY_BRIGHTNESS_AUTO); break;
+ case 0x076: map_key_clear(KEY_CAMERA_ACCESS_ENABLE); break;
+ case 0x077: map_key_clear(KEY_CAMERA_ACCESS_DISABLE); break;
+ case 0x078: map_key_clear(KEY_CAMERA_ACCESS_TOGGLE); break;
+
case 0x079: map_key_clear(KEY_KBDILLUMUP); break;
case 0x07a: map_key_clear(KEY_KBDILLUMDOWN); break;
case 0x07c: map_key_clear(KEY_KBDILLUMTOGGLE); break;
@@ -1131,9 +1135,6 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x0cd: map_key_clear(KEY_PLAYPAUSE); break;
case 0x0cf: map_key_clear(KEY_VOICECOMMAND); break;
- case 0x0d5: map_key_clear(KEY_CAMERA_ACCESS_ENABLE); break;
- case 0x0d6: map_key_clear(KEY_CAMERA_ACCESS_DISABLE); break;
- case 0x0d7: map_key_clear(KEY_CAMERA_ACCESS_TOGGLE); break;
case 0x0d8: map_key_clear(KEY_DICTATE); break;
case 0x0d9: map_key_clear(KEY_EMOJI_PICKER); break;
--
2.41.0.178.g377b9f9a00-goog
--
Dmitry
^ permalink raw reply related
* Re: [PATCH v2 2/7] HID: add mapping for camera access keys
From: Dmitry Torokhov @ 2023-06-27 17:35 UTC (permalink / raw)
To: Eray Orçunus
Cc: platform-driver-x86, linux-kernel, linux-input, ike.pan, jikos,
benjamin.tissoires, hdegoede, mgross, pobrn, Eray Orçunus
In-Reply-To: <20221029120311.11152-3-erayorcunus@gmail.com>
Hi Eray,
On Sat, Oct 29, 2022 at 5:04 AM Eray Orçunus <erayorcunus@gmail.com> wrote:
>
> HUTRR72 added 3 new usage codes for keys that are supposed to enable,
> disable and toggle camera access. These are useful, considering many
> laptops today have key(s) for toggling access to camera.
>
> This patch adds new key definitions for KEY_CAMERA_ACCESS_ENABLE,
> KEY_CAMERA_ACCESS_DISABLE and KEY_CAMERA_ACCESS_TOGGLE. Additionally
> hid-debug is adjusted to recognize this new usage codes as well.
>
> Signed-off-by: Eray Orçunus <erayorcunus@gmail.com>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/hid/hid-debug.c | 3 +++
> drivers/hid/hid-input.c | 3 +++
> include/uapi/linux/input-event-codes.h | 3 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index f48d3534e020..991f880fdbd4 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -936,6 +936,9 @@ static const char *keys[KEY_MAX + 1] = {
> [KEY_ASSISTANT] = "Assistant",
> [KEY_KBD_LAYOUT_NEXT] = "KbdLayoutNext",
> [KEY_EMOJI_PICKER] = "EmojiPicker",
> + [KEY_CAMERA_ACCESS_ENABLE] = "CameraAccessEnable",
> + [KEY_CAMERA_ACCESS_DISABLE] = "CameraAccessDisable",
> + [KEY_CAMERA_ACCESS_TOGGLE] = "CameraAccessToggle",
> [KEY_DICTATE] = "Dictate",
> [KEY_BRIGHTNESS_MIN] = "BrightnessMin",
> [KEY_BRIGHTNESS_MAX] = "BrightnessMax",
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index f197aed6444a..f8e6513e77b8 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -995,6 +995,9 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> case 0x0cd: map_key_clear(KEY_PLAYPAUSE); break;
> case 0x0cf: map_key_clear(KEY_VOICECOMMAND); break;
>
> + case 0x0d5: map_key_clear(KEY_CAMERA_ACCESS_ENABLE); break;
> + case 0x0d6: map_key_clear(KEY_CAMERA_ACCESS_DISABLE); break;
> + case 0x0d7: map_key_clear(KEY_CAMERA_ACCESS_TOGGLE); break;
I was looking at the HID Usages table spec and it came to my attention
that HUTRR72 actually established 0x76, 0x77 and 0x78 as the usages for
then new camera access controls:
https://www.usb.org/sites/default/files/hutrr72_-_usages_to_control_camera_access_0.pdf
Where did 0xd5, 0xd6 and 0xd7 came from?
It looks like we need to update the hid-input mappings as the are
clashing with game recording controls from HUTRR64:
https://www.usb.org/sites/default/files/hutrr64b_-_game_recording_controllers_0.pdf
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: Block a rogue device on ASUS TUF A16
From: Friedrich Vock @ 2023-06-27 14:38 UTC (permalink / raw)
To: Limonciello, Mario, linux-input, Natikar, Basavaraj,
S-k, Shyam-sundar
Cc: Jiri Kosina, Benjamin Tissoires
In-Reply-To: <491ec8a8-efc7-7995-ac82-03a5431f66bd@amd.com>
Hi,
On 26.06.23 17:10, Limonciello, Mario wrote:
>
> On 6/18/2023 10:05 PM, Mario Limonciello wrote:
>> On 6/15/23 07:41, Friedrich Vock wrote:
>>> Hi,
>>>
>>> sorry for taking so long to reply.
>>>
>>> On 02.06.23 20:43, Limonciello, Mario wrote:
>>>> + some AMD guys
>>>>
>>>> On 5/30/2023 10:40 AM, Friedrich Vock wrote:
>>>>> On these laptops, there seems to be a device that, when probed by
>>>>> i2c-hid, constantly sends bogus interrupts and interferes with the
>>>>> keyboard controller. When the device is enabled, it takes the
>>>>> keyboard
>>>>> around 8 seconds to register that keys are being pressed or released.
>>>>
>>>> Do you know what interrupt is firing constantly?
>>>> Presumably it is the GPIO controller master interrupt, right?
>>>> And it's for GPIO 7 (guessed from acpidump on one of the bug
>>>> reports).
>>>>
>>>> To confirm check /proc/interrupts.
>>> Seems likely that you guessed correctly. The corresponsing line in
>>> /proc/interrupts (with the interrupts counts omitted):
>>> 71: amd_gpio 7 ITE5570:00
>>
>> OK.
> I had asked in the past for R/W everything output to compare to
> /sys/kernel/debug/gpio.
>
> I've added more explicit instructions how to get this to
> the kernel bugzilla 217336 – keyboard not working Asus TUF FA617NS
> (kernel.org) <https://bugzilla.kernel.org/show_bug.cgi?id=217336#c126>
Thanks for this. R/W everything didn't work for the other people with
the same models, so I spent this day getting Windows and R/W everything
running myself. I managed to make it run and left a comment with the
results in that bugzilla report.
>>
>>>>
>>>> If it's not obvious which GPIO is firing there is also a dynamic
>>>> debug statement in pinctrl-amd.c that may be helpful to figure
>>>> this out.
>>>>
>>>> I would also suspect in Windows this doesn't happen. If possible
>>>> can you confirm that? Check in Device Manager what driver is bound
>>>> to this device. Is it "inbox" from Microsoft or is it an ASUS
>>>> specific driver?
>>>>
>>>> I wonder if the GPIO controller got programmed differently in
>>>> Windows for some reason. We may want to confirm the values for
>>>> GPIO registers from /sys/kernel/debug/gpio in Linux against those
>>>> that are programmed in Windows.
>>>>
>>>> This can be accomplished using R/W everything in Windows.
>>>
>>> Unfortunately I don't have Windows installed on this system. I know
>>> some
>>> people with the Ryzen 9 7940HS model which might, I'll ask them if they
>>> can give me the configuration on Windows and Linux.
>>
>> OK, I suggest directing everyone over to the bugs I linked and we
>> should gather the relevant GPIO controller register dumps from
>> Windows and Linux on the same hardware there.
>>
>>>
>>>>
>>>>>
>>>>> Nothing I tried seemed to make the device work, and preventing the
>>>>> device from being probed doesn't seem to break any functionality of
>>>>> the laptop.
>>>>>
>>>>> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
>>>>
>>>> There are a few bug reports that popped up around this issue that
>>>> should
>>>> probably also be tagged.
>>>>
>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336
>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217493
>>>>
>>>>> ---
>>>>> drivers/hid/i2c-hid/i2c-hid-core.c | 5 +++
>>>>> drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 48
>>>>> ++++++++++++++++++++++++
>>>>> drivers/hid/i2c-hid/i2c-hid.h | 3 ++
>>>>> 3 files changed, 56 insertions(+)
>>>>>
>>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>> b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>> index efbba0465eef..5f0686d058df 100644
>>>>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>>> @@ -1035,6 +1035,11 @@ int i2c_hid_core_probe(struct i2c_client
>>>>> *client, struct i2chid_ops *ops,
>>>>>
>>>>> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>>>>>
>>>>> + if (i2c_hid_device_blocked(hid->vendor, hid->product)) {
>>>>> + ret = -ENODEV;
>>>>> + goto err_irq;
>>>>> + }
>>>>> +
>>>> The thing I worry about here is that an unserviced interrupt can
>>>> prevent the
>>>> hardware from going into proper low power states; particularly at
>>>> runtime.
>>>>
>>>> I think we should better understand what's going on before going down
>>>> this
>>>> path of just ignoring it.
>>> Yeah, I guess I should've searched more for a proper explanation/fix
>>> before submitting hacks like this. Let's see if this can be fixed in a
>>> cleaner manner than preemptively disabling parts of the system.
>>
>> Can you please have a try with linux-next or apply this series:
>> https://lore.kernel.org/linux-gpio/20230421120625.3366-1-mario.limonciello@amd.com/
>>
>>
>> That does change some of the configuration for the GPIO controller
>> registers to align how windows is handling debouncing.
>>
>> I don't think it will change the outcome of your bug, but I'd love
>> to be surprised.
> Any updates for this?
Tried this out today. You won't be surprised, it didn't change anything.
>>
>>>>> ret = hid_add_device(hid);
>>>>> if (ret) {
>>>>> if (ret != -ENODEV)
>>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>> b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>> index 210f17c3a0be..d2c2806b64b4 100644
>>>>> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>>>>> @@ -440,6 +440,38 @@ static const struct dmi_system_id
>>>>> i2c_hid_dmi_quirk_table[] = {
>>>>> { } /* Terminate list */
>>>>> };
>>>>>
>>>>> +static const struct hid_device_id i2c_hid_blocked_ite_device = {
>>>>> + HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC, USB_VENDOR_ID_ITE,
>>>>> 0x8051)
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * This list contains devices that can't be activated at all, for
>>>>> example
>>>>> + * because activating them breaks other important parts of the
>>>>> system.
>>>>> + */
>>>>> +static const struct dmi_system_id i2c_hid_dmi_block_table[] = {
>>>>> + /*
>>>>> + * On ASUS TUF Gaming A16 laptops, there is a device that will
>>>>> make the
>>>>> + * keyboard controller stop working correctly and flood the CPU
>>>>> with bogus
>>>>> + * interrupts.
>>>>> + */
>>>>> + {
>>>>> + .ident = "ASUS TUF Gaming A16 (Ryzen 7 7735HS)",
>>>>> + .matches = {
>>>>> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
>>>>> INC."),
>>>>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
>>>>> + },
>>>>> + .driver_data = (void *)&i2c_hid_blocked_ite_device,
>>>>> + },
>>>>> + {
>>>>> + .ident = "ASUS TUF Gaming A16 (Ryzen 9 7940HS)",
>>>>> + .matches = {
>>>>> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER
>>>>> INC."),
>>>>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
>>>>> + },
>>>>> + .driver_data = (void *)&i2c_hid_blocked_ite_device,
>>>>> + },
>>>>> + { } /* Terminate list */
>>>> If this *does* end up being the best solution, I think it's better
>>>> to patch in the DMI to gpiolib-acpi.c similar to other quirks for
>>>> floating
>>>> GPIOs. Example:
>>>>
>>>> https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L1654
>>>>
>>>>
>>>>
>>>>> +};
>>>>>
>>>>> struct i2c_hid_desc *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t
>>>>> *i2c_name)
>>>>> {
>>>>> @@ -492,3 +524,19 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor,
>>>>> const u16 product)
>>>>>
>>>>> return quirks;
>>>>> }
>>>>> +
>>>>> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product)
>>>>> +{
>>>>> + const struct dmi_system_id *system_id =
>>>>> + dmi_first_match(i2c_hid_dmi_block_table);
>>>>> +
>>>>> + if (system_id) {
>>>>> + const struct hid_device_id *device_id =
>>>>> + (struct hid_device_id *)(system_id->driver_data);
>>>>> +
>>>>> + if (device_id && device_id->vendor == vendor &&
>>>>> + device_id->product == product)
>>>>> + return true;
>>>>> + }
>>>>> + return false;
>>>>> +}
>>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid.h
>>>>> b/drivers/hid/i2c-hid/i2c-hid.h
>>>>> index 2c7b66d5caa0..e17bdd758f39 100644
>>>>> --- a/drivers/hid/i2c-hid/i2c-hid.h
>>>>> +++ b/drivers/hid/i2c-hid/i2c-hid.h
>>>>> @@ -10,6 +10,7 @@ struct i2c_hid_desc
>>>>> *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name);
>>>>> char *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>>>>> unsigned int *size);
>>>>> u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product);
>>>>> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product);
>>>>> #else
>>>>> static inline struct i2c_hid_desc
>>>>> *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name)
>>>>> @@ -19,6 +20,8 @@ static inline char
>>>>> *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>>>>> { return NULL; }
>>>>> static inline u32 i2c_hid_get_dmi_quirks(const u16 vendor, const
>>>>> u16 product)
>>>>> { return 0; }
>>>>> +static inline bool i2c_hid_device_blocked(const u16 vendor, const
>>>>> u16 product)
>>>>> +{ return false; }
>>>>> #endif
>>>>>
>>>>> /**
>>>>> --
>>>>> 2.40.1
>>>>>
>>>>>
>>
Thanks,
Friedrich
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS bf4ed21778f2920ca91a32fd3a1e1130e843e98f
From: kernel test robot @ 2023-06-27 13:35 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: bf4ed21778f2920ca91a32fd3a1e1130e843e98f Merge branch 'next' into for-linus
elapsed time: 725m
configs tested: 136
configs skipped: 4
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha defconfig gcc
alpha randconfig-r013-20230627 gcc
alpha randconfig-r014-20230627 gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-r036-20230627 gcc
arc randconfig-r043-20230627 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm jornada720_defconfig gcc
arm randconfig-r005-20230627 clang
arm randconfig-r015-20230627 gcc
arm randconfig-r032-20230627 clang
arm randconfig-r046-20230627 gcc
arm64 alldefconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
csky defconfig gcc
csky randconfig-r016-20230627 gcc
csky randconfig-r031-20230627 gcc
hexagon randconfig-r025-20230627 clang
hexagon randconfig-r041-20230627 clang
hexagon randconfig-r045-20230627 clang
i386 allyesconfig gcc
i386 buildonly-randconfig-r004-20230627 gcc
i386 buildonly-randconfig-r005-20230627 gcc
i386 buildonly-randconfig-r006-20230627 gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-i001-20230627 gcc
i386 randconfig-i002-20230627 gcc
i386 randconfig-i003-20230627 gcc
i386 randconfig-i004-20230627 gcc
i386 randconfig-i005-20230627 gcc
i386 randconfig-i006-20230627 gcc
i386 randconfig-i011-20230627 clang
i386 randconfig-i012-20230627 clang
i386 randconfig-i013-20230627 clang
i386 randconfig-i014-20230627 clang
i386 randconfig-i015-20230627 clang
i386 randconfig-i016-20230627 clang
i386 randconfig-r026-20230627 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-r002-20230627 gcc
loongarch randconfig-r034-20230627 gcc
m68k allmodconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
m68k multi_defconfig gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips bcm47xx_defconfig gcc
mips randconfig-r012-20230627 gcc
mips xway_defconfig gcc
nios2 defconfig gcc
nios2 randconfig-r001-20230627 gcc
nios2 randconfig-r035-20230627 gcc
openrisc randconfig-r006-20230627 gcc
openrisc randconfig-r021-20230627 gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc pasemi_defconfig gcc
powerpc randconfig-r015-20230627 clang
powerpc randconfig-r023-20230627 clang
powerpc sam440ep_defconfig gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv randconfig-r011-20230627 clang
riscv randconfig-r021-20230627 clang
riscv randconfig-r022-20230627 clang
riscv randconfig-r023-20230627 clang
riscv randconfig-r042-20230627 clang
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 debug_defconfig gcc
s390 defconfig gcc
s390 randconfig-r004-20230627 gcc
s390 randconfig-r033-20230627 gcc
s390 randconfig-r044-20230627 clang
sh allmodconfig gcc
sh rsk7203_defconfig gcc
sh rsk7264_defconfig gcc
sh se7751_defconfig gcc
sparc alldefconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc randconfig-r003-20230627 gcc
sparc randconfig-r022-20230627 gcc
sparc randconfig-r024-20230627 gcc
sparc64 randconfig-r032-20230627 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allyesconfig gcc
x86_64 buildonly-randconfig-r001-20230627 gcc
x86_64 buildonly-randconfig-r002-20230627 gcc
x86_64 buildonly-randconfig-r003-20230627 gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-r014-20230627 clang
x86_64 randconfig-r024-20230627 clang
x86_64 randconfig-x001-20230627 clang
x86_64 randconfig-x002-20230627 clang
x86_64 randconfig-x003-20230627 clang
x86_64 randconfig-x004-20230627 clang
x86_64 randconfig-x005-20230627 clang
x86_64 randconfig-x006-20230627 clang
x86_64 randconfig-x011-20230627 gcc
x86_64 randconfig-x012-20230627 gcc
x86_64 randconfig-x013-20230627 gcc
x86_64 randconfig-x014-20230627 gcc
x86_64 randconfig-x015-20230627 gcc
x86_64 randconfig-x016-20230627 gcc
x86_64 rhel-8.3-bpf gcc
x86_64 rhel-8.3-func gcc
x86_64 rhel-8.3-kselftests gcc
x86_64 rhel-8.3-kunit gcc
x86_64 rhel-8.3-kvm gcc
x86_64 rhel-8.3-ltp gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa common_defconfig gcc
xtensa generic_kc705_defconfig gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 3/4] Input: st1232 - add overlay touchscreen and buttons handling
From: kernel test robot @ 2023-06-27 9:25 UTC (permalink / raw)
To: Javier Carrasco, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, Bastian Hecht,
Michael Riesch
Cc: oe-kbuild-all, linux-kernel, linux-input, devicetree,
Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v3-3-b4fb7fc4bab7@wolfvision.net>
Hi Javier,
kernel test robot noticed the following build errors:
[auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b]
url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-overlay-Add-touchscreen-overlay-object-handling/20230616-153203
base: ac9a78681b921877518763ba0e89202254349d1b
patch link: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v3-3-b4fb7fc4bab7%40wolfvision.net
patch subject: [PATCH v3 3/4] Input: st1232 - add overlay touchscreen and buttons handling
config: microblaze-randconfig-r051-20230625 (https://download.01.org/0day-ci/archive/20230627/202306271711.B8sjkaYH-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230627/202306271711.B8sjkaYH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306271711.B8sjkaYH-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "ts_overlay_is_button_slot" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_button_press" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_mt_on_touchscreen" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_mapped_buttons" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_button_release" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_map_objects" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_mapped_touchscreen" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_get_touchscreen_abs" [drivers/input/touchscreen/st1232.ko] undefined!
>> ERROR: modpost: "ts_overlay_set_button_caps" [drivers/input/touchscreen/st1232.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] HID: mcp2221: Remove unneeded semicolon
From: baomingtong001 @ 2023-06-27 8:34 UTC (permalink / raw)
To: gupt21, jikos, benjamin.tissoires; +Cc: linux-i2c, linux-input, linux-kernel
./drivers/hid/hid-mcp2221.c:1027:3-4: Unneeded semicolon
Signed-off-by: Mingtong Bao <baomingtong001@208suo.com>
---
drivers/hid/hid-mcp2221.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 72883e0ce757..38e795c58764 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -1024,7 +1024,7 @@ static int mcp_iio_channels(struct mcp2221 *mcp)
break;
default:
continue;
- };
+ }
chan->type = IIO_VOLTAGE;
chan->indexed = 1;
--
2.40.1
^ permalink raw reply related
* [PATCH] samples/hid: hid_surface_dial: Remove unneeded semicolon
From: baomingtong001 @ 2023-06-27 8:19 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel
./samples/hid/hid_surface_dial.c:170:5-6: Unneeded semicolon
Signed-off-by: Mingtong Bao <baomingtong001@208suo.com>
---
samples/hid/hid_surface_dial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/samples/hid/hid_surface_dial.c
b/samples/hid/hid_surface_dial.c
index 4bc97373a708..704f2b639492 100644
--- a/samples/hid/hid_surface_dial.c
+++ b/samples/hid/hid_surface_dial.c
@@ -167,7 +167,7 @@ int main(int argc, char **argv)
"invalid r option %s - expecting a number\n",
optarg ? optarg : "");
exit(EXIT_FAILURE);
- };
+ }
resolution = (int) l;
break;
--
2.40.1
^ permalink raw reply related
* Consumo de combustible
From: Miguel Garcia @ 2023-06-27 8:00 UTC (permalink / raw)
To: linux-input
Buenos dias
Le escribo sobre la gestión de los automóviles de la empresa.
Gracias a las abundantes funcionalidades de la herramienta GPS, que monitorea cada automóvil de manera continua, puede registrar la posición, el tiempo y el kilometraje de los automóviles en tiempo real.
Como resultado, los costos de mantenimiento de la flota de la compañía se reducen en un 20% y el tiempo de viaje o la planificación de la entrega se reduce significativamente. 49 mil están detrás del éxito de nuestras soluciones. Empresas que cooperan con nosotros.
Si el tema le parece interesante, contáctame.
Atentamente,
Miguel Garcia
^ permalink raw reply
* Fotowoltaika- propozycja instalacji
From: Norbert Karecki @ 2023-06-27 7:30 UTC (permalink / raw)
To: linux-input
Dzień dobry,
Czy rozważali Państwo montaż systemu fotowoltaicznego?
Instalacja fotowoltaiczna jest najlepszym sposobem na obniżenie wysokości rachunków za prąd (pozostają tylko opłaty stałe) i zabezpieczenie się przed rosnącymi cenami energii elektrycznej. Jest to w pełni odnawialne i bezemisyjne źródło energii, dzięki czemu przyczyniamy się do ochrony środowiska naturalnego.
Działamy od wielu lat na rynku energetycznym. Przygotujemy projekt, wycenę oraz kompleksowo wykonamy i zgłosimy realizację do zakładu energetycznego.
Czy chcą Państwo poznać naszą propozycję?
Pozdrawiam,
Norbert Karecki
^ permalink raw reply
* [PATCH v4 2/2] Input: cap11xx - add advanced sensitivity settings
From: Jiri Valek - 2N @ 2023-06-27 6:53 UTC (permalink / raw)
To: krzysztof.kozlowski+dt, dmitry.torokhov
Cc: jiriv, devicetree, linux-input, linux-kernel, robh+dt,
u.kleine-koenig
In-Reply-To: <20230627065316.1065911-1-jiriv@axis.com>
Add support for advanced sensitivity settings that allows more precise
tunig of touch buttons. Input-treshold allows to set the sensitivity for
each channel separately. Also add signal guard feature for CAP129x chips.
Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
---
drivers/input/keyboard/cap11xx.c | 250 ++++++++++++++++++++++++-------
1 file changed, 197 insertions(+), 53 deletions(-)
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 040696d0e49c..5972415cc549 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -14,6 +14,7 @@
#include <linux/regmap.h>
#include <linux/i2c.h>
#include <linux/gpio/consumer.h>
+#include <linux/bitfield.h>
#define CAP11XX_REG_MAIN_CONTROL 0x00
#define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT (6)
@@ -24,6 +25,7 @@
#define CAP11XX_REG_NOISE_FLAG_STATUS 0x0a
#define CAP11XX_REG_SENOR_DELTA(X) (0x10 + (X))
#define CAP11XX_REG_SENSITIVITY_CONTROL 0x1f
+#define CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK 0x70
#define CAP11XX_REG_CONFIG 0x20
#define CAP11XX_REG_SENSOR_ENABLE 0x21
#define CAP11XX_REG_SENSOR_CONFIG 0x22
@@ -32,6 +34,7 @@
#define CAP11XX_REG_CALIBRATION 0x26
#define CAP11XX_REG_INT_ENABLE 0x27
#define CAP11XX_REG_REPEAT_RATE 0x28
+#define CAP11XX_REG_SIGNAL_GUARD_ENABLE 0x29
#define CAP11XX_REG_MT_CONFIG 0x2a
#define CAP11XX_REG_MT_PATTERN_CONFIG 0x2b
#define CAP11XX_REG_MT_PATTERN 0x2d
@@ -47,6 +50,8 @@
#define CAP11XX_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
#define CAP11XX_REG_LED_POLARITY 0x73
#define CAP11XX_REG_LED_OUTPUT_CONTROL 0x74
+#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG 0x80
+#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG2 0x81
#define CAP11XX_REG_LED_DUTY_CYCLE_1 0x90
#define CAP11XX_REG_LED_DUTY_CYCLE_2 0x91
@@ -78,12 +83,20 @@ struct cap11xx_led {
struct cap11xx_priv {
struct regmap *regmap;
+ struct device *dev;
struct input_dev *idev;
+ const struct cap11xx_hw_model *model;
+ u8 id;
struct cap11xx_led *leds;
int num_leds;
/* config */
+ u8 analog_gain;
+ u8 sensitivity_delta_sense;
+ u8 signal_guard_inputs_mask;
+ u32 thresholds[8];
+ u32 calib_sensitivities[8];
u32 keycodes[];
};
@@ -181,6 +194,178 @@ static const struct regmap_config cap11xx_regmap_config = {
.volatile_reg = cap11xx_volatile_reg,
};
+static int
+cap11xx_write_calib_sens_config_1(struct cap11xx_priv *priv)
+{
+ return regmap_write(priv->regmap,
+ CAP11XX_REG_CALIB_SENSITIVITY_CONFIG,
+ (priv->calib_sensitivities[3] << 6) |
+ (priv->calib_sensitivities[2] << 4) |
+ (priv->calib_sensitivities[1] << 2) |
+ priv->calib_sensitivities[0]);
+}
+
+static int
+cap11xx_write_calib_sens_config_2(struct cap11xx_priv *priv)
+{
+ return regmap_write(priv->regmap,
+ CAP11XX_REG_CALIB_SENSITIVITY_CONFIG2,
+ (priv->calib_sensitivities[7] << 6) |
+ (priv->calib_sensitivities[6] << 4) |
+ (priv->calib_sensitivities[5] << 2) |
+ priv->calib_sensitivities[4]);
+}
+
+static int
+cap11xx_init_keys(struct cap11xx_priv *priv)
+{
+ struct device_node *node = priv->dev->of_node;
+ struct device *dev = priv->dev;
+ int i, error;
+ u32 u32_val;
+
+ if (!node) {
+ dev_err(dev, "Corresponding DT entry is not available\n");
+ return -ENODEV;
+ }
+
+ if (!of_property_read_u32(node, "microchip,sensor-gain", &u32_val)) {
+ if (priv->model->no_gain) {
+ dev_warn(dev,
+ "This model doesn't support 'sensor-gain'\n");
+ } else if (is_power_of_2(u32_val) && u32_val <= 8) {
+ priv->analog_gain = (u8)ilog2(u32_val);
+
+ error = regmap_update_bits(priv->regmap,
+ CAP11XX_REG_MAIN_CONTROL,
+ CAP11XX_REG_MAIN_CONTROL_GAIN_MASK,
+ priv->analog_gain << CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT);
+ if (error)
+ return error;
+ } else {
+ dev_err(dev, "Invalid sensor-gain value %u\n", u32_val);
+ return -EINVAL;
+ }
+ }
+
+ if (of_property_read_bool(node, "microchip,irq-active-high")) {
+ if (priv->id == CAP1106 ||
+ priv->id == CAP1126 ||
+ priv->id == CAP1188) {
+ error = regmap_update_bits(priv->regmap,
+ CAP11XX_REG_CONFIG2,
+ CAP11XX_REG_CONFIG2_ALT_POL,
+ 0);
+ if (error)
+ return error;
+ } else {
+ dev_warn(dev,
+ "This model doesn't support 'irq-active-high'\n");
+ }
+ }
+
+ if (!of_property_read_u32(node,
+ "microchip,sensitivity-delta-sense", &u32_val)) {
+ if (!is_power_of_2(u32_val) || u32_val > 128) {
+ dev_err(dev, "Invalid sensitivity-delta-sense value %u\n", u32_val);
+ return -EINVAL;
+ }
+
+ priv->sensitivity_delta_sense = (u8)ilog2(u32_val);
+ u32_val = ~(FIELD_PREP(CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK,
+ priv->sensitivity_delta_sense));
+
+ error = regmap_update_bits(priv->regmap,
+ CAP11XX_REG_SENSITIVITY_CONTROL,
+ CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK,
+ u32_val);
+ if (error)
+ return error;
+ }
+
+ if (!of_property_read_u32_array(node, "microchip,input-treshold",
+ priv->thresholds, priv->model->num_channels)) {
+ for (i = 0; i < priv->model->num_channels; i++) {
+ if (priv->thresholds[i] > 127) {
+ dev_err(dev, "Invalid input-treshold value %u\n",
+ priv->thresholds[i]);
+ return -EINVAL;
+ }
+
+ error = regmap_write(priv->regmap,
+ CAP11XX_REG_SENSOR_THRESH(i),
+ priv->thresholds[i]);
+ if (error)
+ return error;
+ }
+ }
+
+ if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
+ priv->calib_sensitivities, priv->model->num_channels)) {
+ if (priv->id == CAP1293 || priv->id == CAP1298) {
+ for (i = 0; i < priv->model->num_channels; i++) {
+ if (!is_power_of_2(priv->calib_sensitivities[i]) ||
+ priv->calib_sensitivities[i] > 4) {
+ dev_err(dev, "Invalid calib-sensitivity value %u\n",
+ priv->calib_sensitivities[i]);
+ return -EINVAL;
+ }
+ priv->calib_sensitivities[i] = ilog2(priv->calib_sensitivities[i]);
+ }
+
+ error = cap11xx_write_calib_sens_config_1(priv);
+ if (error)
+ return error;
+
+ if (priv->id == CAP1298) {
+ error = cap11xx_write_calib_sens_config_2(priv);
+ if (error)
+ return error;
+ }
+ } else {
+ dev_warn(dev,
+ "This model doesn't support 'calib-sensitivity'\n");
+ }
+ }
+
+ for (i = 0; i < priv->model->num_channels; i++) {
+ if (!of_property_read_u32_index(node, "microchip,signal-guard",
+ i, &u32_val)) {
+ if (u32_val > 1)
+ return -EINVAL;
+ if (u32_val)
+ priv->signal_guard_inputs_mask |= 0x01 << i;
+ }
+ }
+
+ if (priv->signal_guard_inputs_mask) {
+ if (priv->id == CAP1293 || priv->id == CAP1298) {
+ error = regmap_write(priv->regmap,
+ CAP11XX_REG_SIGNAL_GUARD_ENABLE,
+ priv->signal_guard_inputs_mask);
+ if (error)
+ return error;
+ } else {
+ dev_warn(dev,
+ "This model doesn't support 'signal-guard'\n");
+ }
+ }
+
+ /* Provide some useful defaults */
+ for (i = 0; i < priv->model->num_channels; i++)
+ priv->keycodes[i] = KEY_A + i;
+
+ of_property_read_u32_array(node, "linux,keycodes",
+ priv->keycodes, priv->model->num_channels);
+
+ /* Disable autorepeat. The Linux input system has its own handling. */
+ error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+ if (error)
+ return error;
+
+ return 0;
+}
+
static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
{
struct cap11xx_priv *priv = data;
@@ -332,11 +517,9 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
const struct i2c_device_id *id = i2c_client_get_device_id(i2c_client);
struct device *dev = &i2c_client->dev;
struct cap11xx_priv *priv;
- struct device_node *node;
const struct cap11xx_hw_model *cap;
- int i, error, irq, gain = 0;
+ int i, error;
unsigned int val, rev;
- u32 gain32;
if (id->driver_data >= ARRAY_SIZE(cap11xx_devices)) {
dev_err(dev, "Invalid device ID %lu\n", id->driver_data);
@@ -355,6 +538,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
if (!priv)
return -ENOMEM;
+ priv->dev = dev;
+
priv->regmap = devm_regmap_init_i2c(i2c_client, &cap11xx_regmap_config);
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);
@@ -384,50 +569,15 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
return error;
dev_info(dev, "CAP11XX detected, model %s, revision 0x%02x\n",
- id->name, rev);
- node = dev->of_node;
-
- if (!of_property_read_u32(node, "microchip,sensor-gain", &gain32)) {
- if (cap->no_gain)
- dev_warn(dev,
- "This version doesn't support sensor gain\n");
- else if (is_power_of_2(gain32) && gain32 <= 8)
- gain = ilog2(gain32);
- else
- dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
- }
+ id->name, rev);
- if (id->driver_data == CAP1106 ||
- id->driver_data == CAP1126 ||
- id->driver_data == CAP1188) {
- if (of_property_read_bool(node, "microchip,irq-active-high")) {
- error = regmap_update_bits(priv->regmap,
- CAP11XX_REG_CONFIG2,
- CAP11XX_REG_CONFIG2_ALT_POL,
- 0);
- if (error)
- return error;
- }
- }
-
- /* Provide some useful defaults */
- for (i = 0; i < cap->num_channels; i++)
- priv->keycodes[i] = KEY_A + i;
-
- of_property_read_u32_array(node, "linux,keycodes",
- priv->keycodes, cap->num_channels);
+ priv->model = cap;
+ priv->id = id->driver_data;
- if (!cap->no_gain) {
- error = regmap_update_bits(priv->regmap,
- CAP11XX_REG_MAIN_CONTROL,
- CAP11XX_REG_MAIN_CONTROL_GAIN_MASK,
- gain << CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT);
- if (error)
- return error;
- }
+ dev_info(dev, "CAP11XX device detected, model %s, revision 0x%02x\n",
+ id->name, rev);
- /* Disable autorepeat. The Linux input system has its own handling. */
- error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+ error = cap11xx_init_keys(priv);
if (error)
return error;
@@ -439,7 +589,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
priv->idev->id.bustype = BUS_I2C;
priv->idev->evbit[0] = BIT_MASK(EV_KEY);
- if (of_property_read_bool(node, "autorepeat"))
+ if (of_property_read_bool(dev->of_node, "autorepeat"))
__set_bit(EV_REP, priv->idev->evbit);
for (i = 0; i < cap->num_channels; i++)
@@ -474,14 +624,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
if (error)
return error;
- irq = irq_of_parse_and_map(node, 0);
- if (!irq) {
- dev_err(dev, "Unable to parse or map IRQ\n");
- return -ENXIO;
- }
-
- error = devm_request_threaded_irq(dev, irq, NULL, cap11xx_thread_func,
- IRQF_ONESHOT, dev_name(dev), priv);
+ error = devm_request_threaded_irq(dev, i2c_client->irq, NULL,
+ cap11xx_thread_func, IRQF_ONESHOT, dev_name(dev), priv);
if (error)
return error;
--
2.25.1
^ permalink raw reply related
* [PATCH v4 0/2] Input: cap11xx add advanced sensitivity settings
From: Jiri Valek - 2N @ 2023-06-27 6:53 UTC (permalink / raw)
To: krzysztof.kozlowski+dt, dmitry.torokhov
Cc: jiriv, devicetree, linux-input, linux-kernel, robh+dt,
u.kleine-koenig
PATCH 1 - add documentation for new options
PATCH 2 - add support for advanced settings into driver
Changes in v2:
- Removed "sensitivity-base-shift" parameter (not HW propertie) in PATCH 2.
- Used IRQ from I2C subsystem instead of parsing it again.
- Fixed some documentation issues in PATCH 1
Changes in v3:
- Remove incorrectly used "Reviewed-by" tag in PATCH 1 and 2
Changes in v4:
- Remove unused variable in PATCH 2
Jiri Valek - 2N (2):
dt-bindings: input: microchip,cap11xx: add advanced sensitivity
settings
Input: cap11xx - add advanced sensitivity settings
.../bindings/input/microchip,cap11xx.yaml | 77 +++++-
drivers/input/keyboard/cap11xx.c | 250 ++++++++++++++----
2 files changed, 269 insertions(+), 58 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH v4 1/2] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings
From: Jiri Valek - 2N @ 2023-06-27 6:53 UTC (permalink / raw)
To: krzysztof.kozlowski+dt, dmitry.torokhov
Cc: jiriv, devicetree, linux-input, linux-kernel, robh+dt,
u.kleine-koenig
In-Reply-To: <20230627065316.1065911-1-jiriv@axis.com>
Add support for advanced sensitivity settings and signal guard feature.
Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
---
.../bindings/input/microchip,cap11xx.yaml | 77 +++++++++++++++++--
1 file changed, 72 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 5fa625b5c5fb..b69dac1fba0e 100644
--- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
+++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
@@ -45,13 +45,13 @@ properties:
Enables the Linux input system's autorepeat feature on the input device.
linux,keycodes:
- minItems: 6
- maxItems: 6
+ minItems: 3
+ maxItems: 8
description: |
Specifies an array of numeric keycode values to
be used for the channels. If this property is
omitted, KEY_A, KEY_B, etc are used as defaults.
- The array must have exactly six entries.
+ The number of entries must correspond to the number of channels.
microchip,sensor-gain:
$ref: /schemas/types.yaml#/definitions/uint32
@@ -70,6 +70,55 @@ properties:
open drain. This property allows using the active
high push-pull output.
+ microchip,sensitivity-delta-sense:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 32
+ enum: [1, 2, 4, 8, 16, 32, 64, 128]
+ description:
+ Optional parameter. Controls the sensitivity multiplier of a touch detection.
+ At the more sensitive settings, touches are detected for a smaller delta
+ capacitance corresponding to a “lighter” touch.
+
+ microchip,signal-guard:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 3
+ maxItems: 8
+ items:
+ minimum: 0
+ maximum: 1
+ description: |
+ Optional parameter supported only for CAP129x.
+ 0 - off
+ 1 - on
+ The signal guard isolates the signal from virtual grounds.
+ If enabled then the behavior of the channel is changed to signal guard.
+ The number of entries must correspond to the number of channels.
+
+ microchip,input-treshold:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 3
+ maxItems: 8
+ items:
+ minimum: 0
+ maximum: 127
+ description:
+ Optional parameter. Specifies the delta threshold that is used to
+ determine if a touch has been detected.
+ The number of entries must correspond to the number of channels.
+
+ microchip,calib-sensitivity:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 3
+ maxItems: 8
+ items:
+ minimum: 1
+ maximum: 4
+ description:
+ Optional parameter supported only for CAP129x. Specifies an array of
+ numeric values that controls the gain used by the calibration routine to
+ enable sensor inputs to be more sensitive for proximity detection.
+ The number of entries must correspond to the number of channels.
+
patternProperties:
"^led@[0-7]$":
type: object
@@ -98,10 +147,23 @@ allOf:
compatible:
contains:
enum:
- - microchip,cap1106
+ - microchip,cap1188
then:
patternProperties:
- "^led@[0-7]$": false
+ "^led@[0-7]$": true
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - microchip,cap1293
+ - microchip,cap1298
+
+ then:
+ properties:
+ microchip,signal-guard: true
+ microchip,calib-sensitivity: true
required:
- compatible
@@ -122,6 +184,11 @@ examples:
reg = <0x28>;
autorepeat;
microchip,sensor-gain = <2>;
+ microchip,sensitivity-delta-sense = <16>;
+
+ microchip,signal-guard = <0>, <0>, <0>, <0>, <0>, <0>;
+ microchip,input-treshold = <21>, <18>, <46>, <46>, <46>, <21>;
+ microchip,calib-sensitivity = <1>, <2>, <2>, <1>, <1>, <2>;
linux,keycodes = <103>, /* KEY_UP */
<106>, /* KEY_RIGHT */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] HID: Add introduction about HID for non-kernel programmers
From: Peter Hutterer @ 2023-06-27 6:04 UTC (permalink / raw)
To: Marco Morandini
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, Jonathan Corbet,
linux-doc
In-Reply-To: <70fdef05-d3b8-e24b-77be-901bd5be369e@polimi.it>
On Thu, Jun 22, 2023 at 10:39:15PM +0200, Marco Morandini wrote:
> Add an introduction about HID
> meant for the casual programmers that is trying
> either to fix his device or to understand what
> is going wrong.
>
> Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
> ---
> v1: https://lore.kernel.org/linux-input/3mbw67akm2xzd2kgzb6sdfh4dly6im5jrz5umuvczjvrgxtf46@q5ooib3zkmfq/T/#m00b625a4d2c605dd7f62a866df7bf97ef2921d63
Thanks for all the fixes, much easier to read and follow already. A few
nitpicks like last time but this is coming together!
> Documentation/hid/hidintro.rst | 543 ++++++++++++++++++++++++
> Documentation/hid/hidreport-parsing.rst | 52 +++
> Documentation/hid/index.rst | 1 +
> include/linux/hid.h | 23 +
> 4 files changed, 619 insertions(+)
> create mode 100644 Documentation/hid/hidintro.rst
> create mode 100644 Documentation/hid/hidreport-parsing.rst
>
> diff --git a/Documentation/hid/hidintro.rst b/Documentation/hid/hidintro.rst
> new file mode 100644
> index 000000000000..cdaa09479147
> --- /dev/null
> +++ b/Documentation/hid/hidintro.rst
> @@ -0,0 +1,543 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +======================================
> +Introduction to HID report descriptors
> +======================================
> +
> +This chapter is meant to give a broad overview
> +of what HID report descriptors are, and of how a casual (non kernel)
> +programmer can deal with an HID device that
> +is not working well with Linux.
> +
> +.. contents::
> + :local:
> + :depth: 2
> +
> +.. toctree::
> + :maxdepth: 2
> +
> + hidreport-parsing
> +
> +
> +Introduction
> +============
> +
> +HID stands for Human Interface Device, and can be whatever device
> +you are using to interact with a computer, be it a mouse,
> +a touchpad, a tablet, a microphone.
> +
> +Many HID devices work out the box, even if their hardware is different.
> +For example, mice can have any number of buttons; they
> +may a wheel; movement sensitivity differs between different
> +models, and so on. Nonetheless,
> +most of the time everything just works, without the need
> +to have specialized code in the kernel for every mouse model
> +developed since 1970.
A general request - can you reformat the paragraphs please to be roughly
the max (or just the same-ish) line lengths. Some of these paragraphs
read more like a poem
with superfluous
linebreaks
sprinkled all over it.
> +
> +This is because modern HID devices do advertise
> +their capabilities through the *HID report descriptor*, a
> +fixed set of bytes describing exactly what *HID reports*
> +may be sent between the device and the host and the meaning of each
> +individual bit in those reports. For example, a HID Report Descriptor
> +may specify that "in a report with ID 3 the second byte is the delta x
> +coordinate of a mouse".
staring at this and if you're doing a v3 anyway, I'd say "bits 8 to 15"
instead of "second byte" to emphasise that HID reports are usually more
bit-y than byte-y. But that's a minor nitpick that doesn't matter too
much.
> +
> +The HID report itself then merely carries the actual data values
> +without any extra meta information. Note that HID reports may be sent
> +from the device ("Input Reports", i.e. input events), to the device
> +("Output Reports" to e.g. change LEDs) or used for device
> +configuration ("Feature reports"). A device may support one or more
> +HID reports.
> +
> +The HID subsystem is in charge of parsing the HID report descriptors,
> +and converts HID events into normal input
> +device interfaces (see Documentation/hid/hid-transport.rst).
> +Devices may misbehave because the HID report descriptor
> +provided by the device is wrong,
> +or because it needs to be dealt with in a special way,
> +or because some special device or interaction mode
> +is not handled by the default code.
> +
> +The format of HID report descriptors is described by two documents,
> +available from the `USB Implementers Forum <https://www.usb.org/>`_
> +at `this <https://www.usb.org/hid>`_ addresses:
> +
> + * the `HID USB Device Class Definition
> + <https://www.usb.org/document-library/device-class-definition-hid-111>`_ (HID Spec from now on)
> + * the `HID Usage Tables <https://usb.org/document-library/hid-usage-tables-14>`_ (HUT from now on)
> +
> +This does not mean that the HID subsystem can deal with USB devices only;
> +rather, different transport drivers, such as I2C or Bluetooth, can be dealt
> +with, see Documentation/hid/hid-transport.rst.
Simpler wording: "The HID subsystem can deal with different transport drivers
(USB, I2C, Bluetooth, etc.). See Documentation/hid/hid-transport.rst."
> +
> +Parsing HID report descriptors
> +==============================
> +
> +The current list of HID devices can be found at ``/sys/bus/hid/devices/``.
> +For each device, say ``/sys/bus/hid/devices/0003\:093A\:2510.0002/``,
> +one can read the corresponding report descriptor::
> +
> + marco@sun:~> hexdump -C /sys/bus/hid/devices/0003\:093A\:2510.0002/report_descriptor
> + 00000000 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 03 |..............).|
> + 00000010 15 00 25 01 75 01 95 03 81 02 75 05 95 01 81 01 |..%.u.....u.....|
> + 00000020 05 01 09 30 09 31 09 38 15 81 25 7f 75 08 95 03 |...0.1.8..%.u...|
> + 00000030 81 06 c0 c0 |....|
> + 00000034
> +
> +Optional stuff: the HID report descriptor can be read also by
> +directly accessing the hidraw driver [#hidraw]_.
> +
> +The basic structure of HID report descriptors is defined in the HID spec, while
> +HUT "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 HID spec,
no comma before "and" in this case
> +while the second one carries the actual value,
superfluous comma-man to the rescue! :)
> +and is described in the HUT.
> +
> +HID report descriptors can, in principle, be painstakingly
> +parsed by hand, byte by byte.
> +
> +A short introduction
> +on how to do this is sketched in Documentation/hid/hidreport-parsing.rst;
> +you need to understand it only if you need to patch HID report descriptors.
I think this should be "you only need to ..." (but I'm ESL too)
> +
> +In practice you should not do parse HID report descriptors by hand;
> +rather, you should use an existing parser.
> +Among all the available ones
> +
> + * the online `USB Descriptor and Request Parser
> + <http://eleccelerator.com/usbdescreqparser/>`_;
> + * `hidrdd <https://github.com/abend0c1/hidrdd>`_,
> + that provides very detailed and somewhat verbose descriptions
> + (verbosity can be useful if you are not familiar with HID report descriptors);
> + * `hid-tools <https://gitlab.freedesktop.org/libevdev/hid-tools>`_,
> + a complete utility set that allows, among other things,
> + to record and replay the raw HID reports and to debug
> + and replay HID devices.
> + It is being actively developed by the Linux HID subsystem mantainers.
> +
> +Parsing the mouse HID report descriptor with `hid-tools <https://gitlab.freedesktop.org/libevdev/hid-tools>`_ leads to
> +(explanations interposed)::
> +
> + marco@sun:~/Programmi/linux/hid-tools (master =)> ./hid-decode /sys/bus/hid/devices/0003\:093A\:2510.0002/report_descriptor
shorten the prompt to just "$ hid-decode ..." and it'll fit better on a
single line
> + # device 0:0
> + # 0x05, 0x01, // Usage Page (Generic Desktop) 0
> + # 0x09, 0x02, // Usage (Mouse) 2
> + # 0xa1, 0x01, // Collection (Application) 4
> + # 0x09, 0x01, // Usage (Pointer) 6
> + # 0xa1, 0x00, // Collection (Physical) 8
> + # 0x05, 0x09, // Usage Page (Button) 10
> + what follows is a button
> +
> + # 0x19, 0x01, // Usage Minimum (1) 12
> + # 0x29, 0x03, // Usage Maximum (3) 14
> + first button is button number 1, last button is button number 3
> +
> + # 0x15, 0x00, // Logical Minimum (0) 16
> + # 0x25, 0x01, // Logical Maximum (1) 18
> + each button can send values from 0 up to including 1
> + (i.e. they are binary buttons)
> +
> + # 0x75, 0x01, // Report Size (1) 20
> + each button is sent as exactly one bit
> +
> + # 0x95, 0x03, // Report Count (3) 22
> + and there are three of those bits
> + (matching the three buttons)
> +
> + # 0x81, 0x02, // Input (Data,Var,Abs) 24
> + it's actual Data (not constant padding), they represent
> + a single variable (Var) and the value are Absolute (not relative);
> + See HID spec Sec. 6.2.2.5 "Input, Output, and Feature Items
> +
> + # 0x75, 0x05, // Report Size (5) 26
> + five additional padding bits, needed to reach a byte
> +
> + # 0x95, 0x01, // Report Count (1) 28
> + those five bits are repeated only once
> +
> + # 0x81, 0x01, // Input (Cnst,Arr,Abs) 30
> + and take Constant (Cnst) values
"i.e. they can be ignored"
> +
> + # 0x05, 0x01, // Usage Page (Generic Desktop) 32
> + # 0x09, 0x30, // Usage (X) 34
> + # 0x09, 0x31, // Usage (Y) 36
> + # 0x09, 0x38, // Usage (Wheel) 38
> + The mouse has also two physical positions (``Usage (X)``, ``Usage (Y)``)
too many spaces here
> + and a wheel (Usage (Wheel))
> +
> + # 0x15, 0x81, // Logical Minimum (-127) 40
> + # 0x25, 0x7f, // Logical Maximum (127) 42
> + each of them can send values ranging from -127 up to including 127
> +
> + # 0x75, 0x08, // Report Size (8) 44
> + is represented by eight bits
> +
> + # 0x95, 0x03, // Report Count (3) 46
> + and there are three of those eight bits, matching X, Y and Wheel
> +
> + # 0x81, 0x06, // Input (Data,Var,Rel) 48
> + This time the data is Relative (Rel), i.e. it represent
> + the change from the previous configuration
"the previously sent report (event)"
> +
> + # 0xc0, // End Collection 50
> + # 0xc0, // End Collection 51
> + #
> + R: 52 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 03 15 00 25 01 75 01 95 03 81 02 75 05 95 01 81 01 05 01 09 30 09 31 09 38 15 81 25 7f 75 08 95 03 81 06 c0 c0
> + N: device 0:0
> + I: 3 0001 0001
> +
> +
> +This Report Descriptor tells us that the mouse input will be
> +transmitted using four bytes:
> +the first one for the buttons (three bits used, five for padding),
> +the last three for the mouse X, Y and wheel changes, respectively.
> +
> +Indeed, for any event, the mouse will send a *report* of four bytes.
> +We can check the values sent by resorting e.g.
> +to the `hid-recorder` tool, from `hid-tools
> +<https://gitlab.freedesktop.org/libevdev/hid-tools>`_:
> +The sequence of bytes sent by clicking and releasing
> +button 1, then button 2, then button 3 is::
> +
> + marco@sun:~/> sudo ./hid-recorder /dev/hidraw1
for consistency please change the prompt here to `$` as well
> +
> + ....
> + output of hid-decode
> + ....
> +
> + # Button: 1 0 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000000.000000 4 01 00 00 00
> + # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000000.183949 4 00 00 00 00
> + # Button: 0 1 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000001.959698 4 02 00 00 00
> + # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000002.103899 4 00 00 00 00
> + # Button: 0 0 1 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000004.855799 4 04 00 00 00
> + # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000005.103864 4 00 00 00 00
> +
> +This example shows that when button 2 is clicked,
> +the bytes ``02 00 00 00`` are sent, and the immediately subsequent
> +event (``00 00 00 00``) is the release of button 2 (no buttons are pressed,
> +remember that the data is *absolute*).
> +
> +If instead one clicks and holds button 1, then clicks and holds button 2,
> +releases button 1, and finally releases button 2, the reports are::
> +
> + # Button: 1 0 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000044.175830 4 01 00 00 00
> + # Button: 1 1 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000045.975997 4 03 00 00 00
> + # Button: 0 1 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000047.407930 4 02 00 00 00
> + # Button: 0 0 0 | # | X: 0 | Y: 0 | Wheel: 0
> + E: 000049.199919 4 00 00 00 00
> +
> +where with ``03 00 00 00`` both buttons are pressed, and with the
> +subsequent ``02 00 00 00`` button 1 is released while button 2 is still
> +active.
> +
> +Output, Input and Feature Reports
> +---------------------------------
> +
> +An HID devices can have Input Reports, like
> +in the mouse example, Output Reports, and Features Reports.
s/Features/Feature/
> +"Output" means that the information is fed
s/fed/sent/
> +to the device. For example,
> +a joystick with force feedback will have
> +some output; the led of a keyboard would
> +need an output as well.
> +"Input" means that data
> +come from the device.
s/come/comes/
Not sure, never got the hang of singular vs plural data, whatever
is used in the rest of the documentation.
> +
> +"Feature"s are not meant to be consumed by the end user
> +and define configuration options for the device.
> +They can be queried from the host;
> +when declared as *Volatile*
> +they should be changed by the host.
> +
> +
> +Collections, Report IDs and Evdev events
> +========================================
> +
> +A single device can logically group
> +data into different, independent sets,
> +called *Collection*.
"called a *Collection*"
> +Collections can be nested, and there
> +are different types of collections
> +(see the HID spec 6.2.2.6
> +"Collection, End Collection Items" for details).
> +
> +Different reports are identified by means
> +of different *Report ID* fields, i.e. a number identifying
> +the structure of the immediately following report.
> +Whenever a Report ID
> +is needed it is transmitted as the first byte of any report.
"A device with only one supported HID report (like the mouse example above)
may omit the report ID."
> +
> +Consider the following HID report descriptor::
> +
> + 05 01 09 02 A1 01 85 01 05 09 19 01 29 05 15 00
> + 25 01 95 05 75 01 81 02 95 01 75 03 81 01 05 01
> + 09 30 09 31 16 00 F8 26 FF 07 75 0C 95 02 81 06
> + 09 38 15 80 25 7F 75 08 95 01 81 06 05 0C 0A 38
> + 02 15 80 25 7F 75 08 95 01 81 06 C0 05 01 09 02
> + A1 01 85 02 05 09 19 01 29 05 15 00 25 01 95 05
> + 75 01 81 02 95 01 75 03 81 01 05 01 09 30 09 31
> + 16 00 F8 26 FF 07 75 0C 95 02 81 06 09 38 15 80
> + 25 7F 75 08 95 01 81 06 05 0C 0A 38 02 15 80 25
> + 7F 75 08 95 01 81 06 C0 05 01 09 07 A1 01 85 05
> + 05 07 15 00 25 01 09 29 09 3E 09 4B 09 4E 09 E3
> + 09 E8 09 E8 09 E8 75 01 95 08 81 02 95 00 81 01
> + C0 05 0C 09 01 A1 01 85 06 15 00 25 01 75 01 95
> + 01 09 3F 81 06 09 3F 81 06 09 3F 81 06 09 3F 81
> + 06 09 3F 81 06 09 3F 81 06 09 3F 81 06 09 3F 81
> + 06 C0 05 0C 09 01 A1 01 85 03 09 05 15 00 26 FF
> + 00 75 08 95 02 B1 02 C0
> +
> +After parsing it (try to parse it on your own using
> +the suggested tools!)
> +one can see that the device presents two ``Mouse``
> +Application Collections
> +(with reports identified by Reports IDs 1 and 2, respectively),
> +a ``Keypad`` Application Collection (whose report is identified
> +by the Report ID 5) and two ``Consumer Controls``
> +Application Collections,
> +(with Report IDs 6 and 3, respectively).
> +Note that, however, that you can have different Report IDs
> +for the same Application Collection.
> +
> +The data sent will begin with the Report ID byte, and will be followed
> +by the corresponding information. For example, the
> +data transmitted for the last consumer
> +control::
> +
> + 0x05, 0x0C, // Usage Page (Consumer)
> + 0x09, 0x01, // Usage (Consumer Control)
> + 0xA1, 0x01, // Collection (Application)
> + 0x85, 0x03, // Report ID (3)
> + 0x09, 0x05, // Usage (Headphone)
> + 0x15, 0x00, // Logical Minimum (0)
> + 0x26, 0xFF, 0x00, // Logical Maximum (255)
> + 0x75, 0x08, // Report Size (8)
> + 0x95, 0x02, // Report Count (2)
> + 0xB1, 0x02, // Feature (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
> + 0xC0, // End Collection
> +
> +will be of three bytes: the first for the Report ID (3), the next two
> +for the headphone, with two (``Report Count (2)``) bytes
> +(``Report Size (8)``), each ranging from 0 (``Logical Minimum (0)`` to 255
> +(``Logical Maximum (255)``).
> +
> +All the Input data sent by the device should be translated into corresponding
> +Evdev events, so that the remaining part of the stack can know what is going on,
> +e.g. that a mouse button was pressed, rather
> +than a mouse has been moved in the X direction.
This example is so obvious that it's actually confusing, a better
example may be: "e.g. the bit for the first button translates into the
EV_KEY/BTN_LEFT evdev event and relative x movement translates into the
EV_REL/REL_X evdev event".
> +
> +Events
> +======
> +
> +In Linux, one ``/dev/input/event*`` is created for each
> +``Application Collection``.
> +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 libinput record /dev/input/event1
$ prompt reminder
> + # libinput record
> + version: 1
> + ndevices: 1
> + libinput:
> + version: "1.23.0"
> + git: "unknown"
> + system:
> + os: "opensuse-tumbleweed:20230619"
> + kernel: "6.3.7-1-default"
> + dmi: "dmi:bvnHP:bvrU77Ver.01.05.00:bd03/24/2022:br5.0:efr20.29:svnHP:pnHPEliteBook64514inchG9NotebookPC:pvr:rvnHP:rn89D2:rvrKBCVersion14.1D.00:cvnHP:ct10:cvr:sku5Y3J1EA#ABZ:"
> + devices:
> + - node: /dev/input/event1
> + evdev:
> + # Name: PixArt HP USB Optical Mouse
> + # ID: bus 0x3 vendor 0x3f0 product 0x94a version 0x111
> + # Supported Events:
> + # Event type 0 (EV_SYN)
> + # Event type 1 (EV_KEY)
> + # Event code 272 (BTN_LEFT)
> + # Event code 273 (BTN_RIGHT)
> + # Event code 274 (BTN_MIDDLE)
> + # Event type 2 (EV_REL)
> + # Event code 0 (REL_X)
> + # Event code 1 (REL_Y)
> + # Event code 8 (REL_WHEEL)
> + # Event code 11 (REL_WHEEL_HI_RES)
> + # Event type 4 (EV_MSC)
> + # Event code 4 (MSC_SCAN)
> + # Properties:
> + name: "PixArt HP USB Optical Mouse"
> + id: [3, 1008, 2378, 273]
> + codes:
> + 0: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15] # EV_SYN
> + 1: [272, 273, 274] # EV_KEY
> + 2: [0, 1, 8, 11] # EV_REL
> + 4: [4] # EV_MSC
> + properties: []
> + hid: [
> + 0x05, 0x01, 0x09, 0x02, 0xa1, 0x01, 0x09, 0x01, 0xa1, 0x00, 0x05, 0x09, 0x19, 0x01, 0x29, 0x03,
> + 0x15, 0x00, 0x25, 0x01, 0x95, 0x08, 0x75, 0x01, 0x81, 0x02, 0x05, 0x01, 0x09, 0x30, 0x09, 0x31,
> + 0x09, 0x38, 0x15, 0x81, 0x25, 0x7f, 0x75, 0x08, 0x95, 0x03, 0x81, 0x06, 0xc0, 0xc0
> + ]
> + udev:
> + properties:
> + - ID_INPUT=1
> + - ID_INPUT_MOUSE=1
> + - LIBINPUT_DEVICE_GROUP=3/3f0/94a:usb-0000:05:00.3-2
> + quirks:
> + events:
> + # Current time is 12:31:56
> + - evdev:
> + - [ 0, 0, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
> + - [ 0, 0, 1, 272, 1] # EV_KEY / BTN_LEFT 1
> + - [ 0, 0, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +0ms
> + - evdev:
> + - [ 1, 207892, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
> + - [ 1, 207892, 1, 273, 1] # EV_KEY / BTN_RIGHT 1
> + - [ 1, 207892, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +1207ms
> + - evdev:
> + - [ 2, 367823, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
> + - [ 2, 367823, 1, 272, 0] # EV_KEY / BTN_LEFT 0
> + - [ 2, 367823, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +1160ms
> + # Current time is 12:32:00
> + - evdev:
> + - [ 3, 247617, 4, 4, 30] # EV_MSC / MSC_SCAN 30 (obfuscated)
> + - [ 3, 247617, 1, 273, 0] # EV_KEY / BTN_RIGHT 0
> + - [ 3, 247617, 0, 0, 0] # ------------ SYN_REPORT (0) ---------- +880ms
> +
> +Note: if ``libinput`` is not available on your system try using ``evemu-record``.
> +
> +When something does not work
> +============================
> +
> +There can be a number of reasons for why a device does not behave
> +correctly. For example
> +
> +* The HID report descriptor provided by the HID device may be wrong
> + because e.g.
> +
> + * it does not follow the standard, so that the kernel
> + will not able to make sense of the HID report descriptor;
> + * it is possible to verify, by reading the raw HID data, that
> + the HID report descriptor *does not match* what is actually
> + sent by the device;
* the HID report descriptor *does not match* what is actually
sent by the device (this can be verified by reading the raw HID
data),
> +
> +or
> +
skip the or, we're already in an enumeration anyway so this can be a
single list
> +* the HID report descriptor may need some "quirks" (see later on);
> +
> +As a consequence, a suitable ``/dev/input/event*`` will not created
s/suitable//
s/will/may/
> +for each Application Collection, and/or the events
> +there will match what you would expect.
s/will/may not/
> +
> +
> +Quirks
> +------
> +
> +There are some known peculiarities of HID devices that the kernel
> +knows how to fix - these are called the HID quirks and a list of those
> +are available in `include/linux/hid.h`.
> +
> +Should this be the case,
> +it should be enough to add the required quirk,
> +in the kernel, for the HID device at hand.
> +This can be done in file `drivers/hid/hid-quirks.c`.
s/file/the file/
> +How to do it should be relatively straightforward
> +after looking into the file.
> +
> +The list of currently defined quirks, from
> +`include/linux/hid.h`, is
> +
> +.. kernel-doc:: include/linux/hid.h
> + :doc: HID quirks
> +
> +Quirks for USB devices can be specified
> +while loading the usbhid module,
> +see ``modinfo usbhid``, although the proper fix
> +should go into hid-quirks.c and **submitted upstream**.
> +See, again, Documentation/process/submitting-patches.rst
s/, again,// (sounds too patronising otherwise)
> +for guidelines on how to do submit a patch.
> +Quirks for other busses need to go into hid-quirks.c
> +
> +Fixing HID report descriptors
> +-----------------------------
> +
> +Should you need to patch HID report descriptors
> +the easiest way is to resort to eBPF, as described
> +in Documentation/hid/hid-bpf.rst.
> +
> +Basically, you can change any byte of the original HID report descriptor.
> +The examples in samples/hid should be a good starting point
> +for your code, 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;
> + }
> +
> +Of course this can be also done within the kernel source
> +code, see e.g. `drivers/hid/hid-aureal.c` or
> +`drivers/hid/hid-samsung.c` for a slightly more complex file.
> +
> +Check Documentation/hid/hidreport-parsing.rst
> +if you need an help navigating the HID manuals and
> +understanding the exact meaning of
> +the HID report descriptor hex numbers.
> +
> +Whatever solution you come up with, please remember to **submit the
> +fix to the HID maintainers**, so that it can be directly
> +integrated in the kernel
> +and that particular HID device will will start
> +working for everyone else.
> +See Documentation/process/submitting-patches.rst
> +for guidelines on how to do this.
> +
> +
> +Modifying the transmitted data on the fly
> +-----------------------------------------
> +
> +Using eBPF it is also possible to modify the data exchanged
> +with the device. See again the examples in `samples/hid`.
> +
> +Again, **please post your fix**, so that
> +it can be integrated in the kernel!
> +
> +Writing a specialized driver
> +----------------------------
> +
> +This should really be your last resort.
> +
> +
> +.. rubric:: Footnotes
> +
> +.. [#hidraw] reading hidraw: see Documentation/hid/hidraw.rst and
> + file `samples/hidraw/hid-example.c` for an example.
> + The output of ``hid-example`` would be, for the same mouse::
> +
> + marco@sun:~> sudo ./hid-example
$ prompt reminder
> + Report Descriptor Size: 52
> + Report Descriptor:
> + 5 1 9 2 a1 1 9 1 a1 0 5 9 19 1 29 3 15 0 25 1 75 1 95 3 81 2 75 5 95 1 81 1 5 1 9 30 9 31 9 38 15 81 25 7f 75 8 95 3 81 6 c0 c0
> +
> + Raw Name: PixArt USB Optical Mouse
> + Raw Phys: usb-0000:05:00.4-2.3/input0
> + Raw Info:
> + bustype: 3 (USB)
> + vendor: 0x093a
> + product: 0x2510
> + ...
> diff --git a/Documentation/hid/hidreport-parsing.rst b/Documentation/hid/hidreport-parsing.rst
> new file mode 100644
> index 000000000000..a31a7aec5947
> --- /dev/null
> +++ b/Documentation/hid/hidreport-parsing.rst
> @@ -0,0 +1,52 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +========================================
> +Manual parsing of HID report descriptors
> +========================================
> +
> +Consider again the mouse HID report descriptor
> +introduced in Documentation/hid/hidintro.rst::
> +
> + marco@sun:~> hexdump -C /sys/bus/hid/devices/0003\:093A\:2510.0002/report_descriptor
> + 00000000 05 01 09 02 a1 01 09 01 a1 00 05 09 19 01 29 03 |..............).|
> + 00000010 15 00 25 01 75 01 95 03 81 02 75 05 95 01 81 01 |..%.u.....u.....|
> + 00000020 05 01 09 30 09 31 09 38 15 81 25 7f 75 08 95 03 |...0.1.8..%.u...|
> + 00000030 81 06 c0 c0 |....|
> + 00000034
> +
> +and try to parse it by hand.
> +
> +Start with the first number, 0x05: according to
> +the HID spec, Sec. 6.2.2.2, "Short Items"
> +
> +Start with the first number, 0x05 which carries 2 bits for the
> +length of the item, 2 bits for the type of the item and 4 bits for the
> +function::
> +
> + +----------+
> + | 00000101 |
> + +----------+
> + ^^
> + ---- Length of data (see HID spec 6.2.2.2)
> + ^^
> + ------ Type of the item (see HID spec 6.2.2.7)
> + ^^^^
> + --------- Function of the item (see HUT Sec 3)
this is misaligned
Cheers,
Peter
> +
> +In our case, the length is 1 byte, the type is ``Global`` and the function
> +is ``Usage Page``, thus we need to refer to HUT Sec 3 which indicates that
> +the value 0x01 in the second byte stands for ``Generic Desktop Page``.
> +
> +The second number is the actual data, and its meaning can
> +be found in the HUT.
> +We have an ``Usage Page``, thus we need to refer to HUT Sec. 3,
> +"Usage Pages"; from there, one sees that the ``0x01``
> +stands for ``Generic Desktop Page``.
> +
> +Moving now to the second two bytes, and following the same scheme, ``0x09``
> +(i.e. ``00001001``) will be followed by one byte (``01``)
> +and is a ``Local`` item.
> +Thus, the meaning of the remaining four bits (``0000``)
> +is given in the HID spec Sec. 6.2.2.8 "Local Items", so that we have an ``Usage``.
> +
> +The following numbers can be parsed in the same way.
> diff --git a/Documentation/hid/index.rst b/Documentation/hid/index.rst
> index b2028f382f11..af02cf7cfa82 100644
> --- a/Documentation/hid/index.rst
> +++ b/Documentation/hid/index.rst
> @@ -7,6 +7,7 @@ Human Interface Devices (HID)
> .. toctree::
> :maxdepth: 1
>
> + hidintro
> hiddev
> hidraw
> hid-sensor
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 7f2e8ba7d783..ad12a36d9993 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -341,6 +341,29 @@ struct hid_item {
> */
> #define MAX_USBHID_BOOT_QUIRKS 4
>
> +/**
> +* DOC: HID quirks
> +* | @HID_QUIRK_NOTOUCH:
> +* | @HID_QUIRK_IGNORE: ignore this device
> +* | @HID_QUIRK_NOGET:
> +* | @HID_QUIRK_HIDDEV_FORCE:
> +* | @HID_QUIRK_BADPAD:
> +* | @HID_QUIRK_MULTI_INPUT:
> +* | @HID_QUIRK_HIDINPUT_FORCE:
> +* | @HID_QUIRK_ALWAYS_POLL:
> +* | @HID_QUIRK_INPUT_PER_APP:
> +* | @HID_QUIRK_X_INVERT:
> +* | @HID_QUIRK_Y_INVERT:
> +* | @HID_QUIRK_SKIP_OUTPUT_REPORTS:
> +* | @HID_QUIRK_SKIP_OUTPUT_REPORT_ID:
> +* | @HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP:
> +* | @HID_QUIRK_HAVE_SPECIAL_DRIVER:
> +* | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE:
> +* | @HID_QUIRK_FULLSPEED_INTERVAL:
> +* | @HID_QUIRK_NO_INIT_REPORTS:
> +* | @HID_QUIRK_NO_IGNORE:
> +* | @HID_QUIRK_NO_INPUT_SYNC:
> +*/
> /* BIT(0) reserved for backward compatibility, was HID_QUIRK_INVERT */
> #define HID_QUIRK_NOTOUCH BIT(1)
> #define HID_QUIRK_IGNORE BIT(2)
> --
> 2.41.0
>
^ permalink raw reply
* Re: [PATCH v9 2/3] Input: Add Novatek NT36xxx touchscreen driver
From: Konrad Dybcio @ 2023-06-26 23:59 UTC (permalink / raw)
To: kholk11, dmitry.torokhov, AngeloGioacchino Del Regno
Cc: robh+dt, rydberg, priv.luk, linux-input, linux-kernel, marijns95,
martin.botka1, phone-devel, devicetree, krzk, andy.shevchenko
In-Reply-To: <20201028221302.66583-3-kholk11@gmail.com>
On 28.10.2020 23:13, kholk11@gmail.com wrote:
> This is a driver for the Novatek in-cell touch controller and
> supports various chips from the NT36xxx family, currently
> including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
>
> Functionality like wake gestures and firmware flashing is not
> included: I am not aware of any of these DrIC+Touch combo
> chips not including a non-volatile memory and it should be
> highly unlikely to find one, since the touch firmware is
> embedded into the DriverIC one, which is obviously necessary
> to drive the display unit.
>
> However, the necessary address for the firmware update
> procedure was included into the address table in this driver
> so, in the event that someone finds the need to implement it
> for a reason or another, it will be pretty straightforward to.
>
> This driver is lightly based on the downstream implementation [1].
> [1] https://github.com/Rasenkai/caf-tsoft-Novatek-nt36xxx
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
So.. this patchset has gone stale for.. a bit.. hopefully to age like
fine wine!
Dmitry, are there any other major outstanding issues you see with it?
I'd be happy to pick it up, fix them up and resubmit. Quite some recent
-ish phones use this touchpanel. And tablets. And other smart rocks!
FWIW there was some discussion regarding regmap usage:
https://patchwork.kernel.org/project/linux-input/patch/20201028221302.66583-3-kholk11@gmail.com/
Konrad
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/nt36xxx.c | 894 ++++++++++++++++++++++++++++
> drivers/input/touchscreen/nt36xxx.h | 122 ++++
> 4 files changed, 1029 insertions(+)
> create mode 100644 drivers/input/touchscreen/nt36xxx.c
> create mode 100644 drivers/input/touchscreen/nt36xxx.h
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 35c867b2d9a7..6d118b967021 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -605,6 +605,18 @@ config TOUCHSCREEN_MTOUCH
> To compile this driver as a module, choose M here: the
> module will be called mtouch.
>
> +config TOUCHSCREEN_NT36XXX
> + tristate "Novatek NT36XXX In-Cell I2C touchscreen controller"
> + depends on I2C
> + help
> + Say Y here if you have a Novatek NT36xxx series In-Cell
> + touchscreen connected to your system over I2C.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called nt36xxx.
> +
> config TOUCHSCREEN_IMX6UL_TSC
> tristate "Freescale i.MX6UL touchscreen controller"
> depends on (OF && GPIOLIB) || COMPILE_TEST
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 30d1e1b42492..424a555e03d5 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_TOUCHSCREEN_MIGOR) += migor_ts.o
> obj-$(CONFIG_TOUCHSCREEN_MMS114) += mms114.o
> obj-$(CONFIG_TOUCHSCREEN_MTOUCH) += mtouch.o
> obj-$(CONFIG_TOUCHSCREEN_MK712) += mk712.o
> +obj-$(CONFIG_TOUCHSCREEN_NT36XXX) += nt36xxx.o
> obj-$(CONFIG_TOUCHSCREEN_HP600) += hp680_ts_input.o
> obj-$(CONFIG_TOUCHSCREEN_HP7XX) += jornada720_ts.o
> obj-$(CONFIG_TOUCHSCREEN_IPAQ_MICRO) += ipaq-micro-ts.o
> diff --git a/drivers/input/touchscreen/nt36xxx.c b/drivers/input/touchscreen/nt36xxx.c
> new file mode 100644
> index 000000000000..a572d2b87464
> --- /dev/null
> +++ b/drivers/input/touchscreen/nt36xxx.c
> @@ -0,0 +1,894 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Novatek NT36xxx series touchscreens
> + *
> + * Copyright (C) 2010 - 2017 Novatek, Inc.
> + * Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/interrupt.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <asm/unaligned.h>
> +
> +/* FW Param address */
> +#define NT36XXX_FW_ADDR 0x01
> +
> +/* Number of bytes for chip identification */
> +#define NT36XXX_ID_LEN_MAX 6
> +
> +/* Touch info */
> +#define TOUCH_DEFAULT_MAX_WIDTH 1080
> +#define TOUCH_DEFAULT_MAX_HEIGHT 2246
> +#define TOUCH_MAX_FINGER_NUM 10
> +#define TOUCH_MAX_PRESSURE 1000
> +
> +/* Point data length */
> +#define POINT_DATA_LEN 65
> +
> +/* Global pages */
> +#define NT36XXX_PAGE_CHIP_INFO 0x0001f64e
> +#define NT36XXX_PAGE_CRC 0x0003f135
> +
> +/* Misc */
> +#define NT36XXX_NUM_SUPPLIES 2
> +#define NT36XXX_MAX_RETRIES 5
> +#define NT36XXX_MAX_FW_RST_RETRY 50
> +
> +struct nt36xxx_abs_object {
> + u16 x;
> + u16 y;
> + u16 z;
> + u8 tm;
> +};
> +
> +struct nt36xxx_fw_info {
> + u8 fw_ver;
> + u8 x_num;
> + u8 y_num;
> + u8 max_buttons;
> + u16 abs_x_max;
> + u16 abs_y_max;
> + u16 nvt_pid;
> +};
> +
> +struct nt36xxx_mem_map {
> + u32 evtbuf_addr;
> + u32 pipe0_addr;
> + u32 pipe1_addr;
> + u32 flash_csum_addr;
> + u32 flash_data_addr;
> +};
> +
> +struct nt36xxx_i2c {
> + struct i2c_client *hw_client;
> + struct i2c_client *fw_client;
> + struct regmap *regmap;
> + struct regmap *fw_regmap;
> + struct input_dev *input;
> + struct regulator_bulk_data *supplies;
> + struct gpio_desc *reset_gpio;
> +
> + struct mutex lock;
> +
> + struct touchscreen_properties prop;
> + struct nt36xxx_fw_info fw_info;
> + struct nt36xxx_abs_object abs_obj;
> +
> + const struct nt36xxx_mem_map *mmap;
> +};
> +
> +enum nt36xxx_chips {
> + NT36525_IC = 0,
> + NT36672A_IC,
> + NT36676F_IC,
> + NT36772_IC,
> + NT36870_IC,
> + NTMAX_IC,
> +};
> +
> +struct nt36xxx_trim_table {
> + u8 id[NT36XXX_ID_LEN_MAX];
> + u8 mask[NT36XXX_ID_LEN_MAX];
> + enum nt36xxx_chips mapid;
> +};
> +
> +enum nt36xxx_cmds {
> + NT36XXX_CMD_ENTER_SLEEP = 0x11,
> + NT36XXX_CMD_ENTER_WKUP_GESTURE = 0x13,
> + NT36XXX_CMD_UNLOCK = 0x35,
> + NT36XXX_CMD_BOOTLOADER_RESET = 0x69,
> + NT36XXX_CMD_SW_RESET = 0xa5,
> + NT36XXX_CMD_SET_PAGE = 0xff,
> +};
> +
> +/**
> + * enum nt36xxx_fw_state - Firmware state
> + * @NT36XXX_STATE_INIT: IC Reset
> + * @NT36XXX_STATE_REK: ReK baseline
> + * @NT36XXX_STATE_REK_FINISH: Baseline is ready
> + * @NT36XXX_STATE_NORMAL_RUN: Firmware is running
> + */
> +enum nt36xxx_fw_state {
> + NT36XXX_STATE_INIT = 0xa0,
> + NT36XXX_STATE_REK,
> + NT36XXX_STATE_REK_FINISH,
> + NT36XXX_STATE_NORMAL_RUN,
> + NT36XXX_STATE_MAX = 0xaf
> +};
> +
> +enum nt36xxx_i2c_events {
> + NT36XXX_EVT_REPORT = 0x00,
> + NT36XXX_EVT_CRC = 0x35,
> + NT36XXX_EVT_CHIPID = 0x4e,
> + NT36XXX_EVT_HOST_CMD = 0x50,
> + NT36XXX_EVT_HS_OR_SUBCMD = 0x51, /* Handshake or subcommand byte */
> + NT36XXX_EVT_RESET_COMPLETE = 0x60,
> + NT36XXX_EVT_FWINFO = 0x78,
> + NT36XXX_EVT_PROJECTID = 0x9a,
> +};
> +
> +static const struct nt36xxx_mem_map nt36xxx_memory_maps[] = {
> + [NT36525_IC] = { 0x11a00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36672A_IC] = { 0x21c00, 0x20000, 0x23000, 0x24000, 0x24002 },
> + [NT36676F_IC] = { 0x11a00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36772_IC] = { 0x11e00, 0x10000, 0x12000, 0x14000, 0x14002 },
> + [NT36870_IC] = { 0x25000, 0x20000, 0x23000, 0x24000, 0x24002 },
> +};
> +
> +static const struct nt36xxx_trim_table trim_id_table[] = {
> + {
> + .id = { 0x0A, 0xFF, 0xFF, 0x72, 0x66, 0x03 },
> + .mask = { 1, 0, 0, 1, 1, 1 },
> + .mapid = NT36672A_IC,
> + },
> + {
> + .id = { 0x55, 0x00, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0x55, 0x72, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xAA, 0x00, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xAA, 0x72, 0xFF, 0x00, 0x00, 0x00 },
> + .mask = { 1, 1, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x72, 0x67, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x67, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x72, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x25, 0x65, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x70, 0x68, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36772_IC,
> + },
> + {
> + .id = { 0xFF, 0xFF, 0xFF, 0x76, 0x66, 0x03 },
> + .mask = { 0, 0, 0, 1, 1, 1 },
> + .mapid = NT36676F_IC,
> + },
> +};
> +
> +/**
> + * nt36xxx_set_page - Set page number for read/write
> + * @ts: Main driver structure
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_set_page(struct nt36xxx_i2c *ts, u32 pageaddr)
> +{
> + u32 data = cpu_to_be32(pageaddr) >> 8;
> + int ret;
> +
> + ret = regmap_noinc_write(ts->fw_regmap, NT36XXX_CMD_SET_PAGE,
> + &data, sizeof(data));
> + if (ret)
> + return ret;
> +
> + usleep_range(100, 200);
> + return ret;
> +}
> +
> +/**
> + * nt36xxx_sw_reset_idle - Warm restart the firmware
> + * @ts: Main driver structure
> + *
> + * This function restarts the running firmware without rebooting to
> + * the bootloader (warm restart)
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_sw_reset_idle(struct nt36xxx_i2c *ts)
> +{
> + int ret;
> +
> + ret = regmap_write(ts->regmap, ts->hw_client->addr,
> + NT36XXX_CMD_SW_RESET);
> + if (ret)
> + return ret;
> +
> + /* Wait until the MCU resets the fw state */
> + usleep_range(15000, 16000);
> + return ret;
> +}
> +
> +/**
> + * nt36xxx_bootloader_reset - Reset MCU to bootloader
> + * @ts: Main driver structure
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_bootloader_reset(struct nt36xxx_i2c *ts)
> +{
> + int ret;
> +
> + ret = regmap_write(ts->regmap, ts->hw_client->addr,
> + NT36XXX_CMD_BOOTLOADER_RESET);
> + if (ret)
> + return ret;
> +
> + /* MCU has to reboot from bootloader: this is the typical boot time */
> + msleep(35);
> + return ret;
> +}
> +
> +/**
> + * nt36xxx_check_reset_state - Check the boot state during reset
> + * @ts: Main driver structure
> + * @fw_state: Enumeration containing firmware states
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_check_reset_state(struct nt36xxx_i2c *ts,
> + enum nt36xxx_fw_state fw_state)
> +{
> + u8 buf[2] = { 0 };
> + int ret, retry = NT36XXX_MAX_FW_RST_RETRY;
> +
> + do {
> + ret = regmap_noinc_read(ts->fw_regmap,
> + NT36XXX_EVT_RESET_COMPLETE,
> + buf, sizeof(buf));
> + if (likely(ret == 0) &&
> + (buf[0] >= fw_state) &&
> + (buf[0] <= NT36XXX_STATE_MAX)) {
> + ret = 0;
> + break;
> + }
> + usleep_range(10000, 11000);
> + } while (--retry);
> +
> + if (!retry) {
> + dev_err(&ts->hw_client->dev, "Firmware reset failed.\n");
> + ret = -EBUSY;
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * nt36xxx_read_pid - Read Novatek Project ID
> + * @ts: Main driver structure
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_read_pid(struct nt36xxx_i2c *ts)
> +{
> + __be16 pid;
> + int ret;
> +
> + ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
> + if (ret)
> + return ret;
> +
> + ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_PROJECTID,
> + &pid, sizeof(pid));
> + if (ret < 0)
> + return ret;
> +
> + ts->fw_info.nvt_pid = be16_to_cpu(pid);
> + return 0;
> +}
> +
> +/**
> + * __nt36xxx_get_fw_info - Get working params from firmware
> + * @ts: Main driver structure
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int __nt36xxx_get_fw_info(struct nt36xxx_i2c *ts)
> +{
> + struct nt36xxx_fw_info *fwi = &ts->fw_info;
> + u8 buf[11] = { 0 };
> + int ret = 0;
> +
> + ret = nt36xxx_set_page(ts, ts->mmap->evtbuf_addr);
> + if (ret)
> + return ret;
> +
> + ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_FWINFO,
> + buf, sizeof(buf));
> + if (ret)
> + return ret;
> +
> + fwi->fw_ver = buf[0];
> + fwi->x_num = buf[2];
> + fwi->y_num = buf[3];
> + fwi->abs_x_max = get_unaligned_be16(&buf[4]);
> + fwi->abs_y_max = get_unaligned_be16(&buf[6]);
> + fwi->max_buttons = buf[10];
> +
> + /* Check fw info integrity and clear x_num, y_num if broken */
> + if ((buf[0] + buf[1]) != 0xFF) {
> + dev_err(&ts->hw_client->dev,
> + "FW info is broken! fw_ver=0x%02X, ~fw_ver=0x%02X\n",
> + buf[0], buf[1]);
> + fwi->fw_ver = 0;
> + fwi->x_num = 18;
> + fwi->y_num = 32;
> + fwi->abs_x_max = TOUCH_DEFAULT_MAX_WIDTH;
> + fwi->abs_y_max = TOUCH_DEFAULT_MAX_HEIGHT;
> + fwi->max_buttons = 0;
> + return -EINVAL;
> + }
> +
> + /* Get Novatek ProjectID */
> + return nt36xxx_read_pid(ts);
> +}
> +
> +static int nt36xxx_get_fw_info(struct nt36xxx_i2c *ts)
> +{
> + struct nt36xxx_fw_info *fwi = &ts->fw_info;
> + int i, ret = 0;
> +
> + for (i = 0; i < NT36XXX_MAX_RETRIES; i++) {
> + ret = __nt36xxx_get_fw_info(ts);
> + if (ret == 0)
> + break;
> + }
> +
> + dev_dbg(&ts->hw_client->dev,
> + "FW Info: PID=0x%x, ver=0x%x res=%ux%u max=%ux%u buttons=%u",
> + fwi->nvt_pid, fwi->fw_ver, fwi->x_num, fwi->y_num,
> + fwi->abs_x_max, fwi->abs_y_max, fwi->max_buttons);
> +
> + return ret;
> +}
> +
> +/**
> + * nt36xxx_report - Report touch events
> + * @ts: Main driver structure
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static void nt36xxx_report(struct nt36xxx_i2c *ts)
> +{
> + struct nt36xxx_abs_object *obj = &ts->abs_obj;
> + struct input_dev *input = ts->input;
> + u8 input_id = 0;
> + u8 point[POINT_DATA_LEN + 1] = { 0 };
> + unsigned int ppos = 0;
> + int i, ret, finger_cnt = 0;
> +
> + mutex_lock(&ts->lock);
> +
> + ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_REPORT,
> + point, sizeof(point));
> + if (ret < 0) {
> + dev_err(&ts->hw_client->dev,
> + "Cannot read touch point data: %d\n", ret);
> + goto xfer_error;
> + }
> +
> + for (i = 0; i < TOUCH_MAX_FINGER_NUM; i++) {
> + ppos = 6 * i;
> + input_id = point[ppos + 0] >> 3;
> + if ((input_id == 0) || (input_id > TOUCH_MAX_FINGER_NUM))
> + continue;
> +
> + if (((point[ppos] & 0x07) == 0x01) ||
> + ((point[ppos] & 0x07) == 0x02)) {
> + obj->x = (point[ppos + 1] << 4) +
> + (point[ppos + 3] >> 4);
> + obj->y = (point[ppos + 2] << 4) +
> + (point[ppos + 3] & 0xf);
> + if ((obj->x > ts->prop.max_x) ||
> + (obj->y > ts->prop.max_y))
> + continue;
> +
> + obj->tm = point[ppos + 4];
> + if (obj->tm == 0)
> + obj->tm = 1;
> +
> + obj->z = point[ppos + 5];
> + if (i < 2) {
> + obj->z += point[i + 63] << 8;
> + if (obj->z > TOUCH_MAX_PRESSURE)
> + obj->z = TOUCH_MAX_PRESSURE;
> + }
> +
> + if (obj->z == 0)
> + obj->z = 1;
> +
> + input_mt_slot(input, input_id - 1);
> + input_mt_report_slot_state(input,
> + MT_TOOL_FINGER, true);
> + touchscreen_report_pos(input, &ts->prop, obj->x,
> + obj->y, true);
> +
> + input_report_abs(input, ABS_MT_TOUCH_MAJOR, obj->tm);
> + input_report_abs(input, ABS_MT_PRESSURE, obj->z);
> +
> + finger_cnt++;
> + }
> + }
> + input_mt_sync_frame(input);
> + input_sync(input);
> +
> +xfer_error:
> + enable_irq(ts->hw_client->irq);
> +
> + mutex_unlock(&ts->lock);
> +}
> +
> +static irqreturn_t nt36xxx_i2c_irq_handler(int irq, void *dev_id)
> +{
> + struct nt36xxx_i2c *ts = dev_id;
> +
> + disable_irq_nosync(ts->hw_client->irq);
> + nt36xxx_report(ts);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static bool nt36xxx_in_crc_reboot_loop(u8 *buf)
> +{
> + return ((buf[0] == 0xFC) && (buf[1] == 0xFC) && (buf[2] == 0xFC)) ||
> + ((buf[0] == 0xFF) && (buf[1] == 0xFF) && (buf[2] == 0xFF));
> +}
> +
> +/**
> + * nt36xxx_stop_crc_reboot - Stop CRC reboot loop and warm-reboot the firmware
> + * @ts: Main driver structure
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_stop_crc_reboot(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[3] = { 0 };
> + u8 val;
> + int ret, retry = NT36XXX_MAX_RETRIES;
> +
> + /* Read dummy buffer to check CRC fail reboot is happening or not */
> +
> + /* Change I2C index to prevent getting 0xFF, but not 0xFC */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
> + if (ret) {
> + dev_dbg(&ts->hw_client->dev,
> + "CRC reset failed: Cannot select page.\n");
> + return ret;
> + }
> +
> + /* If ChipID command returns 0xFC or 0xFF, the MCU is in CRC reboot */
> + ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_CHIPID,
> + buf, sizeof(buf));
> + if (ret)
> + return ret;
> +
> + if (!nt36xxx_in_crc_reboot_loop(buf))
> + return 0;
> +
> + /* IC is in CRC fail reboot loop, needs to be stopped! */
> + do {
> + /* Special reset-idle sequence for CRC failure */
> + ret = regmap_write(ts->regmap, ts->hw_client->addr,
> + NT36XXX_CMD_SW_RESET);
> + if (ret)
> + dev_dbg(&ts->hw_client->dev,
> + "SW Reset 1 failed: may not recover\n");
> +
> + ret = regmap_write(ts->regmap, ts->hw_client->addr,
> + NT36XXX_CMD_SW_RESET);
> + if (ret)
> + dev_dbg(&ts->hw_client->dev,
> + "SW Reset 2 failed: may not recover\n");
> + usleep_range(1000, 1100);
> +
> + /* Clear CRC_ERR_FLAG */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
> + if (ret)
> + continue;
> +
> + val = 0xA5;
> + ret = regmap_raw_write(ts->fw_regmap, NT36XXX_EVT_CRC,
> + &val, sizeof(val));
> + if (ret)
> + continue;
> +
> + /* Check CRC_ERR_FLAG */
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CRC);
> + if (ret)
> + continue;
> +
> + ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_CRC,
> + &buf, sizeof(buf));
> + if (ret)
> + continue;
> +
> + if (buf[0] == 0xA5)
> + break;
> + } while (--retry);
> +
> + if (retry == 0) {
> + dev_err(&ts->hw_client->dev,
> + "CRC reset failed: buf=0x%2ph\n", buf);
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * nt36xxx_i2c_chip_version_init - Detect Novatek NT36xxx family IC
> + * @ts: Main driver structure
> + *
> + * This function reads the ChipID from the IC and sets the right
> + * memory map for the detected chip.
> + *
> + * Return: Always zero for success, negative number for error
> + */
> +static int nt36xxx_i2c_chip_version_init(struct nt36xxx_i2c *ts)
> +{
> + u8 buf[7] = { 0 };
> + int retry = NT36XXX_MAX_RETRIES;
> + int sz = sizeof(trim_id_table) / sizeof(struct nt36xxx_trim_table);
> + int i, list, mapid, ret;
> +
> + ret = nt36xxx_bootloader_reset(ts);
> + if (ret) {
> + dev_err(&ts->hw_client->dev, "Can't reset the nvt IC\n");
> + return ret;
> + }
> +
> + do {
> + ret = nt36xxx_sw_reset_idle(ts);
> + if (ret)
> + continue;
> +
> + ret = regmap_write(ts->regmap, ts->hw_client->addr, NT36XXX_CMD_UNLOCK);
> + if (ret)
> + continue;
> + usleep_range(10000, 11000);
> +
> + ret = nt36xxx_set_page(ts, NT36XXX_PAGE_CHIP_INFO);
> + if (ret)
> + continue;
> +
> + memset(buf, 0, ARRAY_SIZE(buf));
> + ret = regmap_noinc_read(ts->fw_regmap, NT36XXX_EVT_CHIPID,
> + buf, sizeof(buf));
> + if (ret)
> + continue;
> +
> + /* Compare read chip id with trim list */
> + for (list = 0; list < sz; list++) {
> + /* Compare each not masked byte */
> + for (i = 0; i < NT36XXX_ID_LEN_MAX; i++) {
> + if (trim_id_table[list].mask[i] &&
> + buf[i] != trim_id_table[list].id[i])
> + break;
> + }
> +
> + if (i == NT36XXX_ID_LEN_MAX) {
> + mapid = trim_id_table[list].mapid;
> + ts->mmap = &nt36xxx_memory_maps[mapid];
> + return 0;
> + }
> +
> + ts->mmap = NULL;
> + ret = -ENOENT;
> + }
> +
> + /* Stop CRC check to prevent IC auto reboot */
> + if (nt36xxx_in_crc_reboot_loop(buf)) {
> + ret = nt36xxx_stop_crc_reboot(ts);
> + if (ret)
> + continue;
> + }
> +
> + usleep_range(10000, 11000);
> + } while (--retry);
> +
> + return ret;
> +}
> +
> +static const struct regmap_config nt36xxx_i2c_regmap_hw_config = {
> + .name = "nt36xxx_i2c_hw",
> + .reg_bits = 8,
> + .val_bits = 8,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +static const struct regmap_config nt36xxx_i2c_regmap_fw_config = {
> + .name = "nt36xxx_i2c_fw",
> + .reg_bits = 8,
> + .val_bits = 8,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +static void nt36xxx_disable_regulators(void *data)
> +{
> + struct nt36xxx_i2c *ts = data;
> +
> + regulator_bulk_disable(NT36XXX_NUM_SUPPLIES, ts->supplies);
> +}
> +
> +static int nt36xxx_i2c_probe(struct i2c_client *hw_client,
> + const struct i2c_device_id *id)
> +{
> + struct nt36xxx_i2c *ts;
> + struct input_dev *input;
> + int ret;
> +
> + if (!i2c_check_functionality(hw_client->adapter, I2C_FUNC_I2C)) {
> + dev_err(&hw_client->dev, "i2c_check_functionality error\n");
> + return -EIO;
> + }
> +
> + if (!hw_client->irq) {
> + dev_err(&hw_client->dev, "No irq specified\n");
> + return -EINVAL;
> + }
> +
> + ts = devm_kzalloc(&hw_client->dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + ts->supplies = devm_kcalloc(&hw_client->dev,
> + NT36XXX_NUM_SUPPLIES,
> + sizeof(*ts->supplies),
> + GFP_KERNEL);
> + if (!ts->supplies)
> + return -ENOMEM;
> +
> + input = devm_input_allocate_device(&hw_client->dev);
> + if (!input)
> + return -ENOMEM;
> +
> + ts->fw_client = i2c_new_dummy_device(hw_client->adapter,
> + NT36XXX_FW_ADDR);
> + if (IS_ERR(ts->fw_client)) {
> + dev_err(&hw_client->dev, "Cannot add FW I2C device\n");
> + return PTR_ERR(ts->fw_client);
> + }
> +
> + ts->hw_client = hw_client;
> + ts->input = input;
> + i2c_set_clientdata(ts->hw_client, ts);
> + i2c_set_clientdata(ts->fw_client, ts);
> +
> + ts->reset_gpio = devm_gpiod_get_optional(&hw_client->dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(ts->reset_gpio))
> + return PTR_ERR(ts->reset_gpio);
> + gpiod_set_consumer_name(ts->reset_gpio, "nt36xxx reset");
> +
> + /* These supplies are optional */
> + ts->supplies[0].supply = "vdd";
> + ts->supplies[1].supply = "vio";
> + ret = devm_regulator_bulk_get(&hw_client->dev,
> + NT36XXX_NUM_SUPPLIES,
> + ts->supplies);
> + if (ret)
> + return dev_err_probe(&hw_client->dev, ret,
> + "Cannot get supplies: %d\n", ret);
> +
> + ts->regmap = devm_regmap_init_i2c(ts->hw_client,
> + &nt36xxx_i2c_regmap_hw_config);
> + if (IS_ERR(ts->regmap)) {
> + dev_err(&hw_client->dev, "regmap (hw-addr) init failed\n");
> + return PTR_ERR(ts->regmap);
> + }
> +
> + ts->fw_regmap = devm_regmap_init_i2c(ts->fw_client,
> + &nt36xxx_i2c_regmap_fw_config);
> + if (IS_ERR(ts->fw_regmap)) {
> + dev_err(&hw_client->dev, "regmap (fw-addr) init failed\n");
> + return PTR_ERR(ts->fw_regmap);
> + }
> +
> + ret = regulator_bulk_enable(NT36XXX_NUM_SUPPLIES, ts->supplies);
> + if (ret)
> + return ret;
> +
> + usleep_range(10000, 11000);
> +
> + ret = devm_add_action_or_reset(&hw_client->dev,
> + nt36xxx_disable_regulators, ts);
> + if (ret)
> + return ret;
> +
> + mutex_init(&ts->lock);
> +
> + /* Set memory maps for the specific chip version */
> + ret = nt36xxx_i2c_chip_version_init(ts);
> + if (ret) {
> + dev_err(&hw_client->dev, "Failed to check chip version\n");
> + return ret;
> + }
> +
> + /* Reset the MCU */
> + ret = nt36xxx_bootloader_reset(ts);
> + if (ret < 0)
> + return ret;
> +
> + /* Check and eventually wait until the MCU goes in reset state */
> + ret = nt36xxx_check_reset_state(ts, NT36XXX_STATE_INIT);
> + if (ret < 0)
> + return ret;
> +
> + /* Get informations from the TS firmware */
> + ret = nt36xxx_get_fw_info(ts);
> + if (ret < 0)
> + return ret;
> +
> + input->phys = devm_kasprintf(&hw_client->dev, GFP_KERNEL,
> + "%s/input0", dev_name(&hw_client->dev));
> + if (!input->phys)
> + return -ENOMEM;
> +
> + input->name = "Novatek NT36XXX Touchscreen";
> + input->id.bustype = BUS_I2C;
> + input->dev.parent = &hw_client->dev;
> +
> + __set_bit(EV_KEY, input->evbit);
> + __set_bit(EV_ABS, input->evbit);
> + input_set_capability(input, EV_KEY, BTN_TOUCH);
> +
> + ret = input_mt_init_slots(input, TOUCH_MAX_FINGER_NUM,
> + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
> + if (ret) {
> + dev_err(&hw_client->dev, "Cannot init MT slots (%d)\n", ret);
> + return ret;
> + }
> +
> + input_set_abs_params(input, ABS_MT_PRESSURE, 0,
> + TOUCH_MAX_PRESSURE, 0, 0);
> + input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> +
> + input_set_abs_params(input, ABS_MT_POSITION_X, 0,
> + ts->fw_info.abs_x_max - 1, 0, 0);
> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0,
> + ts->fw_info.abs_y_max - 1, 0, 0);
> +
> + /* Override the firmware defaults, if needed */
> + touchscreen_parse_properties(input, true, &ts->prop);
> +
> + input_set_drvdata(input, ts);
> +
> + ret = input_register_device(ts->input);
> + if (ret) {
> + dev_err(&hw_client->dev, "Failed to register input device: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = devm_request_threaded_irq(&hw_client->dev, hw_client->irq, NULL,
> + nt36xxx_i2c_irq_handler, IRQF_ONESHOT,
> + hw_client->name, ts);
> + if (ret) {
> + dev_err(&hw_client->dev, "request irq failed: %d\n", ret);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int __maybe_unused nt36xxx_i2c_suspend(struct device *dev)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(to_i2c_client(dev));
> + int ret;
> +
> + disable_irq(ts->hw_client->irq);
> +
> + ret = regmap_write(ts->fw_regmap, NT36XXX_EVT_HOST_CMD,
> + NT36XXX_CMD_ENTER_SLEEP);
> + if (ret) {
> + dev_err(&ts->hw_client->dev, "Cannot enter suspend!!\n");
> + return ret;
> + }
> +
> + gpiod_set_value(ts->reset_gpio, 1);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused nt36xxx_i2c_resume(struct device *dev)
> +{
> + struct nt36xxx_i2c *ts = i2c_get_clientdata(to_i2c_client(dev));
> + int ret;
> +
> + mutex_lock(&ts->lock);
> +
> + gpiod_set_value(ts->reset_gpio, 0);
> +
> + /* Reboot the MCU (also recalibrates the TS) */
> + ret = nt36xxx_bootloader_reset(ts);
> + if (ret < 0)
> + goto end;
> +
> + ret = nt36xxx_check_reset_state(ts, NT36XXX_STATE_REK);
> + if (ret < 0)
> + goto end;
> +
> + enable_irq(ts->hw_client->irq);
> +end:
> + mutex_unlock(&ts->lock);
> + return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(nt36xxx_i2c_pm,
> + nt36xxx_i2c_suspend, nt36xxx_i2c_resume);
> +
> +static const struct of_device_id nt36xxx_of_match[] = {
> + { .compatible = "novatek,nt36525" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, nt36xxx_of_match);
> +
> +static const struct i2c_device_id nt36xxx_i2c_ts_id[] = {
> + { "NVT-ts", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, nt36xxx_i2c_ts_id);
> +
> +static struct i2c_driver nt36xxx_i2c_ts_driver = {
> + .driver = {
> + .name = "nt36xxx_ts",
> + .pm = &nt36xxx_i2c_pm,
> + .of_match_table = nt36xxx_of_match,
> + },
> + .id_table = nt36xxx_i2c_ts_id,
> + .probe = nt36xxx_i2c_probe,
> +};
> +module_i2c_driver(nt36xxx_i2c_ts_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Novatek NT36XXX Touchscreen Driver");
> +MODULE_AUTHOR("AngeloGioacchino Del Regno <kholk11@gmail.com>");
> diff --git a/drivers/input/touchscreen/nt36xxx.h b/drivers/input/touchscreen/nt36xxx.h
> new file mode 100644
> index 000000000000..6f03dfb45656
> --- /dev/null
> +++ b/drivers/input/touchscreen/nt36xxx.h
> @@ -0,0 +1,122 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2010 - 2017 Novatek, Inc.
> + * Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
> + */
> +
> +#ifndef NT36XXX_H
> +#define NT36XXX_H
> +
> +#define NT36XXX_INPUT_DEVICE_NAME "Novatek NT36XXX Touch Sensor"
> +
> +/* These chips have this fixed address when in bootloader 🙁 */
> +#define NT36XXX_BLDR_ADDR 0x01
> +
> +/* Input device info */
> +#define NVT_TS_NAME "NVTCapacitiveTouchScreen"
> +
> +/* Number of bytes for chip identification */
> +#define NT36XXX_ID_LEN_MAX 6
> +
> +/* Touch info */
> +#define TOUCH_DEFAULT_MAX_WIDTH 1080
> +#define TOUCH_DEFAULT_MAX_HEIGHT 2246
> +#define TOUCH_MAX_FINGER_NUM 10
> +#define TOUCH_MAX_PRESSURE 1000
> +
> +/* Point data length */
> +#define POINT_DATA_LEN 65
> +
> +/* Global pages */
> +#define NT36XXX_PAGE_CHIP_INFO 0x0001f64e
> +#define NT36XXX_PAGE_CRC 0x0003f135
> +
> +/* Misc */
> +#define NT36XXX_NUM_SUPPLIES 2
> +#define NT36XXX_MAX_RETRIES 5
> +#define NT36XXX_MAX_FW_RST_RETRY 50
> +
> +struct nt36xxx_abs_object {
> + u16 x;
> + u16 y;
> + u16 z;
> + u8 tm;
> +};
> +
> +struct nt36xxx_fw_info {
> + u8 fw_ver;
> + u8 x_num;
> + u8 y_num;
> + u8 max_buttons;
> + u16 abs_x_max;
> + u16 abs_y_max;
> + u16 nvt_pid;
> +};
> +
> +struct nt36xxx_mem_map {
> + u32 evtbuf_addr;
> + u32 pipe0_addr;
> + u32 pipe1_addr;
> + u32 flash_csum_addr;
> + u32 flash_data_addr;
> +};
> +
> +struct nt36xxx_i2c {
> + struct i2c_client *client;
> + struct input_dev *input;
> + struct regulator_bulk_data *supplies;
> + struct gpio_desc *reset_gpio;
> +
> + struct work_struct ts_work;
> + struct workqueue_struct *ts_workq;
> + struct mutex lock;
> +
> + struct nt36xxx_fw_info fw_info;
> + struct nt36xxx_abs_object abs_obj;
> +
> + const struct nt36xxx_mem_map *mmap;
> + u8 max_fingers;
> +};
> +
> +enum nt36xxx_chips {
> + NT36525_IC = 0,
> + NT36672A_IC,
> + NT36676F_IC,
> + NT36772_IC,
> + NT36870_IC,
> + NTMAX_IC,
> +};
> +
> +struct nt36xxx_trim_table {
> + u8 id[NT36XXX_ID_LEN_MAX];
> + u8 mask[NT36XXX_ID_LEN_MAX];
> + enum nt36xxx_chips mapid;
> +};
> +
> +enum nt36xxx_cmds {
> + NT36XXX_CMD_ENTER_SLEEP = 0x11,
> + NT36XXX_CMD_ENTER_WKUP_GESTURE = 0x13,
> + NT36XXX_CMD_UNLOCK = 0x35,
> + NT36XXX_CMD_BOOTLOADER_RESET = 0x69,
> + NT36XXX_CMD_SW_RESET = 0xa5,
> + NT36XXX_CMD_SET_PAGE = 0xff,
> +};
> +
> +enum nt36xxx_fw_state {
> + NT36XXX_STATE_INIT = 0xa0, /* IC reset */
> + NT36XXX_STATE_REK, /* ReK baseline */
> + NT36XXX_STATE_REK_FINISH, /* Baseline is ready */
> + NT36XXX_STATE_NORMAL_RUN, /* Normal run */
> + NT36XXX_STATE_MAX = 0xaf
> +};
> +
> +enum nt36xxx_i2c_events {
> + NT36XXX_EVT_CHIPID = 0x4e,
> + NT36XXX_EVT_HOST_CMD = 0x50,
> + NT36XXX_EVT_HS_OR_SUBCMD = 0x51, /* Handshake or subcommand byte */
> + NT36XXX_EVT_RESET_COMPLETE = 0x60,
> + NT36XXX_EVT_FWINFO = 0x78,
> + NT36XXX_EVT_PROJECTID = 0x9a,
> +};
> +
> +#endif
>
^ permalink raw reply
* Re: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: Doug Anderson @ 2023-06-26 22:49 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Frank Rowand, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Sam Ravnborg, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan
In-Reply-To: <y3l4x3kv7jgog3miexati5wbveaynnryzqvj6sc4ul6625f2if@w7nqgojfavfw>
Benjamin,
On Thu, Jun 8, 2023 at 8:37 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> > +static const struct drm_panel_follower_funcs i2c_hid_core_panel_follower_funcs = {
> > + .panel_prepared = i2c_hid_core_panel_prepared,
> > + .panel_unpreparing = i2c_hid_core_panel_unpreparing,
> > +};
>
> Can we make that above block at least behind a Kconfig?
>
> i2c-hid is often used for touchpads, and the notion of drm panel has
> nothing to do with them. So I'd be more confident if we could disable
> that code if not required.
Now that other concerns are addressed, I started trying to write up a
v3 and I found myself writing this as the description of the Kconfig
entry:
--
config I2C_HID_SUPPORT_PANEL_FOLLOWER
bool "Support i2c-hid devices that must be power sequenced with a panel"
Say Y here if you want support for i2c-hid devices that need to
coordinate power sequencing with a panel. This is typically important
when you have a panel and a touchscreen that share power rails or
reset GPIOs. If you say N here then the kernel will not try to honor
any shared power sequencing for your hardware. In the best case,
ignoring power sequencing when it's needed will draw extra power. In
the worst case this will prevent your hardware from functioning or
could even damage your hardware.
If unsure, say Y.
--
I can certainly go that way, but I just wanted to truly make sure
that's what we want. Specifically:
1. If we put the panel follower code behind a Kconfig then we actually
have no idea if a touchscreen was intended to be a panel follower.
Specifically the panel follower API is the one that detects the
connection between the panel and the i2c-hid device, so without being
able to call the panel follower API we have no idea that an i2c-hid
device was supposed to be a panel follower.
2. It is conceivable that power sequencing a device incorrectly could
truly cause hardware damage.
Together, those points mean that if you turn off the Kconfig entry and
then try to boot on a device that needed that Kconfig setting that you
might damage hardware. I can code it up that way if you want, but it
worries me...
Alternatives that I can think of:
a) I could change the panel follower API so that panel followers are
in charge of detecting the panel that they follow. Today, that looks
like:
panel_np = of_parse_phandle(dev->of_node, "panel", 0);
if (panel_np)
/* It's a panel follower */
of_node_put(panel_np);
...so we could put that code in each touchscreen driver and then fail
to probe i2c-hid if we detect that we're supposed to be a panel
follower but the Kconfig is turned off. The above doesn't seem
massively ideal since it duplicates code. Also, one reason why I put
that code in drm_panel_add_follower() is that I think this concept
will eventually be needed even for non-DT cases. I don't know how to
write the non-DT code right now, though...
b) I could open-code detect the panel follower case but leave the
actual linking to the panel follower API. AKA add to i2c-hid:
if (of_property_read_bool(dev->of_node, "panel"))
/* It's a panel follower */
...that's a smaller bit of code, but feels like an abstraction
violation. It also would need to be updated if/when we added support
for non-DT panel followers.
c) I could add a "static inline" implementation of b) to "drm_panel.h".
That sounds great and I started doing it. ...but then realized that it
means adding to drm_panel.h:
#include <linux/device.h>
#include <linux/of.h>
...because otherwise of_property_read_bool() isn't defined and "struct
device" can't be dereferenced. That might be OK, but it looks as if
folks have been working hard to avoid things like this in header
files. Presumably it would get uglier if/when we added the non-DT
case, as well. That being said, I can give it a shot...
--
At this point, I'm hoping for some advice. How important is it for you
to have a Kconfig for "I2C_HID_SUPPORT_PANEL_FOLLOWER"?
NOTE: even if I don't add the Kconfig, I could at least create a
function for registering the panel follower that would get most of the
panel follower logic out of the probe function. Would that be enough?
Thanks!
-Doug
^ permalink raw reply
* Re: [PATCH v3 4/4] input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
From: Jeff LaBundy @ 2023-06-26 21:17 UTC (permalink / raw)
To: Neil Armstrong
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, linux-input,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <94e80a49-11ae-e5b0-7eea-6ed4ec6d2ac8@linaro.org>
Hi Neil,
On Mon, Jun 26, 2023 at 03:20:35PM +0200, Neil Armstrong wrote:
> Hi,
>
> On 26/06/2023 15:01, Jeff LaBundy wrote:
> > Hi Neil,
> >
> > On Mon, Jun 26, 2023 at 09:02:16AM +0200, Neil Armstrong wrote:
> >
> > [...]
> >
> > > > > +static int goodix_berlin_spi_probe(struct spi_device *spi)
> > > > > +{
> > > > > + struct regmap_config *regmap_config;
> > > > > + struct regmap *regmap;
> > > > > + size_t max_size;
> > > > > + int error = 0;
> > > > > +
> > > > > + regmap_config = devm_kmemdup(&spi->dev, &goodix_berlin_spi_regmap_conf,
> > > > > + sizeof(*regmap_config), GFP_KERNEL);
> > > > > + if (!regmap_config)
> > > > > + return -ENOMEM;
> > > >
> > > > Is there any reason we cannot simply pass goodix_berlin_spi_regmap_conf to
> > > > devm_regmap_init() below? Why to duplicate and pass the copy?
> > > >
> > > > For reference, BMP280 in IIO is a similar example of a device with regmap
> > > > sitting atop a bespoke SPI protocol; it does not seem to take this extra
> > > > step.
> > >
> > > The goodix_berlin_spi_regmap_conf copy is modified after with the correct
> > > max raw read/write size, and I'm not a fan of modifying a global structure
> > > that could be use for multiple probes, I can make a copy in a stack variable
> > > if it feels simpler.
> >
> > Ah, that makes sense; in that case, the existing implementation seems fine
> > to me. No changes necessary.
> >
> > Correct me if I'm wrong, but the stack variable method wouldn't work since
> > that memory is gone after goodix_berlin_spi_probe() returns.
>
> The config is only needed for the devm_regmap_init() duration, so keeping
> the memory allocated for the whole lifetime of the device seems useless.
I revisted the regmap code, and you are indeed correct. I agree with your
suggestion.
>
> Neil
>
> >
> > Kind regards,
> > Jeff LaBundy
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH v3 2/2] Input: cap11xx - add advanced sensitivity settings
From: kernel test robot @ 2023-06-26 18:49 UTC (permalink / raw)
To: Jiri Valek - 2N, krzysztof.kozlowski+dt, dmitry.torokhov
Cc: oe-kbuild-all, jiriv, devicetree, linux-input, linux-kernel,
robh+dt, u.kleine-koenig
In-Reply-To: <20230626130006.850254-3-jiriv@axis.com>
Hi Jiri,
kernel test robot noticed the following build warnings:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus linus/master v6.4 next-20230626]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jiri-Valek-2N/dt-bindings-input-microchip-cap11xx-add-advanced-sensitivity-settings/20230626-210123
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20230626130006.850254-3-jiriv%40axis.com
patch subject: [PATCH v3 2/2] Input: cap11xx - add advanced sensitivity settings
config: i386-randconfig-i013-20230626 (https://download.01.org/0day-ci/archive/20230627/202306270246.llRp2LOP-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230627/202306270246.llRp2LOP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306270246.llRp2LOP-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/input/keyboard/cap11xx.c: In function 'cap11xx_i2c_probe':
>> drivers/input/keyboard/cap11xx.c:521:23: warning: unused variable 'irq' [-Wunused-variable]
521 | int i, error, irq;
| ^~~
vim +/irq +521 drivers/input/keyboard/cap11xx.c
514
515 static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
516 {
517 const struct i2c_device_id *id = i2c_client_get_device_id(i2c_client);
518 struct device *dev = &i2c_client->dev;
519 struct cap11xx_priv *priv;
520 const struct cap11xx_hw_model *cap;
> 521 int i, error, irq;
522 unsigned int val, rev;
523
524 if (id->driver_data >= ARRAY_SIZE(cap11xx_devices)) {
525 dev_err(dev, "Invalid device ID %lu\n", id->driver_data);
526 return -EINVAL;
527 }
528
529 cap = &cap11xx_devices[id->driver_data];
530 if (!cap || !cap->num_channels) {
531 dev_err(dev, "Invalid device configuration\n");
532 return -EINVAL;
533 }
534
535 priv = devm_kzalloc(dev,
536 struct_size(priv, keycodes, cap->num_channels),
537 GFP_KERNEL);
538 if (!priv)
539 return -ENOMEM;
540
541 priv->dev = dev;
542
543 priv->regmap = devm_regmap_init_i2c(i2c_client, &cap11xx_regmap_config);
544 if (IS_ERR(priv->regmap))
545 return PTR_ERR(priv->regmap);
546
547 error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
548 if (error)
549 return error;
550
551 if (val != cap->product_id) {
552 dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
553 val, cap->product_id);
554 return -ENXIO;
555 }
556
557 error = regmap_read(priv->regmap, CAP11XX_REG_MANUFACTURER_ID, &val);
558 if (error)
559 return error;
560
561 if (val != CAP11XX_MANUFACTURER_ID) {
562 dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
563 val, CAP11XX_MANUFACTURER_ID);
564 return -ENXIO;
565 }
566
567 error = regmap_read(priv->regmap, CAP11XX_REG_REVISION, &rev);
568 if (error < 0)
569 return error;
570
571 dev_info(dev, "CAP11XX detected, model %s, revision 0x%02x\n",
572 id->name, rev);
573
574 priv->model = cap;
575 priv->id = id->driver_data;
576
577 dev_info(dev, "CAP11XX device detected, model %s, revision 0x%02x\n",
578 id->name, rev);
579
580 error = cap11xx_init_keys(priv);
581 if (error)
582 return error;
583
584 priv->idev = devm_input_allocate_device(dev);
585 if (!priv->idev)
586 return -ENOMEM;
587
588 priv->idev->name = "CAP11XX capacitive touch sensor";
589 priv->idev->id.bustype = BUS_I2C;
590 priv->idev->evbit[0] = BIT_MASK(EV_KEY);
591
592 if (of_property_read_bool(dev->of_node, "autorepeat"))
593 __set_bit(EV_REP, priv->idev->evbit);
594
595 for (i = 0; i < cap->num_channels; i++)
596 __set_bit(priv->keycodes[i], priv->idev->keybit);
597
598 __clear_bit(KEY_RESERVED, priv->idev->keybit);
599
600 priv->idev->keycode = priv->keycodes;
601 priv->idev->keycodesize = sizeof(priv->keycodes[0]);
602 priv->idev->keycodemax = cap->num_channels;
603
604 priv->idev->id.vendor = CAP11XX_MANUFACTURER_ID;
605 priv->idev->id.product = cap->product_id;
606 priv->idev->id.version = rev;
607
608 priv->idev->open = cap11xx_input_open;
609 priv->idev->close = cap11xx_input_close;
610
611 error = cap11xx_init_leds(dev, priv, cap->num_leds);
612 if (error)
613 return error;
614
615 input_set_drvdata(priv->idev, priv);
616
617 /*
618 * Put the device in deep sleep mode for now.
619 * ->open() will bring it back once the it is actually needed.
620 */
621 cap11xx_set_sleep(priv, true);
622
623 error = input_register_device(priv->idev);
624 if (error)
625 return error;
626
627 error = devm_request_threaded_irq(dev, i2c_client->irq, NULL,
628 cap11xx_thread_func, IRQF_ONESHOT, dev_name(dev), priv);
629 if (error)
630 return error;
631
632 return 0;
633 }
634
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox