Attention is currently required from: Ashish Kumar Mishra, Caveh Jalali, Dinesh Gehlot, Eric Herrmann, Forest Mittelberg, Jayvik Desai, Kapil Porwal, 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 19:
(4 comments)
File src/mainboard/google/brox/romstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/0193c6b0_f2ac37c6?usp... : PS19, Line 27: MAX_NONTURBO_PERFORMANCE
did you check BootFrequency UPD dump w/o and w/ this CL? […]
Yes Subrata 1) In normal flow Bootfrequency is set to 2 by default and in ramstage turbo is enabled and I see the typical case of CPU brown out happening. 2) If bootfrequency is set to 1 then i see TurboMode is enabled but TurboModeDisable (MSR_MISC_ENABLES) is set 3) If bootfrequency is set to 2 then i see TurboMode is enabled but TurboModeDisable is not set 4) In turbo.c we are enabling turbo based on IA32_MISC_ENABLE - 0x1a0 5) CPUstrap is read only to check if there is a mismatch in bootfreq and P1 and if reset is required
File src/mainboard/google/brox/variants/baseboard/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/0ced2c1b_ed133a0d?usp... : PS19, Line 57: * PL4 calculations - For USB C charger (Max Power):
if we know that OS can override the PL4 table dynamically and we only need to set a lower P4 when ba […]
For PL4 writeback this is required .I tried keeping a single value - it is not working for all 3 adapters. I followed from how shelly had implemented psys for different power ratings .These values have been tested thoroughly for all corner cases . IMO we can keep it like this - if we are unable to write pl4 back in coreboot - then we can make it static i feel.
https://review.coreboot.org/c/coreboot/+/83752/comment/fffee419_6939275b?usp... : PS19, Line 57: * PL4 calculations - For USB C charger (Max Power):
if we know that OS can override the PL4 table dynamically and we only need to set a lower P4 when ba […]
For PL4 writeback to work on all 3 different chargers it is required to change pl4 based on adapters.
File src/mainboard/google/brox/variants/brox/ramstage.c:
https://review.coreboot.org/c/coreboot/+/83752/comment/505acb21_ba75b128?usp... : PS19, Line 19: .pl1_min_power = 15000,
this change should be part of separate CL
We had discussed that changes reverting psys calculations https://review.coreboot.org/c/coreboot/+/83087 will be taken up as part this patch. If its an issue to keep it here - i will take it as a separate patch. Please let me know
-
Sowmya Aralguppe (Code Review)