27 comments:
SOCs
Patch Set #15, Line 20: simi;ar
similar
Marc Jones
File src/mainboard/amd/padmelon/BiosCallOuts.h:
Patch Set #15, Line 31: fan runs
Either "4-wire fans run at" or "a 4-wire fan runs at". Same for the 2nd half of the sentence.
File src/mainboard/amd/padmelon/Kconfig:
Patch Set #15, Line 18: config HAVE_S3_SUPPORT
Put this under the mainboard part number?
Patch Set #15, Line 22: Select it to change GPIO used for wake.
What? This help makes no sense. HAVE_S3_SUPPORT should be about whether the board supports S3 or not.
Select a bit within LPC_IO_PORT_DECODE_ENABLE that will enable
access to IO port HWM_PORT, which is programmed at bootblock.
I still have no idea what this does or why it should be a Kconfig option. Can this just be hardcoded in a header file?
Patch Set #15, Line 64: config HWM_PORT
Again, does this need to be a Kconfig option? There's no prompt, why not just set it in a header file.
Maybe lead with what HWM stands for and what it's used for.
The Hardware monitor is used for xxxx
File src/mainboard/amd/padmelon/acpi/gpe.asl:
You might want to verify the methods in this file. These look like they're left over from other platforms.
File src/mainboard/amd/padmelon/acpi/routing.asl:
/*
DefinitionBlock ("DSDT.AML","DSDT",0x01,"XXXXXX","XXXXXXXX",0x00010001
)
{
#include "routing.asl"
}
*/
Remove this block?
File src/mainboard/amd/padmelon/acpi/superio.asl:
Patch Set #15, Line 49: #define SUPERIO_PNP_BASE 0x4E
It seems a little odd to define this inside an ASL file. Is this value not used anywhere else? Can this be set by a #define in a header file that gets used here?
Patch Set #15, Line 52: Device(SUPERIO_DEV) {
Why is this in the mainboard instead of the SIO's directory?
File src/mainboard/amd/padmelon/acpi/usb_oc.asl:
/* simple name description */
/*
DefinitionBlock ("DSDT.AML","DSDT",0x01,"XXXXXX","XXXXXXXX",0x00010001
)
{
#include "usb.asl"
}
*/
remove this block?
/* USB Overcurrent GPEs */
/* TODO: Update for Gardenia */
Just update this? Why would you leave a TODO for Gardenia in the padmelon code? Either remove the comment, or update it.
File src/mainboard/amd/padmelon/acpi_tables.c:
You can remove the license header - there's really nothing to license.
If you want to keep the license for some reason, add the line that says that this file is a part of the coreboot project please.
File src/mainboard/amd/padmelon/board_info.txt:
Can't we fill out any other fields?
File src/mainboard/amd/padmelon/bootblock/OemCustomize.c:
Patch Set #15, Line 121: /*DESCRIPTOR_TERMINATE_LIST, */
Remove this comment?
Patch Set #15, Line 140: AzaliaCodecAlc286Table
Is this the correct codec for padmelon?
{ 0x10ec0286, AzaliaCodecAlc286Table},
{ 0x10ec0288, AzaliaCodecAlc286Table},
{ 0x10ec0888, AzaliaCodecAlc286Table},
We use the same table for 3 different codecs?
File src/mainboard/amd/padmelon/bootblock/bootblock.c:
Patch Set #15, Line 35: /* Remove this section if HW handshake is not needed */
Is the HW handshake needed? If so, remove the comment. Otherwise, remove the block.
Looking at the comment below, it seems like they're needed for some reason, but I'm not sure why that would be.
File src/mainboard/amd/padmelon/devicetree.cb:
Patch Set #15, Line 27: #register "uma_size" = "512 * MiB"
Remove this?
Is 51 right (which would be 0xA2) or should this be 52 (which would be 0xA4, as shown in the table above?)
File src/mainboard/amd/padmelon/dsdt.asl:
As i said in the other comment, it seem a little odd to have this defined in the ASL file.
Patch Set #15, Line 73: #include <superio/fintek/f81803a/acpi/superio.asl>
So it is in the SIO directory. Maybe delete the copy out of the mainboard then.
File src/mainboard/amd/padmelon/mainboard.c:
Patch Set #15, Line 35: TODO: Make ACPI use these values too.
Either fix it or remove the TODO. Copying TODOs from one board to another is kind of bad form.
File src/mainboard/amd/padmelon/romstage.c:
* Copyright (C) 2015 Advanced Micro Devices, Inc.
*
What's the copyright on?
Maybe just remove everything and leave a blank file?
To view, visit change 33993. To unsubscribe, or for help writing mail filters, visit settings.