[coreboot-gerrit] [M] Change in coreboot[main]: sio/nuvoton: Implement a common ramstage ACPI LDN helper

[Nico Huber (Code Review)]

Attention is currently required from: Keith Hui, Paul Menzel.

Nico Huber has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/82632?usp=email )

Change subject: sio/nuvoton: Implement a common ramstage ACPI LDN helper
......................................................................

Patch Set 5: Code-Review+1

(4 comments)

File src/superio/nuvoton/common/common.c:

https://review.coreboot.org/c/coreboot/+/82632/comment/52b50741_cde55701?usp... :
PS5, Line 53: 		/* TODO: Show proper message for "keep". */
Let's do that right away? How about an array
```
const char *const power_status_names[] = { "off", "on, "keep" };
```

Also, please skip the setting `if (power_status > 2)`.

https://review.coreboot.org/c/coreboot/+/82632/comment/0d280473_a8bd077b?usp... :
PS5, Line 61: KB_WAKEUP_ANYKEY
I'm not convinced this is what everybody wants/expects. In particular the "any
button or movement"? How about putting the latter behind a Kconfig? (the "One
click of left or right button." option seems like a good default, just any move-
ment would be too much, IMHO).

https://review.coreboot.org/c/coreboot/+/82632/comment/4c5ca946_e03fc284?usp... :
PS5, Line 72: KB_WAKEUP_PSOUT
At least for the mouse option, I don't see how it could be disabled otherwise.

File src/superio/nuvoton/nct5572d/superio.c:

https://review.coreboot.org/c/coreboot/+/82632/comment/ec6f2b19_72765ec2?usp... :
PS5, Line 13: #define MAINBOARD_POWER_OFF	0
            : #define MAINBOARD_POWER_ON	1
            : #define MAINBOARD_POWER_KEEP	2
Not needed anymore.