Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Subrata Banik, Patrick Rudolph. EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59209 )
Change subject: soc/intel/../thermal: Add support for thermal config behind PMC device ......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/common/block/thermal/thermal.c:
https://review.coreboot.org/c/coreboot/+/59209/comment/e13fe30a_23e2a06f PS8, Line 112: struct pmc_thermal_config { : void (*func)(uintptr_t addr, uint32_t data); : uint16_t offset; : uint32_t value; : } config[] = { : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_CTEN, : .value = PMC_PWRM_THERMAL_CTEN_CPDEN | PMC_PWRM_THERMAL_CTEN_CTENLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_ECRPTEN, : .value = PMC_PWRM_THERMAL_ECRPTEN_EN_RPT : | PMC_PWRM_THERMAL_ECRPTEN_ECRPTENLOCK, : }, : { : .func = write32p, : .offset = PMC_PWRM_THERMAL_TL, : .value = pch_get_ltt_value(dev) | PMC_PWRM_THERMAL_TL_TTEN : | PMC_PWRM_THERMAL_TL_TLLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_PHLC, : .value = PMC_PWRM_THERMAL_PHLC_PHLCLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_TLEN, : .value = PMC_PWRM_THERMAL_TLEN_TLENLOCK, : }, : };
Is this really necessary for ~10 10 lines of code ? […]
I kind like this structure, like we do the thermal config. You can just maintain the array to add more setting if needed rather than add in the end of sector. Very clear to me (stupid guys). I really don't like to read long long parameters setbits32(pmc_bar + PMC_PWRM_THERMAL_ECRPTEN, PMC_PWRM_THERMAL_ECRPTEN_EN_RPT | PMC_PWRM_THERMAL_ECRPTEN_ECRPTENLOCK); balbal...