Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Software-based Debounced GPIO #52344

Closed
asemjonovs opened this issue Nov 17, 2022 · 3 comments
Closed

Software-based Debounced GPIO #52344

asemjonovs opened this issue Nov 17, 2022 · 3 comments
Assignees
Labels
area: GPIO Enhancement Changes/Updates/Additions to existing features RFC Request For Comments: want input from the community

Comments

@asemjonovs
Copy link
Collaborator

Introduction

Implement a generic, software-based GPIO debounce driver.

Problem description

Zephyr has GPIO flags for hardware-based debouncing filters. Zephyr currently doesn't support a software-based debounced GPIO driver.

Proposed change

Leverage Zephyr GPIO APIs to implement a software debounced GPIO driver. The algorithm for debouncing will follow closely with Linux gpio_keys implementation. Zephyr already has a gpio-keys device tree binding, but there's no backing driver for it.

Detailed RFC

Proposed change (Detailed)

Generic Debounce Driver API
The interface into the driver will follow Zephyr’s gpio_driver_api. The callback to handle the interrupt will specify the new pin state when it changes.

Device Tree
The Zephyr gpio-keys device tree definition will define the gpio_spec and debounce interval time, which is a subset of the linux gpio-keys schema. The default debounce_interval shall be 30 milliseconds. The chosen node will help identify what the debounced GPIO pin is used for, like in below example, a volume up button.

	chosen {
		cros-ec,vol-up = &gpio_debounce1;
        };
        :
	:
        gpio_debounce1: gpio_debounce1 {
		compatible = "zephyr,gpio-keys";
		gpios = <&gpio0 27 GPIO_ACTIVE_LOW>;
		debounce_interval_us = <30000>;
	};

SW Debounce Algorithm High Level Flow
image
When bounces occur, steps 10 thru 12 in figure above, will keep rescheduling the debounce_change_call_deferred until GPIO pin level settles. At that point we can read the pin for the new state. If the state has changed, the driver will invoke the client’s registered callback with the new pin state.

Dependencies

GPIO

Alternatives

Considered including a type property in device tree nodes to identify volume up/down or recovery buttons. This is not needed with using chosen node, leading to a reduced memory footprint.

The gpio-keys schema has a linux,code property. This is used for reporting input_events from the interrupt which are then passed to designated handlers with this code. This is similar to using a ‘type’ property above. Zephyr has an event implementation that can support this. This adds a thread and an additional layer between driver and client to notify when pin state changes.

@asemjonovs asemjonovs added the RFC Request For Comments: want input from the community label Nov 17, 2022
@carlescufi
Copy link
Member

Looks great! Will you send a Pull Request?

@asemjonovs
Copy link
Collaborator Author

Looks great! Will you send a Pull Request?

Yeah, I'm working on the PR

@cfriedt
Copy link
Member

cfriedt commented Dec 12, 2022

Just thought I would mention my concerns here as well, because I might not be able to make the arch meeting.

In Linux, multiple devices are capable of reporting the same input event. I think that's a good model and it's flexible enough with relatively little technical effort.

We also want to be compatible for the most part with Linux DT bindings.

Concern #1: while I think the idea of using a DT chosen node to perform a 1:1 mapping of device to input event is clever, we likely want an N:1 mapping (N devices, 1 event) in the long run. This could require a generic input layer, which AFAIK, we do not yet have.

Concern #2: as @fabiobaltieri pointed out, using a single device node per input event could make the number of devices balloon. They're cheap but they're not free. Having a single input device capable of reporting one or more input events would be better.

Concern #3: Currently, FWIU, Linux allows any input device to report any input event. That could pose some safety concerns, so the natural conclusion would be to add a DT option that limits which events a device is capable of reporting. One option would be a LUT (with a default base index of 0). Another option would be a runtime-modifiable array. To avoid forcing a potentially non-obvious ordering at declaration time (in DT), and to achieve worst-case O(logN) performance, the array can be sorted with qsort() on init and searched with bsearch().

Sorry to rain on the parade. I'm actually really happy to see this from the CrOS team though. Hopefully we can make it work for everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: GPIO Enhancement Changes/Updates/Additions to existing features RFC Request For Comments: want input from the community
Projects
None yet
Development

No branches or pull requests

4 participants