59 comments:
File src/mainboard/asus/a88xm-e/Kconfig:
Patch Set #62, Line 1: #*****************************************************************************
What's with these line comments?
Patch Set #62, Line 51: config ONBOARD_VGA_IS_PRIMARY
Why?
File src/mainboard/asus/a88xm-e/Makefile.inc:
Patch Set #62, Line 1: #*****************************************************************************
What's with these line comments?
File src/mainboard/asus/a88xm-e/OemCustomize.c:
Patch Set #62, Line 133: /*----------------------------------------------------------------------------------------
What's with these line comments?
Would be good to either enable or remove the code
File src/mainboard/asus/a88xm-e/OptionsIds.h:
empty line in comment
empty line in comment
empty line in comment
Patch Set #62, Line 32: //#define IDSOPT_IDS_ENABLED TRUE
Do we really need to have this many commented-out things?
File src/mainboard/asus/a88xm-e/acpi/cpstate.asl:
/*
#include <acpi/acpi.h>
DefinitionBlock ("DSDT.AML", "DSDT", 0x01, OEM_ID, ACPI_TABLE_CREATOR, 0x00010001)
{
Scope (\_PR) {
Device (CPU0) {
Name (_HID, "ACPI0007")
Name (_UID, 0)
#include "cpstate.asl"
}
Device (CPU1) {
Name (_HID, "ACPI0007")
Name (_UID, 1)
#include "cpstate.asl"
}
Device (CPU2) {
Name (_HID, "ACPI0007")
Name (_UID, 2)
#include "cpstate.asl"
}
Device (CPU3) {
Name (_HID, "ACPI0007")
Name (_UID, 3)
#include "cpstate.asl"
}
}
*/
This is commented-out
Patch Set #62, Line 37: /* P-state support: The maximum number of P-states supported by the */
Why not use a multi-line comment?
Taken
Patch Set #62, Line 63: 0x000009C4,
Why switch to four spaces here?
File src/mainboard/asus/a88xm-e/acpi/gpe.asl:
There's two spaces around the comment delimiters
File src/mainboard/asus/a88xm-e/acpi/mainboard.asl:
Patch Set #62, Line 11: Name(PCLN, Multiply(0x100000, CONFIG_MMCONF_BUS_NUMBER)) /* Length of PCIe config space, 1MB each bus */
Line over 96 characters
File src/mainboard/asus/a88xm-e/acpi/routing.asl:
Patch Set #62, Line 35: F0:SMBus/ACPI,F1:IDE;F2:HDAudio;F3:LPC;F4:PCIBridge;F5:USB
Why is everything squished together?
Patch Set #62, Line 42: * EHCI @ func 2 */
This fits on the previous line just fine
Patch Set #62, Line 98: /* Bus 0, Dev 20 - F0:SMBus/ACPI, F1:IDE; F2:HDAudio; F3:LPC; F4:PCIBridge; F5:USB */
Line over 96 characters
Patch Set #62, Line 106: * EHCI @ func 2 */
Same here
File src/mainboard/asus/a88xm-e/acpi/sata.asl:
Is this needed?
File src/mainboard/asus/a88xm-e/acpi/si.asl:
So, what does this do?
File src/mainboard/asus/a88xm-e/acpi/sleep.asl:
Patch Set #62, Line 7: * \_PTS - Prepare to Sleep method
These comments are missing a space before the asterisk
Patch Set #62, Line 23: /* DBGO("\\_PTS\n") */
Please remove commented-out things
/* Clear sleep SMI status flag and enable sleep SMI trap. */
/*Store(One, CSSM)
Store(One, SSEN)*/
/* On older chips, clear PciExpWakeDisEn */
/*if (LLessEqual(\_SB.SBRI, 0x13)) {
* Store(0,\_SB.PWDE)
*}
*/
Commented-out stuff
/*
* \_BFS OEM Back From Sleep method
*
* Entry:
* Arg0=The value of the sleeping state S1=1, S2=2
*
* Exit:
* -none-
*/
Method(\_BFS, 1) {
/* DBGO("\\_BFS\n") */
/* DBGO("From S") */
/* DBGO(Arg0) */
/* DBGO(" to S0\n") */
}
This does nothing
/* DBGO("\\_WAK\n") */
/* DBGO("From S") */
/* DBGO(Arg0) */
/* DBGO(" to S0\n") */
More dead code
File src/mainboard/asus/a88xm-e/acpi/usb_oc.asl:
Three spaces???
File src/mainboard/asus/a88xm-e/bootblock.c:
Patch Set #62, Line 11: most likely
Would be good to confirm
File src/mainboard/asus/a88xm-e/buildOpts.c:
Training empty line in comment
Two spaces surrounding several comments
Patch Set #62, Line 50: //#define BLDOPT_REMOVE_UDIMMS_SUPPORT TRUE
Do we need to have those commented-out entries?
Patch Set #62, Line 63: //TRUE
...
//This element selects whether P-States should be forced to be independent,
// as reported by the ACPI _PSD object. For single-link processors,
// setting TRUE for OS to support this feature.
But other informative comments are C-style?
Why does this comment end on a new line?
Patch Set #62, Line 143: // Specify the default values for the VRM controlling the VDDNB plane.
Is any of this doing any good?
Patch Set #62, Line 162: #if CONFIG(GFXUMA)
Why check for this? The mainboard selects GFXUMA already...
Patch Set #62, Line 184: // #define BLDCFG_SMBUS0_BASE_ADDRESS 0xB00
Is this good for anything?
Patch Set #62, Line 248: #define DDR400_FREQUENCY 200 ///< DDR 400
Isn't this already defined in another header?
missing a space
Patch Set #62, Line 262: #define QUADRANK_REGISTERED 0 ///< Quadrank registered DIMM
What's with the (lack of) alignment between these options?
File src/mainboard/asus/a88xm-e/cmos.layout:
Patch Set #62, Line 1: #*****************************************************************************
What's with these line comments?
File src/mainboard/asus/a88xm-e/devicetree.cb:
Patch Set #62, Line 1: #*****************************************************************************
What's with these line comments?
Patch Set #62, Line 31: end #chip northbridge/amd/agesa/family15tn # PCI side of HT root complex
over 96 characters
io 0x60 = 0x3f0
irq 0x70 = 6
drq 0x74 = 2
but it's off???
COM1
COM2
io 0x60 = 0x2f8
irq 0x70 = 3
It's off!!
io 0x60 = 0x378
irq 0x70 = 7
It's off!!
Patch Set #62, Line 91: irq 0x70 = 12
It's off!!
Patch Set #62, Line 96: 0x238 #Phony resource IT8603E does not have it
Why not zero?
Patch Set #62, Line 121: { {0xA0, 0x00}, {0xA2, 0x00}, }, // socket 0 - Channel 0 & 1 - 8-bit SPD addresses
Lines over 96 characters
File src/mainboard/asus/a88xm-e/dsdt.asl:
Patch Set #62, Line 14: /* #include <arch/x86/acpi/debug.asl> */ /* Include global debug methods if needed */
Dead code that won't compile
So? Can they be removed?
File src/mainboard/asus/a88xm-e/irq_tables.c:
Patch Set #62, Line 11: u8 slot, u8 rfu)
Fit on the previous line
addr += 15;
addr &= ~15;
ALIGN_UP exists
Patch Set #62, Line 41: /* This table must be between 0xf0000 & 0x100000 */
Is this up-to-date? Why is it not checked?
pirq = (void *)(addr);
v = (u8 *) (addr);
Why create two aliased variables? And why does only one cast have a space after it (should not have any)?
Fits on the previous line
File src/mainboard/asus/a88xm-e/mainboard.c:
Patch Set #62, Line 33: /*****************************************************
What's with these line comments?
To view, visit change 30987. To unsubscribe, or for help writing mail filters, visit settings.