Attention is currently required from: Angel Pons, Evgeny Zinoviev.
Patch set 25:Code-Review +1
7 comments:
Commit Message:
Patch Set #25, Line 47: works with a delay of 500-1000 ms.
It's probably because we always do the "blunt" global reset via
ETR3 + full_reset(). This is only a last resort method and we
should try a global-reset MEI message first.
Probably worth to look into after this is is merged.
Patchset:
This looks very good and I hope you can spare just a little
more patience. The only thing that I see that should be
addressed now is the stale comments in the `cmos.layout`
files.
I had some trouble understand CMOS_ME_CHANGED. It's just
a bit to tell us that we wrote `me_state_prev`... No need
to change that now, but here's a half-baked idea from the
top of my head: Make a single, tri-state CMOS option:
(Keep, Normal, Disable). coreboot would only have to reset
it to `Keep` everytime it processed one of the other states.
This would also allow to keep the ME disabled if CMOS fails
later.
File src/mainboard/lenovo/l520/cmos.layout:
Patch Set #25, Line 37: # coreboot config options: cpu
Sorry, this looks just wrong. Maybe update the comments to say
`me` instead?
File src/southbridge/intel/bd82x6x/me.h:
Patch Set #25, Line 178: state
Nit, macro parameters should always be guarded with additional parentheses.
As I understand the manual, this could also be `u16`. Or more accurately 2x
`u8`.
File src/southbridge/intel/bd82x6x/me.c:
Nit, remove dangling asterisk to comply with coding style.
File src/southbridge/intel/bd82x6x/me_common.c:
Patch Set #25, Line 499: #ifdef __SIMPLE_DEVICE__
This would be much easier in a file that simply defines __SIMPLE_DEVICE__.
Mixing both in one compilation unit was never the idea.
To view, visit change 37115. To unsubscribe, or for help writing mail filters, visit settings.