coreboot-gerrit May 2019

coreboot-gerrit@coreboot.org

1 participants
1485 discussions

Change in ...coreboot[master]: ACPI: Update VT-d DMA remapping structure flags setting
by John Zhao (Code Review) 01 May '19

01 May '19

John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32516 Change subject: ACPI: Update VT-d DMA remapping structure flags setting ...................................................................... ACPI: Update VT-d DMA remapping structure flags setting DMA remapping structure flags settings are Bit 0: INTR_REMAP, Bit 1: X2APIC_OPT_OUT, Bit 2: DMA_CTRL_PLATFORM_OPT_IN_FLAG, Bits 3-7: Reserved (0). BUG=b:130351429 TEST=Image built and kernel booted to kernel. Signed-off-by: John Zhao <john.zhao(a)intel.com> Change-Id: Iaf4764a9062756cadc335d1932b0da8628797f5f --- M src/arch/x86/include/arch/acpi.h 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/32516/1 diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index 1b9438d..79feaad 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -335,9 +335,9 @@ }; enum dmar_flags { - DMAR_INTR_REMAP = 1, - DMAR_X2APIC_OPT_OUT = 2, - DMA_CTRL_PLATFORM_OPT_IN_FLAG = 3, + DMAR_INTR_REMAP = 1 << 0, + DMAR_X2APIC_OPT_OUT = 1 << 1, + DMA_CTRL_PLATFORM_OPT_IN_FLAG = 1 << 2, }; typedef struct dmar_entry { -- To view, visit https://review.coreboot.org/c/coreboot/+/32516 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaf4764a9062756cadc335d1932b0da8628797f5f Gerrit-Change-Number: 32516 Gerrit-PatchSet: 1 Gerrit-Owner: John Zhao <john.zhao(a)intel.com> Gerrit-MessageType: newchange

4 5

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Nico Huber (Code Review) 01 May '19

01 May '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 ) Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ...................................................................... Patch Set 10: > > > We noticed that PCI devices were not visible on the PCI bus, since 'lpss_mode' was set to ACPI. Can not describe details of testing, since PCI/ACPI modes has been corrected in devicetree.cb. > > > > I'm starting to understand. I guess the coreboot code > > simply doesn't allow anything but a 0 or 1 setting of the > > `PcdEnable*` devicetree options and expects that only > > `lpss_acpi_mode` is used to enable ACPI mode. > > > > If you have `lpss_acpi_mode` set to 0 and any `PcdEnable*` > > to 2, I would expect that coreboot won't generate the neces- > > sary ACPI entries. > > > > If you have `lpss_acpi_mode` set to 1, all LPSS devices > > should be in ACPI mode, no matter what the `PcdEnable*` > > options say. > > > > Such problems arise, when we allow direct copies of FSP > > UPD options in the devicetree, and try to make sense of > > them for both worlds, coreboot and FSP. > > > > I'm still interested in detailed test results for this > > patch with `lpss_acpi_mode = 1`. Either it works, then your > > patch could be merged (with a comment that the FSP documen- > > tation is incomplete). Or it doesn't work, then we should > > leave a comment, that the `PcdEnable*` options shouldn't > > be set to 2 (and probably normalize values to 0/1). > > I tested 'lpss_acpi_mode' to 1 and set some PchEnable* to '1': > For devices set to 1: > PCI: Static device PCI: 00:xx.x not found, disabling it. This is what I expected and from your reaction I guess this probably means we are talking past each other. If I'm not mistaken, the code in `lpss.c` won't be run in this case. But this code is vital for proper resource allocation and repor- ting in ACPI. To ask more specifically: Does TEST=Intel Cherry Hill involve booting to an OS and testing if any of these devices in ACPI mode are still usable? -- To view, visit https://review.coreboot.org/c/coreboot/+/29284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Gerrit-Change-Number: 29284 Gerrit-PatchSet: 10 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Wed, 01 May 2019 16:27:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Add SPI QUP driver
by T.Michael Turney (Code Review) 01 May '19

01 May '19

Hello T Michael Turney, Mukesh Savaliya, Vin Kamath, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/27483 to look at the new patch set (#52). Change subject: sdm845: Add SPI QUP driver ...................................................................... sdm845: Add SPI QUP driver This implements the SPI driver for the Qualcomm Universal Peripheral (QUP) core. Change-Id: I35061727d5ccc550eaeb06caef4524bc4cf25b54 Signed-off-by: Mukesh Savaliya <msavaliy(a)codeaurora.org> Signed-off-by: Akash Asthana <akashast(a)codeaurora.org> --- M src/mainboard/google/cheza/bootblock.c M src/soc/qualcomm/sdm845/Makefile.inc M src/soc/qualcomm/sdm845/bootblock.c M src/soc/qualcomm/sdm845/include/soc/addressmap.h A src/soc/qualcomm/sdm845/include/soc/qcom_qup_se.h A src/soc/qualcomm/sdm845/include/soc/spi_qup_qcom.h A src/soc/qualcomm/sdm845/qcom_qup_se.c M src/soc/qualcomm/sdm845/spi.c A src/soc/qualcomm/sdm845/spi_qup.c 9 files changed, 824 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/27483/52 -- To view, visit https://review.coreboot.org/c/coreboot/+/27483 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I35061727d5ccc550eaeb06caef4524bc4cf25b54 Gerrit-Change-Number: 27483 Gerrit-PatchSet: 52 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mukesh Savaliya <msavaliy(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: T Michael Turney <mturney(a)codeaurora.org> Gerrit-Reviewer: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Vin Kamath <vink(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: sdm845: Add QCLib to RomStage to perform IP init
by T.Michael Turney (Code Review) 01 May '19

01 May '19

Hello T Michael Turney, Julius Werner, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/25208 to look at the new patch set (#77). Change subject: sdm845: Add QCLib to RomStage to perform IP init ...................................................................... sdm845: Add QCLib to RomStage to perform IP init CB acts as I/O handler for QCLib (e.g. DDR training data) This interface allows bi-directional data flow between CB and QCLib Tested and working interfaces: DDR Training data QCLib serial console output DDR Information (base & size) limits cfg data TEST=build & run Change-Id: I073186674a1a593547d1ee1d15c7cd4fd8ad5bc1 Signed-off-by: T Michael Turney <mturney(a)codeaurora.org> --- M src/mainboard/google/cheza/chromeos.fmd M src/mainboard/google/cheza/romstage.c M src/soc/qualcomm/sdm845/Makefile.inc M src/soc/qualcomm/sdm845/include/soc/memlayout.ld M src/soc/qualcomm/sdm845/include/soc/mmu.h M src/soc/qualcomm/sdm845/include/soc/symbols.h M src/soc/qualcomm/sdm845/mmu.c A src/soc/qualcomm/sdm845/qclib.c M src/soc/qualcomm/sdm845/soc.c 9 files changed, 84 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/25208/77 -- To view, visit https://review.coreboot.org/c/coreboot/+/25208 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I073186674a1a593547d1ee1d15c7cd4fd8ad5bc1 Gerrit-Change-Number: 25208 Gerrit-PatchSet: 77 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: T Michael Turney <mturney(a)codeaurora.org> Gerrit-Reviewer: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: sdm845: Add QCLib to RomStage to perform IP init
by T Michael Turney (Code Review) 01 May '19

01 May '19

T Michael Turney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25208 ) Change subject: sdm845: Add QCLib to RomStage to perform IP init ...................................................................... Patch Set 76: (4 comments) https://review.coreboot.org/#/c/25208/76/src/mainboard/google/cheza/chromeo… File src/mainboard/google/cheza/chromeos.fmd: https://review.coreboot.org/#/c/25208/76/src/mainboard/google/cheza/chromeo… PS76, Line 27: ( > nit: flags are normally written without space before the parenthesis (compare COREBOOT(CBFS) above). Ack https://review.coreboot.org/#/c/25208/76/src/soc/qualcomm/sdm845/include/so… File src/soc/qualcomm/sdm845/include/soc/memlayout.ld: https://review.coreboot.org/#/c/25208/76/src/soc/qualcomm/sdm845/include/so… PS76, Line 31: 8192 > nit: 8K Ack https://review.coreboot.org/#/c/25208/69/src/soc/qualcomm/sdm845/include/so… File src/soc/qualcomm/sdm845/include/soc/qclib.h: https://review.coreboot.org/#/c/25208/69/src/soc/qualcomm/sdm845/include/so… PS69, Line 52: #define QCLIB_GA_ENABLE_UART_LOGGING 0x00000001 > Yeah, it doesn't really matter, it just makes it easier for me because I don't always get the new Qc […] Just sent yet another email query on this issue and added it to my Daily ToDo list. I will try to have an update Thursday morning as the India team is responsible for these releases. https://review.coreboot.org/#/c/25208/76/src/soc/qualcomm/sdm845/qclib.c File src/soc/qualcomm/sdm845/qclib.c: https://review.coreboot.org/#/c/25208/76/src/soc/qualcomm/sdm845/qclib.c@43 PS76, Line 43: if (size < 0) > This must be a different variable that's ssize_t. Ack Wonder why the compiler didn't alert, must be same size? -- To view, visit https://review.coreboot.org/c/coreboot/+/25208 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I073186674a1a593547d1ee1d15c7cd4fd8ad5bc1 Gerrit-Change-Number: 25208 Gerrit-PatchSet: 76 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: T Michael Turney <mturney(a)codeaurora.org> Gerrit-Reviewer: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 01 May 2019 15:51:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: T Michael Turney <mturney(a)codeaurora.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Add SPI QUP driver
by T Michael Turney (Code Review) 01 May '19

01 May '19

T Michael Turney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27483 ) Change subject: sdm845: Add SPI QUP driver ...................................................................... Patch Set 51: (1 comment) https://review.coreboot.org/#/c/27483/51/src/soc/qualcomm/sdm845/qspi.c File src/soc/qualcomm/sdm845/qspi.c: https://review.coreboot.org/#/c/27483/51/src/soc/qualcomm/sdm845/qspi.c@19 PS51, Line 19: arch > No, this is supposed to be <device/mmio. […] This was a merge error, I will address with next push. -- To view, visit https://review.coreboot.org/c/coreboot/+/27483 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I35061727d5ccc550eaeb06caef4524bc4cf25b54 Gerrit-Change-Number: 27483 Gerrit-PatchSet: 51 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mukesh Savaliya <msavaliy(a)qualcomm.corp-partner.google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: T Michael Turney <mturney(a)codeaurora.org> Gerrit-Reviewer: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Vin Kamath <vink(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Wed, 01 May 2019 14:28:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: (drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support
by Frans Hendriks (Code Review) 01 May '19

01 May '19

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29662 ) Change subject: (drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support ...................................................................... Patch Set 13: (1 comment) https://review.coreboot.org/#/c/29662/13/src/drivers/intel/fsp1_1/Makefile.… File src/drivers/intel/fsp1_1/Makefile.inc: https://review.coreboot.org/#/c/29662/13/src/drivers/intel/fsp1_1/Makefile.… PS13, Line 6: # Copyright (C) 2019 Eltan B.V. > What part of this file does this line apply to? Mistake, Will be removed. (I'm now aware that you should place copyright only in the files where you make 'improvement' and not in every file of a specific patchset of an 'improvement' -- To view, visit https://review.coreboot.org/c/coreboot/+/29662 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab48ad72f1514c93f20d70db5ef4fd8fa2383e8c Gerrit-Change-Number: 29662 Gerrit-PatchSet: 13 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: York Yang <yyang024(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 01 May 2019 13:18:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Frans Hendriks (Code Review) 01 May '19

01 May '19

Hello Patrick Rudolph, Aaron Durbin, Piotr Król, Paul Menzel, build bot (Jenkins), Hannah Williams, Nico Huber, Michał Żygowski, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29284 to look at the new patch set (#10). Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ...................................................................... soc/intel/braswell/chip.c: Configure LPSS devices in correct mode LPSS (SIO 0/24/0 and 0/30/0) are configured to ACPI mode if 'lpss_acpi_mode' = '1' or PcdEnableDma0/PcdEnableDma1 is configured in ACPI mode. When function 0 is configured in ACPI mode, functions > 1 are not available on the PCI bus, even pcdEnableXXXXX is set to PCI mode. For SIO DMA0: - HSUARTS ports not visible on PCI bus. For SIO DMA1: - I2C ports not visible on PCI bus. Force 'function > 1' devices in ACPI mode when function 0 is configured in ACPI mode. Intel FSP documentation does not mention that PcdEnable* devices in LPSS supports ACPI Mode (value 0x2). For these devices value '0x1' is used for PCI Mode. BUG=N/A TEST=Intel Cherry Hill Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Signed-off-by: Frans Hendriks <fhendriks(a)eltan.com> --- M src/soc/intel/braswell/chip.c 1 file changed, 23 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/29284/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/29284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Gerrit-Change-Number: 29284 Gerrit-PatchSet: 10 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: (drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support
by Angel Pons (Code Review) 01 May '19

01 May '19

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29662 ) Change subject: (drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support ...................................................................... Patch Set 13: (1 comment) https://review.coreboot.org/#/c/29662/13/src/drivers/intel/fsp1_1/Makefile.… File src/drivers/intel/fsp1_1/Makefile.inc: https://review.coreboot.org/#/c/29662/13/src/drivers/intel/fsp1_1/Makefile.… PS13, Line 6: # Copyright (C) 2019 Eltan B.V. What part of this file does this line apply to? -- To view, visit https://review.coreboot.org/c/coreboot/+/29662 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab48ad72f1514c93f20d70db5ef4fd8fa2383e8c Gerrit-Change-Number: 29662 Gerrit-PatchSet: 13 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: York Yang <yyang024(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 01 May 2019 12:45:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Frans Hendriks (Code Review) 01 May '19

01 May '19

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 ) Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ...................................................................... Patch Set 9: > Patch Set 9: > > > We noticed that PCI devices were not visible on the PCI bus, since 'lpss_mode' was set to ACPI. Can not describe details of testing, since PCI/ACPI modes has been corrected in devicetree.cb. > > I'm starting to understand. I guess the coreboot code > simply doesn't allow anything but a 0 or 1 setting of the > `PcdEnable*` devicetree options and expects that only > `lpss_acpi_mode` is used to enable ACPI mode. > > If you have `lpss_acpi_mode` set to 0 and any `PcdEnable*` > to 2, I would expect that coreboot won't generate the neces- > sary ACPI entries. > > If you have `lpss_acpi_mode` set to 1, all LPSS devices > should be in ACPI mode, no matter what the `PcdEnable*` > options say. > > Such problems arise, when we allow direct copies of FSP > UPD options in the devicetree, and try to make sense of > them for both worlds, coreboot and FSP. > > I'm still interested in detailed test results for this > patch with `lpss_acpi_mode = 1`. Either it works, then your > patch could be merged (with a comment that the FSP documen- > tation is incomplete). Or it doesn't work, then we should > leave a comment, that the `PcdEnable*` options shouldn't > be set to 2 (and probably normalize values to 0/1). I tested 'lpss_acpi_mode' to 1 and set some PchEnable* to '1': For devices set to 1: PCI: Static device PCI: 00:xx.x not found, disabling it. Will update comment. -- To view, visit https://review.coreboot.org/c/coreboot/+/29284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Gerrit-Change-Number: 29284 Gerrit-PatchSet: 9 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Wed, 01 May 2019 12:38:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2019