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
