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 13:
(21 comments)
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... File src/mainboard/amd/padmelon/BiosCallOuts.c:
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... PS10, Line 16: #include <amdblocks/agesawrapper.h> : #include <amdblocks/BiosCallOuts.h> : #include <soc/southbridge.h> : #include <stdlib.h> : #include <string.h>
Currently this is just a place holder with only an empty function. […]
Done
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... File src/mainboard/amd/padmelon/acpi/routing.asl:
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... PS10, Line 18: DefinitionBlock ("DSDT.AML","DSDT",0x01,"XXXXXX","XXXXXXXX",0x00010001
Not sure why it's not there, it's on my local main tree. Will fix. Here's my local branch: […]
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 34: device pci 2.0 on end # PCIe Host Bridge
It's a real bridge used to control the bridges. There's a second one at 3. […]
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 35: device pci 2.1 on end # No x4 PCIe slot
Actually mini PCIe half slot. And it's x1.
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 36: device pci 2.2 on end # Half mPCIe slo
Does not exists.
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 37: device pci 2.3 on end # NC
Actually LAN 1
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 38: device pci 2.4 on end # RTL8111F
LAN 2
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 39: device pci 3.0 on end # PCIe Host Bridge
GFX, x8, slotted.
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 43: device pci 10.0 on end # USB
yes, xhci
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 45: device pci 12.0 on end # USB
Yes, EHCI
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 51: device i2c 52 on end
Not sure I get it. Please explain.
Done
https://review.coreboot.org/c/coreboot/+/33993/9/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/33993/9/src/mainboard/amd/padmelon/... PS9, Line 19: #if CONFIG(HAVE_MERLINFALCON_BINARIES) : # { {0xA0, 0x00}, {0xA2, 0x00} }, // socket 0 - Channel 0 & 1, slot 0 : #else : { {0xA0, 0x00} }, // socket 0 - Channel 0, slot 0 : #endif :
This is a bad solution, it shows what needs to happen, but the "#" has to be moved manually. […]
Done
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... File src/mainboard/amd/padmelon/fan_init.c:
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... PS10, Line 17: include <console/console.h>
Will remove. Was being used for debug.
Done
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... File src/mainboard/amd/padmelon/gpio.h:
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... PS10, Line 21: size_t
gpio_banks does include stddef. […]
Done
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... File src/mainboard/amd/padmelon/gpio.c:
https://review.coreboot.org/c/coreboot/+/33993/10/src/mainboard/amd/padmelon... PS10, Line 20: #include <soc/gpio.h>
They are 2 different files with same name. […]
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/irq_tables.c:
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 41: unsigned long write_pirq_routing_table(unsigned long addr)
Ok... […]
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 65: pirq->rtr_devfn = PCI_DEVFN(0x14, 4);
Will check.
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 79: /* pci bridge */
Actually code started even earlier... all family 15h use the same code.
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/mainboard.c:
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 35: * MP Tables. TODO: Make ACPI use these values too.
Seems to be correct, only extensive testing will confirm though.
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... File src/mainboard/amd/padmelon/mptable.c:
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 116: /* PCI slots */
All family 15h have it, not my decision. […]
Done
https://review.coreboot.org/c/coreboot/+/33993/7/src/mainboard/amd/padmelon/... PS7, Line 143: PCI_INT(0x0, 0x15, 0x0, 0x10);
No.
Done