Attention is currently required from: Eric Herrmann, Forest Mittelberg, Kapil Porwal, Karthik Ramasubramanian, Keith Short, Nick Vaccaro, Paul Menzel, Rishika Raj, Shelley Chen, Sowmya Aralguppe, Subrata Banik.
Ashish Kumar Mishra has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83752?usp=email )
Change subject: mb/google/brox: Fix Booting to kernel without battery ......................................................................
Patch Set 4:
(9 comments)
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/41d0602a_98f0c836?usp... : PS4, Line 82: if (rv == 0 && type == USB_CHG_TYPE_PD) { No need for {} for single `if` statements.
https://review.coreboot.org/c/coreboot/+/83752/comment/0f4c2b3b_4bd4da5c?usp... : PS4, Line 84: watts = ((u32)current_ma * volts_mv) / 1000000; We can use a macro #define instead of '1000000' for better readability.
https://review.coreboot.org/c/coreboot/+/83752/comment/7f4fc31e_dba719e4?usp... : PS4, Line 87: if (watts == 0) { No need for {} for single if statements.
https://review.coreboot.org/c/coreboot/+/83752/comment/5178aafb_7fa5e14a?usp... : PS4, Line 88: return; It'd be useful to add a debug print for returning reason.
https://review.coreboot.org/c/coreboot/+/83752/comment/540b91aa_d04f50d7?usp... : PS4, Line 91: /* set PL4 to efficiency% of adapter rating */ No need for {} for single if statements.
https://review.coreboot.org/c/coreboot/+/83752/comment/6a536be7_794a0180?usp... : PS4, Line 96: printk(BIOS_INFO, "CPU PL4 %d for %d Watts Charger\n", soc_config->tdp_pl4, watts); overflow issue. Using %d for unsigned ints. Please use %u.
https://review.coreboot.org/c/coreboot/+/83752/comment/4e08e1ab_a36972a0?usp... : PS4, Line 113: settings->pl2.max_power = soc_config->tdp_pl2_override * 1000; Please use a #define macro instead of 1000.
https://review.coreboot.org/c/coreboot/+/83752/comment/711af3f1_919af350?usp... : PS4, Line 117: printk(BIOS_INFO, "Overriding power limits PL1 (%d, %d) PL2 (%d, %d) PL4 (%d)\n", Use %u instead of %d, since args are unsigned ints.
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/e9ad0a0d_4f153581?usp... : PS4, Line 364: m_cfg->BootFrequency); We may not need to print the value with "%d" in addition, to just printing about the same change in text.