Attention is currently required from: Angel Pons. Michael Büchler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45317 )
Change subject: mb/asrock: Add ASRock H77 Pro4-M mainboard ......................................................................
Patch Set 8:
(4 comments)
Patchset:
PS8: Thanks once again! I also tested a build based on a current master, but it failed to boot. Probably because it was inbetween commit 29c7622 and commit 7d925c5? I had no serial port with me to get any debugging output.
Next test on 19th December. Will it be ready this year? :D
File src/mainboard/asrock/h77pro4-m/early_init.c:
https://review.coreboot.org/c/coreboot/+/45317/comment/bc0bc584_813206e3 PS6, Line 59: // TODO check if it's okay to have this in devicetree.cb : /* Power RAM in S3 */ : pnp_set_logical_device(ACPI_DEV); : pnp_wr
I'm pretty sure it's needed. Setting it in devicetree. […]
First I tested it without these lines here. You're right, it is needed. Without it there is no +5V_DUAL during suspend and the DRAM goes down. I moved this to devicetree.cb and will test it again.
https://review.coreboot.org/c/coreboot/+/45317/comment/90b0609b_f898c561 PS6, Line 73: cpu_fan_header = get_uint_option("cpu_fan_header", CPU_FAN_HEADER_1); : switch (cpu_fan_header) { : case CPU_FAN_HEADER_NONE: : reg = 0xff; : break; : case CPU_FAN_HEADER_1: : reg = 0x7f; : break; : case CPU_FAN_HEADER_2: : reg = 0xbf; : break; : case CPU_FAN_HEADER_BOTH: : reg = 0x3f; : break; : }
Good function name, I like it! I'll let you mark this one as resolved, I'm not sure if you've tested […]
Glad to hear it! I did the test and it still works as expected. I also saw no reason to keep the intermediate variable `reg` so I removed it, I hope that's fine. The resulting binary is identical.
File src/mainboard/asrock/h77pro4-m/early_init.c:
https://review.coreboot.org/c/coreboot/+/45317/comment/bd1ce40a_ce6052e7 PS7, Line 44: /* The tachometer signal that goes to CPUFANIN of the Super I/O is set via : * GPIOs. : * When GP77 (register E1h[7]) is '0', CPU_FAN1 is connected. : * When GP76 (register E1h[6]) is '0', CPU_FAN2 is connected. : * When both are '0' and both fans are connected, wrong readings will : * be reported. */
Please adapt this comment's style: https://doc.coreboot.org/contributing/coding_style. […]
Ha! I was actually looking for this section but I was too impatient to find it. You were referring to the newlines only, not the content, right? I added three of them. One too many?