[M] Change in coreboot[main]: soc/mediatek/mt8196: Initialize MCUPM

[Yu-Ping Wu (Code Review)]

Attention is currently required from: Hung-Te Lin, Jarried Lin.

Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85888?usp=email )

Change subject: soc/mediatek/mt8196: Initialize MCUPM
......................................................................

Patch Set 2:

(13 comments)

File src/soc/mediatek/mt8196/include/soc/mcupm_plat.h:

https://review.coreboot.org/c/coreboot/+/85888/comment/ee1b8c04_ed36f9c9?usp... :
PS2, Line 42: 1b
What does this mean?

https://review.coreboot.org/c/coreboot/+/85888/comment/624d7733_5dc8e77b?usp... :
PS2, Line 70: REPLACE_15
Please define a meaningful name. What does bit 15 mean?

https://review.coreboot.org/c/coreboot/+/85888/comment/6a1ee285_35a743eb?usp... :
PS2, Line 84: SIZE
Is this supposed to be `NUM`? From the comment, this is the number of slots, not the size of a slot.

https://review.coreboot.org/c/coreboot/+/85888/comment/2f5d4429_9b5ed7c7?usp... :
PS2, Line 93: 0x1000BC7C
What does this stand for?

File src/soc/mediatek/mt8196/mcupm.c:

https://review.coreboot.org/c/coreboot/+/85888/comment/f237061e_7c804626?usp... :
PS1, Line 39: 	/* set CPUEB_STATE_VALID = 1 */
            : 	setbits32p(SPM_MCUPM_SPMC_CON, CPUEB_STATE_VALID);
            : 
            : 	/* polling CPUEB_STATE_FINISH_ACK = 1 */
            : 	if (!retry(POLLING_ACK_TIME,
            : 		   (read32p(SPM_MCUPM_SPMC_CON) & CPUEB_STATE_FINISH_ACK) ==
            : 		   CPUEB_STATE_FINISH_ACK, udelay(1))) {
            : 		printk(BIOS_WARNING,
            : 		       "[EB_SPMC] Polling ACK timeout, %#x\n",
            : 		       read32p(SPM_MCUPM_SPMC_CON));
            : 		return -1;
            : 	}
            : 
            : 	/* set CPUEB_STATE_VALID = 0 */
            : 	clrbits32p(SPM_MCUPM_SPMC_CON, CPUEB_STATE_VALID);
            : 
            : 	/* polling CPUEB_STATE_FINISH_ACK = 0 */
            : 	if (!retry(POLLING_ACK_TIME,
            : 		   (read32p(SPM_MCUPM_SPMC_CON) & CPUEB_STATE_FINISH_ACK) !=
            : 		   CPUEB_STATE_FINISH_ACK, udelay(1))) {
            : 		printk(BIOS_WARNING,
            : 		       "[EB_SPMC] Polling ACK timeout, %#x\n",
            : 		       read32p(SPM_MCUPM_SPMC_CON));
            : 		return -1;
            : 	}
> What I mean is this. Then we use the same logic to set/clear CPUEB_STATE_VALID value. […]

How about

```
static int set_cpueb_state(bool state_valid)
{
    if (state_valid)
        clrbits32p(SPM_MCUPM_SPMC_CON, CPUEB_STATE_VALID);
    else
        setbits32p(SPM_MCUPM_SPMC_CON, CPUEB_STATE_VALID);

...
}
```

File src/soc/mediatek/mt8196/mcupm.c:

https://review.coreboot.org/c/coreboot/+/85888/comment/267ec204_ac2d37f1?usp... :
PS2, Line 16: Polling
Poll

https://review.coreboot.org/c/coreboot/+/85888/comment/c756e130_51007c44?usp... :
PS2, Line 20: WARNING
Should this be `ERR`?

https://review.coreboot.org/c/coreboot/+/85888/comment/78695baf_eba11f64?usp... :
PS2, Line 46: WARNING
ERR? Same below.

https://review.coreboot.org/c/coreboot/+/85888/comment/b3de61eb_76bb465d?usp... :
PS2, Line 65: eb_sleep_protect
return eb_sleep_protect();

https://review.coreboot.org/c/coreboot/+/85888/comment/990693f3_1d2ad5e2?usp... :
PS2, Line 81: %s:
Let's use `[EB_SPMC]` for consistency.

https://review.coreboot.org/c/coreboot/+/85888/comment/68e83795_42b7532f?usp... :
PS2, Line 85: %s 
remove

https://review.coreboot.org/c/coreboot/+/85888/comment/7b8bed1b_7541444f?usp... :
PS2, Line 124: %s()
`%s:`

https://review.coreboot.org/c/coreboot/+/85888/comment/c44af5bb_683ee963?usp... :
PS2, Line 127: %s()
`%s:`