[coreboot-gerrit] [M] Change in coreboot[main]: ec/google/chromeec: Optimize battery string readout with caching

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84354?usp=email )

Change subject: ec/google/chromeec: Optimize battery string readout with caching
......................................................................

ec/google/chromeec: Optimize battery string readout with caching

This commit refactors the long battery string implementation to include
caching of the EC response for battery information (model, serial, and
manufacturer).

This optimization reduces resume time by approximately 63ms by
minimizing communication overhead between the AP and EC.

BUG=b:366338622
TEST=Verified on google/tivviks_ufs:
    * Long battery string is displayed when
      EC_GOOGLE_CHROMEEC_READ_BATTERY_LONG_STRING is enabled.
    * Short battery string is displayed when
      EC_GOOGLE_CHROMEEC_READ_BATTERY_LONG_STRING=n.

Change-Id: I32ae5b5e618f20335f3d344811a97f1416df529e
Signed-off-by: Subrata Banik 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/84354
Tested-by: build bot (Jenkins) 
Reviewed-by: Matt DeVillier 
Reviewed-by: Paul Menzel 
Reviewed-by: Caveh Jalali 
---
M src/ec/google/chromeec/acpi/battery.asl
1 file changed, 58 insertions(+), 17 deletions(-)

Approvals:
  Matt DeVillier: Looks good to me, approved
  build bot (Jenkins): Verified
  Caveh Jalali: Looks good to me, but someone else must approve
  Paul Menzel: Looks good to me, but someone else must approve

diff --git a/src/ec/google/chromeec/acpi/battery.asl b/src/ec/google/chromeec/acpi/battery.asl
index 30fe0fc..7d52088 100644
--- a/src/ec/google/chromeec/acpi/battery.asl
+++ b/src/ec/google/chromeec/acpi/battery.asl
@@ -60,6 +60,14 @@
 #else
 Name(BRSS, 0x0)
 #endif
+// Cached battery string response indicator
+Name(BRI1, 0)
+Name(BRI2, 0)
+Name(BRI3, 0)
+// Cached battery string response data to save suspend-resume time
+Name(BRS1, Buffer(32) {0})
+Name(BRS2, Buffer(32) {0})
+Name(BRS3, Buffer(32) {0})
 // Read extended battery strings from the selected battery.
 //   Arg0 = string index
 //
@@ -82,11 +90,27 @@
 Method(BRSX, 1, Serialized)
 {
    // It doesn't make sense to read the FIFO support indicator.
-	if (Arg0 == 0)
+	if (Arg0 == 0 || Arg0 > 3)
    {
    	Return ("")
    }

+	// Check if response is already cached
+	If (Arg0 == 1 && BRI1 == 1)
+	{
+		Return (BRS1) /* battery model name */
+	}
+
+	If (Arg0 == 2 && BRI2 == 1)
+	{
+		Return (BRS2) /* battery serial number */
+	}
+
+	If (Arg0 == 3 && BRI3 == 1)
+	{
+		Return (BRS3) /* battery manufacturer's name */
+	}
+
    If (BRSS == 0xff)
    {
    	// Write 0 to BSRF to read back a support indicator; nonzero and
@@ -109,34 +133,51 @@
    {
    	If (Arg0 == 1)
    	{
-			Return (ToString (Concatenate (BMOD, 0)))
+			Local0 = ToString (Concatenate (BMOD, 0))
    	}
    	ElseIf (Arg0 == 2)
    	{
-			Return (ToString (Concatenate (BSER, 0)))
+			Local0 = ToString (Concatenate (BSER, 0))
    	}
    	ElseIf (Arg0 == 3)
    	{
-			Return (ToString (Concatenate (BMFG, 0)))
+			Local0 = ToString (Concatenate (BMFG, 0))
    	}
-		Else
+	}
+	Else
+	{
+		// Select requested parameter to read
+		BSRF = Arg0
+
+		// Read to end of string, or up to a reasonable maximum length. Reads of
+		// BSRF consume bytes from the FIFO, so take care to read it only once
+		// per byte of data.
+		Local0 = ""
+		Local1 = BSRF
+		While (Local1 != 0 && SizeOf (Local0) < 32)
    	{
-			Return ("")
+			Local0 = Concatenate (Local0, ToString (Local1))
+			Local1 = BSRF
    	}
    }

-	// Select requested parameter to read
-	BSRF = Arg0
-
-	// Read to end of string, or up to a reasonable maximum length. Reads of
-	// BSRF consume bytes from the FIFO, so take care to read it only once
-	// per byte of data.
-	Local0 = ""
-	Local1 = BSRF
-	While (Local1 != 0 && SizeOf (Local0) < 32)
+	// Store the result in the cache
+	If (Arg0 == 1)
    {
-		Local0 = Concatenate (Local0, ToString (Local1))
-		Local1 = BSRF
+		BRS1 = Local0
+		BRI1 = 1
+	}
+
+	If (Arg0 == 2)
+	{
+		BRS2 = Local0
+		BRI2 = 1
+	}
+
+	If (Arg0 == 3)
+	{
+		BRS3 = Local0
+		BRI3 = 1
    }

Return (Local0)