Ice Lake /o\
22 comments:
Patch Set #3, Line 11: SuperIO
ITE website disagrees:
IT8320 : Highly Integrated Embedded Controller
Sure, ITE ECs also have a SuperIO-like interface, but they are ECs.
Only two LPDDR4 chips?
File src/mainboard/razer/blade_stealth_icl/Kconfig:
Patch Set #3, Line 37: UART_FOR_CONSOLE
Where is the UART routed to?
File src/mainboard/razer/blade_stealth_icl/Makefile.inc:
Patch Set #3, Line 22: CONFIG_SOC_INTEL_COMMON_BLOCK_HDA_VERB
This is enforced by the mainboard selection.
File src/mainboard/razer/blade_stealth_icl/acpi/mainboard.asl:
Is this file needed, or is it a stub?
File src/mainboard/razer/blade_stealth_icl/acpi_tables.c:
There's a change dropping these empty files
File src/mainboard/razer/blade_stealth_icl/devicetree.cb:
nit: PCH
Patch Set #3, Line 164: Enable
Really?
Please align these `end`. That way, it looks prettier :D
Another misalignment around here
File src/mainboard/razer/blade_stealth_icl/dsdt.asl:
Patch Set #3, Line 28: // Some generic macros
I got rid of these comments recently! Please don't add them again
Patch Set #3, Line 34: // CPU
This comment doesn't really add any value, does it?
Scope (\_SB) {
Device (PCI0)
Device (\_SB.PCI0)
Patch Set #3, Line 45: // Chipset specific sleep states
I removed these comments as well. The sleepstates are no longer specific to each chipset.
Patch Set #3, Line 49: #include "acpi/mainboard.asl"
If you don't use that file, this can be dropped
File src/mainboard/razer/blade_stealth_icl/hda_verb.h:
/* ALC 700 */
0x10EC0700, 0xFFFFFFFF, 0x00000023,
You can add comments to these values. Also, please use lowercase for hex constants. The third number is a length value, so it should be decimal:
0x10ec0700, /* Realtek ALC700 */
0xffffffff, /* Subsystem ID */
35, /* Number of four-dword sets */
Patch Set #3, Line 25: AZALIA_PIN_CFG(0, 0x01, 0x00000000),
Looks bogus. NID 0x01 is the subsystem ID, AFAIK, so it shouldn't have any pin_cfg macro
Patch Set #3, Line 25: 0x10EC10F2
You can also put this value in place of the 0xffffffff above
AZALIA_PIN_CFG(0, 0x12, 0x40000000), AZALIA_PIN_CFG(0, 0x13, 0x40000000),
AZALIA_PIN_CFG(0, 0x14, 0x411111F0), AZALIA_PIN_CFG(0, 0x15, 0x411111F0),
AZALIA_PIN_CFG(0, 0x16, 0x411111F0), AZALIA_PIN_CFG(0, 0x17, 0x90170110),
AZALIA_PIN_CFG(0, 0x18, 0x411111F0), AZALIA_PIN_CFG(0, 0x19, 0x04A11030),
AZALIA_PIN_CFG(0, 0x1A, 0x411111F0), AZALIA_PIN_CFG(0, 0x1B, 0x411111F0),
AZALIA_PIN_CFG(0, 0x1D, 0x40622005), AZALIA_PIN_CFG(0, 0x1E, 0x411111F0),
AZALIA_PIN_CFG(0, 0x1F, 0x411111F0), AZALIA_PIN_CFG(0, 0x21, 0x04211020),
AZALIA_PIN_CFG(0, 0x29, 0x411111F0),
Please put these on individual lines. Also, please use lowercase for the hex values
Patch Set #3, Line 40: // Widget node 0X20 for ALC1305 20160603 update
Are all of these hex spells applicable to the ALC700? If so, please group them in quadruples (groups of four). That way, one can easily check if there are actually 35 groups.
File src/mainboard/razer/blade_stealth_icl/hda_verb.c:
Patch Set #3, Line 15: #include "hda_verb.h"
Why don't you just put the hda_verb.h contents here?
File src/mainboard/razer/blade_stealth_icl/romstage.c:
Patch Set #3, Line 28: // Only one memory setup exists
Please use consistent comment styles
To view, visit change 37565. To unsubscribe, or for help writing mail filters, visit settings.