Attention is currently required from: Arthur Heymans, Kyösti Mälkki.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74441 )
Change subject: cpu/intel/speedstep: Use acpigen_write_processor_device() ......................................................................
Patch Set 4:
(3 comments)
File src/southbridge/intel/i82801gx/fadt.c:
https://review.coreboot.org/c/coreboot/+/74441/comment/617fb54c_662de538 PS4, Line 29: fadt->p_lvl3_lat = chip->c3_latency; This leaves the `c3_latency` field blind. Would it be much hassle to port all ports setting it to _CST and drop it completely?
File src/southbridge/intel/i82801jx/fadt.c:
https://review.coreboot.org/c/coreboot/+/74441/comment/856c237e_0082e37e PS4, Line 25: fadt->p_lvl3_lat = 0; /* FIXME: Is this correct? */ ACPI spec: "A value > 1000 indicates the system does not support a C3 state.". I think it's ok to change spurious values like this 0 to ffff or similar. But why should we drop valid values like the 1 above?
File src/southbridge/intel/i82801jx/lpc.c:
https://review.coreboot.org/c/coreboot/+/74441/comment/ab2eddd6_fbb8d8a0 PS4, Line 159: return 0; AIUI, this is to keep the current behavior and not to reflect the hardware support? Then a comment would be good.
(there's also a chance that the CPU emulates it. on newer CPUs it's part of the MSR_PMG_IO_CAPTURE_ADDR configuration (even though there's the +4 offset given). couldn't find any documentation for older ones, it seems coreboot code preceded public documentation.)