[L] Change in coreboot[main]: mb/hp: Add Pro 3400

[Angel Pons (Code Review)]

Attention is currently required from: Václav Straka.

Angel Pons has posted comments on this change by Václav Straka. ( https://review.coreboot.org/c/coreboot/+/85825?usp=email )

Change subject: mb/hp: Add Pro 3400
......................................................................

Patch Set 1:

(9 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/85825/comment/55975a96_28fe75c9?usp... :
PS1, Line 11: As a side effect fixed 3500's USB. (broken since 81878)
This should belong to a separate commit. I'm not sure what exactly fixed the USB ports.

Also, "81878" is not a meaningful reference. Do you mean CB:81878 ? If so, use the commit hash as reference:
> commit 943b5409147 (sb/intel/bd82x6x: Make space for USB port config in devicetree)

File Documentation/mainboard/hp/pro_3x00_series.md:

PS1: 
It's really annoying that Gerrit doesn't show this file as being renamed. Would it be possible for you to rename it in a follow-up, so that it's easy to see the changes in contents?

File src/mainboard/hp/pro_3x00_series/Kconfig:

https://review.coreboot.org/c/coreboot/+/85825/comment/44c905f7_4442b414?usp... :
PS1, Line 47: 	default "src/mainboard/$(MAINBOARDDIR)/variants/$(CONFIG_VARIANT_DIR)/data.vbt"
I think this is the default path

File src/mainboard/hp/pro_3x00_series/cmos.default:

https://review.coreboot.org/c/coreboot/+/85825/comment/a5a4a33c_d89a9560?usp... :
PS1, Line 6: nmi=Enable
I would disable NMI by default, IIRC the state isn't properly saved/restored so NMI ends up disabled anyway.

File src/mainboard/hp/pro_3x00_series/cmos.layout:

https://review.coreboot.org/c/coreboot/+/85825/comment/dcb1531b_3b50e215?usp... :
PS1, Line 26: # SandyBridge MRC Scrambler Seed values
            : 896		32	r	0	mrc_scrambler_seed
            : 928		32	r	0	mrc_scrambler_seed_s3
            : 960		16	r	0	mrc_scrambler_seed_chk
Unused with native raminit, which is forced on (`select USE_NATIVE_RAMINIT`). Please remove.

File src/mainboard/hp/pro_3x00_series/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/85825/comment/be96f5e5_f7e91e75?usp... :
PS1, Line 3: #define BRIGHTNESS_UP _SB.PCI0.GFX0.INCB
           : #define BRIGHTNESS_DOWN _SB.PCI0.GFX0.DECB
For a follow-up: this doesn't make sense on a desktop and can be removed.

https://review.coreboot.org/c/coreboot/+/85825/comment/b0667892_f74ff4d6?usp... :
PS1, Line 26: 		#include <drivers/intel/gma/acpi/default_brightness_levels.asl>
For a follow-up: this doesn't make sense on a desktop and can be removed.

File src/mainboard/hp/pro_3x00_series/variants/pro_3400_series/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/85825/comment/1008e0e2_95260145?usp... :
PS1, Line 24: 			device ref mei1 off end
Why is this off? If you used me_cleaner or the FDO jumper, the ME stops working properly and vendor firmware will typically hide it from the OS.

https://review.coreboot.org/c/coreboot/+/85825/comment/4a244937_889a7eae?usp... :
PS1, Line 25: 			device ref pci_bridge off end
This is the default state. This line can be removed.