Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31133 )
Change subject: soc/intel/cannonlake: Add Whiskeylake SoC kconfig
......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/#/c/31133/7/src/soc/intel/cannonlake/Kconfig
File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31133/7/src/soc/intel/cannonlake/Kconfig@14
PS7, Line 14: and SOC_INTEL_CANNONLAKE Kconfig are in FSP Header Files. Hence this Kconfig
: might help to select required SoC support FSP headers. Any future Intel SoC would
> Are these lines too long?
i thought its <80, i will check
https://review.coreboot.org/#/c/31133/7/src/soc/intel/cannonlake/Kconfig@266
PS7, Line 266: 3rdparty/fsp/CoffeeLakeFspBinPkg/Include/
> Have to be careful about this. […]
this may not holds true for CML because that will have CML FSP package, so i have to add CFL and WHL Kconfig specifically here
--
To view, visit https://review.coreboot.org/c/coreboot/+/31133
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I66b63361841f5a16615ddce4225c4f6182eabdb3
Gerrit-Change-Number: 31133
Gerrit-PatchSet: 7
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: PraveenX Hodagatta Pranesh <praveenx.hodagatta.pranesh(a)intel.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 05 Feb 2019 19:26:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31133 )
Change subject: soc/intel/cannonlake: Add Whiskeylake SoC kconfig
......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/#/c/31133/7/src/soc/intel/cannonlake/Kconfig
File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/#/c/31133/7/src/soc/intel/cannonlake/Kconfig@14
PS7, Line 14: and SOC_INTEL_CANNONLAKE Kconfig are in FSP Header Files. Hence this Kconfig
: might help to select required SoC support FSP headers. Any future Intel SoC would
Are these lines too long?
https://review.coreboot.org/#/c/31133/7/src/soc/intel/cannonlake/Kconfig@266
PS7, Line 266: 3rdparty/fsp/CoffeeLakeFspBinPkg/Include/
Have to be careful about this. If SOC_INTEL_COMMON_CANNONLAKE_BASE is expected to be selected by all new SoCs using CNL PCH, then you need to ensure that they use the right FSP headers and paths.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31133
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I66b63361841f5a16615ddce4225c4f6182eabdb3
Gerrit-Change-Number: 31133
Gerrit-PatchSet: 7
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: PraveenX Hodagatta Pranesh <praveenx.hodagatta.pranesh(a)intel.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 05 Feb 2019 19:22:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31206
Change subject: Kconfig: Add system type entries for convertible and tablet
......................................................................
Kconfig: Add system type entries for convertible and tablet
These are two common system types and in some cases it is important
to know when a device is a convertible or a tablet instead of just
a laptop.
This change will select the appropriate SMBIOS enclosure type based
on the selected system type.
This is important for the Intel Virtual Button driver as it does a
check on the SMBIOS enclosure type and only enables the tablet mode
events if it is set to convertible:
https://patchwork.kernel.org/patch/10236253/
Change-Id: I148ec2329a1dd38ad55c60ba277a514c66376fcc
Signed-off-by: Duncan Laurie <dlaurie(a)google.com>
---
M src/Kconfig
1 file changed, 13 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/31206/1
diff --git a/src/Kconfig b/src/Kconfig
index a069f63..dc960eb 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -335,6 +335,14 @@
default n
bool
+config SYSTEM_TYPE_TABLET
+ default n
+ bool
+
+config SYSTEM_TYPE_CONVERTIBLE
+ default n
+ bool
+
config CBFS_AUTOGEN_ATTRIBUTES
default n
bool
@@ -654,11 +662,14 @@
hex
depends on GENERATE_SMBIOS_TABLES
default 0x09 if SYSTEM_TYPE_LAPTOP
+ default 0x1e if SYSTEM_TYPE_TABLET
+ default 0x1f if SYSTEM_TYPE_CONVERTIBLE
default 0x03
help
System Enclosure or Chassis Types as defined in SMBIOS specification.
- The default value is SMBIOS_ENCLOSURE_DESKTOP (0x03) or
- SMBIOS_ENCLOSURE_LAPTOP (0x09) if SYSTEM_TYPE_LAPTOP is set.
+ The default value is SMBIOS_ENCLOSURE_DESKTOP (0x03) but laptop,
+ convertible, or tablet enclosure will be used if the appropriate
+ system type is selected.
endmenu
--
To view, visit https://review.coreboot.org/c/coreboot/+/31206
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I148ec2329a1dd38ad55c60ba277a514c66376fcc
Gerrit-Change-Number: 31206
Gerrit-PatchSet: 1
Gerrit-Owner: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-MessageType: newchange
Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31207
Change subject: mb/google/sarien: Set system type for the board variants
......................................................................
mb/google/sarien: Set system type for the board variants
Select the appropriate system type for the different variants of
the Sarien board.
This will allow the Arcada variant to use the tablet mode feature
of the Intel Virtual Button driver.
Change-Id: I8a829aab012256ec196c8ec0fa298fd2bc77f2e1
Signed-off-by: Duncan Laurie <dlaurie(a)google.com>
---
M src/mainboard/google/sarien/Kconfig
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/31207/1
diff --git a/src/mainboard/google/sarien/Kconfig b/src/mainboard/google/sarien/Kconfig
index 67f879f..785c8a7 100644
--- a/src/mainboard/google/sarien/Kconfig
+++ b/src/mainboard/google/sarien/Kconfig
@@ -21,7 +21,8 @@
select SOC_INTEL_COMMON_BLOCK_HDA_VERB
select SOC_INTEL_COMMON_BLOCK_SMM_ESPI_ACPI_DIS
select SPD_READ_BY_WORD
- select SYSTEM_TYPE_LAPTOP
+ select SYSTEM_TYPE_LAPTOP if BOARD_GOOGLE_SARIEN
+ select SYSTEM_TYPE_CONVERTIBLE if BOARD_GOOGLE_ARCADA
select TPM2
select MAINBOARD_USES_IFD_EC_REGION
select MAINBOARD_USES_IFD_GBE_REGION if BOARD_GOOGLE_SARIEN
--
To view, visit https://review.coreboot.org/c/coreboot/+/31207
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8a829aab012256ec196c8ec0fa298fd2bc77f2e1
Gerrit-Change-Number: 31207
Gerrit-PatchSet: 1
Gerrit-Owner: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-MessageType: newchange
Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31208
Change subject: ec/google/wilco: Add virtual button support
......................................................................
ec/google/wilco: Add virtual button support
Add an ACPI device that is compatible with the the Intel Virtual
Button kernel driver for reporting tablet mode state and various
virtual button events that may come from the EC.
This driver is used in Windows and in the Linux kernel at
drivers/platform/x86/intel-vbtn.c
Because of a check in the kernel driver it expects the board to
define the SMBIOS enclosure type as convertible for the check at
driver load time for tablet/laptop and dock/undock to work.
The virtual tablet mode button will proxy the tablet mode state
sent from the Sensor Hub to a SW_TABLET_MODE event in the kernel.
The virtual power buton is used during S0ix for the EC to wake
the system with an SCI. There are separate press and release
events which are sent for completeness, although the kernel driver
will ignore the release event.
BUG=b:73137291
TEST=Test that the power button can wake the system from S0ix.
Also verify that the device is reported as laptop mode at boot.
Change-Id: I0d5dc985a3cfb1d01ff164c4e67f17e6b1cdd619
Signed-off-by: Duncan Laurie <dlaurie(a)google.com>
---
M src/ec/google/wilco/acpi/ec.asl
M src/ec/google/wilco/acpi/ec_ram.asl
M src/ec/google/wilco/acpi/event.asl
A src/ec/google/wilco/acpi/vbtn.asl
4 files changed, 116 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/31208/1
diff --git a/src/ec/google/wilco/acpi/ec.asl b/src/ec/google/wilco/acpi/ec.asl
index be80b2d..5aca187 100644
--- a/src/ec/google/wilco/acpi/ec.asl
+++ b/src/ec/google/wilco/acpi/ec.asl
@@ -164,6 +164,7 @@
#include "event.asl"
#include "lid.asl"
#include "platform.asl"
+ #include "vbtn.asl"
#ifdef EC_ENABLE_DPTF
#include "dptf.asl"
#endif
diff --git a/src/ec/google/wilco/acpi/ec_ram.asl b/src/ec/google/wilco/acpi/ec_ram.asl
index 1e5d7cb..1c030af 100644
--- a/src/ec/google/wilco/acpi/ec_ram.asl
+++ b/src/ec/google/wilco/acpi/ec_ram.asl
@@ -115,6 +115,7 @@
Name (DRTQ, Package () { 0x38, 0xff, RD }) /* DPTF: Read Trip Query */
Name (ORST, Package () { 0x39, 0xff, RD }) /* Orientation State */
+Name (OTBL, Package () { 0x39, 0x02, RD }) /* Orientation: Tablet mode */
Name (OREV, Package () { 0x3a, 0xff, RD }) /* Orientation Events */
Name (OECH, Package () { 0x3a, 0x01, RD }) /* Event: Orientation */
Name (OERL, Package () { 0x3a, 0x02, RD }) /* Event: Rotation Lock */
diff --git a/src/ec/google/wilco/acpi/event.asl b/src/ec/google/wilco/acpi/event.asl
index 754ed26..a10a388 100644
--- a/src/ec/google/wilco/acpi/event.asl
+++ b/src/ec/google/wilco/acpi/event.asl
@@ -81,6 +81,16 @@
Printf ("QS EVENT")
Notify (^WLCO, 0x90)
}
+
+ If (EBIT (E2OR, Arg0)) {
+ If (R (OTBL)) {
+ Printf ("EC event indicates tablet mode")
+ Notify (^VBTN, ^VTBL)
+ } Else {
+ Printf ("EC event indicates laptop mode")
+ NotifY (^VBTN, ^VLAP)
+ }
+ }
}
/* Handle events in PmEv3 */
@@ -88,6 +98,16 @@
{
Printf ("EVT3: %o", Arg0)
+ If (EBIT (E3CP, Arg0)) {
+ If (R (P2PB)) {
+ Printf ("Power button pressed")
+ Notify (^VBTN, ^VPPB)
+ } Else {
+ Printf ("Power button released")
+ Notify (^VBTN, ^VRPB)
+ }
+ }
+
#ifdef EC_ENABLE_DPTF
/* Theraml Events */
If (EBIT (E3TH, Arg0)) {
diff --git a/src/ec/google/wilco/acpi/vbtn.asl b/src/ec/google/wilco/acpi/vbtn.asl
new file mode 100644
index 0000000..201ab51
--- /dev/null
+++ b/src/ec/google/wilco/acpi/vbtn.asl
@@ -0,0 +1,94 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright 2019 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 of
+ * the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/*
+ * Intel Virtual Button driver compatible with the driver found in
+ * the Linux kernel at drivers/platform/x86/intel-vbtn.c
+ *
+ * For tablet/laptop and dock/undock events to work the board must
+ * select SYSTEM_TYPE_CONVERTIBLE for the SMBIOS enclosure type to
+ * indicate the device is a convertible.
+ */
+
+Name (FLAP, 0x40) /* Flag indicating device is in laptop mode */
+
+/* Virtual events */
+Name (VPPB, 0xc0) /* Power Button press */
+Name (VRPB, 0xc1) /* Power Button release */
+Name (VPSP, 0xc2) /* Special key press (LEFTMETA in Linux) */
+Name (VRSP, 0xc3) /* Special key release (LEFTMETA in Linux) */
+Name (VPVU, 0xc4) /* Volume Up press */
+Name (VRVU, 0xc5) /* Volume Up release */
+Name (VPVD, 0xc6) /* Volume Down press */
+Name (VRVD, 0xc7) /* Volume Down release */
+Name (VPRL, 0xc8) /* Rotate Lock press */
+Name (VRRL, 0xc9) /* Rotate Lock release */
+Name (VDOC, 0xca) /* Docked */
+Name (VUND, 0xcb) /* Undocked */
+Name (VTBL, 0xcc) /* Tablet Mode */
+Name (VLAP, 0xcd) /* Laptop Mode */
+
+Device (VBTN)
+{
+ Name (_HID, "INT33D6")
+ Name (_UID, One)
+ Name (_DDN, "Intel Virtual Button Driver")
+
+ /*
+ * This method is called at driver probe time and must exist or
+ * the driver will not load.
+ */
+ Method (VBDL)
+ {
+ }
+
+ /*
+ * This method returns flags indicating tablet and dock modes.
+ * It is called at driver probe time so the OS knows what the
+ * state of the device is at boot.
+ */
+ Method (VGBS)
+ {
+ Local0 = Zero
+
+ /* Check EC orientation for tablet mode flag */
+ If (R (OTBL)) {
+ Printf ("EC reports tablet mode at boot")
+ } Else {
+ Printf ("EC reports laptop mode at boot")
+ Local0 |= ^^FLAP
+ }
+ Return (Local0)
+ }
+
+ Method(_STA, 0)
+ {
+ Return (0xF)
+ }
+}
+
+Device (VBTO)
+{
+ Name (_HID, "INT33D3")
+ Name (_CID, "PNP0C60")
+ Name (_UID, One)
+ Name (_DDN, "Laptop/tablet mode indicator driver")
+
+ Method (_STA, 0)
+ {
+ Return (0xF)
+ }
+}
--
To view, visit https://review.coreboot.org/c/coreboot/+/31208
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0d5dc985a3cfb1d01ff164c4e67f17e6b1cdd619
Gerrit-Change-Number: 31208
Gerrit-PatchSet: 1
Gerrit-Owner: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-MessageType: newchange
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31101 )
Change subject: src/soc/intel/cannonlake: Add _DSM methods for LPIT table
......................................................................
Patch Set 15:
> Patch Set 15:
>
> > Patch Set 15:
> >
> > (1 comment)
>
> I have a quick compare the dsdt.dsl generated and that's identical for patch 12 and patch 15, I may assume the didn't work will be caused by other regression on S0ix. ASL 2.0 had been introduced for several years and widely adopted in coreboot as well.
Agree, I tested on my local build. Result is passed and work well.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31101
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia908969decf7cf12f505becb4f4a4a9caa7ed6db
Gerrit-Change-Number: 31101
Gerrit-PatchSet: 15
Gerrit-Owner: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Assignee: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 05 Feb 2019 15:54:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Matt Delco has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31162
Change subject: arch/x86/acpigen: wrap _PLD in a package
......................................................................
arch/x86/acpigen: wrap _PLD in a package
The ACPI spec has an asl example for _PLD in the form:
Name (_PLD, Package (0x01) { ToPLD (PLD_Revision = 0x2) })
When I ported this to acpigen and diffed the results I noticed that
the binary blob was no longer provided within a package. The ACPI
spec (section 6.1.8 in version 6.2) defines _PLD as "a variable-length
Package containing a list of Buffers". This commit changes
acpigen_write_pld to use a package (the one existing caller I found
isn't wrapping the result in a package so it doesn't look like
it was intended for the callers of acpigen_write_pld to be responsible
for using a package.
BUG=none
BRANCH=none
TEST=Verified that after this change a package is use and the result
of acpigen matches what was used in the original asl.
Change-Id: Ie2db63c976100109bfe976553e52565fb2d2d9df
Signed-off-by: Matt Delco <delco(a)chromium.org>
---
M src/arch/x86/acpigen.c
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/31162/1
diff --git a/src/arch/x86/acpigen.c b/src/arch/x86/acpigen.c
index 290893d..0eacf05 100644
--- a/src/arch/x86/acpigen.c
+++ b/src/arch/x86/acpigen.c
@@ -1261,7 +1261,9 @@
return;
acpigen_write_name("_PLD");
+ acpigen_write_package(1);
acpigen_write_byte_buffer(buf, ARRAY_SIZE(buf));
+ acpigen_pop_len();
}
void acpigen_write_dsm(const char *uuid, void (**callbacks)(void *),
--
To view, visit https://review.coreboot.org/c/coreboot/+/31162
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie2db63c976100109bfe976553e52565fb2d2d9df
Gerrit-Change-Number: 31162
Gerrit-PatchSet: 1
Gerrit-Owner: Matt Delco <delco(a)chromium.org>
Gerrit-MessageType: newchange
Matt Delco has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31161
Change subject: acpi: device: avoid empty property list in acpi_dp_write
......................................................................
acpi: device: avoid empty property list in acpi_dp_write
If an acpi_dp table has children but no properties then acpi_dp_write()
will write out a properties UUID and package that contains no properties.
The existing function will avoid writing out a UUID and empty package
when no children exist, but it seems to assume that properties will
always be used. With this change properties are handled in a manner
akin to children so that a UUID and package are only written if
properties exist.
BUG=none
BRANCH=none
TEST=Confirmed that prior to this change a UUID and empty package was
present for a device that had children but no properties. Verified that
after this change the UUID and empty package are no longer present but
the child UUID and package are still present.
Change-Id: I6f5597713a1e91ca26b409f36b3ff9eb90a010af
Signed-off-by: Matt Delco <delco(a)chromium.org>
---
M src/arch/x86/acpi_device.c
1 file changed, 26 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/31161/1
diff --git a/src/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c
index 48b7fee..c62d6f3 100644
--- a/src/arch/x86/acpi_device.c
+++ b/src/arch/x86/acpi_device.c
@@ -632,7 +632,7 @@
void acpi_dp_write(struct acpi_dp *table)
{
struct acpi_dp *dp, *prop;
- char *dp_count, *prop_count;
+ char *dp_count, *prop_count = NULL;
int child_count = 0;
if (!table || table->type != ACPI_DP_TYPE_TABLE)
@@ -644,32 +644,42 @@
/* Device Property list starts with the next entry */
prop = table->next;
- /* Package (DP), default to 2 elements (assuming no children) */
- dp_count = acpigen_write_package(2);
-
- /* ToUUID (ACPI_DP_UUID) */
- acpigen_write_uuid(ACPI_DP_UUID);
-
- /* Package (PROP), element count determined as it is populated */
- prop_count = acpigen_write_package(0);
+ /* Package (DP), default to assuming no properties or children */
+ dp_count = acpigen_write_package(0);
/* Print base properties */
for (dp = prop; dp; dp = dp->next) {
if (dp->type == ACPI_DP_TYPE_CHILD) {
child_count++;
} else {
+ /*
+ * The UUID and package is only added when
+ * we come across the first property. This
+ * is to avoid creating a zero-length package
+ * in situations where there are only children.
+ */
+ if (!prop_count) {
+ *dp_count += 2;
+ /* ToUUID (ACPI_DP_UUID) */
+ acpigen_write_uuid(ACPI_DP_UUID);
+ /*
+ * Package (PROP), element count determined as
+ * it is populated
+ */
+ prop_count = acpigen_write_package(0);
+ }
(*prop_count)++;
acpi_dp_write_property(dp);
}
}
-
- /* Package (PROP) length */
- acpigen_pop_len();
+ if (prop_count) {
+ /* Package (PROP) length, if a package was written */
+ acpigen_pop_len();
+ }
if (child_count) {
- /* Update DP package count to 4 */
- *dp_count = 4;
-
+ /* Update DP package count to 2 or 4 */
+ *dp_count += 2;
/* ToUUID (ACPI_DP_CHILD_UUID) */
acpigen_write_uuid(ACPI_DP_CHILD_UUID);
@@ -679,7 +689,7 @@
for (dp = prop; dp; dp = dp->next)
if (dp->type == ACPI_DP_TYPE_CHILD)
acpi_dp_write_property(dp);
-
+ /* Package (CHILD) length */
acpigen_pop_len();
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/31161
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6f5597713a1e91ca26b409f36b3ff9eb90a010af
Gerrit-Change-Number: 31161
Gerrit-PatchSet: 1
Gerrit-Owner: Matt Delco <delco(a)chromium.org>
Gerrit-MessageType: newchange