[coreboot-gerrit] [L] Change in coreboot[main]: mb/hp: Add Pro 3500 series (Sandy/Ivy Bridge)

[Matt DeVillier (Code Review)]

Attention is currently required from: Joel Linn, Nicholas Chin.

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81368?usp=email )

Change subject: mb/hp: Add Pro 3500 series (Sandy/Ivy Bridge)
......................................................................

Patch Set 2: Code-Review+1

(9 comments)

File src/mainboard/hp/pro_3500_series/Kconfig:

https://review.coreboot.org/c/coreboot/+/81368/comment/bfa0f16f_4fdc18c7 :
PS2, Line 37: # FIXME: check this
if checked, remove fixme

File src/mainboard/hp/pro_3500_series/acpi_tables.c:

https://review.coreboot.org/c/coreboot/+/81368/comment/7cb94ace_9340ecfb :
PS2, Line 6: /* FIXME: check this function.  */
if this is from autoport and you've verified the temps, can probably remove the comment

File src/mainboard/hp/pro_3500_series/common_defines.h:

https://review.coreboot.org/c/coreboot/+/81368/comment/e8ce5707_ce1b335d :
PS2, Line 8: #define IT8772F_BASE 0x2e
           : #define EC_DEV PNP_DEV(IT8772F_BASE, IT8772F_EC)
           : #define GPIO_DEV PNP_DEV(IT8772F_BASE, IT8772F_GPIO)
tab align for readability

File src/mainboard/hp/pro_3500_series/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/81368/comment/01a409a2_43194d11 :
PS2, Line 3: register "gpu_cpu_backlight" = "0x00000000"
           : 	register "gpu_dp_b_hotplug" = "4"
           : 	register "gpu_dp_c_hotplug" = "0"
           : 	register "gpu_dp_d_hotplug" = "0"
           : 	register "gpu_panel_port_select" = "0"
           : 	register "gpu_panel_power_backlight_off_delay" = "0"
           : 	register "gpu_panel_power_backlight_on_delay" = "0"
           : 	register "gpu_panel_power_cycle_delay" = "4"
           : 	register "gpu_panel_power_down_delay" = "0"
           : 	register "gpu_panel_power_up_delay" = "0"
           : 	register "gpu_pch_backlight" = "0x00000000"
these values look very suspect - the fixme comment might be applicable here

https://review.coreboot.org/c/coreboot/+/81368/comment/cc524ca9_2671f541 :
PS2, Line 78: #register "gpio_set1" = "0x00"
            : 					#register "gpio_set2" = "0xff"
            : 					#register "gpio_set3" = "0x00"
            : 					#register "gpio_set4" = "0x00"
            : 					#register "gpio_set5" = "0x00"
can be removed since commented out

File src/mainboard/hp/pro_3500_series/early_init.c:

https://review.coreboot.org/c/coreboot/+/81368/comment/3ba52bb3_a81d1465 :
PS2, Line 18: FIXME: Unknown current: RCBA(0x350c)=0x350c
address fixme comments

https://review.coreboot.org/c/coreboot/+/81368/comment/1f6682c0_9d1657f0 :
PS2, Line 42: // ite_kill_watchdog(GPIO_DEV);
remove commented line

File src/mainboard/hp/pro_3500_series/mainboard.c:

https://review.coreboot.org/c/coreboot/+/81368/comment/934e32f2_8a5af0d3 :
PS2, Line 9: * FIXME: untested. */
tested? handler needed at all?

File src/superio/ite/common/early_serial.c:

PS2:
> I would split the changes to src/superio/ite/common into a separate patch

2nd