Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54132 )
Change subject: drivers/acpi: Add device tree driver to generate thermal zone ......................................................................
Patch Set 2:
(2 comments)
File src/drivers/acpi/thermal_zone/thermal_zone.c:
https://review.coreboot.org/c/coreboot/+/54132/comment/313388bb_e4872bfc PS2, Line 98: !config->passive_config.time_constant_1 : || !config->passive_config.time_constant_2 : || !config->passive_config.time_sampling_period) { : acpigen_write_name_integer("_TC1", 2); : acpigen_write_name_integer("_TC2", 5); : acpigen_write_name_integer("_TSP", SECONDS_TO_DECI_SECONDS(10));
Should there be an error instead of emitting defaults?
The same values have been copied so many times, I wanted to centralize it:
git grep _TC1, src/ec/lenovo/h8/acpi/thermal.asl: Name (_TC1, 0x02) src/ec/lenovo/h8/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/getac/p470/acpi/thermal.asl: Method (_TC1, 0, Serialized) src/mainboard/gigabyte/ga-b75m-d3h/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/auron/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/beltino/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/butterfly/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/jecht/variants/guado/include/variant/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/jecht/variants/jecht/include/variant/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/jecht/variants/rikku/include/variant/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/jecht/variants/tidus/include/variant/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/kahlee/variants/baseboard/include/baseboard/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/link/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/parrot/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/slippy/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/stout/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/google/zork/variants/baseboard/include/baseboard/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/intel/baskingridge/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/intel/emeraldlake2/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/intel/wtm2/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/kontron/ktqm77/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/roda/rk886ex/acpi/thermal.asl: Method (_TC1, 0, Serialized) src/mainboard/roda/rk9/acpi/thermal.asl: Method (_TC1, 0, Serialized) src/mainboard/roda/rv11/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/samsung/lumpy/acpi/thermal.asl: Name (_TC1, 0x02) src/mainboard/samsung/stumpy/acpi/thermal.asl: Name (_TC1, 0x02)
Do you think I should still error out?
https://review.coreboot.org/c/coreboot/+/54132/comment/341b74ff_c9b4061e PS2, Line 114: : config->temperature_controller->ops->acpigen_write_TMP(dev);
Hmm... I'm not a big fan of adding a new callback to `struct device *`... […]
That would also work. The reason I went with the callback is that we can actually perform error checking to see if it's been implemented. We don't have a way to verify that the temperature controller has implemented _TMP.
You think I should still go with the pure ACPI version?