22 comments:
File src/mainboard/amd/padmelon/bootblock/OemCustomize.c:
Patch Set #7, Line 18: #define FILECODE PROC_GNB_PCIE_FAMILY_0X15_F15PCIECOMPLEXCONFIG_FILECODE
Can we get rid of this FILECODE stuff.
File src/mainboard/amd/padmelon/bootblock/bootblock.c:
Patch Set #7, Line 28: #define SERIAL_DEV PNP_DEV(0x4e, F81803A_SP1)
This and fintek includes seem unnecessary, since you are not doing any PNP config actions.
Patch Set #7, Line 31: static void full_serial(unsigned int base_port, unsigned int io_enable)
full ?
Patch Set #7, Line 36: temp |= DECODE_ENABLE_SERIAL_PORT0;
Probably wrong
Patch Set #7, Line 38: reg = inb(base_port + UART8250_MCR);
arch/io.h
Patch Set #7, Line 39: reg |= UART8250_MCR_DTR | UART8250_MCR_RTS;
Why? Hardware flow control is not desired for boot firmware.
File src/mainboard/amd/padmelon/devicetree.cb:
Patch Set #7, Line 34: device pci 2.0 on end # PCIe Host Bridge
Is this really a PCI bridge or a "placeholder" to configure 0:2.1 to 0:2.4 ?
Patch Set #7, Line 35: device pci 2.1 on end # No x4 PCIe slot
Slot or no slot?
Patch Set #7, Line 37: device pci 2.3 on end # NC
If not routed, why on?
Patch Set #7, Line 43: device pci 10.0 on end # USB
xHCI ?
Patch Set #7, Line 45: device pci 12.0 on end # USB
EHCI ?
Patch Set #7, Line 51: device i2c 52 on end
Contradicts SPD map above.
File src/mainboard/amd/padmelon/fan_init.c:
Patch Set #7, Line 24: #include <soc/pci_devs.h>
some spurious includes there?
Patch Set #7, Line 30: u8 cpu_boudaries[FINTEK_BOUNDARIES_SIZE] = {
static
boundaries ?
File src/mainboard/amd/padmelon/gpio.h:
Patch Set #7, Line 19: const struct soc_amd_gpio *early_gpio_table(size_t *size);
Missing include for the struct.
File src/mainboard/amd/padmelon/gpio.c:
Patch Set #7, Line 29: const struct soc_amd_gpio gpio_set_stage_reset[] = {
static
File src/mainboard/amd/padmelon/irq_tables.c:
Patch Set #7, Line 41: unsigned long write_pirq_routing_table(unsigned long addr)
In general, I'd just submit without MP and PIRQ tables since they are probably not much tested at all.
Patch Set #7, Line 65: pirq->rtr_devfn = PCI_DEVFN(0x14, 4);
I don't remember what this field in PIRQ structure is supposed to be. But 0:14.4 used to be the parallel PCI bus bridge.
Patch Set #7, Line 79: /* pci bridge */
Hmm.. bogus entries?
File src/mainboard/amd/padmelon/mainboard.c:
Patch Set #7, Line 35: * MP Tables. TODO: Make ACPI use these values too.
Well ACPI is the more important one to get correct. Perhaps just drop MP and PIRQ table support.
File src/mainboard/amd/padmelon/mptable.c:
Patch Set #7, Line 116: /* PCI slots */
Where are they?
Patch Set #7, Line 143: PCI_INT(0x0, 0x15, 0x0, 0x10);
Is pci 0:15.x stil there?
To view, visit change 33993. To unsubscribe, or for help writing mail filters, visit settings.