Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34475 )
Change subject: Add Razer Blade Stealth (2016) H2U ......................................................................
Patch Set 33: Code-Review+2
(10 comments)
Juuuust stupid details
https://review.coreboot.org/c/coreboot/+/34475/33//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34475/33//COMMIT_MSG@13 PS33, Line 13: memory in dualchannel mode I'd say these lines are too long for a commit message, mind splitting them?
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/Kconfi... File src/mainboard/razer/Kconfig:
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/Kconfi... PS33, Line 13: MAINBOARD_VENDOR defined here already
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/Kconfi... File src/mainboard/razer/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/Kconfi... PS33, Line 2: Razer please make it uppercase "RAZER" for consistency
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... File src/mainboard/razer/blade_stealth_kbl/Kconfig:
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... PS33, Line 33: : : config MAINBOARD_VENDOR : string : default "RAZER" Shouldn't need to redefine this
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... PS33, Line 59: config CPU_MICROCODE_CBFS_LEN : hex : default 0x18000 : : config CPU_MICROCODE_CBFS_LOC : hex : default 0xFFE115A0 Is it needed to specify the microcode length and location?
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... PS33, Line 22: # For now no way to choose the correct the available RAM : config BOARD_RAZER_BLADE_STEALTH_KBL_16GB : bool "16GB RAM (4x MT52L1G32D4PG)" : default n : : config VGA_BIOS_ID : string : default "8086,5916" : : config IRQ_SLOT_COUNT : int : default 18 : : config MAINBOARD_VENDOR : string : default "RAZER" : : config MAINBOARD_FAMILY : string : default "BLADE_STEALTH" : : config MAINBOARD_PART_NUMBER : string : default "H2U" : : config MAINBOARD_VERSION : string : default "1.0" : : config MAINBOARD_DIR : string : default "razer/blade_stealth_kbl" : : config MAX_CPUS : int : default 4 : : config CPU_MICROCODE_CBFS_LEN : hex : default 0x18000 : : config CPU_MICROCODE_CBFS_LOC : hex : default 0xFFE115A0 : : config CBFS_SIZE : hex : default 0x5c0000 These are usually not indented
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... File src/mainboard/razer/blade_stealth_kbl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... PS33, Line 39: register "SataPortsDevSlp[0]" = "0" : register "SataPortsDevSlp[2]" = "0" : register "SataSpeedLimit" = "2" I don't think you need to set these with SATA ports disabled
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... PS33, Line 199: [ Is this used?
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... PS33, Line 216: d Is this used?
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... File src/mainboard/razer/blade_stealth_kbl/mainboard.c:
https://review.coreboot.org/c/coreboot/+/34475/33/src/mainboard/razer/blade_... PS33, Line 47: MAX_SERIAL_LENGTH); Minor: fits on the previous line