zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.21k stars 6.26k forks source link

Only the first five gpio-keys of wio-terminal seem to work #67049

Closed maxhbr closed 6 months ago

maxhbr commented 7 months ago

Describe the bug Using two different wio_terminal I could reproduce the weird behaviour, that only the first 5 entries in gpio-keys or buttons seem to work.

They are ordererd as:

    /* Buttons */
    gpio_keys {
        compatible = "gpio-keys";
        user_button_0: button_0 {...};
        user_button_1: button_1 {...};
        user_button_2: button_2 {...};
        joy_sel: joystick_selection {...};
        joy_down: joystick_down {...};
        joy_up: joystick_up {...};
        joy_left: joystick_left {...};
        joy_right: joystick_right {...};
    };

and just the buttons _0, _1, _2 and _selection and _down are working. This can be observed in the lvgl sample or with the following code:

static void input_cb(struct input_event *evt)
{

  char buf[1000];
  snprintf(buf, sizeof(buf), "type: %d, code: %d, value: %d", evt->type, evt->code, evt->value);
  ... // print buf to display
}
INPUT_CALLBACK_DEFINE(NULL, input_cb);

Using this code only the first five entries show any sign of reaction.

The weird behaviour is, that reordering the entries to

    /* Buttons */
    gpio_keys {
        compatible = "gpio-keys";
        user_button_0: button_0 {...};
        joy_sel: joystick_selection {...};
        joy_left: joystick_left {...};
        joy_right: joystick_right {...};
        user_button_1: button_1 {...};
        user_button_2: button_2 {...};
        joy_down: joystick_down {...};
        joy_up: joystick_up {...};
    };

makes _0, _selection, _left, _right and _1 available and allows to actually use the button matrix of the lvgl example.

To Reproduce

$ west build -b wio_terminal samples/subsys/display/lvgl
$ west flash

observe that button grid is displayed but only the select (press on joystick) has a visual reaction.

Reorder the entries as above, rebuild and flash and observe that right and left joystick movement is now working as expected.

Expected behavior I would expect that all gpio_keys are working

Impact Some buttons of the wio_terminal are not usable for no clear reason.

Environment (please complete the following information):

maxhbr commented 7 months ago

Additionally, for testing with the lvgl sample one needs to add the following files:

$ cat samples/subsys/display/lvgl/boards/wio_terminal.conf
CONFIG_GPIO=y
CONFIG_INPUT=y

and

$ cat samples/subsys/display/lvgl/boards/wio_terminal.overlay
/*
 * SPDX-License-Identifier: Apache-2.0
 */
#include <zephyr/dt-bindings/input/input-event-codes.h>
#include <zephyr/dt-bindings/lvgl/lvgl.h>
 / {

        lvgl_button_input {
                compatible = "zephyr,lvgl-button-input";
                //input = <&gpio_keys>;
                input-codes = <INPUT_KEY_B>;
                coordinates = <160 120>;
        };

        lvgl_keypad_input {
                compatible = "zephyr,lvgl-keypad-input";
                //input = <&gpio_keys>;
                input-codes = <INPUT_KEY_LEFT INPUT_KEY_RIGHT INPUT_KEY_ENTER>;
                lvgl-codes = <LV_KEY_LEFT LV_KEY_RIGHT LV_KEY_ENTER>;
        };
};
henrikbrixandersen commented 7 months ago

Ah, to get the lvgl sample working one needs

So this issue is resolved?

maxhbr commented 7 months ago

Oh, sorry for the unclear communication. No, this does not solve the issue but just adds the necessary overlay to map the keypad.

The issue is still existent and depending on order just the first 5 entries of gpio_keys seem to have any effect.

Edit: I reformulated my above comment to make that clear.

kartben commented 7 months ago

@joelguittet

joelguittet commented 7 months ago

Hello, Not seen this during the initial integration of the wio terminal, but it was tested only with one gpio one by one to ensure the assignment of the pins is correct. Here the issue you describe is different, not sure how it is related to the wio terminal itself, or the gpio keys? I will have a look when I will be available after the new year celebration, if not solved in the meantime :-) Joel

maxhbr commented 7 months ago

I also checked that all pin assignments are correct.

faxe1008 commented 7 months ago

@maxhbr can you maybe check if the input events are raised in the first place? You can enable some logging for them with the CONFIG_INPUT_EVENT_DUMP Kconfig option.

joelguittet commented 7 months ago

@maxhbr

I'm trying to reproduce your setup, I have added the two files mentioned above samples/subsys/display/lvgl/boards/wio_terminal.conf and samples/subsys/display/lvgl/boards/wio_terminal.overlay. Note CONFIG_GPIO is already defined in the defconfig of the Wio terminal board so it's not required.

Now I get the following error while building samples/subsys/display/lvgl :

/home/ubuntu/zephyrproject/zephyr/samples/subsys/display/lvgl/boards/wio_terminal.overlay:5:10: fatal error: zephyr/dt-bindings/lvgl/lvgl.h: No such file or directory
    5 | #include <zephyr/dt-bindings/lvgl/lvgl.h>

Seems really strange to me and I didn't found any samples where you may have copy/paste this include. Which Zephyr version are you using ? I'm on 3.5.0.

maxhbr commented 7 months ago

I am on https://github.com/zephyrproject-rtos/zephyr/commit/bf2bdcf6f79cf9711b1fdee3f2d91a5e61603f29. This should be a recent master commit.

joelguittet commented 7 months ago

Ok, seems more recent than release 3.5.0, I need to update on my side. I will let you known. No other addition/modification you have made except the two files above ?

maxhbr commented 7 months ago

let me recreate this example as a branch based on 3.5.0

maxhbr commented 7 months ago

Hey, I created https://github.com/maxhbr/zephyr/tree/maxhbr/exampleFor%2367049 and it

faxe1008 commented 7 months ago

@maxhbr can you give us also the full part of the dts with gpio_keys just to make sure that they all have the correct zephyr,code property?

@joelguittet the keypad input device as well as the dts-bindings for the special lvgl keys have been introduced after 3.5.0.

kartben commented 7 months ago

FWIW the issue is not with LVGL IMO, and can easily be reproduced with the input_dump sample:

west build -p auto -b wio_terminal samples/subsys/input/input_dump
maxhbr commented 7 months ago

@maxhbr can you give us also the full part of the dts with gpio_keys just to make sure that they all have the correct zephyr,code property?

after a west build -b wio_terminal samples/subsys/display/lvgl I see the following in build/zephyr/zephyr.dts:

 557   │     gpio_keys {
 558   │         compatible = "gpio-keys";
 559   │         user_button_0: button_0 {
 560   │             label = "User Button 0";
 561   │             gpios = < &portc 0x1a 0x1 >;
 562   │             zephyr,code = < 0xb >;
 563   │         };
 564   │         user_button_1: button_1 {
 565   │             label = "User Button 1";
 566   │             gpios = < &portc 0x1b 0x1 >;
 567   │             zephyr,code = < 0x2 >;
 568   │         };
 569   │         user_button_2: button_2 {
 570   │             label = "User Button 2";
 571   │             gpios = < &portc 0x1c 0x1 >;
 572   │             zephyr,code = < 0x3 >;
 573   │         };
 574   │         joy_sel: joystick_selection {
 575   │             label = "joystick selection";
 576   │             gpios = < &portd 0xa 0x1 >;
 577   │             zephyr,code = < 0x1c >;
 578   │         };
 579   │         joy_down: joystick_down {
 580   │             label = "joystick down";
 581   │             gpios = < &portd 0x8 0x1 >;
 582   │             zephyr,code = < 0x6c >;
 583   │         };
 584   │         joy_up: joystick_up {
 585   │             label = "joystick up";
 586   │             gpios = < &portd 0x14 0x1 >;
 587   │             zephyr,code = < 0x67 >;
 588   │         };
 589   │         joy_left: joystick_left {
 590   │             label = "joystick left";
 591   │             gpios = < &portd 0xc 0x1 >;
 592   │             zephyr,code = < 0x69 >;
 593   │         };
 594   │         joy_right: joystick_right {
 595   │             label = "joystick right";
 596   │             gpios = < &portd 0x9 0x1 >;
 597   │             zephyr,code = < 0x6a >;
 598   │         };
 599   │     };
maxhbr commented 7 months ago

I can not reproduce the same behaviour on other boards. I tested it on a stm32h747i_disco which also has a similar joystick and with the input_dump example I saw all 6 gpio_keys working.

I needed to apply the following patch: ```diff diff --git a/boards/arm/stm32h747i_disco/stm32h747i_disco_m7.dts b/boards/arm/stm32h747i_disco/stm32h747i_ disco_m7.dts index 514047c8a5..94de4db849 100644 --- a/boards/arm/stm32h747i_disco/stm32h747i_disco_m7.dts +++ b/boards/arm/stm32h747i_disco/stm32h747i_disco_m7.dts @@ -44,6 +44,21 @@ wake_up: button { status = "okay"; }; + joy_center: joystick_center { + status = "okay"; + }; + joy_down: joystick_down { + status = "okay"; + }; + joy_up: joystick_up { + status = "okay"; + }; + joy_left: joystick_left { + status = "okay"; + }; + joy_right: joystick_right { + status = "okay"; + }; }; ```
kartben commented 7 months ago

looks like gpio-keys driver is throwing an error:

D: gpio_keys_interrupt_configure [0x0xb7c4, 26]
D: gpio_keys_interrupt_configure [0x0xb7c4, 27]
D: gpio_keys_interrupt_configure [0x0xb7c4, 28]
D: gpio_keys_interrupt_configure [0x0xb7b0, 10]
D: gpio_keys_interrupt_configure [0x0xb7b0, 8]
D: gpio_keys_interrupt_configure [0x0xb7b0, 20]
E: interrupt configuration failed: -16
E: Pin 5 interrupt configuration failed: -16
joelguittet commented 6 months ago

Agree with @kartben I see the same issue. The SAMD51P19A has only 16 external interrupt. Maybe they are all used ? Trying to verify....

joelguittet commented 6 months ago

Ok issue found @maxhbr @kartben @faxe1008

The EIC has 16 external interrupts available, and they are affected to GPIOs. However, there are much more GPIO of course, so a single EIC input is on multiple GPIOs at the same time. Of course to get it working, it's not possible to activate multiple source to the same EIC input.

The pins used for buttons on the Wio Terminal have the following configuration (from the datasheet https://ww1.microchip.com/downloads/aemDocuments/documents/MCU32/ProductDocuments/DataSheets/SAM-D5x-E5x-Family-Data-Sheet-DS60001507.pdf, chapter 6):

Disable either Button 0 (button 1 on Seeed schematic) or Joystick UP and this is now working as expected.

This is an hardware limitation of the MCU.

joelguittet commented 6 months ago

PS: you will likely disable Button 0, depending of your application maybe it's possible to use polling on this GPIO instead of interrupt, if you really need this.

kartben commented 6 months ago

Given the current gpio-key configuration can't work as-is, I wonder if it should be reworked in two different nodes (one for button 0, 1, 2, "okay"'d by default, and another disabled one for the 5-way switch buttons?).

[...] depending of your application maybe it's possible to use polling on this GPIO instead of interrupt, if you really need this

@fabiobaltieri would it make sense for gpio-keys driver to have a polling mode similar to (shared with?) the one available for kbd-matrix?

fabiobaltieri commented 6 months ago

@fabiobaltieri would it make sense for gpio-keys driver to have a polling mode similar to (shared with?) the one available for kbd-matrix?

Yeah it does, stm32 have a similar limitations, plus some mcu don't support triggering on both edges, so it's useful to have a fallback mode. Linux has one as well (https://elixir.bootlin.com/linux/latest/source/drivers/input/keyboard/gpio_keys_polled.c) we could either have a separate driver or a "polling" mode.

fabiobaltieri commented 6 months ago

Had a look at the code, should be fairly easy to add polling mode, same schema as with the keyboard matrix one. I'll give it a shot, stay tuned.

kartben commented 6 months ago

reopening as the linked PR makes a fix for this issue possible, but does not fix it per se. @joelguittet over to you for an update of the devicetree? :)

joelguittet commented 6 months ago

Yes will do !