Attention is currently required from: Felix Held, Maximilian Brune.
Felix Singer has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83375?usp=email )
Change subject: Revert "Makefile.mk: Remove bc dependency"
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
> I am terribly sorry about this. I meant to add the WIP tag and test it first. That's on me. […]
Please test your changes before you push them, or push them as WIP or private by default.
Commit Message:
https://review.coreboot.org/c/coreboot/+/83375/comment/8756ff50_90285e01?us… :
PS1, Line 13: Change-Id: Ibf9041b0095ac486d4ee7e8925be8cebb13d2c8f
Missing Signed-off-by line.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83375?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibf9041b0095ac486d4ee7e8925be8cebb13d2c8f
Gerrit-Change-Number: 83375
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jul 2024 13:22:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Attention is currently required from: Felix Held, Felix Singer.
Maximilian Brune has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83375?usp=email )
Change subject: Revert "Makefile.mk: Remove bc dependency"
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I am terribly sorry about this. I meant to add the WIP tag and test it first. That's on me. Should probably make this a quick revert, since it is not apparent when building.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83375?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibf9041b0095ac486d4ee7e8925be8cebb13d2c8f
Gerrit-Change-Number: 83375
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jul 2024 13:14:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Maximilian Brune has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/83313?usp=email )
Change subject: Makefile.mk: Remove bc dependency
......................................................................
--
To view, visit https://review.coreboot.org/c/coreboot/+/83313?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: revert
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6ab4bc2bd7a45e84b923d4fe7ec473e6c7db2146
Gerrit-Change-Number: 83313
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82775?usp=email )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: chromeec: support reading long battery strings
......................................................................
chromeec: support reading long battery strings
The Chrome EC currently supports two ways to read battery strings on
ACPI platforms:
* Read up to 8 bytes from EC shared memory BMFG, BMOD, ...
* Send a EC_CMD_BATTERY_GET_STATIC host command and read strings from
the response. This is assumed to be exclusively controlled by the OS,
because host commands' use of buffers is prone to race conditions.
To support readout of longer strings via ACPI mechanisms, this change
adds support for EC_ACPI_MEM_STRINGS_FIFO (https://crrev.com/c/5581473)
and allows ACPI firmware to read strings of arbitrary length (currently
limited to 64 characters in the implementation) from the EC and to
determine whether this function is supported by the EC (falling back to
shared memory if not).
BUG=b:339171261
TEST=on yaviks, the EC console logs FIFO readout messages when used in
ACPI and correct strings are shown in the OS. If EC support is
removed, correct strings are still shown in the OS.
BRANCH=nissa
Change-Id: Ia29cacb7d86402490f9ac458f0be50e3f2192b04
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/82775
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Subrata Banik <subratabanik(a)google.com>
---
M src/ec/google/chromeec/acpi/battery.asl
M src/ec/google/chromeec/acpi/ec.asl
2 files changed, 92 insertions(+), 6 deletions(-)
Approvals:
build bot (Jenkins): Verified
Subrata Banik: Looks good to me, approved
diff --git a/src/ec/google/chromeec/acpi/battery.asl b/src/ec/google/chromeec/acpi/battery.asl
index 5344111..f5047cb 100644
--- a/src/ec/google/chromeec/acpi/battery.asl
+++ b/src/ec/google/chromeec/acpi/battery.asl
@@ -54,6 +54,91 @@
Return (Local0)
}
+// Cached flag for BSRF FIFO readout support from EC.
+Name(BRSS, 0xff)
+
+// Read extended battery strings from the selected battery.
+// Arg0 = string index
+//
+// If supported by the EC, strings of arbitrary length are read using the
+// FIFO readout method. Otherwise short (no more than 8 bytes) strings are
+// read from the EC shared memory map. The desired battery index should be
+// selected with BTSW before calling this method.
+//
+// Currently supported string indexes:
+// * 1 = EC_ACPI_MEM_STRINGS_FIFO_ID_BATTERY_MODEL: battery model name,
+// equivalent to BMOD.
+// * 2 = EC_ACPI_MEM_STRINGS_FIFO_ID_BATTERY_SERIAL: battery serial number,
+// equivalent to BSER.
+// * 3 = EC_ACPI_MEM_STRINGS_FIFO_ID_BATTERY_MANUFACTURER: battery
+// manufacturer's name, equivalent to BMFG.
+//
+// These are assumed to be supported if the EC reports at least version 1 of
+// string readout (it returns an integer greater than 0 and less than 255 when
+// reading FIFO index 0). Future strings may require higher FIFO versions.
+Method(BRSX, 1, Serialized)
+{
+ // It doesn't make sense to read the FIFO support indicator.
+ if (Arg0 == 0)
+ {
+ Return ("")
+ }
+
+ If (BRSS == 0xff)
+ {
+ // Write 0 to BSRF to read back a support indicator; nonzero and
+ // non-0xFF if FIFO readout is supported, assuming minimum v1 support
+ // for strings 1 through 3.
+ BSRF = 0
+ BRSS = BSRF
+
+ // 0xff readback also means no support for FIFO readout, when the EC
+ // doesn't even know what this command is.
+ if (BRSS == 0xff)
+ {
+ BRSS = 0
+ }
+ }
+
+ // If FIFO readout through BSRF is not supported, fall back to reading
+ // the short strings in EMEM.
+ If (BRSS == 0)
+ {
+ If (Arg0 == 1)
+ {
+ Return (ToString (Concatenate (BMOD, 0)))
+ }
+ ElseIf (Arg0 == 2)
+ {
+ Return (ToString (Concatenate (BSER, 0)))
+ }
+ ElseIf (Arg0 == 3)
+ {
+ Return (ToString (Concatenate (BMFG, 0)))
+ }
+ Else
+ {
+ Return ("")
+ }
+ }
+
+ // 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)
+ {
+ Local0 = Concatenate (Local0, ToString (Local1))
+ Local1 = BSRF
+ }
+
+ Return (Local0)
+}
+
// _BIF implementation.
// Arg0 = battery index
// Arg1 = PBIF
@@ -86,9 +171,9 @@
Arg1[6] = Local2
// Get battery info from mainboard
- Arg1[9] = ToString(Concatenate(BMOD, 0x00))
- Arg1[10] = ToString(Concatenate(BSER, 0x00))
- Arg1[12] = ToString(Concatenate(BMFG, 0x00))
+ Arg1[9] = BRSX (1)
+ Arg1[10] = BRSX (2)
+ Arg1[12] = BRSX (3)
Release (^BATM)
Return (Arg1)
@@ -129,9 +214,9 @@
Arg1[8] = BTCC
// Get battery info from mainboard
- Arg1[16] = ToString(Concatenate(BMOD, 0x00))
- Arg1[17] = ToString(Concatenate(BSER, 0x00))
- Arg1[19] = ToString(Concatenate(BMFG, 0x00))
+ Arg1[16] = BRSX (1)
+ Arg1[17] = BRSX (2)
+ Arg1[19] = BRSX (3)
Release (^BATM)
Return (Arg1)
diff --git a/src/ec/google/chromeec/acpi/ec.asl b/src/ec/google/chromeec/acpi/ec.asl
index d6d33b2..741ad59 100644
--- a/src/ec/google/chromeec/acpi/ec.asl
+++ b/src/ec/google/chromeec/acpi/ec.asl
@@ -100,6 +100,7 @@
USPP, 8, // USB Port Power
RFWU, 8, // Retimer Firmware Update
PBOK, 8, // Power source change count from dptf
+ BSRF, 8, // Battery string readout FIFO
}
#if CONFIG(EC_GOOGLE_CHROMEEC_ACPI_MEMMAP)
--
To view, visit https://review.coreboot.org/c/coreboot/+/82775?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia29cacb7d86402490f9ac458f0be50e3f2192b04
Gerrit-Change-Number: 82775
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Martin L Roth, Nico Huber.
Felix Singer has posted comments on this change by Felix Singer. ( https://review.coreboot.org/c/coreboot/+/83347?usp=email )
Change subject: xcompile: Apply -Wextra with temporary exceptions to GCC
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
Could we continue here?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83347?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I60915cb66581dc2c9b6807335fd0e214b45e76d6
Gerrit-Change-Number: 83347
Gerrit-PatchSet: 6
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Mon, 08 Jul 2024 13:00:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83313?usp=email )
Change subject: Makefile.mk: Remove bc dependency
......................................................................
Makefile.mk: Remove bc dependency
bc was added as dependency in commit 229e021110 ("Makefile.inc: Add left shift macro")
bc is not stated as dependency in our docs (e.g. package installation).
If you don't have bc installed you can easily get false positives on
coreboot builds. For example you build a mainboard and coreboot tells
you the build succeeded, even though you don't have bc installed.
This patch is from julius comment on CB:21601.
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
Change-Id: I6ab4bc2bd7a45e84b923d4fe7ec473e6c7db2146
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83313
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
---
M Makefile.mk
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Felix Singer: Looks good to me, approved
diff --git a/Makefile.mk b/Makefile.mk
index 68fc8c9..592365e 100644
--- a/Makefile.mk
+++ b/Makefile.mk
@@ -178,7 +178,7 @@
int-multiply=$(if $(filter 1,$(words $1)),$(strip $1),$(call int-multiply,$(call _int-multiply2,$(word 1,$1),$(word 2,$1)) $(wordlist 3,$(words $1),$1)))
int-divide=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) / $(call _toint,$(word 2,$1))))
int-remainder=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) % $(call _toint,$(word 2,$1))))
-int-shift-left=$(shell echo "$(call _toint,$(word 1, $1)) * (2 ^ $(call _toint,$(word 2, $1)))" | bc)
+int-shift-left=$(shell expr $(call _toint,$(word 1, $1)) << $(call _toint,$(word 2, $1)))
int-lt=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) \< $(call _toint,$(word 2,$1))))
int-gt=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) \> $(call _toint,$(word 2,$1))))
int-eq=$(if $(filter 1,$(words $1)),$(strip $1),$(shell expr $(call _toint,$(word 1,$1)) = $(call _toint,$(word 2,$1))))
--
To view, visit https://review.coreboot.org/c/coreboot/+/83313?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6ab4bc2bd7a45e84b923d4fe7ec473e6c7db2146
Gerrit-Change-Number: 83313
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Attention is currently required from: Ronald Claveau.
Felix Singer has posted comments on this change by Ronald Claveau. ( https://review.coreboot.org/c/coreboot/+/83104?usp=email )
Change subject: mainboard/dell: Add new mainboard XPS 8300 (Sandy Bridge)
......................................................................
Patch Set 15: Code-Review+2
(1 comment)
Patchset:
PS15:
Looks good!
--
To view, visit https://review.coreboot.org/c/coreboot/+/83104?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7d394794fec580bc7aed3f6396ceb47d4a6fd059
Gerrit-Change-Number: 83104
Gerrit-PatchSet: 15
Gerrit-Owner: Ronald Claveau <sousmangoosta(a)aliel.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ronald Claveau <sousmangoosta(a)aliel.fr>
Gerrit-Comment-Date: Mon, 08 Jul 2024 12:50:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes