coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: mb/intel/adlrvp_m: Enable CR50 TPM support over SPI
by Anil Kumar K (Code Review) 10 May '21

10 May '21

Attention is currently required from: Bora Guvendik, Cliff Huang, Selma Bensaid, Bernardo Perez Priego, Thejaswani Putta. Anil Kumar K has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54023 ) Change subject: mb/intel/adlrvp_m: Enable CR50 TPM support over SPI ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: commit message. Bug=None Add Hashtag "SYSCROS-57029" -- To view, visit https://review.coreboot.org/c/coreboot/+/54023 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icaedf9f17e35e82c35cbabd6d2938c167e42e9e8 Gerrit-Change-Number: 54023 Gerrit-PatchSet: 1 Gerrit-Owner: Thejaswani Putta <thejaswani.putta(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Thejaswani Putta <thejaswani.putta(a)intel.com> Gerrit-Comment-Date: Mon, 10 May 2021 21:19:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: enable DPTF functionality for brya
by Tim Wawrzynczak (Code Review) 10 May '21

10 May '21

Attention is currently required from: Furquan Shaikh, Sumeet R Pawnikar. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52859 ) Change subject: mb/google/brya: enable DPTF functionality for brya ...................................................................... Patch Set 2: Code-Review+1 (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/52859/comment/5a9772fd_567c586f PS2, Line 13: tested What all did you test? File src/mainboard/google/brya/variants/brya0/overridetree.cb: https://review.coreboot.org/c/coreboot/+/52859/comment/27e8360d_a0a401d8 PS2, Line 11: # Default DPTF Policy for all Brya boards if not overridden This comment is not true anymore (this isn't the baseboard) https://review.coreboot.org/c/coreboot/+/52859/comment/62b62871_68396120 PS2, Line 23: nit: remove a few of these tabs? https://review.coreboot.org/c/coreboot/+/52859/comment/09520a48_2ce1c39b PS2, Line 12: ## sensor information : register "options.tsr[0].desc" = ""DRAM"" : register "options.tsr[1].desc" = ""Charger"" : register "options.tsr[2].desc" = ""Ambient"" : : # TODO: below values are initial reference values only : ## Active Policy : register "policies.active" = "{ : [0] = { : .target = DPTF_CPU, : .thresholds = { : TEMP_PCT(85, 90), : TEMP_PCT(80, 80), : TEMP_PCT(75, 70), : } : }, : [1] = { : .target = DPTF_TEMP_SENSOR_2, : .thresholds = { : TEMP_PCT(70, 90), : TEMP_PCT(60, 80), : TEMP_PCT(50, 70), : } : } : }" : : ## Passive Policy : register "policies.passive" = "{ : [0] = DPTF_PASSIVE(CPU, CPU, 95, 5000), : [1] = DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 75, 5000), : [2] = DPTF_PASSIVE(CHARGER, TEMP_SENSOR_1, 75, 5000), : [3] = DPTF_PASSIVE(CPU, TEMP_SENSOR_2, 75, 5000) : }" : : ## Critical Policy : register "policies.critical" = "{ : [0] = DPTF_CRITICAL(CPU, 105, SHUTDOWN), : [1] = DPTF_CRITICAL(TEMP_SENSOR_0, 85, SHUTDOWN), : [2] = DPTF_CRITICAL(TEMP_SENSOR_1, 85, SHUTDOWN), : [3] = DPTF_CRITICAL(TEMP_SENSOR_2, 85, SHUTDOWN) : }" : : register "controls.power_limits" = "{ : .pl1 = { : .min_power = 3000, : .max_power = 15000, : .time_window_min = 28 * MSECS_PER_SEC, : .time_window_max = 32 * MSECS_PER_SEC, : .granularity = 200, : }, : .pl2 = { : .min_power = 55000, : .max_power = 55000, : .time_window_min = 28 * MSECS_PER_SEC, : .time_window_max = 32 * MSECS_PER_SEC, : .granularity = 1000, : } : }" : : ## Charger Performance Control (Control, mA) : register "controls.charger_perf" = "{ : [0] = { 255, 1700 }, : [1] = { 24, 1500 }, : [2] = { 16, 1000 }, : [3] = { 8, 500 } : }" : : ## Fan Performance Control (Percent, Speed, Noise, Power) : register "controls.fan_perf" = "{ : [0] = { 90, 6700, 220, 2200, }, : [1] = { 80, 5800, 180, 1800, }, : [2] = { 70, 5000, 145, 1450, }, : [3] = { 60, 4900, 115, 1150, }, : [4] = { 50, 3838, 90, 900, }, : [5] = { 40, 2904, 55, 550, }, : [6] = { 30, 2337, 30, 300, }, : [7] = { 20, 1608, 15, 150, }, : [8] = { 10, 800, 10, 100, }, : [9] = { 0, 0, 0, 50, } : }" : : ## Fan options : register "options.fan.fine_grained_control" = "1" : register "options.fan.step_size" = "2" : : device generic 0 on end indent this one more level -- To view, visit https://review.coreboot.org/c/coreboot/+/52859 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I33266c85aa30869466034ccbab04a3c7820ae2b0 Gerrit-Change-Number: 52859 Gerrit-PatchSet: 2 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alan Lee <alan_lee(a)compal.corp-partner.google.com> Gerrit-CC: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-CC: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Gerrit-CC: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Comment-Date: Mon, 10 May 2021 21:19:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Configure wake resource for WiFi
by Karthik Ramasubramanian (Code Review) 10 May '21

10 May '21

Attention is currently required from: Raul Rangel, Furquan Shaikh, Martin Roth, Mathew King. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52908 ) Change subject: mb/google/guybrush: Configure wake resource for WiFi ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52908/comment/3e293ff9_a638198d PS1, Line 13: is added to SSDT. > At what point did you check it? IIUC, it gets set on the suspend path in kernel. Checked it in the resume path (as part of event log SMI handler and logged in the eventlog). > Did you probe the WAKE# line in hardware? Does that toggle at all? I shorted the Wake# pin to the ground explicitly and see if that wakes-up the system. But the system still did not wake up. -- To view, visit https://review.coreboot.org/c/coreboot/+/52908 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic238d9606aea20c058e9b47093693f10b14e6288 Gerrit-Change-Number: 52908 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mathew King <mathewk(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Mathew King <mathewk(a)chromium.org> Gerrit-Comment-Date: Mon, 10 May 2021 21:18:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/i2c/cs42l42: Make HS_BIAS_SENSE_EN optional
by Angel Pons (Code Review) 10 May '21

10 May '21

Attention is currently required from: Furquan Shaikh, Vitaly Rodionov, Marco Chen, Karthik Ramasubramanian. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52981 ) Change subject: drivers/i2c/cs42l42: Make HS_BIAS_SENSE_EN optional ...................................................................... Patch Set 4: (1 comment) File src/drivers/i2c/cs42l42/chip.h: https://review.coreboot.org/c/coreboot/+/52981/comment/83c2b0f2_ad93012a PS4, Line 118: Default = > I think it would be helpful to have a comment before this struct (on lines 36-37) saying that the "D […] In this case, coreboot will always generate the property, so the kernel default is meaningless. -- To view, visit https://review.coreboot.org/c/coreboot/+/52981 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I87c6f01af1bdb5b1cb8e399191519598d7fbe9ea Gerrit-Change-Number: 52981 Gerrit-PatchSet: 4 Gerrit-Owner: Vitaly Rodionov <vitaly.rodionov(a)cirrus.corp-partner.google.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marco Chen <marcochen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alan Lee <alan_lee(a)compal.corp-partner.google.com> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Vitaly Rodionov <vitaly.rodionov(a)cirrus.corp-partner.google.com> Gerrit-Attention: Marco Chen <marcochen(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 10 May 2021 21:16:05 +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/guybrush: Configure wake resource for WiFi
by Furquan Shaikh (Code Review) 10 May '21

10 May '21

Attention is currently required from: Raul Rangel, Martin Roth, Mathew King, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52908 ) Change subject: mb/google/guybrush: Configure wake resource for WiFi ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52908/comment/9136031f_541f1ed7 PS1, Line 13: is added to SSDT. > I checked the GPE_EN bit and observed that GEVENT_8 is not set/enabled At what point did you check it? IIUC, it gets set on the suspend path in kernel. > Later when the system resumes due to other wake sources, I can see that the bit corresponding to GEVENT_8 status is not set. Did you probe the WAKE# line in hardware? Does that toggle at all? -- To view, visit https://review.coreboot.org/c/coreboot/+/52908 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic238d9606aea20c058e9b47093693f10b14e6288 Gerrit-Change-Number: 52908 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mathew King <mathewk(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Mathew King <mathewk(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 10 May 2021 21:13:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/i2c/cs42l42: Make HS_BIAS_SENSE_EN optional
by Furquan Shaikh (Code Review) 10 May '21

10 May '21

Attention is currently required from: Vitaly Rodionov, Angel Pons, Marco Chen, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52981 ) Change subject: drivers/i2c/cs42l42: Make HS_BIAS_SENSE_EN optional ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52981 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I87c6f01af1bdb5b1cb8e399191519598d7fbe9ea Gerrit-Change-Number: 52981 Gerrit-PatchSet: 4 Gerrit-Owner: Vitaly Rodionov <vitaly.rodionov(a)cirrus.corp-partner.google.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marco Chen <marcochen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alan Lee <alan_lee(a)compal.corp-partner.google.com> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Vitaly Rodionov <vitaly.rodionov(a)cirrus.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Marco Chen <marcochen(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 10 May 2021 21:11:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/i2c/cs42l42: Make HS_BIAS_SENSE_EN optional
by Furquan Shaikh (Code Review) 10 May '21

10 May '21

Attention is currently required from: Vitaly Rodionov, Angel Pons, Marco Chen, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52981 ) Change subject: drivers/i2c/cs42l42: Make HS_BIAS_SENSE_EN optional ...................................................................... Patch Set 4: (1 comment) File src/drivers/i2c/cs42l42/chip.h: https://review.coreboot.org/c/coreboot/+/52981/comment/f0691276_947f0a83 PS4, Line 118: Default = I think it would be helpful to have a comment before this struct (on lines 36-37) saying that the "Default =" really means that the kernel enforces the default if the corresponding field is not present in ACPI table. It can be confusing when reading the comment as to what entity is really enforcing that and I see a similar comment from Angel already. -- To view, visit https://review.coreboot.org/c/coreboot/+/52981 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I87c6f01af1bdb5b1cb8e399191519598d7fbe9ea Gerrit-Change-Number: 52981 Gerrit-PatchSet: 4 Gerrit-Owner: Vitaly Rodionov <vitaly.rodionov(a)cirrus.corp-partner.google.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marco Chen <marcochen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alan Lee <alan_lee(a)compal.corp-partner.google.com> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Vitaly Rodionov <vitaly.rodionov(a)cirrus.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Marco Chen <marcochen(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 10 May 2021 21:11:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Configure wake resource for WiFi
by Karthik Ramasubramanian (Code Review) 10 May '21

10 May '21

Attention is currently required from: Raul Rangel, Furquan Shaikh, Martin Roth, Mathew King. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52908 ) Change subject: mb/google/guybrush: Configure wake resource for WiFi ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52908/comment/5500313c_5bad9738 PS1, Line 13: is added to SSDT. > Can you please try configuring the pad to `PAD_SCI` instead of `PAD_NF_SCI`. Even with PAD_SCI, system was not waking up. > 1. Is the mapping of GPIO_2 to GEVENT_8 looks correct in the OS? Looked at the SCI mapping and looks correct in the OS. Read the SCI mapping register for WAKE_L from the OS and the concerned event map is correct GEVENT_8 > Is the GPE_EN bit set when going into suspend? (probably will have to use S3 for this so that you can dump the state just before suspending). I checked the GPE_EN bit and observed that GEVENT_8 is not set/enabled Here are the GEVENTs enabled: 0x81400028 GEVENT_31 | GEVENT_24 | GEVENT_22 (Trackpad) | GEVENT_5 (FP) | GEVENT_3 (EC) > 3. Is GPE_STS bit set on resume? GPE_STS bit is also not set. I tried to wake first using the WLAN events. Later when the system resumes due to other wake sources, I can see that the bit corresponding to GEVENT_8 status is not set. > 4. Is the behavior same in case of S3 and S0i3? We could not match the behavior with S3 since it is not fully functional yet. -- To view, visit https://review.coreboot.org/c/coreboot/+/52908 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic238d9606aea20c058e9b47093693f10b14e6288 Gerrit-Change-Number: 52908 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mathew King <mathewk(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Mathew King <mathewk(a)chromium.org> Gerrit-Comment-Date: Mon, 10 May 2021 21:09:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/{common,picasso}: Enable PCIE_GPP_DRIVER
by Raul Rangel (Code Review) 10 May '21

10 May '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held. Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/54028 ) Change subject: soc/amd/{common,picasso}: Enable PCIE_GPP_DRIVER ...................................................................... soc/amd/{common,picasso}: Enable PCIE_GPP_DRIVER This will change the names of the GPP bridges, but this ok since there is no hand written ASL that references these names. BUG=b:184766519 TEST=Boot picasso and dump ACPI Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: Ic09200156e8a37bd1a29ca95a17c8f8ae2b92bd3 --- M src/soc/amd/common/block/pci/pcie_gpp.c M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/pcie_gpp.c 3 files changed, 10 insertions(+), 192 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/54028/1 diff --git a/src/soc/amd/common/block/pci/pcie_gpp.c b/src/soc/amd/common/block/pci/pcie_gpp.c index a3ac1cf..a1cc3d2 100644 --- a/src/soc/amd/common/block/pci/pcie_gpp.c +++ b/src/soc/amd/common/block/pci/pcie_gpp.c @@ -59,10 +59,17 @@ .acpi_fill_ssdt = acpi_device_write_gpp_pci_dev, }; +static const unsigned short internal_pci_gpp_ids[] = { + PCI_DEVICE_ID_AMD_FAM17H_MODEL18H_PCIE_GPP_BUSA, + PCI_DEVICE_ID_AMD_FAM17H_MODEL18H_PCIE_GPP_BUSB, + PCI_DEVICE_ID_AMD_FAM17H_MODEL60H_PCIE_GPP_BUSABC, + 0 +}; + static const struct pci_driver internal_pcie_gpp_driver __pci_driver = { .ops = &internal_pcie_gpp_ops, .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_FAM17H_MODEL60H_PCIE_GPP_BUSABC, + .devices = internal_pci_gpp_ids, }; static struct device_operations external_pcie_gpp_ops = { @@ -76,6 +83,7 @@ }; static const unsigned short external_pci_gpp_ids[] = { + PCI_DEVICE_ID_AMD_FAM17H_MODEL18H_PCIE_GPP, PCI_DEVICE_ID_AMD_FAM17H_MODEL60H_PCIE_GPP_D1, PCI_DEVICE_ID_AMD_FAM17H_MODEL60H_PCIE_GPP_D2, 0 diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index e69fb62..daac4d6 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -42,6 +42,7 @@ select SOC_AMD_COMMON_BLOCK_LPC select SOC_AMD_COMMON_BLOCK_NONCAR select SOC_AMD_COMMON_BLOCK_PCI + select SOC_AMD_COMMON_BLOCK_PCIE_GPP_DRIVER select SOC_AMD_COMMON_BLOCK_PM select SOC_AMD_COMMON_BLOCK_PM_CHIPSET_STATE_SAVE select SOC_AMD_COMMON_BLOCK_PSP_GEN2 diff --git a/src/soc/amd/picasso/pcie_gpp.c b/src/soc/amd/picasso/pcie_gpp.c index eabaa77..c46135f 100644 --- a/src/soc/amd/picasso/pcie_gpp.c +++ b/src/soc/amd/picasso/pcie_gpp.c @@ -1,16 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ -#include <acpi/acpigen.h> -#include <acpi/acpigen_pci.h> -#include <arch/ioapic.h> -#include <assert.h> #include <amdblocks/amd_pci_util.h> -#include <device/device.h> -#include <device/pci.h> -#include <device/pciexp.h> -#include <device/pci_ids.h> #include <soc/pci_devs.h> -#include <stdio.h> /* See AMD PPR 55570 - IOAPIC Initialization for the table that AGESA sets up */ const struct pci_routing_info pci_routing_table[] = { @@ -30,185 +21,3 @@ *entries = ARRAY_SIZE(pci_routing_table); return pci_routing_table; } - -static const char *pcie_gpp_acpi_name(const struct device *dev) -{ - if (dev->path.type != DEVICE_PATH_PCI) - return NULL; - - switch (dev->path.pci.devfn) { - case PCIE_GPP_0_DEVFN: - return "PBR0"; - case PCIE_GPP_1_DEVFN: - return "PBR1"; - case PCIE_GPP_2_DEVFN: - return "PBR2"; - case PCIE_GPP_3_DEVFN: - return "PBR3"; - case PCIE_GPP_4_DEVFN: - return "PBR4"; - case PCIE_GPP_5_DEVFN: - return "PBR5"; - case PCIE_GPP_6_DEVFN: - return "PBR6"; - case PCIE_GPP_A_DEVFN: - return "PBRA"; - case PCIE_GPP_B_DEVFN: - return "PBRB"; - } - - return NULL; -} - -/* - * This function writes a PCI device with _ADR, _STA, and _PRT objects: - * Example: - * Scope (\_SB.PCI0) - * { - * Device (PBRA) - * { - * Name (_ADR, 0x0000000000080001) // _ADR: Address - * Method (_STA, 0, NotSerialized) // _STA: Status - * { - * Return (0x0F) - * } - * - * Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table - * { - * If (PICM) - * { - * Return (Package (0x04) - * { - * Package (0x04) - * { - * 0x0000FFFF, - * 0x00, - * 0x00, - * 0x00000034 - * }, - * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x01, - * 0x00, - * 0x00000035 - * }, - * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x02, - * 0x00, - * 0x00000036 - * }, - * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x03, - * 0x00, - * 0x00000037 - * } - * }) - * } - * Else - * { - * Return (Package (0x04) - * { - * Package (0x04) - * { - * 0x0000FFFF, - * 0x00, - * \_SB.INTE, - * 0x00000000 - * }, - * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x01, - * \_SB.INTF, - * 0x00000000 - * }, - * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x02, - * \_SB.INTG, - * 0x00000000 - * }, - * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x03, - * \_SB.INTH, - * 0x00000000 - * } - * }) - * } - * } - * } - * } - */ -static void acpi_device_write_gpp_pci_dev(const struct device *dev) -{ - const char *scope = acpi_device_scope(dev); - const char *name = acpi_device_name(dev); - - assert(dev->path.type == DEVICE_PATH_PCI); - assert(name); - assert(scope); - - acpigen_write_scope(scope); - acpigen_write_device(name); - - acpigen_write_ADR_pci_device(dev); - acpigen_write_STA(acpi_device_status(dev)); - - acpigen_write_pci_GNB_PRT(dev); - - acpigen_pop_len(); /* Device */ - acpigen_pop_len(); /* Scope */ -} - - -static struct device_operations internal_pcie_gpp_ops = { - .read_resources = pci_bus_read_resources, - .set_resources = pci_dev_set_resources, - .enable_resources = pci_bus_enable_resources, - .scan_bus = pci_scan_bridge, - .reset_bus = pci_bus_reset, - .acpi_name = pcie_gpp_acpi_name, - .acpi_fill_ssdt = acpi_device_write_gpp_pci_dev, -}; - -static const unsigned short pci_device_ids[] = { - PCI_DEVICE_ID_AMD_FAM17H_MODEL18H_PCIE_GPP_BUSA, - PCI_DEVICE_ID_AMD_FAM17H_MODEL18H_PCIE_GPP_BUSB, - 0 -}; - -static const struct pci_driver internal_pcie_gpp_driver __pci_driver = { - .ops = &internal_pcie_gpp_ops, - .vendor = PCI_VENDOR_ID_AMD, - .devices = pci_device_ids, -}; - -static struct device_operations external_pcie_gpp_ops = { - .read_resources = pci_bus_read_resources, - .set_resources = pci_dev_set_resources, - .enable_resources = pci_bus_enable_resources, - .scan_bus = pciexp_scan_bridge, - .reset_bus = pci_bus_reset, - .acpi_name = pcie_gpp_acpi_name, - .acpi_fill_ssdt = acpi_device_write_gpp_pci_dev, -}; - -static const struct pci_driver external_pcie_gpp_driver __pci_driver = { - .ops = &external_pcie_gpp_ops, - .vendor = PCI_VENDOR_ID_AMD, - .device = PCI_DEVICE_ID_AMD_FAM17H_MODEL18H_PCIE_GPP, -}; -- To view, visit https://review.coreboot.org/c/coreboot/+/54028 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic09200156e8a37bd1a29ca95a17c8f8ae2b92bd3 Gerrit-Change-Number: 54028 Gerrit-PatchSet: 1 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: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/common/block/pci: Capitalize PCI ACPI names
by Raul Rangel (Code Review) 10 May '21

10 May '21

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021