Ken Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38860 )
Change subject: mb/google/hatch: Create palkia variant ......................................................................
Patch Set 3:
(8 comments)
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/palkia/include/variant/acpi/dptf.asl:
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 80: 90, 70, 50, 50
Do we need to add this?
Add these temporary setting to make FAN working when CPU temperature raise up .
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/palkia/memory.c:
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 55: Helios
Which RCOMP resistors does Palkia use?
Palkia use the same RCOMP resistors as Helios .
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 58: Helios
Are these the same for Palkia?
Palkia should use the same setting as Helios on Rcomp target values .
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 87: /* : * For boards with id 0 or unknown, memory straps 3 and 4 are : * incorrectly stuffed in hardware. This is a workaround for these : * boards to override memory strap 3 to 0 and 4 to 1. : */
Does this apply to Palkia?
Yes , Palkia use the same memory config table as Helios . It should be suitable as Palkia .
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/palkia/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 53: .i2c[3] = { : .speed = I2C_SPEED_FAST, : .rise_time_ns = 150, : .fall_time_ns = 150, : },
It's disabled in the devicetree
According to this CL https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... i2c[4] setting would effect by i2c[3] setting show up .
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 121: register "generic.reset_gpio" = : "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" : register "generic.reset_delay_ms" = "120" : register "generic.reset_off_delay_ms" = "1"
Remove this. From schematics, it looks like we don't use this pin.
We do use GPP_D15 for touchscreen reset control .
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 137: chip drivers/generic/gpio_keys : register "name" = ""PENH"" : register "gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(GPP_A8)" : register "key.wake" = "GPE0_DW0_08" : register "key.wakeup_event_action" = "EV_ACT_ASSERTED" : register "key.dev_name" = ""EJCT"" : register "key.linux_code" = "SW_PEN_INSERTED" : register "key.linux_input_type" = "EV_SW" : register "key.label" = ""pen_eject"" : device generic 0 on end : end
Remove this.
We plan to support touch pen . Should we remove it ?
https://review.coreboot.org/c/coreboot/+/38860/2/src/mainboard/google/hatch/... PS2, Line 157: register "generic.reset_gpio" = : "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" : register "generic.reset_delay_ms" = "120" : register "generic.reset_off_delay_ms" = "1"
Remove this. From schematics, it looks like we don't use this pin.
We do use GPP_D15 for touchscreen reset control .