It looks much better now, thanks!
Patch set 8:Code-Review +1
5 comments:
File src/soc/intel/common/block/acpi/acpi/northbridge.asl:
Both _BAS and _LEN get updated now, so this comment should be updated.
Also, I really like this approach. Even though I know very little ASL, it's very easy to understand :D
File src/soc/intel/common/block/systemagent/Kconfig:
Patch Set #8, Line 12: default 14
I understand that, as most platforms use the same value, one would want to make it the default setting. But what happens if, when adding a new platform, somebody forgets to specify the correct value? Everything builds, but the used value can be incorrect.
IMHO, it would be a good idea to put invalid defaults here, so that each SoC has to define the correct value. That way, the build process can be aborted if an invalid configuration is detected. For example, zero: it is not a valid value, and is a common "default" value.
To implement this check, I would add to `soc/intel/common/block/systemagent/Makefile.inc`:
ifeq ($(CONFIG_SA_MCHBAR_GRANULARITY),0)
$(error SA_MCHBAR_GRANULARITY value is invalid. SoC must provide this)
endif
(this also applies to the other fields below)
Patch Set #8, Line 14: reserve
reserve*d*
Patch Set #8, Line 32: reserve
reserve*d*
File src/soc/intel/common/block/systemagent/systemagent_early.c:
Patch Set #4, Line 42: switch (CONFIG_SA_PCIEX_LENGTH) {
Ack
Sorry if my previous message was confusing. I meant to say that, since all the values here are known at compile-time, the switch() statement could be avoided. In any case, this belongs to a separate change.
To view, visit change 38456. To unsubscribe, or for help writing mail filters, visit settings.