linux-kernel - Re: [PATCH] Input: keypad-nomadik-ske

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZtszE9A-576SmvsX@google.com>
Date: Fri, 6 Sep 2024 09:51:31 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Arnd Bergmann <arnd@...db.de>,
	Michael Hennerich <michael.hennerich@...log.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Lee Jones <lee@...nel.org>, linux-arm-kernel@...ts.infradead.org,
	Utsav Agarwal <utsav.agarwal@...log.com>
Subject: Re: [PATCH] Input: keypad-nomadik-ske - remove the driver

Hi Nuno,

On Fri, Sep 06, 2024 at 10:38:35AM +0200, Nuno Sá wrote:
> 
> Hi Dmitry,
> 
> This is not forgotten and I plan to start working on this early next week.
> 
> One thing I noticed and I might as well just ask before starting the work, is that
> the platform data allows, in theory, for you to have holes in your keymap [1]. Like
> enabling row 1 and 3 skipping 2. AFAICT, the matrix stuff does not allow this out of
> the box as we just define the number of rows/cols and then without any other property
> we assume (I think) that the map is contiguous. 
> 
> This is just early thinking but one way to support the current behavior would be 2
> custom DT properties that would be 2 u32 arrays specifying the enabled columns and
> rows. Out of it, we could build row and col masks and get the total number of cols
> and rows that we could pass to matrix_keypad_build_keymap().

I'd ask DT maintainers but in my opinion we could add 2 u32 scalar
properties to specify row and col masks (either enabled or disabled,
whatever is more convenient) and then indeed we could figure out the
resulting size of key matrix and use matrix_keypad_build_keymap() to
load it.

> 
> The question is... is it worth it? I'm aware that if we just assume a contiguous
> keymap we could break some old users. But I guess it would be only out of tree ones
> as we don't have any in kernel user of the platform data. On top of it, I guess it's
> sane to assume that one just wants a contiguous keymap...
> 
> [1]: https://elixir.bootlin.com/linux/v6.10.8/source/drivers/input/keyboard/adp5589-keys.c#L630

I think in practice it's just a few extra lines of code, so shoudl be
fairly easy to keep supporting this.

But we can actually split the binding and the driver implementation,
with binding defining all capabilities of the hardware and driver
implementing just a subset of it (i.e. complain if row and column mask
properties are specified and abort probe).

Thanks.

-- 
Dmitry