add gpio-key overlay

[shawaj]

as discussed in #2312 this is my first attempt at a generic gpio-key overlay with overrides for all of the necessary changes.

The one thing I am unsure of (other than generally whether it will work or not!!) is the key@0 part - have I done that right?

Also - perhaps it is better to not have the active-low on by default and to configure it to something less unexpected than a shutdown? Since gpio-shutdown overlay already does that. Not sure it matters hugely, but could just map it to an up arrow or something and have active low off. Let me know what you think?

P.S. this is untested so far with any buttons. I will test tomorrow when I am close to some hardware.

P.P.S i just noticed that I have accidentally deleted some stuff from the last commit (the mcp3202 overlay stuff) - off to fix that now!

[shawaj]

ok fixed now. let me know if I have made any obvious errors!

[pelwell]

I've been playing with this for an hour or so, and apart from a minor syntax error (a missing semicolon in the parameter declarations) there are a some more serious problems:

1. The overlay creates the buttons node with the same name every time, which is OK if the firmware is applying the overlay but an error if applying the overlay at runtime. I can live with this restriction for now, but would like to overcome it eventually. One way, which would work today, is to have a separate overlay to create the buttons node, then another to populate it with a key, but see point 2...

2. The pinctrl pin declarations are there to prevent the pins being exported as general purpose GPIOs. The problem here is that there is only one pins declaration that is overwritten each time, and because there is only one buttons node which is only referencing one pinctrl node there is no way of recording the fact that we have now claimed multiple pins. We could do without the pin declarations altogether, but then you would lose the ability to set a pull (it's such a shame this wasn't considered part of GPIO functionality...). A more drastic approach would be to have multiple buttons nodes, each of which defines one key with one pinctrl node, but our use of reg to create unique names causes some warnings.

3. The gpio-keys driver doesn't talk directly to any hardware, so it doesn't belong in the /soc node - / is a better home. If we use the reg property to manage multiple buttons nodes then the #address-cells needs to go up a level to prevent warnings, at which point the #size-cells ends up overwriting the top-level settings in the base DTB.

A trivial change that would allow us to remove the need for #address-cells would be to make the property name magic - it would work like reg, but set the whole name of the node as a string. In this overlay we could then use the label property value as a node name. Unfortunately this needs a small change in the firmware so can't be done immediately, but I'll start the process.

Another simple change would be to keep the use of reg as a way of modifying an address, but to only modify the reg property if it already exists. Actually, removing the default reg property is enough to remove the #address-cells warnings.

I've added another commit that brings the PR up to date with as far as I've got, not relying on new firmware features. Give it a try - although the overlay looks like it applies OK I haven't tested that it works. For example, it is conceivable that gpio-keys doesn't like multiple instantiations.

This has turned into a bit of a brain dump - sorry - but this needs more time and I didn't want to leave you hanging.

[shawaj]

i have just tested this with the buttons on our media center HAT and PaPiRus HAT and it works perfectly with both distinct buttons and a joystick.

FYI - i tested PaPiRus with no pull ups set (as it has on board ones) and used media center hat with raspi pull ups set (it doesnt have on board ones)

[pelwell]

That's great. Does it work if you load the overlays at runtime instead? Comment out the config.txt entries and run something like:

sudo dtoverlay gpio-key gpio=13 keycode=59 label="KEY_F1"
sudo dtoverlay gpio-key gpio=17 keycode=60 label="KEY_F2"
...

It's not a train smash if they don't, but if we have a mechanism that works in both cases then I'm more likely to merge it as is.

[shawaj]

i tried those at runtime on the command line. this is exactly how i did it:

sudo dtoverlay gpio-key gpio=21 keycode=4 level="KEY_4" gpio_pull=0

it didn't seem to respond at all to that. it didn't give an error message but the buttons did not respond when pressed.

is there a way I can check if it is loaded correctly?

[shawaj]

@pelwell actually, i just tested it again and it works perfectly at runtime. don't know what i was doing wrong before. think possibly the HAT PCB wasn't properly connected or something.

[pelwell]

level is not a valid parameter, but assuming that is a transcription error then it should work - I managed to test it and it worked for me. Grounding GPIO 21 displays a 3 (scancode 4 is key 3 - obviously...). The default pull=2 is safer, of course, otherwise the line will float which may cause spurious readings. If the "key" appears to be pressed from the start then it seems you won't see any output until it is first released.

It's quite cool being able to add and change keys like this on the fly. I'm happy to merge if you are.

[shawaj]

sorry, "level" should have been "label"!

and yes - i had gpio_pull=0 because my board has hardware pullups on it so no need to use the RPi ones :-)

yeah - very nice indeed and super useful! will be making use of this a lot. happy to merge also.

couple further questions i have for you though...for our media-center-overlay - is it preferred to add keymappings directly to that overlay for the joystick/buttons? or to use a separate overlay to set them up? if the latter, is there a way we can add multiple device tree overlays to a device tree blob for the eeprom on the product? just trying to work out the most user-friendly way to have the buttons set up.

[pelwell]

HATs are expected to have a single overlay associated with them. This can either be built into or, recently, just named. In order to make use of the generic gpio-key overlay directly in your application you would need to be able to pass parameters to the multiple invocations - you would really need to embed a config.txt fragment, and this is even less likely to be supported in the future.

What is needed is a tool to combine a few overlays, with optional parameters, and generate a standalone overlay. We would then check the output (in source format) into the kernel tree - I wouldn't want require the use of this tool to build the kernel and its DTBs - but build into it enough information to be able to rerun the merge in the event that something changes. A checker script could then verify whether or not an update is required.

The dtmerge utility applies overlays, with parameters, to a base DTB, but then one would need to create an overlay from the diff of the merged DTB and the base. This is tricky - it would need to identify suitable labels (or paths) for use as fragment targets - and is a lossy operation because the labels get turned into phandles. Alternatively, one could try to stay within the source domain, effectively concatenating the overlays with some fragment renumbering, but this would require code to apply overlays to the source, and care would have to be taken with labels that are used by multiple overlays. Either way, it sounds like a fun project.

[shawaj]

@pelwell so if the overlays are in RasPi kernel, in the device EEPROM you can just name the overlay? Do you have an example of how that works? Is there a benefit to using the full overlay in the EEPROM instead of this way?

Thanks for the info on the rest of the stuff too :-)

[pelwell]

To embed the name, create a file containing the name of the overlay and pass that to eepmake, just as you would pass the .dtbo. Alternatively, include the blob in the settings file:

...
#setgpio  26    INPUT     DEFAULT
#setgpio  27    INPUT     DEFAULT

# DT blob contains the name of the overlay "hello"
dt_blob
68 65 6c 6c 6f 00

I don't see any real advantage to embedding the overlay, provided the overlay is part of the standard firmware.

[shawaj]

Guess the only benefit would be for an out-of-date kernel from a downstream application - OSMC or other distro - or perhaps a distro that is not derived from the official RPi one?

[pelwell]

Since most overlay PRs usually include config changes they tend to be tied to new kernels.

[pelwell]

@shawaj That fun project for combining overlays I mentioned earlier is now entering the beta phase. It can be used to:

• flatten a .dts file and its includes into a new .dts
• apply parameters to an overlay to create a new overlay (with the parameters removed)
• combine multiple overlays, optionally applying parameters, to create a new one, renaming fragments and labels to avoid a clash
• include a comment in the output to allow the same command to be rerun, and
• list the dts[i] files included by another dts[i] file.

There is minimal usage information in the file. It is written in Perl (sorry), but I've tried to keep it readable (just don't look at the regexp in the tokeniser...)

You can find the current version here.

Feedback is welcome. My plan is to make this tool generally availabe once it's ready.

[shawaj]

looks great! had a quick play and seems to work really nicely but haven't had time to give it more than a "play"

definitely will be useful for future though - near tool :-)

by generally available - does that mean listed on the RasPi site / github as a tool? Just wondering whether to bookmark this page :-)

[pelwell]

The ovmerge script is now available as (the only) part of a new "utils" repo.

[kog8790]

Ok, so I don’t want to sound like a complete noob which I clearly am, but when I was playing around with the

sudo dtoverlay gpio-key gpio=21 keycode=4 label="KEY_4" gpio_pull=0

Command from terminal it got stuck. I tried to kill the command, change the command, etc... nothing works.

when I kill the command it appears to work but after about ten seconds it starts printing 3333333333333 again.

When I try to change the command I get this error:

Failed to apply overlay ‘1_gpio-key’ (kernel)

any idea what I might be missing or messing up?

[pelwell]

Having no pull (gpio_pull=0) is a bad idea unless there's an external pull resistor - with neither the pin can float and generate spurious keystrokes.

Where did you get your keycode from? They are defined in https://github.com/raspberrypi/linux/blob/rpi-5.10.y/include/uapi/linux/input-event-codes.h, which shows that the keycode for KEY_4 is actually 5.