Attention is currently required from: Ashish Kumar Mishra, Caveh Jalali, Dinesh Gehlot, Eric Herrmann, Forest Mittelberg, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Keith Short, Nick Vaccaro, Rishika Raj, 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 11:
(9 comments)
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/4e5e0c15_7337e187?usp... : PS11, Line 15: #define SET_PSYSPL2(e, w) ((e) * (w) / 100)
If not setting Psys please remove it.
Other variant's build were failing - just to make sure other variants are not affected i added this back
https://review.coreboot.org/c/coreboot/+/83752/comment/d687260c_6b6b20fb?usp... : PS11, Line 16: SET_PL2
If not setting PL2, please remove it.
Other variant's build were failing - just to make sure other variants are not affected i added this back
https://review.coreboot.org/c/coreboot/+/83752/comment/b7efa4d4_c5145a98?usp... : PS11, Line 19: #define TENPOWERSIX 1000000 : #define TENPOWERTHREE 1000 :
Instead of TENPOWERSIX and TENPOWERTHREE, please define MILLIVOLT_TO_VOLT and MILLIAMP_TO_AMP. […]
Yes right - will do
https://review.coreboot.org/c/coreboot/+/83752/comment/85543f25_6a690fbc?usp... : PS11, Line 82: MILLIWATTS_TO_WATTS);
Indent it after the DIV_ROUND_UP open parenthesis.
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/e9953990_ba2d9c30?usp... : PS11, Line 87: &volts_mv);
Same comment as above.
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/5c58b834_4540c733?usp... : PS11, Line 98: soc_config->tdp_pl4 = SET_PL4(40, watts);
Can you please add a table at the start of this function to provide an idea of what the PL4 will loo […]
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/ed662272_8213a9cf?usp... : PS11, Line 102: watts);
Same comment as above.
will do
https://review.coreboot.org/c/coreboot/+/83752/comment/45ca2c4b_f942a1c7?usp... : PS11, Line 146: void variant_update_psys_power_limits
It seems not used anymore. If so, can we remove it. Also remove the function prototype in variants. […]
Other variant's build were failing - just to make sure other variants are not affected i added this back
File src/soc/intel/alderlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/1b208368_76d6034e?usp... : PS9, Line 359: if (CONFIG(EC_GOOGLE_CHROMEEC) && (!google_chromeec_is_battery_present_and_above_critical_threshold())) {
How about moving inside mainboard_memory_init_params override in brox mainboard. […]
This should be written in romstage .so I am not very sure about writing in mainboard_memory_init_params .let me sync up internally about this and come back with an answer. The only way i could think of is to keep this exclusive for Brox using Kconfig - but adding too many configuration parameters is not the right approach also ..so i am not pursuing that approach.