Attention is currently required from: Andrey Petrov, Patrick Rudolph.
Hello Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/61431
to look at the new patch set (#3).
Change subject: soc/intel/common/cse: Rework `heci_disable` function
......................................................................
soc/intel/common/cse: Rework `heci_disable` function
This patch provides the possible options for SoC users to choose the
applicable interface to make HECI1 function disable at pre-boot.
`SOC_INTEL_COMMON_BLOCK_HECI_DISABLE_USING_SBI` config is used for
disabling heci1 using non-posted sideband write (inside SMM) after
FSP-S sets the postboot_sai attribute. Applicable from CNL PCH onwards.
`SOC_INTEL_COMMON_BLOCK_HECI_DISABLE_USING_PMC_IPC` config is used for
disabling heci1 using PMC IPC command `0xA9`. Applicable from TGL PCH
onwards.
`SOC_INTEL_COMMON_BLOCK_HECI_DISABLE_USING_PCR` config is used for
disabling heci1 using private configuration register (PCR) write.
Applicable for SoC platform prior to CNL PCH.
Additionally, add PID_CSME0 macro for SKL, Xeon_SP and APL to fix the
compilation failure.
BUG=none
TEST=Able to build and boot brya.
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Change-Id: I7e0bab0004013b999ec1e054310763427d7b9348
---
M src/soc/intel/apollolake/include/soc/pcr_ids.h
M src/soc/intel/common/block/cse/Kconfig
M src/soc/intel/common/block/cse/Makefile.inc
M src/soc/intel/common/block/cse/disable_heci.c
M src/soc/intel/common/block/include/intelblocks/cse.h
M src/soc/intel/skylake/include/soc/pcr_ids.h
M src/soc/intel/xeon_sp/include/soc/pcr_ids.h
7 files changed, 71 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/61431/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/61431
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7e0bab0004013b999ec1e054310763427d7b9348
Gerrit-Change-Number: 61431
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Tim Wawrzynczak.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61432 )
Change subject: Redrix4es: remove unwanted entries in devicetree
......................................................................
Patch Set 1:
(2 comments)
File src/mainboard/google/brya/variants/baseboard/brya/devicetree.cb:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-139322):
https://review.coreboot.org/c/coreboot/+/61432/comment/7dac5baa_1dff34b3
PS1, Line 29: register "usb2_ports[4]" = "USB2_PORT_EMPTY"
trailing whitespace
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-139322):
https://review.coreboot.org/c/coreboot/+/61432/comment/caec6ba6_e351fc8c
PS1, Line 30: register "usb2_ports[6]" = "USB2_PORT_EMPTY"
trailing whitespace
--
To view, visit https://review.coreboot.org/c/coreboot/+/61432
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaed4e2dd84aab0dffbf2e3825ad30d2efa846f23
Gerrit-Change-Number: 61432
Gerrit-PatchSet: 1
Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Thu, 27 Jan 2022 21:39:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Felix Held.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61086 )
Change subject: soc/amd/sabrina: add additional UART controllers
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/61086/comment/8948e451_d5b42d98
PS5, Line 8:
Please mention the datasheet name and revision, and if it’s not public also say how many controllers are added.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61086
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I628b1a7a0930f3409acdcabda2b864d42bf6bd23
Gerrit-Change-Number: 61086
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 27 Jan 2022 21:36:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Anil Kumar K, Cliff Huang, Selma Bensaid, Thejaswani Putta.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61354 )
Change subject: drivers/wwan/fm: Add Fibocom 5G WWAN ACPI support
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/61354/comment/61e79ef8_1aa810e6
PS2, Line 10: when
on
https://review.coreboot.org/c/coreboot/+/61354/comment/4cfb688c_c6951206
PS2, Line 9: Support PXSX._RST and PXSX.MRST._RST for warm and cold reset.
: PXSX._RST is invoked when driver removal.
:
: Test:
: Add chip entry to the corresponding root port and check PXSX Device
: is generated in ssdt.
Please do not indent.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e0b9fd405f6cfb1e216ea27558bb9299a09e566
Gerrit-Change-Number: 61354
Gerrit-PatchSet: 2
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Comment-Date: Thu, 27 Jan 2022 21:34:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Anil Kumar K, Cliff Huang, Selma Bensaid, Thejaswani Putta.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61354 )
Change subject: drivers/wwan/fm: Add Fibocom 5G WWAN ACPI support
......................................................................
Patch Set 2:
(11 comments)
Patchset:
PS2:
Thanks Cliff!
One thing to note; right now we have the WWAN shut-down sequence hardcoded into the DSDT (https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr…), so after this lands, we have the opportunity to make the shut-down sequence into its own Method and then brya's `MPTS` method can just call that instead of hardcoding the sequence there 😊
File src/drivers/wwan/fm/acpi_fm350gl.c:
https://review.coreboot.org/c/coreboot/+/61354/comment/995f5cb8_32c9a60b
PS2, Line 29:
Can you add a comment here about what FHRF does?
https://review.coreboot.org/c/coreboot/+/61354/comment/77b3a674_01eff461
PS2, Line 38: assert
`deassert`
(it is active-low, so driving it high is deassertion, and the If just above means this code will only get called if PERST# is already low, i.e. active)
https://review.coreboot.org/c/coreboot/+/61354/comment/36c026e8_f9a7ad95
PS2, Line 42: assert
`deassert`
same reason as above
https://review.coreboot.org/c/coreboot/+/61354/comment/1e03d250_4c93bdd8
PS2, Line 44: warm reset
Arg0 == 0, means cold reset and
Arg0 == 1, means warm reset?
https://review.coreboot.org/c/coreboot/+/61354/comment/90e8e004_f08c4a1b
PS2, Line 61:
Can you add a comment here about what SHRF does?
https://review.coreboot.org/c/coreboot/+/61354/comment/a07ba0d2_f026d80e
PS2, Line 90: acpi_device_path_join(parent_dev, "RTD3._OFS")
nit: this is used twice, extract to a local variable
https://review.coreboot.org/c/coreboot/+/61354/comment/350a525e_ccd683a1
PS2, Line 90: acpigen_write_store_int_to_namestr(1, acpi_device_path_join(parent_dev, "RTD3._OFS"));
To ensure that these _ON and _OFF calls are properly paired, I would rather see these incremented and decremented rather than set to 0 and 1
https://review.coreboot.org/c/coreboot/+/61354/comment/d6fc81fa_d621810d
PS2, Line 106: acpi_device_path_join(parent_dev, "RTD3._OFS")
nit: this is used twice, extract to a local variable
https://review.coreboot.org/c/coreboot/+/61354/comment/c3da10d6_6e5e3ee6
PS2, Line 106: acpigen_write_store_int_to_namestr(1, acpi_device_path_join(parent_dev, "RTD3._OFS"));
To ensure that these _ON and _OFF calls are properly paired, I would rather see these incremented and decremented rather than set to 0 and 1
https://review.coreboot.org/c/coreboot/+/61354/comment/d000ca39_abd652c4
PS2, Line 167: .read_resources = noop_read_resources,
: .set_resources = noop_set_resources,
: .acpi_fill_ssdt = wwan_fm350gl_acpi_fill_ssdt,
: .acpi_name = wwan_fm350gl_acpi_name,
: };
nit: line these up like this (with tabs)
```
.read_resources = noop_read_resources,
.set_resources = noop_set_resources,
.acpi_fill_ssdt = wwan_fm350gl_acpi_fill_ssdt,
.acpi_name = wwan_fm350gl_acpi_name,
};
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/61354
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e0b9fd405f6cfb1e216ea27558bb9299a09e566
Gerrit-Change-Number: 61354
Gerrit-PatchSet: 2
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Comment-Date: Thu, 27 Jan 2022 21:29:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph.
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/61431
to look at the new patch set (#2).
Change subject: soc/intel/common/cse: Rework `heci_disable` function
......................................................................
soc/intel/common/cse: Rework `heci_disable` function
This patch provides the possible options for SoC users to choose the
applicable interface to make HECI1 function disable at pre-boot.
`SOC_INTEL_COMMON_BLOCK_HECI_DISABLE_USING_SBI` config is used for
disabling heci1 using non-posted sideband write (inside SMM) after
FSP-S sets the postboot_sai attribute. Applicable from CNL PCH onwards.
`SOC_INTEL_COMMON_BLOCK_HECI_DISABLE_USING_PMC_IPC` config is used for
disabling heci1 using PMC IPC command `0xA9`. Applicable from TGL PCH
onwards.
`SOC_INTEL_COMMON_BLOCK_HECI_DISABLE_USING_PCR` config is used for
disabling heci1 using private configuration register (PCR) write.
Applicable for SoC platform prior to CNL PCH.
Additionally, add PID_CSME0 macro for SKL and APL to fix the
compilation failure.
BUG=none
TEST=Able to build and boot brya.
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Change-Id: I7e0bab0004013b999ec1e054310763427d7b9348
---
M src/soc/intel/apollolake/include/soc/pcr_ids.h
M src/soc/intel/common/block/cse/Kconfig
M src/soc/intel/common/block/cse/Makefile.inc
M src/soc/intel/common/block/cse/disable_heci.c
M src/soc/intel/common/block/include/intelblocks/cse.h
M src/soc/intel/skylake/include/soc/pcr_ids.h
6 files changed, 70 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/61431/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/61431
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7e0bab0004013b999ec1e054310763427d7b9348
Gerrit-Change-Number: 61431
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Bora Guvendik, Anil Kumar K, Cliff Huang, Selma Bensaid, Thejaswani Putta, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61353 )
Change subject: soc/intel/common/block/pcie/rtd3: Add optional _OFF and _ON skip control
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
So the use case for this is for some other driver to consume/set the _OFS and _ONS variables?
File src/soc/intel/common/block/pcie/rtd3/rtd3.c:
https://review.coreboot.org/c/coreboot/+/61353/comment/63d16d69_4bcc4066
PS1, Line 428: _ONS
suggestion:
names that start with `_` are reserved for ACPI spec use; you could just use
`ONSK`
and
`OFSK`
for example
--
To view, visit https://review.coreboot.org/c/coreboot/+/61353
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic32d151d65107bfc220258c383a575e40a496b6f
Gerrit-Change-Number: 61353
Gerrit-PatchSet: 1
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 27 Jan 2022 21:18:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Lance Zhao, Anil Kumar K, Cliff Huang, Selma Bensaid, Paul Menzel, Thejaswani Putta, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61352 )
Change subject: soc/intel/common/block/pcie/rtd3: Add PM methods to the device.
......................................................................
Patch Set 1:
(1 comment)
File src/soc/intel/common/block/pcie/rtd3/chip.h:
https://review.coreboot.org/c/coreboot/+/61352/comment/c3d80970_ea579787
PS1, Line 53: ext_pm_support
> Yup, that will be good.
Then there is still an invalid state; perhaps we can make an enumeration out of the legal combinations?
e.g.
```
enum acpi_pcie_rp_pm_emit {
ACPI_PCIE_RP_EMIT_NORMAL, /* disable_l23=false, ext_pm_support=false */
ACPI_PCIE_RP_EMIT_NONE, /* disable_l23=true, ext_pm_support=false */
ACPI_PCIE_RP_EMIT_EXPORT, /* disable_l23=false, ext_pm_support=true */
};
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/61352
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I79de76f26c8424b036cb7d2719df68937599ca2f
Gerrit-Change-Number: 61352
Gerrit-PatchSet: 1
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lance Zhao
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 27 Jan 2022 21:15:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Lance Zhao
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-MessageType: comment