Hello Caleb and Michael,
On 2024-01-11 10:38, Michael Walle wrote:
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 apologize for my delayed response.
Using CONFIG_EXTRA_ENV_SETTINGS should be good enough to provide
the fallback defaults. However, the users can still mess the things up,
but again, they can do that already in many places.
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.
Having compile-time constants is also a viable option, IMHO. We should
just need some fallback mechanism, and the simpler the better.
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.
I also agree, but it should be added eventually.
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.
Yes, all this opens quite a few questions. IMHO, we should simply have
some fallback mechanism, for which using CONFIG_EXTRA_ENV_SETTINGS seems
fine to me, and leave everything else to as it already is. Sure, the
users could mess it up, but that can already happen in many places.
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.
To me, having an option to restore the default environment isn't a bad
idea, but not automatically and always. Perhaps some other mechanism
that
triggers the restoration should be devised.
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.
I agree.
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.
Sorry, this is a bit confusing to me. Didn't you write above that
the users should be able to change the associated commands through
the environment variables?
