Attention is currently required from: Tim Wawrzynczak, Sugnan Prabhu S, Ravindra, Sridhar Siricilla, Patrick Rudolph, Ravindra N. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57783 )
Change subject: cpu/intel/common: Update CPPCv3 Nominal Frequency entry ......................................................................
Patch Set 7:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57783/comment/f0f30491_73174166 PS7, Line 9: DWORD buffer there is no such thing, it can either be the one or the other
File src/cpu/intel/common/common_init.c:
https://review.coreboot.org/c/coreboot/+/57783/comment/f4f129de_d694ead5 PS7, Line 136: CPPC_DWORD(0);
Isn't this functionally the same thing? the field reads back as zero, and the kernel interprets this […]
The difference is: CPPC_UNSUPPORTED resulting in `{Register {(SystemMemory, 0, 0, 0, 0)}` which the OSPM interprets as `unsupported`, while `DWORD(0)` is interpreted as actual integer value `0`. However, depending on the implementation `0` could be interpreted as `unsupported`, too, for this specific field.
Ravindra, elaborate, please
From the ACPI spec:
``` Nominal Frequency Type: Integer (DWORD) or Buffer Optional. [...] If this element is an Integer, OSPM reads the integer value directly. If this element is a Buffer (and supported), it must contain a Resource Descriptor with a single Register() to read the value from. ```
``` Optional _CPC package fields that are not supported by the platform should be encoded as follows: - Integer fields: Integer 0 - Register fields: the following NULL register descriptor should be used: ResourceTemplate() {Register {(SystemMemory, 0, 0, 0, 0)}} ```
-
Michael Niewöhner (Code Review)