| 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
| ||
|
Message-ID: <72897af0-6f03-cf25-d84b-830020973a4c@linaro.org>
Date: Wed, 11 May 2022 18:46:40 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Gireesh.Hiremath@...bosch.com, krzysztof.kozlowski+dt@...aro.org
Cc: m.felsch@...gutronix.de, linux-omap@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-input@...r.kernel.org, bcousson@...libre.com,
tony@...mide.com, robh+dt@...nel.org, dmitry.torokhov@...il.com,
mkorpershoek@...libre.com, davidgow@...gle.com,
swboyd@...omium.org, fengping.yu@...iatek.com,
y.oudjana@...tonmail.com, rdunlap@...radead.org,
colin.king@...el.com, sjoerd.simons@...labora.co.uk,
VinayKumar.Shettar@...bosch.com,
Govindaraji.Sivanantham@...bosch.com, anaclaudia.dias@...bosch.com
Subject: Re: [PATCH v2 2/4] Input: mt-matrix-keypad: Add Bosch mt matrix
keypad driver
On 10/05/2022 16:13, Gireesh.Hiremath@...bosch.com wrote:
> From: Gireesh Hiremath <Gireesh.Hiremath@...bosch.com>
>
> Hi Krzysztof,
>
>>>>> both matric_keypad.c and mt_matrix_kepad.c logically operate differently,
>>>>> my openion is not to merge both.
>>>>
>>>> IMHO from the user/system-integrator pov it is looking the same and so
>>>> one driver should be fine. To distinguish between both modes we could
>>>> add dt-property or add a new dt-compatible like "gpio-matrix-keypad-v2".
>>>>
>>>
>>> as mentioned above our keypad is not complete matrix keypad and it will
>>> not be compatible with matrix_keypad diver. that is the reason we derived
>>> mt matrix keypad driver.
>>>
>>> to avoid confusion, we will rename the driver as bosch_mt_keypad.c
>>> if you suggest.
>>
>> Sending a new version while discussions are ongoing is not how we reach
>> consensus.
>
> I apologize for sending new version.
>
>>
>> Make the driver as part of matrix-keypad driver or bring real arguments
>> why it cannot be merged.
>
> I tryied to put real hardware scenario which used in
> Bosch Power tool measuring devices.
> Keypad schematic as below, it is reduced matrix keypad compared
> to standard matrix keypad
>
> Pin8 (gpio1 16)-----------------------
> Pin7 (gpio1 20)--------------------- |
> Pin6 (gpio1 22)------------------- | |
> Pin5 (gpio2 21)----------------- | | |
> Pin4 (ground )--------------- | | | |
> Pin3 (gpio1 31)------------- | | | | |
> Pin2 (gpio1 23)----------- | | | | | |
> Pin1 (gpio1 24)--------- | | | | | | |
> | | | | | | | |
> | | | | | | | |
> | | | | | | | |
> |------------|---------|----------------- | | | | | | |-----------|
> | Button1 | | Button2 | | | | | | Button3 |
> | _|_ | | _|_ | | | | | | _|_ |
> | |--o o--| |--o o-----------| | | | | |------o o--|
> | | | | | | | | |
> | | |----------------------------| | | | | | |
> | | Button4 | Button5 | | | | | Button6 |
> | | _|_ | _|_ | | | | | _|_ |
> | |--o o--| |---o o------------| | | | |---o o-----|
> | | | | | | |
> | | |------------------|---| | |-----------| |
> | | | | | |
> | |------------------------------| | |---------| | |
> | | | | | |
> | Button7 Button8 | | Button9 | | |
> | _|_ _|_ | | _|_ | | |
> |-----o o-----|--------o o----| |------o o----| | |
> | | | |
> | |---------------------------| |
> | |
> |-------------------------------------------------|
>
>
> ____________________________________
> | Button | Pin activation| Keymap |
> |----------------------------------|
> |Button1 | 1,6 | KEY_7 |
> |----------------------------------|
> |Button2 | 1,2 | KEY_8 |
> |----------------------------------|
> |Button3 | 7,8 | KEY_9 |
> |----------------------------------|
> |Button4 | 2,6 | KEY_4 |
> |----------------------------------|
> |Button5 | 3,4 | KEY_5 |
> |----------------------------------|
> |Button6 | 6,7 | KEY_6 |
> |----------------------------------|
> |Button7 | 1,8 | KEY_1 |
> |----------------------------------|
> |Button8 | 6,8 | KEY_2 |
> |----------------------------------|
> |Button9 | 4,5 | KEY_3 |
> |----------------------------------|
>
> for Button5 and Button9 we used standard gpio_keys.c driver.
>
> Button1,2,3,4,6,7,8 are not in standard row and column format,
> found difficulty to apply matrix keypad drive to these button.
>
> to solve this we came with vendor specific driver like
> mt_matrix_keypad.c by taking matrix_keypad as reference.
>
> after your review comment I felt it should named as
> bosch_keypad.c to show as vendor specific.
>
> in this driver all gpio lines act as row as well as column,
> a key can be placed at each intersection of a unique row
> number not equal to a unique column and they are diagonally
> symmetric.
> we can skip keymap for the valid intersection of gpio and
> invalid keymap for row equal to column.
>
> the matrix table as below for above schematic
>
> ------------------------------------------------------
> |Row\Col |GPIO 0 | GPIO 1 | GPIO 2 | GPIO 3 | GPIO 4 |
> ------------------------------------------------------
> | GPIO 0 | X | KEY_9 | KEY_2 | X | KEY_1 |
> ------------------------------------------------------
> | GPIO 1 | KEY_9 | X | KEY_6 | X | X |
> ------------------------------------------------------
> | GPIO 2 | KEY_2 | KEY_6 | X | KEY_4 | KEY_7 |
> ------------------------------------------------------
> | GPIO 3 | X | X | KEY_4 | X | KEY_8 |
> ------------------------------------------------------
> | GPIO 4 | KEY_1 | X | KEY_7 | KEY_8 | X |
> ------------------------------------------------------
> X - invalid key
> KEY_x - preferred key code
>
>
> in Device tree we avoided row and column
> and passed gpio info as line-gpios
>
> line-gpios = <
> &gpio1 24 1 /*gpio_56*/
> &gpio1 23 1 /*gpio_55*/
> &gpio1 22 1 /*gpio_54*/
> &gpio1 20 1 /*gpio_52*/
> &gpio1 16 1 /*gpio_48*/
> >;
> linux,keymap = <
> 0x00000000 /* row 0, col 0, KEY_RESERVED */
> 0x0001000a /* row 0, col 1, KEY_9 */
> 0x00020003 /* row 0, col 2, KEY_2 */
> 0x00030000 /* row 0, col 3, KEY_RESERVED */
> 0x00040002 /* row 0, col 4, KEY_1 */
> 0x0100000a /* row 1, col 0, KEY_9 */
> 0x01010000 /* row 1, col 1, KEY_RESERVED */
> 0x01020007 /* row 1, col 2, KEY_6 */
> 0x01030000 /* row 1, col 3, KEY_RESERVED */
> 0x01040000 /* row 1, col 4, KEY_RESERVED */
> 0x02000003 /* row 2, col 0, KEY_2 */
> 0x02010007 /* row 2, col 1, KEY_6 */
> 0x02020000 /* row 2, col 2, KEY_RESERVED */
> 0x02030005 /* row 2, col 3, KEY_4 */
> 0x02040008 /* row 2, col 4, KEY_7 */
> 0x03000000 /* row 3, col 0, KEY_RESERVED */
> 0x03010000 /* row 3, col 1, KEY_RESERVED */
> 0x03020005 /* row 3, col 2, KEY_4 */
> 0x03030000 /* row 3, col 3, KEY_RESERVED */
> 0x03040009 /* row 3, col 4, KEY_8 */
> 0x04000002 /* row 4, col 0, KEY_1 */
> 0x04010000 /* row 4, col 1, KEY_RESERVED */
> 0x04020008 /* row 4, col 2, KEY_7 */
> 0x04030009 /* row 4, col 3, KEY_8 */
> 0x04040000 /* row 4, col 4, KEY_RESERVED */
> >;
>
> this driver approch may be usefull for the embadded device
> which are using reduced matrix keypad
You wrote pretty long message explaining how the device works, but I
still do not see the answer to questions - why it cannot be part of
matrix keypad?
"It looks like this driver has smaller number of features than
matrix-keypad, so it should be integrated into the matrix-keypad.
matrix-keypad features are superset to this one."
"But anyway this should be just merged into matrix-keypad. It's a
simpler set of that binding."
Best regards,
Krzysztof
Powered by blists - more mailing lists