Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk.
1 comment:
Commit Message:
however config options can be disabled and in that case test should
not be run.
I'd even say it's better to return back to this later. Why switch
to another scheme before we have seen how this one turns out? I
grasp from the name `init_shutdown.c` that the idea was to have
individual files per tested path. Once we try to add another file,
we'll probably much easier see what the advantages and disadvan-
tages are.As stated earlier, missing dependencies might be a reason to favor
per programmer files. For instance if we need some library's
header file to compile the test code. But that's a problem that
I'd try to solve only once it's clear to exist :)
I fully agree. Predicting all potential advantages and disadvantages of some approach is hard (at least for me), and I find it easier to wait until the issues manifest themselves.
As for weak functions. I'd rather clutter the code with
if (CONFIG(MEC1308))than with weak declarations. In the only project I know that makes
heavily use of weak symbols (coreboot) they often turn bad.
I concur. Most uses of weak symbols in coreboot cause more trouble than benefits, and this is why I try to get rid of weak symbols where possible.
For instance, CB:55812 could have added a weak declaration for the `mb_get_lpddr3_dq_dqs_map()` function instead of a new Kconfig option. However, mainboards with LPDDR3 *must* provide the DQ/DQS map, or else raminit will always fail. If one forgets to define the `mb_get_lpddr3_dq_dqs_map()` function, the current Kconfig approach will result in a build error (linker complains about undefined reference). Using a weak symbol would result in a runtime failure, and finding out what went wrong wouldn't be trivial (raminit is done by the MRC.bin blob, which doesn't log nor return useful error information).
To view, visit change 55295. To unsubscribe, or for help writing mail filters, visit settings.