Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c... PS9, Line 853: if (cbi_get_uint32(&ssfc, CBI_TAG_SSFC)) I think the whole SSFC is still under review to evaluate whether it is really required. I am not sure where we will end up on it, but we might have to support a single FW_CONFIG field which is 64-bit wide instead of 32-bit FW_CONFIG and 32-bit SSFC.
I am thinking should we just assume there is no SSFC for now and fill 0s in the upper 32-bit? When we get to supporting SSFC, it should be easy to add this call to get CBI_TAG_SSFC.
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c@456 PS9, Line 456: uint64_t value The way SSFC support is being added in this CL expects that the devicetree representation takes care of transforming the SSFC fields into a 64-bit field when defining the fw_config field and options. We had some discussion (captured in b/169182605) and the conclusion was that we should allow mainboard devicetree to represent fw_config and fw_source separately and have sconfig handle the transformation from 2 32-bit fields into a single 64-bit field. This would allow the devicetree field/options to match what is really configured in CBI without having to handle that manually.
Given that the fate of SSFC is still undetermined, I think we can continue with what you have. But, if we ever decide to support SSFC as a separate field, we might have to support in sconfig to handle fw_source as well.