coreboot-gerrit April 2021

coreboot-gerrit@coreboot.org

1 participants
6269 discussions

Change in coreboot[master]: soc/amd/cezanne: Set Power state after power failure
by Karthik Ramasubramanian (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52049 ) Change subject: soc/amd/cezanne: Set Power state after power failure ...................................................................... Patch Set 1: (1 comment) File src/soc/amd/cezanne/bootblock.c: https://review.coreboot.org/c/coreboot/+/52049/comment/81563294_1f5de288 PS1, Line 115: fch_early_init > Is this something we can put in fch_early_init? Makes sense to move it into fch_early_init since the concerned register is part of FCH::PM register space. I will move there in the follow-up CL. -- To view, visit https://review.coreboot.org/c/coreboot/+/52049 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21c5da08c82156d6239450ef6921771da74cbaa1 Gerrit-Change-Number: 52049 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> 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-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 02 Apr 2021 15:52:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common: Handle power resume after power failure
by Karthik Ramasubramanian (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Furquan Shaikh, Martin Roth, Marshall Dawson, Felix Held. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51924 ) Change subject: soc/amd/common: Handle power resume after power failure ...................................................................... Patch Set 2: (2 comments) File src/soc/amd/common/block/pm/pmlib.c: https://review.coreboot.org/c/coreboot/+/51924/comment/656fb33a_10904592 PS2, Line 21: get_option > Do we have a cmos_layout.bin? […] I can defer this i.e. defer using option table until we define the cmos_layout.bin. Anyways we are not using option table from the quick grep. So removing this does not change the flow. https://review.coreboot.org/c/coreboot/+/51924/comment/d41311dd_f48fcdaf PS2, Line 42: PM_RTC_SHADOW_REG > Writing to these four bits will set the value onto bits [7:4]. Software […] In the PPR, there is a mention for BIOS to set it to 4 i.e. set bit 2. Also Marshall and Felix commented about the same in the earlier patchsets. -- To view, visit https://review.coreboot.org/c/coreboot/+/51924 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea4ea57d747425fe6714d40ba6e60f2447febf28 Gerrit-Change-Number: 51924 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.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)users.sourceforge.net> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> 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: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 02 Apr 2021 15:48:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Add Bluetooth configuration
by Mathew King (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Martin Roth, Mathew King, Bhanu Prakash Maiya, Karthik Ramasubramanian. Mathew King has uploaded a new patch set (#5) to the change originally created by Karthik Ramasubramanian. ( https://review.coreboot.org/c/coreboot/+/51963 ) Change subject: mb/google/guybrush: Add Bluetooth configuration ...................................................................... mb/google/guybrush: Add Bluetooth configuration Configure the BT disable GPIO to logic low in order to enable Bluetooth. Add USB ACPI configuration for BT device. BUG=b:182201890 TEST=Build and boot to OS. Change-Id: I647c301e2db6d4a7c5c8cb31cbc47a44cba5e734 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb M src/mainboard/google/guybrush/variants/baseboard/gpio.c 2 files changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/51963/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/51963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I647c301e2db6d4a7c5c8cb31cbc47a44cba5e734 Gerrit-Change-Number: 51963 Gerrit-PatchSet: 5 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Bhanu Prakash Maiya <bhanumaiya(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-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Mathew King <mathewk(a)chromium.org> Gerrit-Attention: Bhanu Prakash Maiya <bhanumaiya(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: coreboot_tables: Print strapping IDs when adding them to coreboot table
by Tim Wawrzynczak (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Furquan Shaikh, Julius Werner. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52011 ) Change subject: coreboot_tables: Print strapping IDs when adding them to coreboot table ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifdbfdd29d25a0937c27113ace776f7aec231a57d Gerrit-Change-Number: 52011 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Apr 2021 15:22:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Enable VT-d
by Tim Wawrzynczak (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Furquan Shaikh, Maulik V Vaghela, Subrata Banik, Meera Ravindranath, Ronak Kanabar, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51849 ) Change subject: soc/intel/alderlake: Enable VT-d ...................................................................... Patch Set 5: (1 comment) File src/soc/intel/alderlake/romstage/fsp_params.c: https://review.coreboot.org/c/coreboot/+/51849/comment/76673806_f3abf3d4 PS5, Line 204: #if (GFXVT_BASE_ADDRESS == 0) : #error "Error: GFXVT_BASE_ADDRESS should be non-zero for enabling VT-d!" : #endif > @Meera, you can add this check at stating of the file, no need to do inside exact code if you want Sure, so if you check that the user wants VT-d enabled (by checking that `InternalGfx == 1`), and you still find the base address is 0, then just set VtdIgdEnable to 0 and print a warning, then there's no hang, can do the same for the other Vtd options. These BARs are defined in soc/iomap.h, the odds of them getting redefined to 0 seems small to me? -- To view, visit https://review.coreboot.org/c/coreboot/+/51849 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I96a9f3df185002a4e58faa910f867ace0b97ec2b Gerrit-Change-Number: 51849 Gerrit-PatchSet: 5 Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 02 Apr 2021 15:21:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Documentation/mb/ocp: Update Delta Lake documentation
by Jonathan Zhang (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Marc Jones, Anjaneya "Reddy" Chagam, Johnny Lin, Paul Menzel, Stefan Reinauer, Angel Pons, ron minnich, Tim Chu. Hello build bot (Jenkins), Marc Jones, Anjaneya "Reddy" Chagam, Johnny Lin, Christian Walter, Stefan Reinauer, Angel Pons, ron minnich, Tim Chu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51824 to look at the new patch set (#6). Change subject: Documentation/mb/ocp: Update Delta Lake documentation ...................................................................... Documentation/mb/ocp: Update Delta Lake documentation Update OCP Delta Lake documentation following OSF (Open System Firmware) solution reaching DVT exit parity. This alternative host firmware solution uses FSP/coreboot/Linuxboot stack. Signed-off-by: Jonathan Zhang <jonzhang(a)fb.com> Change-Id: Ifd6ab251cd7806cf8cd3f984ad88c091f85035cf --- M Documentation/mainboard/ocp/deltalake.md 1 file changed, 38 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/51824/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/51824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd6ab251cd7806cf8cd3f984ad88c091f85035cf Gerrit-Change-Number: 51824 Gerrit-PatchSet: 6 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: Documentation/mb/ocp: Update Delta Lake documentation
by Jonathan Zhang (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Marc Jones, Anjaneya "Reddy" Chagam, Johnny Lin, Paul Menzel, Stefan Reinauer, Angel Pons, ron minnich, Tim Chu. Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51824 ) Change subject: Documentation/mb/ocp: Update Delta Lake documentation ...................................................................... Patch Set 5: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/51824/comment/0479c4de_e705dbb5 PS5, Line 10: OSF (Open System Firmware) solution. > I have no idea, what the second part means. […] Done File Documentation/mainboard/ocp/deltalake.md: https://review.coreboot.org/c/coreboot/+/51824/comment/0b82c016_382f5093 PS5, Line 31: Intel. > Or even move it to the previous line (it should fit) Done https://review.coreboot.org/c/coreboot/+/51824/comment/8aee1d76_8f71dae0 PS5, Line 66: mapped to verbose, 0 to 4 and 9 would be mapped to quiet. > Use two spaces? Done https://review.coreboot.org/c/coreboot/+/51824/comment/2633f288_3704ca29 PS5, Line 103: - KM/BPM signing > Yeah, I'd use 8 spaces Done https://review.coreboot.org/c/coreboot/+/51824/comment/5beee6d8_d11fa9cc PS5, Line 151: CLTT > CLTT = Closed Loop Thermal Throttling? I'd add the full form in parentheses: […] Done https://review.coreboot.org/c/coreboot/+/51824/comment/db03c1e1_a9afefb7 PS5, Line 152: - ProcHot > PROCHOT# is a signal that is used to report "Processor Hot", and activate thermal protections. […] CLTT is thermal protection for DIMM modules, PROCHOT is thermal protection for processor. There are thermal sensors on them. When overheating is detected, the DIMM/processor frequency will be reduced to protect the hardware. -- To view, visit https://review.coreboot.org/c/coreboot/+/51824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd6ab251cd7806cf8cd3f984ad88c091f85035cf Gerrit-Change-Number: 51824 Gerrit-PatchSet: 5 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 02 Apr 2021 15:16:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT
by Tim Wawrzynczak (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Nico Huber, Furquan Shaikh, Patrick Rudolph. Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51159 to look at the new patch set (#8). Change subject: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT ...................................................................... soc/intel/common/block/irq: Add support for intel_write_pci0_PRT Add a new function to fill out the data structures necessary to generate a _PRT table. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I21a4835890ca03bff83ed0e8791441b3af54cb62 --- M src/soc/intel/common/block/include/intelblocks/irq.h M src/soc/intel/common/block/irq/irq.c 2 files changed, 39 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/51159/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/51159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21a4835890ca03bff83ed0e8791441b3af54cb62 Gerrit-Change-Number: 51159 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT
by Tim Wawrzynczak (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Nico Huber, Furquan Shaikh, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51159 ) Change subject: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT ...................................................................... Patch Set 7: (1 comment) File src/soc/intel/common/block/irq/irq.c: https://review.coreboot.org/c/coreboot/+/51159/comment/4508ff45_f1b3e5a8 PS7, Line 378: /* Map INTA->PIRQ_A, INTB->PIRQ_B, INTC->PIRQ_C, INTD->PIRQ_D */ > This is tying a knot in my head... it can only be true for […] Sorry I can make the comment a little more clear, this is how Intel does the mapping for PIC-mode IRQs (IRQ11 for INTA,INTC,INTD, and IRQ10 for INTB). Take another look at the CB:50857, `pic_pirq` is eventually used as an index into `pirq_map->{gsi,source_path}` for the PIC-mode table. The global_least_used_pirq is used for APIC-mode. However, about your theory, I think that it is valid, I have not 100% confirmed it, but I believe that the "unique IRQ" devices just won't work correctly in legacy mode. The only reason I know of that anyone would care about the PIC-mode IRQs is for, e.g., testing `noapic` in Linux? I really don't know of a good argument to keep the legacy PIC mappings around anymore... not for this patch, but should be investigated and possibly removed if it's just not useful anymore. -- To view, visit https://review.coreboot.org/c/coreboot/+/51159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21a4835890ca03bff83ed0e8791441b3af54cb62 Gerrit-Change-Number: 51159 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 02 Apr 2021 15:14:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: sb/intel/common: Refactor _PRT generation to support GSI-based tables
by Tim Wawrzynczak (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Nico Huber, Angel Pons, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50857 ) Change subject: sb/intel/common: Refactor _PRT generation to support GSI-based tables ...................................................................... Patch Set 15: (9 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/50857/comment/39d406d3_170b8280 PS14, Line 10: insteade > Nit, *instead* Done. Patchset: PS14: > Sorry for leaving this unattended for long. Ping me to test […] Agreed, I am hesitant to submit without testing it on at least 1 of the older platforms. If this is broken, the OS will have a lot of trouble (experienced this plenty of times while this was in development...) I need to get my hands on some of these older mainboards... File src/southbridge/intel/common/acpi_pirq_gen.h: https://review.coreboot.org/c/coreboot/+/50857/comment/e0a59bb8_714346ac PS14, Line 28: PIRQ_MAX = PIRQ_H, > With `PIRQ_NONE` removed, this is the last valid index now. Ack, added PIRQ_COUNT instead of PIRQ_MAX. https://review.coreboot.org/c/coreboot/+/50857/comment/9bd087cf_8ea1d6ec PS14, Line 47: dveices > *devices* Done https://review.coreboot.org/c/coreboot/+/50857/comment/163f671a_c3d246cf PS14, Line 69: PIRQ_MAX > PIRQ_MAX is the last valid index, not the count of PIRQs. Ouch, that's a good catch Nico! Will fix this. https://review.coreboot.org/c/coreboot/+/50857/comment/448cf277_7ce6b748 PS14, Line 69: unsigned int gsi[PIRQ_MAX]; : char source_path[PIRQ_MAX][DEVICE_PATH_MAX]; > This could be a union, just to emphasize that they are mutually exclusive. That's a good hint to the user, will modify. https://review.coreboot.org/c/coreboot/+/50857/comment/ee318c3e_259e8bb9 PS14, Line 79: num_devs > This made me think for a second what it's about. I suppose it's […] You're right, it's map entries (or pins, that's valid too), will rename it. File src/southbridge/intel/common/acpi_pirq_gen.c: https://review.coreboot.org/c/coreboot/+/50857/comment/9beab3c4_e95f2952 PS14, Line 58: 16 > This little number is nowhere to be found in the new implementation. Good catch! related to your other comment about apic_gsi ;) File src/southbridge/intel/common/rcba_pirq.c: https://review.coreboot.org/c/coreboot/+/50857/comment/2e3aea70_4ab887e3 PS14, Line 85: pin_irq_map[num_devs].pic_pirq = map_pirq(dev, int_pin); > I don't think gen_apic_route() would be too happy if we leave . […] Whoops this is where that little `16` was supposed to go 😄 -- To view, visit https://review.coreboot.org/c/coreboot/+/50857 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica420a3d12fd1d64c8fe6e4b326fd779b3f10868 Gerrit-Change-Number: 50857 Gerrit-PatchSet: 15 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 02 Apr 2021 15:09:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2021