coreboot-gerrit April 2019

coreboot-gerrit@coreboot.org

1 participants
1219 discussions

Change in ...coreboot[master]: cbfstool: Add ifittool
by Duncan Laurie (Code Review) 20 Apr '19

20 Apr '19

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31493 ) Change subject: cbfstool: Add ifittool ...................................................................... Patch Set 9: (6 comments) https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c File util/cbfstool/fit.c: https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@249 PS9, Line 249: int this could just return void since the return code has no meaning https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@299 PS9, Line 299: mcu_header->total_size I know this is left over from the original, but if this were left out and it just used "? :" then this would fit on one line. https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@353 PS9, Line 353: The There https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@354 PS9, Line 354: Type 2 entry I know it is defined in the header, but it would be useful to use FIT_TYPE_BIOS_ACM (etc) in these comments so it was easier to follow without having to remember the numbering. https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@399 PS9, Line 399: entry->size_reserved = lcp_policy_size; This is overwriting the setting from the previous line, except it is not converting to multiple of 16 bytes which does not seem like what you want. https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@446 PS9, Line 446: struct microcode_entry *mcu This threw me for a minute trying to find where it was declared, style guide suggests this should be at the top of the function. -- To view, visit https://review.coreboot.org/c/coreboot/+/31493 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0fe8cd70611d58823aca1147d5b830722ed72bd5 Gerrit-Change-Number: 31493 Gerrit-PatchSet: 9 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jens Drenhaus <jens.drenhaus(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kai Michaelis <kai.michaelis(a)9elements.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Roy Wen <rgzwen(a)arista.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sat, 20 Apr 2019 00:40:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Add AOP firmware support
by build bot (Jenkins) (Code Review) 19 Apr '19

19 Apr '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25210 ) Change subject: sdm845: Add AOP firmware support ...................................................................... Patch Set 76: (1 comment) https://review.coreboot.org/#/c/25210/76/src/soc/qualcomm/sdm845/mmu.c File src/soc/qualcomm/sdm845/mmu.c: https://review.coreboot.org/#/c/25210/76/src/soc/qualcomm/sdm845/mmu.c@36 PS76, Line 36: void soc_mmu_dram_config_post_dram_init() Bad function definition - void soc_mmu_dram_config_post_dram_init() should probably be void soc_mmu_dram_config_post_dram_init(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/25210 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9845c8638e4b905de5d6985dc9f1fddd8b1a8942 Gerrit-Change-Number: 25210 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: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Comment-Date: Fri, 19 Apr 2019 21:01:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sdm845: Port I2C driver
by T.Michael Turney (Code Review) 19 Apr '19

19 Apr '19

Hello T Michael Turney, Akash Asthana, Julius Werner, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29104 to look at the new patch set (#28). Change subject: sdm845: Port I2C driver ...................................................................... sdm845: Port I2C driver Add I2C functionality in coreboot Change-Id: Ifb76564d8a11427423dd14d8ba7c8c7d500ef346 Signed-off-by: Akash Asthana <akashast(a)codeaurora.org> --- M src/mainboard/google/cheza/mainboard.c M src/soc/qualcomm/sdm845/Makefile.inc A src/soc/qualcomm/sdm845/i2c.c A src/soc/qualcomm/sdm845/include/soc/i2c.h M src/soc/qualcomm/sdm845/include/soc/qcom_qup_se.h M src/soc/qualcomm/sdm845/include/soc/qupv3_fw_config.h M src/soc/qualcomm/sdm845/qcom_qup_se.c M src/soc/qualcomm/sdm845/qupv3_fw_config.c M src/soc/qualcomm/sdm845/spi_qup.c 9 files changed, 341 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/29104/28 -- To view, visit https://review.coreboot.org/c/coreboot/+/29104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifb76564d8a11427423dd14d8ba7c8c7d500ef346 Gerrit-Change-Number: 29104 Gerrit-PatchSet: 28 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Akash Asthana <akashast(a)qualcomm.corp-partner.google.com> 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: 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-MessageType: newpatchset

1 0

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

19 Apr '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 (#48). 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/qspi.c M src/soc/qualcomm/sdm845/spi.c A src/soc/qualcomm/sdm845/spi_qup.c 10 files changed, 825 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/27483/48 -- 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: 48 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) 19 Apr '19

19 Apr '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 (#73). 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, 126 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/25208/73 -- 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: 73 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 USB support on cheza platform
by T.Michael Turney (Code Review) 19 Apr '19

19 Apr '19

Hello T Michael Turney, Julius Werner, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth, Ciluveru chandana kishori, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/25213 to look at the new patch set (#68). Change subject: sdm845: Add USB support on cheza platform ...................................................................... sdm845: Add USB support on cheza platform This patch adds code to initialize two USB DWC3.0 controllers and its associated QUSB V2 10nm PHYs to the SDM845 SOC, and uses them to initialize USB3.0 on the cheza mainboard. Synopsis controller initialization and configuration sequences taken from USB 3.0 HPG chapter 2.2 and refer PHY HPG chapter 10.2 for QUSB phy programming. Includes Super speed mode support. TEST=USB keypad and mass-storage device enumeration tested with this patch Change-Id: I475a7757239acb8ef22a4d61afd59b304a7f0acc Signed-off-by: Chandana Kishori Chiluveru <cchiluve(a)codeaurora.org> --- M src/mainboard/google/cheza/mainboard.c M src/mainboard/google/cheza/romstage.c M src/soc/qualcomm/sdm845/Makefile.inc M src/soc/qualcomm/sdm845/include/soc/addressmap.h M src/soc/qualcomm/sdm845/include/soc/clock.h A src/soc/qualcomm/sdm845/include/soc/efuse.h A src/soc/qualcomm/sdm845/include/soc/usb.h A src/soc/qualcomm/sdm845/usb.c 8 files changed, 1,114 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/25213/68 -- To view, visit https://review.coreboot.org/c/coreboot/+/25213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I475a7757239acb8ef22a4d61afd59b304a7f0acc Gerrit-Change-Number: 25213 Gerrit-PatchSet: 68 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Ciluveru chandana kishori <cchiluve(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: Chandana Kishori Chiluveru <cchiluve(a)qualcomm.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: ich7/i945: Write XCAP for D28:F0/F1/F2/F3/F4/F5
by HAOUAS Elyes (Code Review) 19 Apr '19

19 Apr '19

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32362 Change subject: ich7/i945: Write XCAP for D28:F0/F1/F2/F3/F4/F5 ...................................................................... ich7/i945: Write XCAP for D28:F0/F1/F2/F3/F4/F5 This write 1 into R/WO [Bit 8] XCAP registry. Probably this needs to move to southbridge. Change-Id: Ibacdf07d8f6410bffbf6080b9aead0ac8e4814b1 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/intel/i945/early_init.c 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32362/1 diff --git a/src/northbridge/intel/i945/early_init.c b/src/northbridge/intel/i945/early_init.c index 0c92c4b..f946970 100644 --- a/src/northbridge/intel/i945/early_init.c +++ b/src/northbridge/intel/i945/early_init.c @@ -333,6 +333,7 @@ { u16 reg16; u32 reg32; + int i; reg16 = RCBA16(LCTL); reg16 &= ~(3 << 0); @@ -342,10 +343,11 @@ RCBA32(V0CTL) = 0x80000001; RCBA32(V1CAP) = 0x03128010; - pci_write_config16(PCI_DEV(0, 0x1c, 0), 0x42, 0x0141); - pci_write_config16(PCI_DEV(0, 0x1c, 4), 0x42, 0x0141); - pci_write_config16(PCI_DEV(0, 0x1c, 5), 0x42, 0x0141); - + for (i = 0; i < 6; ++i) { /* D28:F0/F1/F2/F3/F4/F5 */ + reg16 = read_config16(PCI_DEV(0, 0x1c, i), 0x42); + reg16 |= (1 << 8); + pci_write_config16(PCI_DEV(0, 0x1c, i), 0x42, reg16); + } pci_write_config32(PCI_DEV(0, 0x1c, 4), 0x54, 0x00480ce0); pci_write_config32(PCI_DEV(0, 0x1c, 5), 0x54, 0x00500ce0); -- To view, visit https://review.coreboot.org/c/coreboot/+/32362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibacdf07d8f6410bffbf6080b9aead0ac8e4814b1 Gerrit-Change-Number: 32362 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

1 2

Change in ...coreboot[master]: sdm845: Add interface between CB & QCLib
by T Michael Turney (Code Review) 19 Apr '19

19 Apr '19

1 0

Change in ...coreboot[master]: sdm845: Add interface between CB & QCLib
by T Michael Turney (Code Review) 19 Apr '19

19 Apr '19

T Michael Turney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27360 ) Change subject: sdm845: Add interface between CB & QCLib ...................................................................... Patch Set 35: (38 comments) I believe all comments from this patch (patch no longer in train) are addressed by current patches, when next patch-train is pushed, should be 19.April.2019. https://review.coreboot.org/#/c/27360/26/src/mainboard/google/cheza/chromeo… File src/mainboard/google/cheza/chromeos.fmd: https://review.coreboot.org/#/c/27360/26/src/mainboard/google/cheza/chromeo… PS26, Line 36: RW_LIMITS_CFG 4K > There aren't any security concerns. […] Done https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/include/so… File src/soc/qualcomm/sdm845/include/soc/qclib.h: https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/include/so… PS30, Line 74: qclib_cb_if_table_header > This is the whole table, not just the header, so just call it qclib_cb_if_table. Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/include/so… File src/soc/qualcomm/sdm845/include/soc/qclib.h: https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/include/so… PS26, Line 32: #if IS_ENABLED(CONFIG_QC_SDI_ENABLE) > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/include/so… PS26, Line 41: te_index_ddr_information > ACK, removing on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/include/so… PS26, Line 70: #define MAX_NUMBER_OF_ENTRIES 16 > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/include/so… PS26, Line 72: #define QCLIB_MAGIC_NUMBER 0x51434C49425F4342 /* QCLIB_CB */ > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/include/so… PS26, Line 92: fmap_region > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/include/so… PS26, Line 97: fmap_name > Actually, this is the FMAP region name, will create new LENGTH #define rather than overload TE_NAME_ […] Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… File src/soc/qualcomm/sdm845/qclib_execute.c: https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 46: .te = { > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 69: struct fmap_region fmap_region_table[fmap_region_max] = { > Will add the two fmap_ APIs here, on next submission as trial, when they look good, they can be move […] Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 94: "RW_DDR_TRAINING"); > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 139: if (fr->size != sram_size) { > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 199: RamStage > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 200: ddr_mmu = (struct ddr_mmu_information *)_ddr_information; > Removed mmu from names. […] Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 203: ddr_mmu->size = ddr_size * MiB; > This comes out of QCLib interface, either it gets changed here or in QCLib. […] QCLib is still passing back block size and this multiply is still occurring, please confirm you are OK with this. https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 215: u8 > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 266: #if IS_ENABLED(CONFIG_QC_SDI_ENABLE) > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 293: GA_BMASK_ENABLE_UART_LOGGING; > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 303: size > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 307: 1 > ACK on next submission Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 318: PBL_DATA_PTR > Currently working on this, legacy QCLib code needs to be reworked to remove this dependency. Done https://review.coreboot.org/#/c/27360/26/src/soc/qualcomm/sdm845/qclib_exec… PS26, Line 351: > ACK on next submission Done https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/qclib_exec… File src/soc/qualcomm/sdm845/qclib_execute.c: https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/qclib_exec… PS30, Line 200: static void write_qclib_serial_log(int te_index) > ACK Done Protection is via table entry only being added with if (CONFIG_CONSOLE_CBMEM). https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/qclib_exec… PS30, Line 212: int te_index > Ack Done https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/qclib_exec… PS30, Line 295: e.g. console log copy > Ack Done If enabled, the console buffer is first table entry added so should be first checked on return and copied to output. https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/qclib_exec… PS30, Line 319: init_and_read_fmap_region(QCLIB_FR_DDR_TRAINING); > Ack Done https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/qclib_exec… PS30, Line 376: printk(BIOS_DEBUG, "QCLib completed\n\n\n"); > Ack Done https://review.coreboot.org/#/c/27360/30/src/soc/qualcomm/sdm845/qclib_exec… PS30, Line 386: dump_te_table(); > Ack Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… File src/soc/qualcomm/sdm845/qclib_execute.c: https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 61: /* after validation, move to src/lib/fmap.c */ > Yes), please move these to src/lib/fmap. […] Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 68: return rdev_readat(&rdev, buffer, 0, MIN(size, region_device_sz(&rdev))); > line over 80 characters Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 78: rdev_eraseat(&rdev, 0, region_device_sz(&rdev)); > This can also return an error value that needs to be checked. Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 79: size > This should imply a (size <= region_device_sz(&rdev)) check, but I think it would still be better to […] Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 102: cbmemc_tx_byte > I think this may still not compile if CBMEM_CONSOLE is not enabled, because even though we know it c […] Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 107: printk(BIOS_INFO, "[%s]:[%s]\n", __func__, te->name); > I don't think we need this anymore? (I'm trying to get rid of all prints that will be out-of-order i […] Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 124: te->size > Just to clarify, QcLib will change this size value to contain the actual size of the data structure […] Done If QCLib sets "dirty" bit the size should reflect correct amount of data to write-back. If it doesn't this is bug in QCLib. https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 133: printk(BIOS_INFO, " not implemented\n"); > ...and when we take the string above out, this should be changed to print the names. […] Done https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 223: dump_te_table(); > Do we still need this? Need it, probably not, I find it good diagnostic data to have in the log output. Please confirm you want it removed. https://review.coreboot.org/#/c/27360/32/src/soc/qualcomm/sdm845/qclib_exec… PS32, Line 262: printk(BIOS_DEBUG, "QCLib completed\n\n\n"); > Okay, if we can't put this in write_qclib_log_to_cbmemc() (and I agree that would be bad for the cas […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/27360 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8d86f360d9e5f1a50f052ab99e821a587e091946 Gerrit-Change-Number: 27360 Gerrit-PatchSet: 35 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-Comment-Date: Fri, 19 Apr 2019 16:31:01 +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> Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: sb/intel/lynxpoint: Enable LPC/SIO setup in bootblock
by Duncan Laurie (Code Review) 19 Apr '19

19 Apr '19

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30315 ) Change subject: sb/intel/lynxpoint: Enable LPC/SIO setup in bootblock ...................................................................... Patch Set 46: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/30315 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c6e107c267a7acb5bf9cbeb54eb5361af3b6db4 Gerrit-Change-Number: 30315 Gerrit-PatchSet: 46 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tristan Corrick <tristan(a)corrick.kiwi> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Fri, 19 Apr 2019 14:55:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2019