[coreboot-gerrit] [M] Change in coreboot[main]: mb/google/brox: Fix booting to kernel without battery

[Sowmya Aralguppe (Code Review)]

Attention is currently required from: Ashish Kumar Mishra, Caveh Jalali, Dinesh Gehlot, Eric Herrmann, Forest Mittelberg, Jayvik Desai, Karthik Ramasubramanian, Keith Short, Krishna P Bhat D, Nick Vaccaro, Paul Menzel, Rishika Raj, Ronak Kanabar, Shelley Chen, Sowmya Aralguppe, Subrata Banik.

Sowmya Aralguppe 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 28:

(4 comments)

File src/mainboard/google/brox/Kconfig:

https://review.coreboot.org/c/coreboot/+/83752/comment/6713f88d_ea8e91fa?usp... :
PS27, Line 124: 	  Select this if the variant has to boot for low battery, battery at critical threshold
              : 	  or when battery is disconnected physically. PL4 which is processor Instataneous Power
              : 	  or Absolute Peak Power controls the highest power draw from processor at any given
              : 	  instant of time within Pl1Tau duration. Hence PL4 acts as a fail safe mechanism to set an
              : 	  upper threshold limit for processor instantaneous power draw. For a 30W adapter, the
              : 	  maximum peak power is set at 9 watts which is 30W multiplied by 32 percent efficiency.
              : 	  This default pl4 value is set for 30W or any higher adapter rating such as 45W or 65W.
> Select this if the variant has to boot even with low battery, critical battery threshold, or when th […]

Done

File src/mainboard/google/brox/romstage.c:

https://review.coreboot.org/c/coreboot/+/83752/comment/576ecf3e_bfaeef85?usp... :
PS27, Line 16: 	FSP_M_CONFIG *mem_cfg = &memupd->FspmConfig;
> you can move this line inside `if` case in line 27

Done

File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:

https://review.coreboot.org/c/coreboot/+/83752/comment/f110fd5b_e6c08900?usp... :
PS27, Line 74: else
             : 		soc_config->tdp_pl4 = CONFIG_PL4_LIMIT_FOR_CRITICAL_BAT_BOOT;
> please use braces as we have multi-line `if` case as well.

Acknowledged

https://review.coreboot.org/c/coreboot/+/83752/comment/19057e5a_1aa1d990?usp... :
PS27, Line 70: 	if (google_chromeec_is_battery_present_and_above_critical_threshold()) {
             : 		if (soc_config->tdp_pl4 == 0)
             : 			soc_config->tdp_pl4 = DIV_ROUND_UP(limits[brox_idx].pl4_power,
             : 								MILLIWATTS_TO_WATTS);
             : 	} else
             : 		soc_config->tdp_pl4 = CONFIG_PL4_LIMIT_FOR_CRITICAL_BAT_BOOT;
             :
> can we move this into a local function like `variant_pl4_override(soc_config)` […]

Done