Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33993 )
Change subject: mainboard/amd: Add padmelon board code ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/33993/3/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/Kconfig:
https://review.coreboot.org/c/coreboot/+/33993/3/src/mainboard/amd/padmelon/... PS3, Line 64: Oddly enough,
Sure, I just mean remove "Oddly enough" It's not needed.
Ok, will do.
https://review.coreboot.org/c/coreboot/+/33993/3/src/mainboard/amd/padmelon/... PS3, Line 68: # Don't use AMD's Secure OS : config USE_PSPSECUREOS : def_bool n
Sorry, I don't understand. […]
I'm not 100% sure now... It was one of the things Marshall told me to remove when we were trying to figure out why it was not coming out of reset, though not the one that solved it. OTOH, as I'm using pre-installed OS on my board, it might cause problems to complete the boot if I enable it. I'll leave to whoever clone this code for their own board to decide if they should use it.
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/Kconfig:
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 21: # select MERLINFALCON_BINARIES_PRESENT
Not needed. Just set it to Y when you run menuconfig.
This is temporary until the binaries become available... it makes it easy for me when building for debug. I could also simply remove it from here and leave it only in my debug branch.
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/fan_init.c:
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 29: celcius
sorry - my last comment was about the spelling of the word celsius.
Opss...sorry myself. Will fix. Though now that I separated part of the code back to SIO folder, maybe this message should be duplicated there too.