This is simply awesome, but I see one possible issue -- the need to have proper environment variables defined for a particular board or device, to make the buttons work as expected. Obviously, those environment variables can be absent or can become missing for numerous reasons.
Is CFG_EXTRA_ENV_SETTINGS not persistent enough?
IMHO no. Because a user might accidentially mess up the environment variables.
I think that we should have an additional mechanism in place that defines the buttons and the associated commands even if no environment variables are found. Like a set of fallback defaults for a particular board or device, built into the U-Boot image. For example, Rockchip boards have those defaults pretty well defined.
A programmatic API for register button/cmd mapping from board_late_init() (for example) sounds sensible to me.
I don't know if it has to be that complex, or if it will be enough to just have some compile-time constants like CONFIG_BUTTON_CMD_N.
Is this really an issue that invalidates the implementation proposed here though? It feels much more like a nice-to-have addition that maybe we could leave out for now?
Agreed. Looks like one can add it to get_button_cmd() later.
It also has a MUCH wider scope imo - should board override env or vice versa? What about triggering default AND custom actions for one button press? What if a board wants to register a callback function instead of running a command?) - these are questions I don't want to answer with this patch.
One use case I have is restoring the default environment *in any case*; regardless what the state of the board is. In this case, the environment must not override the button settings. This can be a compiled-in command, which always takes precedence.
If you want to be able to overwrite a button command, then I guess you can already do that with the environment setting. Provide sane defaults via CFG_EXTRA_ENV_SETTINGS and a user can then overwrite it.
In summary, the registered (compiled-in) command should always take precedence. If one wants to supply a default command which can be changed later, that can go via the (compiled-in) default environment.
-michael