[S] Change in coreboot[main]: ec/google/chromeec: Add read_battery_status to get dynamic battery flags
Attention is currently required from: Boris Mittelberg, Brandon Breitenstein, Caveh Jalali, Hannah Williams, Kapil Porwal, Subrata Banik, Sukumar Ghorai.
Jamie Ryu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78321?usp=email )
Change subject: ec/google/chromeec: Add read_battery_status to get dynamic battery flags ......................................................................
Patch Set 4:
(2 comments)
File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/78321/comment/1b208f46_81aa8d37 : PS4, Line 1538: dynamic_p
can u please explain what is dynamic_p and _r refers to ??
I used the same variable names from ec/util/ectool.cc, and this reads dynamic battery info, and I think p refers to params and r refers to response. I can change this to 'params' and 'resp' if this looks better.
https://review.coreboot.org/c/coreboot/+/78321/comment/31d1f1ed_0447f801 : PS4, Line 1543: if (ec_cmd_battery_get_dynamic(PLAT_EC, &dynamic_p, &dynamic_r) != 0)
We only care if the Battery is cutoff right?
The change is currently added to check the battery presence only and I think we need more discussion if we need to check the battery level as well.
if this host command fails, we got bigger problems.
Do you have any suggestion how to handle the failure at here? This is a utility function to get the data and result, so I think the next action to handle the failure should be done by the caller?
-
Jamie Ryu (Code Review)