coreboot-gerrit June 2021

coreboot-gerrit@coreboot.org

1 participants
7127 discussions

Change in coreboot[master]: soc/intel/tigerlake: enable ACPI S0ix DSM for Intel PEP
by Cliff Huang (Code Review) 03 Jun '21

03 Jun '21

Cliff Huang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55151 ) Change subject: soc/intel/tigerlake: enable ACPI S0ix DSM for Intel PEP ...................................................................... soc/intel/tigerlake: enable ACPI S0ix DSM for Intel PEP This is to enable S0ix device specific method _DSM ( UUID: 57a6512e-3979-4e9d-9708-ff13b2508972) for Intel Power Engine Plug-in. Alone with this change, one coreboot and two kernel changes are also required: https://review.coreboot.org/c/coreboot/+/55127 https://chromium-review.googlesource.com/2800280 https://chromium-review.googlesource.com/2800281 Once done, substate_requirement_registers is created under /sys/kernel/debug/pmc_core/ Use: 'cat /sys/kernel/debug/pmc_core/substate_requirement_registers' to check the content. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I87fa55e4003e789d55ddce11d1f76e9ca8b08f18 --- M src/soc/intel/tigerlake/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/55151/1 diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index 0c472d9..2e6034b 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -45,6 +45,7 @@ select SOC_INTEL_COMMON_BLOCK_ACPI select SOC_INTEL_COMMON_BLOCK_ACPI_GPIO select SOC_INTEL_COMMON_BLOCK_ACPI_LPIT + select SOC_INTEL_COMMON_BLOCK_ACPI_PEP_S0IX select SOC_INTEL_COMMON_BLOCK_CAR select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CNVI -- To view, visit https://review.coreboot.org/c/coreboot/+/55151 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I87fa55e4003e789d55ddce11d1f76e9ca8b08f18 Gerrit-Change-Number: 55151 Gerrit-PatchSet: 1 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/alderlake: enable ACPI S0ix DSM for Intel PEP
by Cliff Huang (Code Review) 03 Jun '21

03 Jun '21

Attention is currently required from: Cliff Huang, Tim Wawrzynczak, Patrick Rudolph. Hello build bot (Jenkins), Tim Wawrzynczak, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55150 to look at the new patch set (#2). Change subject: soc/intel/alderlake: enable ACPI S0ix DSM for Intel PEP ...................................................................... soc/intel/alderlake: enable ACPI S0ix DSM for Intel PEP This is to enable S0ix device specific method _DSM ( UUID: 57a6512e-3979-4e9d-9708-ff13b2508972) for Intel Power Engine Plug-in. Alone with this change, one coreboot and two kernel changes are also required: https://review.coreboot.org/c/coreboot/+/55127 https://chromium-review.googlesource.com/2800280 https://chromium-review.googlesource.com/2800281 Once done, substate_requirement_registers is created under /sys/kernel/debug/pmc_core/ Use: 'cat /sys/kernel/debug/pmc_core/substate_requirement_registers' to check the content. BUG=b:185437326 brya: _DSM method need to implemented in coreboot for PMC requirement register. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I127c695eed9e2842996381c7559695f289cf4585 --- M src/soc/intel/alderlake/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/55150/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55150 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I127c695eed9e2842996381c7559695f289cf4585 Gerrit-Change-Number: 55150 Gerrit-PatchSet: 2 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/alderlake: enable ACPI S0ix DSM for Intel PEP
by Cliff Huang (Code Review) 02 Jun '21

02 Jun '21

Cliff Huang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55150 ) Change subject: soc/intel/alderlake: enable ACPI S0ix DSM for Intel PEP ...................................................................... soc/intel/alderlake: enable ACPI S0ix DSM for Intel PEP This is to enable S0ix device specific method _DSM ( UUID: 57a6512e-3979-4e9d-9708-ff13b2508972) for Intel Power Engine Plug-in. Alone with this change, one coreboot and two kernel changes are also required: https://review.coreboot.org/c/coreboot/+/55127 https://chromium-review.googlesource.com/2800280 https://chromium-review.googlesource.com/2800281 Once done, substate_requirement_registers is created under /sys/kernel/debug/pmc_core/ Use: 'cat /sys/kernel/debug/pmc_core/substate_requirement_registers' to check the content. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I127c695eed9e2842996381c7559695f289cf4585 --- M src/soc/intel/alderlake/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/55150/1 diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index 79e775d..eb62b11 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -80,6 +80,7 @@ select UDK_202005_BINDING select DISPLAY_FSP_VERSION_INFO select HECI_DISABLE_USING_SMM + select SOC_INTEL_COMMON_PEP_S0IX config MAX_CPUS int -- To view, visit https://review.coreboot.org/c/coreboot/+/55150 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I127c695eed9e2842996381c7559695f289cf4585 Gerrit-Change-Number: 55150 Gerrit-PatchSet: 1 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/guybrush: Add helpers for cbi fw_config settings.
by Martin Roth (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Raul Rangel, Furquan Shaikh, Paul Menzel, Karthik Ramasubramanian. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54639 ) Change subject: mb/google/guybrush: Add helpers for cbi fw_config settings. ...................................................................... Patch Set 5: (1 comment) File src/mainboard/google/guybrush/variants/guybrush/helpers.c: https://review.coreboot.org/c/coreboot/+/54639/comment/5ce91136_0f91dc28 PS4, Line 44: get_variant_wwan_type > fw_config is used to allow OEMs to provide configuration information […] My counter proposal to your change is this: - Each variant still supplies the option bit definitions as we do now. - All fw_config settings are defined in devicetree at a platform level - not about which bits are used for the settings, just having a definition so that it can be accessed by all variants. - All variants are expected to use the same names for things that exist. This isn't strictly required, but would make things less confusing - The fw configs bits can be defined as "always present", so the cbi FP_CONFIG doesn't even need to be checked, - The top level helpers can just check fw_config for anything they need to, and it's expected that they'd get back a response. If the config option doesn't exist on that platform, it's just returned as it's not enabled. Looking at your example of the fingerprint on guybrush: 1) OEMs with no fingerprint support 2) OEMs with all SKUs having fingerprint support 3) OEMs with some SKUs having fingerprint support whereas others with no fingerprint support Option 1: The FP option isn't even included in the variant devicetree - always returns false Option 2: The FP option is set to ENABLED in the variant devicetree - always returns true Option 3: The FP option is actually in the CBI, which is checked to see if the option is enabled or disabled. This solution doesn't require that OEMs define fields that they don't care about. This solution doesn't add any significant overhead. This solution uses the current flow, and doesn't require that platforms be changed significantly. The values can be obtained and acted upon without having to find or define the devicetree structure. I have no objection to your solution for ramstage and determining if a device in a platform is enabled - It seems like a nice way to do it. My issue with your solution is that it seems heavier than is needed in bootblock/romstage and doesn't give any benefit there. There are other ways that are less intrusive that it can be done. What I've proposed above doesn't require that we throw away all of the experience we have about this right now - it's an almost trivial expansion on what we already have. -- To view, visit https://review.coreboot.org/c/coreboot/+/54639 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I212e7f413b4d8a7d15122cde90100a0ec28e88a6 Gerrit-Change-Number: 54639 Gerrit-PatchSet: 5 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 23:40:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Override I2C3 pad configuration
by Karthik Ramasubramanian (Code Review) 02 Jun '21

02 Jun '21

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55149 ) Change subject: mb/google/guybrush: Override I2C3 pad configuration ...................................................................... mb/google/guybrush: Override I2C3 pad configuration I2C bus to Google Security Chip (GSC) should be at 1.8V level. By default, all the I2C buses are initialized to operate at 3.3 V. Override the I2C3 pad control configuration. BUG=b:188538373 TEST=Build and boot to OS in Guybrush. Ensure that the communication with GSC is fine. Change-Id: I595a64736fdac0274abffb68c5e521302275b845 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/guybrush/Makefile.inc A src/mainboard/google/guybrush/i2c.c 2 files changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/55149/1 diff --git a/src/mainboard/google/guybrush/Makefile.inc b/src/mainboard/google/guybrush/Makefile.inc index d4eeaf5..c57b14a 100644 --- a/src/mainboard/google/guybrush/Makefile.inc +++ b/src/mainboard/google/guybrush/Makefile.inc @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later bootblock-y += bootblock.c +bootblock-y += i2c.c ifneq ($(wildcard $(MAINBOARD_BLOBS_DIR)/APCB_CZN_D4.bin),) $(info APCB sources present.) @@ -15,6 +16,7 @@ ramstage-y += mainboard.c ramstage-y += ec.c ramstage-$(CONFIG_CHROMEOS) += chromeos.c +ramstage-y += i2c.c verstage-y += verstage.c diff --git a/src/mainboard/google/guybrush/i2c.c b/src/mainboard/google/guybrush/i2c.c new file mode 100644 index 0000000..b2ec4f2 --- /dev/null +++ b/src/mainboard/google/guybrush/i2c.c @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <soc/i2c.h> +#include <soc/southbridge.h> + +#define GSC_I2C_BUS 3 + +void mainboard_i2c_override(int bus, uint32_t *pad_ctrl) +{ + if (bus != GSC_I2C_BUS) + return; + + *pad_ctrl &= ~I2C_PAD_CTRL_RX_SEL_MASK; + *pad_ctrl |= I2C_PAD_CTRL_RX_SEL_1_8V; +} -- To view, visit https://review.coreboot.org/c/coreboot/+/55149 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I595a64736fdac0274abffb68c5e521302275b845 Gerrit-Change-Number: 55149 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/guybrush/var/guybrush: Update GPIO configuration
by Karthik Ramasubramanian (Code Review) 02 Jun '21

02 Jun '21

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55148 ) Change subject: mb/google/guybrush/var/guybrush: Update GPIO configuration ...................................................................... mb/google/guybrush/var/guybrush: Update GPIO configuration Some of the GPIOs are either re-purposed for different use-cases or are unused in upcoming board phase (board version 2). Update the GPIO configuration accordingly. Here are the GPIOs that are updated: GPIO Board Id 1 Board Id 2 ============================================= GPIO31 TP183 EN_SPKR GPIO69 EN_SPKR SD_AUX_REST_L GPIO70 SD_AUX_RESET_L Unused TP27 GPIO74 RAM_ID_CHAN_SEL Unused TP49 BUG=b:189327557, b:188542649, b:188542497 TEST=Build Guybrush mainboard. Change-Id: I31523b3e03d2b59577f33eae548747834cfc98aa Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/guybrush/variants/baseboard/gpio.c M src/mainboard/google/guybrush/variants/guybrush/gpio.c 2 files changed, 12 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/55148/1 diff --git a/src/mainboard/google/guybrush/variants/baseboard/gpio.c b/src/mainboard/google/guybrush/variants/baseboard/gpio.c index c89ded2..af19e50 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/gpio.c +++ b/src/mainboard/google/guybrush/variants/baseboard/gpio.c @@ -64,8 +64,8 @@ PAD_GPO(GPIO_29, LOW), /* ESPI_CS_L */ PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), - /* SPI_CS3_L */ - PAD_NF(GPIO_31, SPI_CS3_L, PULL_NONE), + /* EN_SPKR */ + PAD_GPO(GPIO_31, HIGH), /* EN_PWR_FP */ PAD_GPO(GPIO_32, HIGH), /* GPIO_33 - GPIO_39: Not available */ @@ -79,13 +79,11 @@ PAD_GPI(GPIO_67, PULL_NONE), /* EN_PP3300_TCHSCR */ PAD_GPO(GPIO_68, LOW), - /* EN_SPKR */ - PAD_GPO(GPIO_69, HIGH), /* SD_AUX_RESET_L */ - PAD_GPO(GPIO_70, HIGH), + PAD_GPO(GPIO_69, HIGH), + /* GPIO_70 Unused TP27 */ /* GPIO_71 - GPIO_73: Not available */ - /* RAM_ID_CHAN_SEL */ - PAD_GPI(GPIO_74, PULL_NONE), + /* GPIO_74 Unused TP49 */ /* RAM_ID_2 / DEV_BEEP_LRCLK */ PAD_GPI(GPIO_75, PULL_NONE), /* EN_PP3300_CAM */ diff --git a/src/mainboard/google/guybrush/variants/guybrush/gpio.c b/src/mainboard/google/guybrush/variants/guybrush/gpio.c index 64afa60..3ca77f7 100644 --- a/src/mainboard/google/guybrush/variants/guybrush/gpio.c +++ b/src/mainboard/google/guybrush/variants/guybrush/gpio.c @@ -8,8 +8,14 @@ /* This table is used by guybrush variant with board version < 2. */ static const struct soc_amd_gpio bid1_gpio_table[] = { + /* SPI_CS3_L */ + PAD_NF(GPIO_31, SPI_CS3_L, PULL_NONE), /* EN_SPKR */ - PAD_GPO(GPIO_69, LOW), + PAD_GPO(GPIO_69, HIGH), + /* SD_AUX_RESET_L */ + PAD_GPO(GPIO_70, HIGH), + /* RAM_ID_CHAN_SEL */ + PAD_GPI(GPIO_74, PULL_NONE), }; const struct soc_amd_gpio *variant_override_gpio_table(size_t *size) -- To view, visit https://review.coreboot.org/c/coreboot/+/55148 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31523b3e03d2b59577f33eae548747834cfc98aa Gerrit-Change-Number: 55148 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/brya/brya0: Fix irq and CS lines for FPMCU
by Tim Wawrzynczak (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: EricR Lai. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55147 ) Change subject: mb/google/brya/brya0: Fix irq and CS lines for FPMCU ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55147/comment/e7b5cf83_91b254e1 PS1, Line 13: (0 > unrelated? oops -- To view, visit https://review.coreboot.org/c/coreboot/+/55147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ef6c99f011969fc444e0c12b806529cb82bba3d Gerrit-Change-Number: 55147 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 22:54:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/brya0: Fix irq and CS lines for FPMCU
by Tim Wawrzynczak (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Tim Wawrzynczak, EricR Lai. Hello build bot (Jenkins), Furquan Shaikh, EricR Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55147 to look at the new patch set (#2). Change subject: mb/google/brya/brya0: Fix irq and CS lines for FPMCU ...................................................................... mb/google/brya/brya0: Fix irq and CS lines for FPMCU The entries in the ACPI tables for the fingerprint module's SPI configuration were incorrect. 1) The GPIO is routed to IOAPIC (and SCI), therefore in ACPI, it must be described by Interrupt(), not GpioInt() 2) The chip-select signal was selected as 1, not 0 `device spi 0/1 on` BUG=b:181635081 TEST=verified in kernel logs: localhost # ~ dmesg|egrep 'cros-ec-dev|cros-ec-spi' [ 4.569412] cros-ec-dev cros-ec-dev.1.auto: CrOS Fingerprint MCU detected [ 4.575303] cros-ec-spi spi-PRP0001:00: Chrome EC device registered Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I9ef6c99f011969fc444e0c12b806529cb82bba3d --- M src/mainboard/google/brya/variants/brya0/overridetree.cb 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/55147/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ef6c99f011969fc444e0c12b806529cb82bba3d Gerrit-Change-Number: 55147 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/brya/brya0: Fix irq and CS lines for FPMCU
by Furquan Shaikh (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Tim Wawrzynczak, EricR Lai. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55147 ) Change subject: mb/google/brya/brya0: Fix irq and CS lines for FPMCU ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55147/comment/cbdbf0cc_6d32b8df PS1, Line 13: (0 unrelated? -- To view, visit https://review.coreboot.org/c/coreboot/+/55147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ef6c99f011969fc444e0c12b806529cb82bba3d Gerrit-Change-Number: 55147 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Wed, 02 Jun 2021 22:52:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/pcie/rtd3/device: Add PCIe RTD3 driver
by Raul Rangel (Code Review) 02 Jun '21

02 Jun '21

Attention is currently required from: Jason Glenesk, Furquan Shaikh, Marshall Dawson, Tim Wawrzynczak, Karthik Ramasubramanian, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54966 ) Change subject: drivers/pcie/rtd3/device: Add PCIe RTD3 driver ...................................................................... Patch Set 4: (1 comment) File src/soc/amd/common/block/rtd3/rtd3.c: https://review.coreboot.org/c/coreboot/+/54966/comment/bb2d53cb_f8199ab1 PS1, Line 210: pci_dev_read_resources > +1 I like it So with this approach, the rtd3 driver doesn't write the actual Device node, but just writes a PowerResource into the device's scope. acpigen_write_scope(acpi_device_path(config->target)); acpi_device_add_power_res(&power_res_params); ... acpigen_write_scope_end(); Ideally the default pci opts would have acpi_device_write_pci_dev so the Device node gets written... but we can't really do that since the device could be defined in the DSDT. I'm not sure what the best way to approach this is... suggestions? -- To view, visit https://review.coreboot.org/c/coreboot/+/54966 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2adfc925183ff7a19ab97e89212bc87c40d552d0 Gerrit-Change-Number: 54966 Gerrit-PatchSet: 4 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Duncan Laurie <dlaurie(a)gmail.com> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Mario Limonciello <mario.limonciello(a)amd.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 02 Jun 2021 22:45:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2021