Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of in.bosch.com designates
 139.15.153.200 as permitted sender) receiver=protection.outlook.com;
 client-ip=139.15.153.200; helo=eop.bosch-org.com;
From:   <Gireesh.Hiremath@in.bosch.com>
To:     <krzysztof.kozlowski+dt@linaro.org>
CC:     <m.felsch@pengutronix.de>, <linux-omap@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-input@vger.kernel.org>, <bcousson@baylibre.com>,
        <tony@atomide.com>, <robh+dt@kernel.org>,
        <dmitry.torokhov@gmail.com>, <mkorpershoek@baylibre.com>,
        <davidgow@google.com>, <swboyd@chromium.org>,
        <fengping.yu@mediatek.com>, <y.oudjana@protonmail.com>,
        <rdunlap@infradead.org>, <colin.king@intel.com>,
        <Gireesh.Hiremath@in.bosch.com>, <sjoerd.simons@collabora.co.uk>,
        <VinayKumar.Shettar@in.bosch.com>,
        <Govindaraji.Sivanantham@in.bosch.com>,
        <anaclaudia.dias@de.bosch.com>
Subject: Re: [PATCH v2 2/4] Input: mt-matrix-keypad: Add Bosch mt matrix keypad driver
Date:   Tue, 10 May 2022 14:13:06 +0000
Message-ID: <20220510141306.2431-1-Gireesh.Hiremath@in.bosch.com>
In-Reply-To: <20220506072737.1590-2-Gireesh.Hiremath@in.bosch.com>
References: <20220506072737.1590-2-Gireesh.Hiremath@in.bosch.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2022 14:13:33.4835
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 9c010ec4-f57f-4252-abff-08da328f4467
X-MS-Exchange-CrossTenant-Id: 0ae51e19-07c8-4e4b-bb6d-648ee58410f4
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=0ae51e19-07c8-4e4b-bb6d-648ee58410f4;Ip=[139.15.153.200];Helo=[eop.bosch-org.com]
X-MS-Exchange-CrossTenant-AuthSource: AM5EUR03FT059.eop-EUR03.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR1001MB1076
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Gireesh Hiremath <Gireesh.Hiremath@in.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
		
>
>Best regards,
>Krzysztof

Best regards,
Gireesh Hiremath