Re: [PATCH v2] boot: add support for button commands

[Michael Walle <mwalle@kernel.org>]

>> 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