Message-Id: <91593303-4a6a-49c9-87a0-bb6f72f512a1@app.fastmail.com>
Date: Wed, 10 Apr 2024 22:48:10 -0400
From: "Mark Pearson" <mpearson-lenovo@...ebb.ca>
To: "Dmitry Torokhov" <dmitry.torokhov@...il.com>
Cc: "Peter Hutterer" <peter.hutterer@...hat.com>,
 "Hans de Goede" <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 "Henrique de Moraes Holschuh" <hmh@....eng.br>,
 ibm-acpi-devel@...ts.sourceforge.net,
 "platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
 linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
 "Nitin Joshi1" <njoshi1@...ovo.com>, "Vishnu Sankar" <vsankar@...ovo.com>
Subject: Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes

Hi Dmitry,

On Wed, Apr 10, 2024, at 8:02 PM, Dmitry Torokhov wrote:
> On Tue, Apr 09, 2024 at 10:17:05PM -0400, Mark Pearson wrote:
>> Hi Dmitry
>> 
>> On Tue, Apr 9, 2024, at 9:20 PM, Dmitry Torokhov wrote:
>> > On Tue, Apr 09, 2024 at 02:47:05PM -0700, Dmitry Torokhov wrote:
>> >> On Tue, Apr 09, 2024 at 03:23:52PM +1000, Peter Hutterer wrote:
>> >> > On 09/04/2024 09:31, Dmitry Torokhov wrote:
>> >> > > Hi Mark,
>> >> > > 
>> >> > > On Sun, Mar 24, 2024 at 05:07:58PM -0400, Mark Pearson wrote:
>> >> > > > Add support for new input events on Lenovo laptops that need exporting to
>> >> > > > user space.
>> >> > > > 
>> >> > > > Lenovo trackpoints are adding the ability to generate a doubletap event.
>> >> > > > Add a new keycode to allow this to be used by userspace.
>> >> > > 
>> >> > > What is the intended meaning of this keycode? How does it differ from
>> >> > > the driver sending BTN_LEFT press/release twice?
>> >> > > > 
>> >> > > > Lenovo support is using FN+N with Windows to collect needed details for
>> >> > > > support cases. Add a keycode so that we'll be able to provide similar
>> >> > > > support on Linux.
>> >> > > 
>> >> > > Is there a userspace consumer for this?
>> >> > 
>> >> > Funnily enough XKB has had a keysym for this for decades but it's not
>> >> > hooked up anywhere due to the way it's pointer keys accessibility
>> >> > feature was implemented. Theory is that most of userspace just needs
>> >> > to patch the various pieces together for the new evdev code + keysym,
>> >> > it's not really any different to handling a volume key (except this
>> >> > one needs to be assignable).
>> >> 
>> >> What is the keysym? If we can make them relatable to each other that
>> >> would be good. Or maybe we could find a matching usage from HID usage
>> >> tables...
>> >
>> > I was looking through the existing codes and I see:
>> >
>> > #define KEY_INFO		0x166	/* AL OEM Features/Tips/Tutorial */
>> >
>> > We also have KEY_VENDOR used in a few drivers/plafrom/x86, including
>> > thinkkpad_acpi.c and I wonder if it would be suitable for this vendor
>> > specific debug info collection application (which I honestly doubt will
>> > materialize).
>> >
>> 
>> That's a somewhat disappointing note on your doubts, is that based on
>> anything? Just wondering what we've done to deserve that criticism.
>
> Sorry, this was not meant as a criticism really, but you mentioned
> yourself that there isn't anything in the works yet, you just have some
> plans.

All good - I was just worried we'd promised something previously and not delivered. We're often slow at delivering - but I try not to fail completely. I try especially hard not to annoy hard working kernel maintainers :)

We do have something developed internally, but it's pretty basic and we'll need to have discussion with 'userspace' fol as to if we release that as a standalone application, or if we tie into gnome (which we don't have a lot of experience with)...or something else.
Kernel world is much easier :)

>
> For such a project to succeed Lenovo needs to invest into selling
> devices with Linux as a primary operating system, and it has to be
> consumer segment (or small business, because for corporate they
> typically roll their own support channels). The case of retrofitting
> Linux onto a that device originally came with Windows OS rarely gets
> much if any response from the normal support channels.
>
> Is this something that is actually happening?

This is way off topic for the patch set, but as you asked :)

I'm afraid I'm going to disagree. What you're suggesting is probably the more common and easier route vendors take, but it has some issues and I think some longer term limitations.
For me one of the things I like the most about our program using the same exact HW as Windows is it means:
 - I have a business lever to get Linux support from HW vendors. This makes a difference when you're dealing with 'minor' components - panels, fingerprint readers, touchpads etc - the smaller devices (though it helps for the CPU vendors too). We have requirements that state the upstream Linux support is needed or the chip vendor will not be considered for the platform...and that's a big deal.
 - It gets FW teams thinking about Linux support. Yeah, there are still a ton of issues there and it's far from perfect, but it means they have to think about Linux support and at least helps us shine a bit of light on what is going on in that horrible closed box.
 - It means I get Linux running at a good level on a wider range of HW then would be otherwise possible. It means that when there is new technology I get to go and ask "how about Linux" and have that discussion (including schedules). It might come later than I'd like - but at least we get to put Linux on the roadmap rather than being excluded for being so niche. Shrug.

I think our Linux program still has a long way to go before it's even close to good - but it's in better shape than it (at least I think so, I don't love the way the industry is going with more being stuffed in FW...but that's above my paygrade)

>
>> 
>> That aside, I guess KEY_INFO or KEY_VENDOR could be a good fit (I
>> personally don't think KEY_CONFIG matches well), but I would be
>> worried about clashing with existing functionality.
>> 
>> Peter - do you have any opinion from the user space side of things, or
>> are these likely unused? KEY_VENDOR seems the safer bet to me (but I
>> don't love it).
>> 
>> Dmitry - What are the downsides or concerns of introducing a new code?
>> I'd like to evaluate that against the potential to cause conflicts by
>> re-using existing codes. If you feel strongly about it then I'll defer
>> to your judgement, but I'd like to understand better the context.
>
> The keycode space is finite and extending bitmaps leads to more memory
> consumption and weird breakages (like uevent generation exceeding 4K
> memory page resulting in failures). I am trying to balance need for new
> keycodes with how likely they are to be used.
>
Ack.
So I've been refactoring my patches to implement the feedback from Hans for the latter patches and just need to figure out what works here.

For the SYS_DEBUG_INFO, if you'd rather we use KEY_VENDOR (I think that's my preference - as its intended for a Lenovo support role it seems the best fit), and Peter has no objections, I will make that change.

I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.

Is that OK?

Mark

Powered by blists - more mailing lists