18 comments:
File Documentation/mainboard/asus/a88xm-e.md:
Patch Set #47, Line 64: the vendor EFI binary
Are you sure? the command looks wrong
Patch Set #47, Line 98: Kaveri/Godavary
Don't these use a different AGESA? (well, binaryPI)
File src/mainboard/asus/a88xm-e/Kconfig:
Patch Set #47, Line 55: config BOARD_ASUS_A88XM_E_DDR3_VOLT_VAL
I don't recall, was this ever tested? By tested, I mean all three voltages. 1.50V is the default for DDR3
File src/mainboard/asus/a88xm-e/OemCustomize.c:
/* TODO: is this OK for DDR3 socket FM2? */
/*
MEMCLK_DIS_MAP(ANY_SOCKET, ANY_CHANNEL, 0x01, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00),
CKE_TRI_MAP(ANY_SOCKET, ANY_CHANNEL, 0x05, 0x0A),
ODT_TRI_MAP(ANY_SOCKET, ANY_CHANNEL, 0x01, 0x02, 0x00, 0x00),
CS_TRI_MAP(ANY_SOCKET, ANY_CHANNEL, 0x01, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00),
*/
NEW IDEA: although coreboot's AGESA code seems to be not XMP-capable, it could be possible to use a […]
You can also rewrite AGESA and add support for XMP memory profiles. And when I say rewrite, I mean it. I am afraid to touch this code at all.
File src/mainboard/asus/a88xm-e/OemCustomize.c:
Patch Set #47, Line 147: * CUSTOMER OVERIDES MEMORY TABLE
'OVERIDES' may be misspelled - perhaps 'OVERRIDES'?
Please fix.
File src/mainboard/asus/a88xm-e/acpi/cpstate.asl:
Patch Set #47, Line 16: /* This
This comment should start like this:
/*
* This ...
Needs a space
Patch Set #47, Line 23: DefinitionBlock ("DSDT.AML", "DSDT", 0x01, OEM_ID, ACPI_TABLE_CREATOR, 0x00010001)
Why is this commented out?
File src/mainboard/asus/a88xm-e/acpi_tables.c:
/*
* This file is part of the coreboot project.
*
* Copyright (C) 2012 Advanced Micro Devices, Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; version 2 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
Please use SPDX for all license headers.
File src/mainboard/asus/a88xm-e/bootblock.c:
Patch Set #47, Line 34: pnp_devfn_t uart = PNP_DEV(0x2e, IT8728F_SP1);
Make these const?
File src/mainboard/asus/a88xm-e/buildOpts.c:
Patch Set #47, Line 63: //#define BLDOPT_REMOVE_UDIMMS_SUPPORT TRUE
Can this be aligned with tabs please?
Patch Set #47, Line 197: // #define BLDCFG_SMBUS0_BASE_ADDRESS 0xB00
Are these commented-out entries needed?
Patch Set #47, Line 238: { AMD_AP_MTRR_FIX16k_A0000, 0x0000000000000000 },
nit: Add a space before the hex numbers to align them
File src/mainboard/asus/a88xm-e/irq_tables.c:
Patch Set #47, Line 21: static void write_pirq_info(struct irq_info *pirq_info, u8 bus, u8 devfn,
This function makes things much less clear than directly writing the fields
File src/mainboard/asus/a88xm-e/mainboard.c:
Patch Set #47, Line 47: the dedicated function
Which dedicated function? Maybe look up the MSR meanings?
File src/mainboard/asus/a88xm-e/mptable.c:
Is this even correct?
LAN (and add a space after it)
Patch Set #47, Line 137: /*Local Ints: Type Polarity Trigger Bus ID IRQ APIC ID PIN# */
This comment does not correspond to the code below...
To view, visit change 30987. To unsubscribe, or for help writing mail filters, visit settings.