Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"
From: Werner Sembach @ 2023-02-28 11:07 UTC (permalink / raw)
  To: Hans de Goede, dmitry.torokhov, swboyd, gregkh, mkorpershoek,
	chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <a2216cc2-b719-12e1-264c-374fc467db14@redhat.com>


Am 28.02.23 um 09:41 schrieb Hans de Goede:
> Hi Werner,
>
> On 2/27/23 19:59, Werner Sembach wrote:
>> This is a continuation of
>> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
>>
>> That fix did fix the keyboard not responding at all sometimes after resume,
>> but at the price of it being laggy for some time after boot. Additionally
>> setting atkbd.reset removes that lag.
>>
>> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
>> atkbd.reset and the second one then applies that and the i8042 quirks to
>> the affected devices.
> Can you please rework this series so that the quirk based setting of
> the "atkbd.reset" equivalent on the kernel commandline becomes another
> SERIO_QUIRK_* flag and avoid the duplication of the DMI ids?

I'm not sure how to cleanly do this, since atkbd is an own module?

Kind Regards,

Werner

>
> Regards,
>
> Hans
>
>
>

^ permalink raw reply

* Re: [PATCH v2 1/2] Input: atkbd - Make it possible to apply atkbd.reset via kernel quirk
From: Werner Sembach @ 2023-02-28 10:58 UTC (permalink / raw)
  To: Greg KH
  Cc: dmitry.torokhov, swboyd, hdegoede, mkorpershoek, chenhuacai,
	wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <Y/2ueXSFK0iOR5M5@kroah.com>


Am 28.02.23 um 08:34 schrieb Greg KH:
> On Mon, Feb 27, 2023 at 07:59:06PM +0100, Werner Sembach wrote:
>> atkbd.reset was only a command line parameter. Some devices might have a
>> known bug that can be worked around by just permanently applying this
>> quirk.
>>
>> This patch adds the ability to do this on the kernel level for known buggy
>> devices.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> Cc: stable@vger.kernel.org
>> ---
>>   drivers/input/keyboard/atkbd.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
>> index 246958795f60..ef65c46c4efe 100644
>> --- a/drivers/input/keyboard/atkbd.c
>> +++ b/drivers/input/keyboard/atkbd.c
>> @@ -1731,6 +1731,12 @@ static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
>>   	return 1;
>>   }
>>   
>> +static int __init atkbd_reset_fixup(const struct dmi_system_id *id)
>> +{
>> +	atkbd_reset = true;
>> +	return 1;
> Why is this returning 1?  Who calls this?

This function is following the format of the other fixup functions directly 
above it.

It is there to be called as a callback via the atkbd_dmi_quirk_table on a per 
device basis. See 2nd patch of this patchset.

Greetings,

Werner

>
> And this should be a per-device attribute, not a global one, right?
>
> thanks,
>
> greg k-h

^ permalink raw reply

* Re: [PATCH] HID: usbhid: enable remote wakeup for mice
From: Oliver Neukum @ 2023-02-28  9:03 UTC (permalink / raw)
  To: Limonciello, Mario, Oliver Neukum, Greg KH, Michael Wu
  Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
	linux-usb@vger.kernel.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, Gong, Richard
In-Reply-To: <MN0PR12MB6101B1BF9748F0602684FED5E2AB9@MN0PR12MB6101.namprd12.prod.outlook.com>

On 23.02.23 20:41, Limonciello, Mario wrote:

Hi,

>> As a system wakeup source a mouse that generates events when
>> it is moved, however, would make the system unsuspendable, whenever
>> even
>> a bit of vibration is acting on the system.
>> And as S4 is used in many setups to prevent an uncontrolled shutdown
>> at low power, this must work.
> 
> At least in my version of the series, this is part of the reason that it was
> only intended to be used with s2idle.

Yes, that is sensible. If these patches are to be taken at all, that will
be a necessary condition to meet. But it is not sufficient.
  
> The kernel driver is well aware of what power state you're in the suspend
> callback (pm_suspend_target_state).
> 
> What about if we agreed to treat this one special by examining that?
> 
> If the sysfs is set to "enabled"

If user space needs to manipulate sysfs at all, we can have user space
tell kernel space exactly what to do. Hence I see no point in
conditional interpretations values in sysfs at that point.

We are discussing the kernel's default here.

> * During suspend if your target is s2idle -> program it
> * During suspend if your target is mem -> disable it
> * During suspend if your target is hibernate -> disable it

To my mind these defaults make sense.
However, do they make much more sense than what we are doing now?

> With that type of policy on how to handle the suspend call in place
> perhaps we could set it to enabled by default?

It pains me to say, but I am afraid in that regard the only
decision that will not cause ugly surprises is to follow Windows.
Yet, what is wrong about the current defaults?
  
> Turning on "autosuspend" for USB mice makes them behave pretty
> similarly to how they work when they're marked for remote wakeup.

Because it is exactly the same mechanism.
  
> On some mice the lasers turn off, and they only wakeup when you
> press a button or roll a wheel.

Yes. And _some_ is the exact problem. If we could tell, _how_ mice
react, this discussion were unnecessary.

	Regards
		Oliver

^ permalink raw reply

* Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"
From: Hans de Goede @ 2023-02-28  8:41 UTC (permalink / raw)
  To: Werner Sembach, dmitry.torokhov, swboyd, gregkh, mkorpershoek,
	chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <20230227185907.569154-1-wse@tuxedocomputers.com>

Hi Werner,

On 2/27/23 19:59, Werner Sembach wrote:
> This is a continuation of
> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
> 
> That fix did fix the keyboard not responding at all sometimes after resume,
> but at the price of it being laggy for some time after boot. Additionally
> setting atkbd.reset removes that lag.
> 
> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
> atkbd.reset and the second one then applies that and the i8042 quirks to
> the affected devices.

Can you please rework this series so that the quirk based setting of
the "atkbd.reset" equivalent on the kernel commandline becomes another
SERIO_QUIRK_* flag and avoid the duplication of the DMI ids?

Regards,

Hans




^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: input: atmel,maxtouch: add linux,keycodes
From: Krzysztof Kozlowski @ 2023-02-28  8:18 UTC (permalink / raw)
  To: André Apitzsch, Nick Dyer, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, Linus Walleij, linux-input, devicetree,
	linux-arm-kernel, linux-kernel
  Cc: ~postmarketos/upstreaming
In-Reply-To: <20230227205035.18551-1-git@apitzsch.eu>

On 27/02/2023 21:50, André Apitzsch wrote:
> In some configurations the touch controller can support the touch keys.
> Document the linux,keycodes property that enables those keys and
> specifies the keycodes that should be used to report the key events.
> 
> Signed-off-by: André Apitzsch <git@apitzsch.eu>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/2] Input: atkbd - Make it possible to apply atkbd.reset via kernel quirk
From: Greg KH @ 2023-02-28  7:34 UTC (permalink / raw)
  To: Werner Sembach
  Cc: dmitry.torokhov, swboyd, hdegoede, mkorpershoek, chenhuacai,
	wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <20230227185907.569154-2-wse@tuxedocomputers.com>

On Mon, Feb 27, 2023 at 07:59:06PM +0100, Werner Sembach wrote:
> atkbd.reset was only a command line parameter. Some devices might have a
> known bug that can be worked around by just permanently applying this
> quirk.
> 
> This patch adds the ability to do this on the kernel level for known buggy
> devices.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/input/keyboard/atkbd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> index 246958795f60..ef65c46c4efe 100644
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -1731,6 +1731,12 @@ static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
>  	return 1;
>  }
>  
> +static int __init atkbd_reset_fixup(const struct dmi_system_id *id)
> +{
> +	atkbd_reset = true;
> +	return 1;

Why is this returning 1?  Who calls this?

And this should be a per-device attribute, not a global one, right?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Hector Martin @ 2023-02-28  3:05 UTC (permalink / raw)
  To: Sasha Finkelstein, Rob Herring
  Cc: Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
	Krzysztof Kozlowski, -, Henrik Rydberg, linux-arm-kernel,
	linux-input, devicetree, linux-kernel
In-Reply-To: <CAMT+MTTH-DC+1wMFUGNwVM3cYm0B+XVK7wL07khdy1hSjyhDGA@mail.gmail.com>

On 28/02/2023 07.25, Sasha Finkelstein wrote:
> On Mon, 27 Feb 2023 at 23:14, Rob Herring <robh@kernel.org> wrote:
>> I know little about libinput, but how would it know about
>> 'apple,z2-device-name'?
>>
> The idea was to forward the contents of this property
> into the input device name.

Then you want "label", as Rob said.

But I also agree with Rob that we want per-device compatibles, even if
we don't use them upfront in the driver. That's what we do everywhere
else, and it has served us well. I suggest this hierarchy:

compatible = "apple,j293-touchbar", "apple,z2-touchbar",
"apple,z2-multitouch";
label = "Apple J293 Touch Bar";

Then let's say a hypothetical touchscreen + touchbar MacBook comes out,
we end up with:

compatible = "apple,j789-touchbar", "apple,z2-touchbar",
"apple,z2-multitouch";
label = "Apple J789 Touch Bar";

compatible = "apple,j789-touchscreen", "apple,z2-touchscreen",
"apple,z2-multitouch";
label = "Apple J789 Touchscreen";

And it all is nicely future-proof. If libinput needs a hint other than
the device name to figure out what should be treated as a touchscreen or
not, the driver can use the "apple,z2-touchbar" vs
"apple,z2-touchscreen" distinction level for that. And if per-device
quirks are never necessary, we just ignore the model number compatible,
which is already what we do all over the place in other drivers (but the
day it becomes necessary, it's ready for us). And if it turns out we
don't need any of this for a while, the driver can just bind to
"apple,z2-multitouch" and call it a day.

- Hector

^ permalink raw reply

* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Hector Martin @ 2023-02-28  2:58 UTC (permalink / raw)
  To: Sven Peter, Sasha Finkelstein, Mark Kettenis
  Cc: Alyssa Rosenzweig, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, asahi, Henrik Rydberg, linux-arm-kernel,
	linux-input, devicetree, linux-kernel
In-Reply-To: <e6c7eb27-1b60-4894-a623-28ca3bccdea5@app.fastmail.com>

On 24/02/2023 20.08, Sven Peter wrote:
> Hi,
> 
> 
> On Fri, Feb 24, 2023, at 12:04, Sasha Finkelstein wrote:
>> On Fri, 24 Feb 2023 at 11:55, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>
>>> What is the motivation for including the firmware name in the device
>>> tree rather than constructing it in the driver like what is done for
>>> the broadcom wireless?
>> There is no way to identify the device subtype before the firmware is
>> uploaded, and so i need some way of figuring out which firmware to use.
> 
> Some Broadcom bluetooth boards use the compatible of the root node (see
> btbcm_get_board_name in drivers/bluetooth/btbcm.c) which would be "apple,jXXX"
> for Apple Silicon. I believe the Broadcom WiFi driver has similar logic as well
> which marcan had to extend to instead of "brcm,board-type" because different
> WiFi boards can me matched to different Apple Silicon boards. I don't think
> that's the case for this touchscreen though.

The reason why the brcmfmac stuff needs to construct the firmware name
itself is that parts of it come from the OTP contents, so there is no
way to know from the bootloader what the right firmware is.

That is not the case here, so it makes perfect sense to specify the
firmware with `firmware-name` (which is a standard DT property).

As for the layout, both bare names and paths are in common use:

qcom/sm8450-qrd.dts:    firmware-name = "qcom/sm8450/slpi.mbn";
ti/k3-am64-main.dtsi:   firmware-name = "am64-main-r5f0_0-fw";

... but the bare names in particular, judging by some Google searches,
are *actually* mapped to bare files in /lib/firmware anyway. So the
firmware-name property contains the firmware path in the linux-firmware
standard hierarchy, in every case.

I already did the same thing for the touchpad on M2s (which requires
analogous Z2 firmware passed to it, just in a different format):

dts/apple/t8112-j413.dts: firmware-name = "apple/tpmtfw-j413.bin";

Why is having a directory a problem for OpenBSD? Regardless of how
firmware is handled behind the scenes, it seems logical to organize it
by vendor somehow. It seems to me that gratuitously diverging from the
standard firmware hierarchy is only going to cause trouble for OpenBSD.
Obviously it's fine to store it somewhere other than /lib/firmware or
use a completely unrelated mechanism other than files, but why does the
*organization* of the firmware have to diverge? There can only be one DT
binding, so we need to agree on a way of specifying firmwares that works
cross-OS, and I don't see why "apple/foo.bin" couldn't be made to work
for everyone in some way or another.

- Hector

^ permalink raw reply

* Re: [PATCH RFC 0/4] Driver for Apple Z2 touchscreens.
From: Hector Martin @ 2023-02-28  2:43 UTC (permalink / raw)
  To: fnkl.kernel, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
	Rob Herring, Krzysztof Kozlowski, -, Henrik Rydberg
  Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel
In-Reply-To: <20230223-z2-for-ml-v1-0-028f2b85dc15@gmail.com>

On 24/02/2023 19.20, Sasha Finkelstein via B4 Relay wrote:
> Hi.
> 
> This series adds support for Apple touchscreens using the Z2 protocol.
> Those are used as the primary touchscreen on mobile Apple devices, and for the
> touchbar on laptops using the M-series chips. (T1/T2 laptops have a coprocessor
> in charge of speaking Z2 to the touchbar).
> 
> Sending this as a RFC for now, since this series requires the SPI controller
> support which is not upstream yet:
> https://lore.kernel.org/all/20211212034726.26306-1-marcan@marcan.st/
> 
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>

Just FWIW, I'm happy to see this get RFCed early (early review is always
good), but I don't think we should submit it until it actually gets some
testing in real-world scenarios. That is naturally blocked on having a
MIPI display driver, since a touchscreen above a dead screen isn't
terribly useful. Otherwise, we might realize that we have some
binding/API issues we missed that are harder to fix after the fact.

So let's hold off on submission proper until we have the screen working
and some basic userspace tooling to go with all this shipping downstream
and it gets some real-world testing. Hopefully by then I'll have sent
out the SPI controller driver too :)

- Hector

^ permalink raw reply

* Re: [PATCH] Input: xpad - fix PowerA EnWired Controller guide button
From: Vicki Pfau @ 2023-02-28  0:32 UTC (permalink / raw)
  To: Dmitry Torokhov, Lyude Paul; +Cc: Michael Cullen, Marcos Alano, linux-input
In-Reply-To: <Y/pS+NAqCwdQNJNQ@google.com>

Hi Dmitry,

On 2/25/23 10:27, Dmitry Torokhov wrote:
> Hi Lyude,
> 
> On Sat, Feb 25, 2023 at 02:14:27AM +0100, Lyude Paul wrote:
>> Poke, Dmitry - any chance we could get this pushed?
> 
> I was waiting for Vicki to respond to Mattijs' comments...
> 
> Thanks.
> 

I'm a little confused as to what's happened here. This appears to be the thread for the v1 of this patch, but the v2 appeared later in a separate series (https://lore.kernel.org/linux-input/20230203022758.3982393-1-vi@endrift.com/T/#m98b35653c34a180e08cebb63b43b3690943f6e77) that was reviewed already. It moves the check into a separate function that will wind up properly freeing the URB if it fails. I tried moving it directly into probe, but that broke resume; it works properly when put in this function.

Vicki

^ permalink raw reply

* Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support
From: Andy Shevchenko @ 2023-02-27 23:07 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
	bartosz.golaszewski, dmitry.torokhov, devicetree, linux-input,
	ethan.twardy
In-Reply-To: <20230227140758.1575-4-kaehndan@gmail.com>

On Mon, Feb 27, 2023 at 08:07:58AM -0600, Danny Kaehn wrote:
> Bind I2C and GPIO interfaces to subnodes with names
> "i2c" and "gpio" if they exist, respectively. This
> allows the GPIO and I2C controllers to be described
> in firmware as usual. Additionally, support configuring the
> I2C bus speed from the clock-frequency device property.

A bit shorten indentation...

Nevertheless what I realized now is that this change, despite being OF
independent by used APIs, still OF-only.

Would it be possible to allow indexed access to child nodes as well, so if
there are no names, we may still be able to use firmware nodes from the correct
children?

P.S. The problem with ACPI is that "name" of the child node will be in capital
letters as it's in accordance with the specification.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Sasha Finkelstein @ 2023-02-27 22:25 UTC (permalink / raw)
  To: Rob Herring
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
	Krzysztof Kozlowski, -, Henrik Rydberg, linux-arm-kernel,
	linux-input, devicetree, linux-kernel
In-Reply-To: <20230227221426.GA1048218-robh@kernel.org>

On Mon, 27 Feb 2023 at 23:14, Rob Herring <robh@kernel.org> wrote:
> I know little about libinput, but how would it know about
> 'apple,z2-device-name'?
>
The idea was to forward the contents of this property
into the input device name.

^ permalink raw reply

* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Rob Herring @ 2023-02-27 22:23 UTC (permalink / raw)
  To: Sasha Finkelstein
  Cc: Sven Peter, Hector Martin, Alyssa Rosenzweig, Dmitry Torokhov,
	Krzysztof Kozlowski, -, Henrik Rydberg, linux-arm-kernel,
	linux-input, devicetree, linux-kernel
In-Reply-To: <CAMT+MTQ0bZiGpEFK9w3zncB-ZcCLY_tp1d5Wc0MJUMjWqc61jA@mail.gmail.com>

On Fri, Feb 24, 2023 at 12:08:42PM +0100, Sasha Finkelstein wrote:
> On Fri, 24 Feb 2023 at 12:04, Sven Peter <sven@svenpeter.dev> wrote:
> > Now that I thought about this again after the brief discussion we already had:
> > Do we even need to specify the device name? Is there any reason we can't just
> > always use something like "Apple Z2 TouchBar"?
> A similar protocol is used for primary touchscreen on idevices, which

Similar? So close, but somehow different.

> need different
> userspace handling. This is to make the driver potentially useful for
> people who run
> linux on checkra1n-able devices.

It is sounding to me like you need different compatibles. Don't try to 
parameterize the differences with properties. Unless you can define all 
the differences up front (hint: you can't).

Rob

^ permalink raw reply

* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Rob Herring @ 2023-02-27 22:14 UTC (permalink / raw)
  To: Sasha Finkelstein
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
	Krzysztof Kozlowski, -, Henrik Rydberg, linux-arm-kernel,
	linux-input, devicetree, linux-kernel
In-Reply-To: <CAMT+MTRodB_+sMtoPxv-gP_+sJAwNb36XuLOGo=HvuCQq6h+gQ@mail.gmail.com>

On Mon, Feb 27, 2023 at 09:06:28PM +0100, Sasha Finkelstein wrote:
> On Mon, 27 Feb 2023 at 20:51, Rob Herring <robh@kernel.org> wrote:
> >
> > > +properties:
> > > +  compatible:
> > > +    const: apple,z2-touchscreen
> >
> > Is 'z2' anything other than a touchscreen? If not, '-touchscreen' is
> > redundant. If so, then what else is there? You should be describing
> > physical devices, not just a protocol for touchscreen.
> >
> 
> This is a class of touchscreen controllers that talk the z2 protocol
> over spi.

Yes, you already said that much. So nothing else for this piece of h/w? 
Then 'apple,z2' is sufficient. Well maybe. You are assuming all h/w in 
the world speaking 'z2' is the same (to software). Usually that's not a 
safe assumption, but maybe Apple is better at not changing the h/w...

Normally, the 'protocol' to talk to a device is only part of it. There's 
other pieces like how to turn the device on and off which need h/w 
specific knowledge. If you need any of that, then you need specific 
compatibles. Adding properties for each variation doesn't end up well.


> 
> > > +                    touchscreen-size-y = <640>;
> > > +                    apple,z2-device-name = "MacBookPro17,1 Touch Bar";
> >
> > Why do we need this string? If you want a human consumed label for
> > some identification, we have a property for that purpose. It's called
> > 'label'. But when there is only 1 instance, I don't really see the
> > point.
> 
> I want a libinput-consumed label to distinguish between devices
> using this protocol. 

I know little about libinput, but how would it know about 
'apple,z2-device-name'?

> It is used both for 'normal' touchscreens, and,
> as is in this example a 'touchbar', which absolutely should not be
> treated as a normal touchscreen, and needs special handling in
> userspace.

Meaning there are both touchscreens and touchbars using this? That 
sounds like s/w needs this information. From a DT perspective, 
'compatible' is how DT defines exactly what the h/w is and how to use 
it. That also doesn't sound like a unique issue. Doesn't the kernel 
provide a standard way to tell userspace what's a touchscreen vs. 
touchpad vs. ???

Rob


^ permalink raw reply

* [PATCH 1/2] dt-bindings: input: atmel,maxtouch: add linux,keycodes
From: André Apitzsch @ 2023-02-27 20:50 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Linus Walleij,
	linux-input, devicetree, linux-arm-kernel, linux-kernel
  Cc: ~postmarketos/upstreaming, André Apitzsch

In some configurations the touch controller can support the touch keys.
Document the linux,keycodes property that enables those keys and
specifies the keycodes that should be used to report the key events.

Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 .../devicetree/bindings/input/atmel,maxtouch.yaml          | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
index 3ec579d63570..c40799355ed7 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
@@ -14,6 +14,9 @@ description: |
   Atmel maXTouch touchscreen or touchpads such as the mXT244
   and similar devices.
 
+allOf:
+  - $ref: input.yaml#
+
 properties:
   compatible:
     const: atmel,maxtouch
@@ -60,6 +63,10 @@ properties:
       or experiment to determine which bit corresponds to which input. Use
       KEY_RESERVED for unused padding values.
 
+  linux,keycodes:
+    minItems: 1
+    maxItems: 8
+
   atmel,wakeup-method:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: |

base-commit: 982818426a0ffaf93b0621826ed39a84be3d7d62
-- 
2.39.2


^ permalink raw reply related

* [PATCH 2/2] Input: atmel_mxt_ts - support capacitive keys
From: André Apitzsch @ 2023-02-27 20:50 UTC (permalink / raw)
  To: Nick Dyer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, Linus Walleij,
	linux-input, devicetree, linux-arm-kernel, linux-kernel
  Cc: ~postmarketos/upstreaming, André Apitzsch
In-Reply-To: <20230227205035.18551-1-git@apitzsch.eu>

Add support for touch keys found in some Atmel touch controller
configurations.

Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 85 ++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 996bf434e1cb..eb368dd1abf0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -55,6 +55,7 @@
 #define MXT_TOUCH_KEYARRAY_T15		15
 #define MXT_TOUCH_PROXIMITY_T23		23
 #define MXT_TOUCH_PROXKEY_T52		52
+#define MXT_TOUCH_PTC_KEYS_T97		97
 #define MXT_PROCI_GRIPFACE_T20		20
 #define MXT_PROCG_NOISE_T22		22
 #define MXT_PROCI_ONETOUCH_T24		24
@@ -326,9 +327,13 @@ struct mxt_data {
 	u16 T71_address;
 	u8 T9_reportid_min;
 	u8 T9_reportid_max;
+	u8 T15_reportid_min;
+	u8 T15_reportid_max;
 	u16 T18_address;
 	u8 T19_reportid;
 	u16 T44_address;
+	u8 T97_reportid_min;
+	u8 T97_reportid_max;
 	u8 T100_reportid_min;
 	u8 T100_reportid_max;
 
@@ -344,6 +349,9 @@ struct mxt_data {
 	u32 *t19_keymap;
 	unsigned int t19_num_keys;
 
+	u32 *t15_keymap;
+	unsigned int t15_num_keys;
+
 	enum mxt_suspend_mode suspend_mode;
 
 	u32 wakeup_method;
@@ -375,6 +383,7 @@ static bool mxt_object_readable(unsigned int type)
 	case MXT_TOUCH_KEYARRAY_T15:
 	case MXT_TOUCH_PROXIMITY_T23:
 	case MXT_TOUCH_PROXKEY_T52:
+	case MXT_TOUCH_PTC_KEYS_T97:
 	case MXT_TOUCH_MULTITOUCHSCREEN_T100:
 	case MXT_PROCI_GRIPFACE_T20:
 	case MXT_PROCG_NOISE_T22:
@@ -891,6 +900,25 @@ static void mxt_proc_t9_message(struct mxt_data *data, u8 *message)
 	data->update_input = true;
 }
 
+static void mxt_proc_t15_messages(struct mxt_data *data, u8 *message)
+{
+	struct input_dev *input_dev = data->input_dev;
+	unsigned long keystates = get_unaligned_le32(&message[2]);
+	int key;
+
+	for (key = 0; key < data->t15_num_keys; key++) {
+		input_report_key(input_dev, data->t15_keymap[key],
+			!!(keystates & BIT(key)));
+	}
+
+	data->update_input = true;
+}
+
+static void mxt_proc_t97_messages(struct mxt_data *data, u8 *message)
+{
+	mxt_proc_t15_messages(data, message);
+}
+
 static void mxt_proc_t100_message(struct mxt_data *data, u8 *message)
 {
 	struct device *dev = &data->client->dev;
@@ -1017,6 +1045,12 @@ static int mxt_proc_message(struct mxt_data *data, u8 *message)
 	} else if (report_id >= data->T9_reportid_min &&
 		   report_id <= data->T9_reportid_max) {
 		mxt_proc_t9_message(data, message);
+	} else if (report_id >= data->T15_reportid_min &&
+		   report_id <= data->T15_reportid_max) {
+		mxt_proc_t15_messages(data, message);
+	} else if (report_id >= data->T97_reportid_min &&
+		   report_id <= data->T97_reportid_max) {
+		mxt_proc_t97_messages(data, message);
 	} else if (report_id >= data->T100_reportid_min &&
 		   report_id <= data->T100_reportid_max) {
 		mxt_proc_t100_message(data, message);
@@ -1689,9 +1723,13 @@ static void mxt_free_object_table(struct mxt_data *data)
 	data->T71_address = 0;
 	data->T9_reportid_min = 0;
 	data->T9_reportid_max = 0;
+	data->T15_reportid_min = 0;
+	data->T15_reportid_max = 0;
 	data->T18_address = 0;
 	data->T19_reportid = 0;
 	data->T44_address = 0;
+	data->T97_reportid_min = 0;
+	data->T97_reportid_max = 0;
 	data->T100_reportid_min = 0;
 	data->T100_reportid_max = 0;
 	data->max_reportid = 0;
@@ -1764,6 +1802,10 @@ static int mxt_parse_object_table(struct mxt_data *data,
 						object->num_report_ids - 1;
 			data->num_touchids = object->num_report_ids;
 			break;
+		case MXT_TOUCH_KEYARRAY_T15:
+			data->T15_reportid_min = min_id;
+			data->T15_reportid_max = max_id;
+			break;
 		case MXT_SPT_COMMSCONFIG_T18:
 			data->T18_address = object->start_address;
 			break;
@@ -1773,6 +1815,10 @@ static int mxt_parse_object_table(struct mxt_data *data,
 		case MXT_SPT_GPIOPWM_T19:
 			data->T19_reportid = min_id;
 			break;
+		case MXT_TOUCH_PTC_KEYS_T97:
+			data->T97_reportid_min = min_id;
+			data->T97_reportid_max = max_id;
+			break;
 		case MXT_TOUCH_MULTITOUCHSCREEN_T100:
 			data->multitouch = MXT_TOUCH_MULTITOUCHSCREEN_T100;
 			data->T100_reportid_min = min_id;
@@ -2050,6 +2096,7 @@ static int mxt_initialize_input_device(struct mxt_data *data)
 	int error;
 	unsigned int num_mt_slots;
 	unsigned int mt_flags = 0;
+	int i;
 
 	switch (data->multitouch) {
 	case MXT_TOUCH_MULTI_T9:
@@ -2095,6 +2142,10 @@ static int mxt_initialize_input_device(struct mxt_data *data)
 	input_dev->open = mxt_input_open;
 	input_dev->close = mxt_input_close;
 
+	input_dev->keycode = data->t15_keymap;
+	input_dev->keycodemax = data->t15_num_keys;
+	input_dev->keycodesize = sizeof(data->t15_keymap[0]);
+
 	input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
 
 	/* For single touch */
@@ -2162,6 +2213,12 @@ static int mxt_initialize_input_device(struct mxt_data *data)
 				     0, 255, 0, 0);
 	}
 
+	/* For T15 and T97 Key Array */
+	if (data->T15_reportid_min || data->T97_reportid_min) {
+		for (i = 0; i < data->t15_num_keys; i++)
+			input_set_capability(input_dev, EV_KEY, data->t15_keymap[i]);
+	}
+
 	input_set_drvdata(input_dev, data);
 
 	error = input_register_device(input_dev);
@@ -3080,8 +3137,10 @@ static void mxt_input_close(struct input_dev *dev)
 static int mxt_parse_device_properties(struct mxt_data *data)
 {
 	static const char keymap_property[] = "linux,gpio-keymap";
+	static const char buttons_property[] = "linux,keycodes";
 	struct device *dev = &data->client->dev;
 	u32 *keymap;
+	u32 *buttonmap;
 	int n_keys;
 	int error;
 
@@ -3111,6 +3170,32 @@ static int mxt_parse_device_properties(struct mxt_data *data)
 		data->t19_num_keys = n_keys;
 	}
 
+	if (device_property_present(dev, buttons_property)) {
+		n_keys = device_property_count_u32(dev, buttons_property);
+		if (n_keys <= 0) {
+			error = n_keys < 0 ? n_keys : -EINVAL;
+			dev_err(dev, "invalid/malformed '%s' property: %d\n",
+				buttons_property, error);
+			return error;
+		}
+
+		buttonmap = devm_kmalloc_array(dev, n_keys, sizeof(*buttonmap),
+					       GFP_KERNEL);
+		if (!buttonmap)
+			return -ENOMEM;
+
+		error = device_property_read_u32_array(dev, buttons_property,
+						       buttonmap, n_keys);
+		if (error) {
+			dev_err(dev, "failed to parse '%s' property: %d\n",
+				buttons_property, error);
+			return error;
+		}
+
+		data->t15_keymap = buttonmap;
+		data->t15_num_keys = n_keys;
+	}
+
 	return 0;
 }
 
-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Sasha Finkelstein @ 2023-02-27 20:06 UTC (permalink / raw)
  To: Rob Herring
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
	Krzysztof Kozlowski, -, Henrik Rydberg, linux-arm-kernel,
	linux-input, devicetree, linux-kernel
In-Reply-To: <20230227195139.GA677578-robh@kernel.org>

On Mon, 27 Feb 2023 at 20:51, Rob Herring <robh@kernel.org> wrote:
>
> > +properties:
> > +  compatible:
> > +    const: apple,z2-touchscreen
>
> Is 'z2' anything other than a touchscreen? If not, '-touchscreen' is
> redundant. If so, then what else is there? You should be describing
> physical devices, not just a protocol for touchscreen.
>

This is a class of touchscreen controllers that talk the z2 protocol
over spi.

> > +                    touchscreen-size-y = <640>;
> > +                    apple,z2-device-name = "MacBookPro17,1 Touch Bar";
>
> Why do we need this string? If you want a human consumed label for
> some identification, we have a property for that purpose. It's called
> 'label'. But when there is only 1 instance, I don't really see the
> point.

I want a libinput-consumed label to distinguish between devices
using this protocol. It is used both for 'normal' touchscreens, and,
as is in this example a 'touchbar', which absolutely should not be
treated as a normal touchscreen, and needs special handling in
userspace.

^ permalink raw reply

* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Rob Herring @ 2023-02-27 19:51 UTC (permalink / raw)
  To: Sasha Finkelstein
  Cc: Hector Martin, Sven Peter, Alyssa Rosenzweig, Dmitry Torokhov,
	Krzysztof Kozlowski, -, Henrik Rydberg, linux-arm-kernel,
	linux-input, devicetree, linux-kernel
In-Reply-To: <20230223-z2-for-ml-v1-1-028f2b85dc15@gmail.com>

On Fri, Feb 24, 2023 at 11:20:06AM +0100, Sasha Finkelstein wrote:
> Add bindings for touchscreen controllers attached using the Z2 protocol.
> Those are present in most Apple devices.
> 
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
>  .../input/touchscreen/apple,z2-touchscreen.yaml    | 81 ++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> new file mode 100644
> index 000000000000..695594494b1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/apple,z2-touchscreen.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/apple,z2-touchscreen.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple touchscreens attached using the Z2 protocol.
> +
> +maintainers:
> +  - asahi@lists.linux.dev
> +  - Sasha Finkelstein <fnkl.kernel@gmail.com>
> +
> +description: A series of touschscreen controllers used in Apple products.
> +
> +allOf:
> +  - $ref: touchscreen.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: apple,z2-touchscreen

Is 'z2' anything other than a touchscreen? If not, '-touchscreen' is 
redundant. If so, then what else is there? You should be describing 
physical devices, not just a protocol for touchscreen.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts-extended:

Just 'interrupts' here. 'interrupts-extended' is implicitly supported.

> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  cs-gpios:

There is a standard way to do GPIO based chip-selects. It happens to be 
'cs-gpios', but this is in the wrong place. It goes in the SPI 
controller node.

> +    maxItems: 1
> +
> +  firmware-name:
> +    maxItems: 1
> +
> +  apple,z2-device-name:
> +    description: The name to be used for the input device
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +  touchscreen-size-x: true
> +  touchscreen-size-y: true
> +  spi-max-frequency: true
> +
> +required:
> +  - compatible
> +  - interrupts-extended
> +  - reset-gpios
> +  - cs-gpios
> +  - firmware-name
> +  - apple,z2-device-name
> +  - touchscreen-size-x
> +  - touchscreen-size-y
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;

4 space indentation is preferred here.

> +
> +            touchscreen@0 {
> +                    compatible = "apple,z2-touchscreen";
> +                    reg = <0>;
> +                    spi-max-frequency = <11500000>;
> +                    reset-gpios = <&pinctrl_ap 139 0>;
> +                    cs-gpios = <&pinctrl_ap 109 0>;
> +                    interrupts-extended = <&pinctrl_ap 194 IRQ_TYPE_EDGE_FALLING>;
> +                    firmware-name = "apple/dfrmtfw-j293.bin";
> +                    touchscreen-size-x = <23045>;
> +                    touchscreen-size-y = <640>;
> +                    apple,z2-device-name = "MacBookPro17,1 Touch Bar";

Why do we need this string? If you want a human consumed label for 
some identification, we have a property for that purpose. It's called 
'label'. But when there is only 1 instance, I don't really see the 
point.

Rob

^ permalink raw reply

* [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"
From: Werner Sembach @ 2023-02-27 18:59 UTC (permalink / raw)
  To: dmitry.torokhov, wse, swboyd, gregkh, hdegoede, mkorpershoek,
	chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel

This is a continuation of
https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/

That fix did fix the keyboard not responding at all sometimes after resume,
but at the price of it being laggy for some time after boot. Additionally
setting atkbd.reset removes that lag.

This patch comes in 2 parts: The first one adds a quirk to atkbd to set
atkbd.reset and the second one then applies that and the i8042 quirks to
the affected devices.



^ permalink raw reply

* [PATCH v2 1/2] Input: atkbd - Make it possible to apply atkbd.reset via kernel quirk
From: Werner Sembach @ 2023-02-27 18:59 UTC (permalink / raw)
  To: dmitry.torokhov, wse, swboyd, gregkh, hdegoede, mkorpershoek,
	chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <20230227185907.569154-1-wse@tuxedocomputers.com>

atkbd.reset was only a command line parameter. Some devices might have a
known bug that can be worked around by just permanently applying this
quirk.

This patch adds the ability to do this on the kernel level for known buggy
devices.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: stable@vger.kernel.org
---
 drivers/input/keyboard/atkbd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 246958795f60..ef65c46c4efe 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -1731,6 +1731,12 @@ static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
 	return 1;
 }
 
+static int __init atkbd_reset_fixup(const struct dmi_system_id *id)
+{
+	atkbd_reset = true;
+	return 1;
+}
+
 /*
  * NOTE: do not add any more "force release" quirks to this table.  The
  * task of adjusting list of keys that should be "released" automatically
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 2/2] Input: i8042 - add 2 more TUXEDO devices to i8042 and atkbd quirk tables
From: Werner Sembach @ 2023-02-27 18:59 UTC (permalink / raw)
  To: dmitry.torokhov, wse, swboyd, gregkh, hdegoede, mkorpershoek,
	chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <20230227185907.569154-1-wse@tuxedocomputers.com>

A lot of modern Clevo barebones have touchpad and/or keyboard issues after
suspend fixable with nomux + reset + noloop + nopnp. Luckily, none of them
have an external PS/2 port so this can safely be set for all of them.

I'm not entirely sure if every device listed really needs all four quirks,
but after testing and production use. No negative effects could be
observed when setting all four.

Setting SERIO_QUIRK_NOMUX or SERIO_QUIRK_RESET_ALWAYS makes the keyboard
very laggy for ~5 seconds after boot and sometimes also after resume. To
fix this atkbd_reset_fixup is also applied for this device via a quirk,
which fixes it again.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: stable@vger.kernel.org
---
 drivers/input/keyboard/atkbd.c        | 24 ++++++++++++++++++++++++
 drivers/input/serio/i8042-acpipnpio.h | 26 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index ef65c46c4efe..e89918347357 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -1886,6 +1886,30 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 		},
 		.callback = atkbd_deactivate_fixup,
 	},
+	/*
+	 * Some Clevo devices need this reset, otherwise they keyboard may
+	 * be laggy after boot and/or resume for ~5 seconds.
+	 */
+	{
+		/*
+		 * Clevo device, DMI_BOARD_VENDOR and DMI_SYSTEM_VENDOR
+		 * differ between resellers.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "N150CU"),
+		},
+		.callback = atkbd_reset_fixup,
+	},
+	{
+		/*
+		 * Clevo device, DMI_BOARD_VENDOR and DMI_SYSTEM_VENDOR
+		 * differ between resellers.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NHxxRZQ"),
+		},
+		.callback = atkbd_reset_fixup,
+	},
 	{ }
 };
 
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index efc61736099b..6ed1044b4ba5 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -1116,6 +1116,19 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
 					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
 	},
+	{
+		/*
+		 * Setting SERIO_QUIRK_NOMUX or SERIO_QUIRK_RESET_ALWAYS makes the
+		 * keyboard very laggy for ~5 seconds after boot and sometimes also
+		 * after resume. To fix this atkbd_reset_fixup is also applied for
+		 * this device via a quirk, which fixes it again.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "N150CU"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
+					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
+	},
 	{
 		.matches = {
 			DMI_MATCH(DMI_BOARD_NAME, "NH5xAx"),
@@ -1123,6 +1136,19 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
 					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
 	},
+	{
+		/*
+		 * Setting SERIO_QUIRK_NOMUX or SERIO_QUIRK_RESET_ALWAYS makes the
+		 * keyboard very laggy for ~5 seconds after boot and sometimes also
+		 * after resume. To fix this atkbd_reset_fixup is also applied for
+		 * this device via a quirk, which fixes it again.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NHxxRZQ"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
+					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
+	},
 	{
 		.matches = {
 			DMI_MATCH(DMI_BOARD_NAME, "NL5xRU"),
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH AUTOSEL 5.15 10/25] HID: multitouch: Add quirks for flipped axes
From: Sasha Levin @ 2023-02-27 14:59 UTC (permalink / raw)
  To: Jean Delvare
  Cc: linux-kernel, stable, Allen Ballway, Jiri Kosina, jikos,
	benjamin.tissoires, groeck, alistair, dmitry.torokhov, jk,
	Jonathan.Cameron, cmo, u.kleine-koenig, linux-input
In-Reply-To: <20230227132300.4a3c3fad@endymion.delvare>

On Mon, Feb 27, 2023 at 01:23:00PM +0100, Jean Delvare wrote:
>Hi Sasha,
>
>On Sun, 26 Feb 2023 21:08:33 -0500, Sasha Levin wrote:
>> From: Allen Ballway <ballway@chromium.org>
>>
>> [ Upstream commit a2f416bf062a38bb76cccd526d2d286b8e4db4d9 ]
>>
>> Certain touchscreen devices, such as the ELAN9034, are oriented
>> incorrectly and report touches on opposite points on the X and Y axes.
>> For example, a 100x200 screen touched at (10,20) would report (90, 180)
>> and vice versa.
>>
>> This is fixed by adding device quirks to transform the touch points
>> into the correct spaces, from X -> MAX(X) - X, and Y -> MAX(Y) - Y.
>>
>> Signed-off-by: Allen Ballway <ballway@chromium.org>
>> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>> ---
>>  drivers/hid/hid-multitouch.c             | 39 ++++++++++++++++++---
>>  drivers/hid/hid-quirks.c                 |  6 ++++
>>  drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 43 ++++++++++++++++++++++++
>>  drivers/hid/i2c-hid/i2c-hid.h            |  3 ++
>>  4 files changed, 87 insertions(+), 4 deletions(-)
>> (...)
>
>Second rule of acceptance for stable patches:
>
> - It cannot be bigger than 100 lines, with context.
>
>Clearly not met here.

About a quarter of stable commits in the last 12 months breat the rule
above.

>To me, this commit is something distributions may want to backport if
>their users run are likely to run the affected hardware. But it's out
>of scope for stable kernel branches.

Why? We explicitly call out new device IDs and quirks in the same doc
you quoted the 100 line rule from.

-- 
Thanks,
Sasha

^ permalink raw reply

* Re: [PATCH AUTOSEL 5.15 10/25] HID: multitouch: Add quirks for flipped axes
From: Greg KH @ 2023-02-27 14:34 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Sasha Levin, linux-kernel, stable, Allen Ballway, Jiri Kosina,
	jikos, benjamin.tissoires, groeck, alistair, dmitry.torokhov, jk,
	Jonathan.Cameron, cmo, u.kleine-koenig, linux-input
In-Reply-To: <20230227132300.4a3c3fad@endymion.delvare>

On Mon, Feb 27, 2023 at 01:23:00PM +0100, Jean Delvare wrote:
> Hi Sasha,
> 
> On Sun, 26 Feb 2023 21:08:33 -0500, Sasha Levin wrote:
> > From: Allen Ballway <ballway@chromium.org>
> > 
> > [ Upstream commit a2f416bf062a38bb76cccd526d2d286b8e4db4d9 ]
> > 
> > Certain touchscreen devices, such as the ELAN9034, are oriented
> > incorrectly and report touches on opposite points on the X and Y axes.
> > For example, a 100x200 screen touched at (10,20) would report (90, 180)
> > and vice versa.
> > 
> > This is fixed by adding device quirks to transform the touch points
> > into the correct spaces, from X -> MAX(X) - X, and Y -> MAX(Y) - Y.
> > 
> > Signed-off-by: Allen Ballway <ballway@chromium.org>
> > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > ---
> >  drivers/hid/hid-multitouch.c             | 39 ++++++++++++++++++---
> >  drivers/hid/hid-quirks.c                 |  6 ++++
> >  drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 43 ++++++++++++++++++++++++
> >  drivers/hid/i2c-hid/i2c-hid.h            |  3 ++
> >  4 files changed, 87 insertions(+), 4 deletions(-)
> > (...)
> 
> Second rule of acceptance for stable patches:
> 
>  - It cannot be bigger than 100 lines, with context.
> 
> Clearly not met here.
> 
> To me, this commit is something distributions may want to backport if
> their users run are likely to run the affected hardware. But it's out
> of scope for stable kernel branches.

For new quirks and ids, this is totally fine, I do not see why we would
NOT want to take such a thing.

The 100 lines is a guideline, to make things easy to review.

thanks,

greg k-h

^ permalink raw reply

* [PATCH v8 3/3] HID: cp2112: Fwnode Support
From: Danny Kaehn @ 2023-02-27 14:07 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: bartosz.golaszewski, andriy.shevchenko, dmitry.torokhov,
	devicetree, linux-input, ethan.twardy
In-Reply-To: <20230227140758.1575-1-kaehndan@gmail.com>

Bind I2C and GPIO interfaces to subnodes with names
"i2c" and "gpio" if they exist, respectively. This
allows the GPIO and I2C controllers to be described
in firmware as usual. Additionally, support configuring the
I2C bus speed from the clock-frequency device property.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 drivers/hid/hid-cp2112.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 27cadadda7c9..491e3c83af12 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1234,6 +1234,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	u8 buf[3];
 	struct cp2112_smbus_config_report config;
 	struct gpio_irq_chip *girq;
+	struct i2c_timings timings;
 	int ret;
 
 	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1292,6 +1293,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_power_normal;
 	}
 
+	device_set_node(&dev->adap.dev, device_get_named_child_node(&hdev->dev, "i2c"));
+	i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
+
+	config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
 	config.retry_time = cpu_to_be16(1);
 
 	ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
@@ -1300,7 +1305,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hid_err(hdev, "error setting SMBus config\n");
 		if (ret >= 0)
 			ret = -EIO;
-		goto err_power_normal;
+		goto err_free_i2c_of;
 	}
 
 	hid_set_drvdata(hdev, (void *)dev);
@@ -1322,7 +1327,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	if (ret) {
 		hid_err(hdev, "error registering i2c adapter\n");
-		goto err_power_normal;
+		goto err_free_i2c_of;
 	}
 
 	hid_dbg(hdev, "adapter registered\n");
@@ -1336,6 +1341,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	dev->gc.ngpio			= 8;
 	dev->gc.can_sleep		= 1;
 	dev->gc.parent			= &hdev->dev;
+	dev->gc.fwnode			= device_get_named_child_node(&hdev->dev, "gpio");
 
 	dev->irq.name = "cp2112-gpio";
 	dev->irq.irq_startup = cp2112_gpio_irq_startup;
@@ -1376,7 +1382,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 err_gpiochip_remove:
 	gpiochip_remove(&dev->gc);
 err_free_i2c:
+	fwnode_handle_put(dev->gc.fwnode);
 	i2c_del_adapter(&dev->adap);
+err_free_i2c_of:
+	fwnode_handle_put(dev_fwnode(&dev->adap.dev));
 err_power_normal:
 	hid_hw_power(hdev, PM_HINT_NORMAL);
 err_hid_close:
@@ -1391,6 +1400,8 @@ static void cp2112_remove(struct hid_device *hdev)
 	struct cp2112_device *dev = hid_get_drvdata(hdev);
 	int i;
 
+	fwnode_handle_put(dev->gc.fwnode);
+	fwnode_handle_put(dev_fwnode(&dev->adap.dev));
 	sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
 	i2c_del_adapter(&dev->adap);
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v8 2/3] HID: usbhid: Share USB device firmware node with child HID device
From: Danny Kaehn @ 2023-02-27 14:07 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: bartosz.golaszewski, andriy.shevchenko, dmitry.torokhov,
	devicetree, linux-input, ethan.twardy
In-Reply-To: <20230227140758.1575-1-kaehndan@gmail.com>

USB HID core now shares its fwnode with its child HID device.
Since there can only be one HID device on a USB interface, it is redundant
to specify a hid node under the USB device. This allows usb HID device
drivers to be described in firmware and make use of device properties.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/hid/usbhid/hid-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 257dd73e37bf..090260d99c84 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
+#include <linux/property.h>
 #include <linux/spinlock.h>
 #include <asm/unaligned.h>
 #include <asm/byteorder.h>
@@ -1374,6 +1375,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
 	hid->hiddev_report_event = hiddev_report_event;
 #endif
 	hid->dev.parent = &intf->dev;
+	device_set_node(&hid->dev, dev_fwnode(&intf->dev));
 	hid->bus = BUS_USB;
 	hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
 	hid->product = le16_to_cpu(dev->descriptor.idProduct);
-- 
2.25.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox