On 10/05/2022 16:13, Gireesh.Hiremath@xxxxxxxxxxxx wrote: > From: Gireesh Hiremath <Gireesh.Hiremath@xxxxxxxxxxxx> > > 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
