Matthew Garrett has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32531 )
Change subject: Add support for the 51nb X210 ......................................................................
Patch Set 10:
(7 comments)
Reworked based on comments, EC is now in a subdirectory, io port addresses are provided in devicetree, using an fmap to reserve space for the EC firmware (and copying it in using cbfstool rather than using a separate utility), using generic HDA code. Tested as working.
https://review.coreboot.org/#/c/32531/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32531/8//COMMIT_MSG@10 PS8, Line 10: firmware protection
What does this refer to? Is the IFD fully unlocked? Are all protected ranges unset? Does it not use […]
Done
https://review.coreboot.org/#/c/32531/8//COMMIT_MSG@15 PS8, Line 15: EC firmware is contained within the system SPI flash, and so a blob of : EC firmware must be injected to a defined location during image build.
I would suggest using a fmap to define this location. […]
Done
https://review.coreboot.org/#/c/32531/8//COMMIT_MSG@27 PS8, Line 27:
I'd suggest listing what works/does not work/is untested.
Done
https://review.coreboot.org/#/c/32531/8/src/ec/51nb/Kconfig File src/ec/51nb/Kconfig:
https://review.coreboot.org/#/c/32531/8/src/ec/51nb/Kconfig@43 PS8, Line 43: 0x00200000
Is that the beginning of the BIOS region? Some other different Nuvoton EC I have has the firmware at […]
Done
https://review.coreboot.org/#/c/32531/8/src/ec/51nb/ec.h File src/ec/51nb/ec.h:
https://review.coreboot.org/#/c/32531/8/src/ec/51nb/ec.h@20 PS8, Line 20: #define SETUP_COMMAND 0x4e
I recall seeing 0x4e/0x4f somewhere else... […]
Done
https://review.coreboot.org/#/c/32531/8/src/mainboard/51nb/x210/acpi/ec.asl File src/mainboard/51nb/x210/acpi/ec.asl:
https://review.coreboot.org/#/c/32531/8/src/mainboard/51nb/x210/acpi/ec.asl@... PS8, Line 126: Device (BAT)
Maybe this could go in acpi/battery. […]
Done
https://review.coreboot.org/#/c/32531/8/src/mainboard/51nb/x210/mainboard.c File src/mainboard/51nb/x210/mainboard.c:
https://review.coreboot.org/#/c/32531/8/src/mainboard/51nb/x210/mainboard.c@... PS8, Line 57: /* Route 0x4e/4f to LPC */
Ah, this is what I recall 0x4e/0x4f from.
Done