Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31461 )
Change subject: intel/apollolake: Add smbus defines ......................................................................
Patch Set 3:
gentle ping
I have no idea how the defines could be useful for mainboard code; and it seems there is no push of the latter to look at how they are used?
Generally I wonder, why the mainboard code would have to poke registers. We have APIs for device access (maybe something is missing for APL, though).
In my mainboard KConfig I have this now:
config BOARD_SPECIFIC_OPTIONS def_bool y select SOC_INTEL_APOLLOLAKE select SOC_INTEL_COMMON_BLOCK_SMBUS <--- ..
As it was not accepted to select SOC_INTEL_COMMON_BLOCK_SMBUS in generic APL code I need to do it in my mainboard code cause of GPIO settings. And this ends in this compile error.
This might be a misunderstanding. A `select SOC_INTEL_ COMMON_BLOCK_SMBUS` always belongs into the soc/ code. It reflects that this SoC supports the common SMBus implementation. What I asked to move into the mainboard code was the early call to initialize it in romstage.
Also, when we ask for the mainboard code: in the end it should be a reviewed patch on Gerrit, before we merge compilation fixes that wouldn't exist without it. Simply because there is no reason to fix something upstream that is not build tested upstream.