Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41894 )
Change subject: vendorcode/google/chromeos: Add vendor_dptf_* functions ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41894/7/src/vendorcode/google/chrom... File src/vendorcode/google/chromeos/dptf.c:
https://review.coreboot.org/c/coreboot/+/41894/7/src/vendorcode/google/chrom... PS7, Line 178: \_SB.DPTF" note to self: this should probably be exported from acpigen_dptf.h instead and used here too.
https://review.coreboot.org/c/coreboot/+/41894/7/src/vendorcode/google/chrom... PS7, Line 194: acpigen_write_if(); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_byte(LOCAL0_OP); : acpigen_write_integer(i); /* TMPI */
is this acpigen_write_if_lequal_op_int()?
Done
https://review.coreboot.org/c/coreboot/+/41894/7/src/vendorcode/google/chrom... PS7, Line 198: acpigen_emit_byte(NOTIFY_OP); : acpigen_emit_namestring(name); : acpigen_write_integer(THERMAL_EVENT);
this seems like a good candidate for a new function like acpigen_notify(namestr, val) since it gets […]
Done
https://review.coreboot.org/c/coreboot/+/41894/7/src/vendorcode/google/chrom... PS7, Line 276: "TMPI"
could this just use tsr_index to build a name instead of relying on TMPI to be defined?
It sure could; I wasn't sure whether TMPI was defined for some other reason or not... I didn't see anything, and the DPTF spec doesn't mention anything. It also didn't "need" to exist in the original implementation either, since each TSR is written out by hand, so I was a little hesitant to just remove it. But I guess since it isn't mentioned in the spec I can remove it.