[coreboot-gerrit] Change in coreboot[master]: tpch: Introduce new thermal control mechanism for pch device

[Sumeet R Pawnikar (Code Review)]

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph, Karthik Ramasubramanian.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57096 )

Change subject: tpch: Introduce new thermal control mechanism for pch device
......................................................................

Patch Set 2:

(4 comments)

Patchset:

PS1:
> Why did you break this out into a separate chip driver? […]

ACK

File src/drivers/intel/tpch/tpch.h:

https://review.coreboot.org/c/coreboot/+/57096/comment/15059077_c2d2c1d5 
PS1, Line 15: /* IPC command to control FIVR Configuration */
            : #define	V_PMC_PWRM_IPC_CMD_COMMAND_FIVR			0xA3
            : /* IPC subcommand to write FIVR Register */
            : #define	V_PMC_PWRM_IPC_CMD_CMD_ID_FIVR_WRITE	0x01
            : /* IPC subcommand to control RFI Control 0 register logic write */
            : #define	V_PMC_PWRM_IPC_SUBCMD_RFI_CTRL0_LOGIC	0x00
            : /* IPC subcommand to control RFI Control 4 register logic write */
            : #define V_PMC_PWRM_IPC_SUBCMD_RFI_CTRL4_LOGIC	0x01
> These should probably go in  […]

Done

File src/drivers/intel/tpch/tpch.c:

https://review.coreboot.org/c/coreboot/+/57096/comment/cdd241e3_ffe03abb 
PS1, Line 26: 	acpigen_emit_namestring("IPCS");
            : 	acpigen_write_integer(V_PMC_PWRM_IPC_CMD_COMMAND_FIVR);
            : 	acpigen_write_integer(V_PMC_PWRM_IPC_CMD_CMD_ID_FIVR_WRITE);
            : 	acpigen_write_integer(0x8);
            : 	acpigen_write_integer(V_PMC_PWRM_IPC_SUBCMD_RFI_CTRL0_LOGIC);
            : 	acpigen_write_dword(3);
            : 	acpigen_emit_byte(RETURN_OP);
            : 	acpigen_write_package(0);
            : 	acpigen_write_method_end();
            : 	acpigen_pop_len();
            : 
            : 	acpigen_write_method_serialized("RFC1", 1);
            : 	acpigen_emit_namestring("IPCS");
            : 	acpigen_write_integer(V_PMC_PWRM_IPC_CMD_COMMAND_FIVR);
            : 	acpigen_write_integer(V_PMC_PWRM_IPC_CMD_CMD_ID_FIVR_WRITE);
            : 	acpigen_write_integer(0x8);
            : 	acpigen_write_integer(V_PMC_PWRM_IPC_SUBCMD_RFI_CTRL4_LOGIC);
> Got it, thanks! Should the GFC0 and GFC1 methods be implemented too?

Welcome. Yes, I'm working phase wise manner, will submit separate new CL for GFC0 and GFC1 methods on top of this.

https://review.coreboot.org/c/coreboot/+/57096/comment/57ce1790_64a8c763 
PS1, Line 51: devices
> nit: space after `devices`

Ack