coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: soc/qualcomm/common: Update helper function to know size of memchipinfo
by Ravi Kumar Bokka (Code Review) 12 Apr '22

12 Apr '22

Attention is currently required from: Paul Menzel, Julius Werner, Yu-Ping Wu. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63026 ) Change subject: soc/qualcomm/common: Update helper function to know size of memchipinfo ...................................................................... Patch Set 10: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63026/comment/ec687345_d26c68ba PS5, Line 7: soc/qualcomm/common: update size of memchipinfo structure > hi julius updated commit message with details Ack Commit Message: https://review.coreboot.org/c/coreboot/+/63026/comment/a9b5d739_8a97cdc3 PS6, Line 8: > hi paul, […] corrected commit message File src/soc/qualcomm/common/qclib.c: https://review.coreboot.org/c/coreboot/+/63026/comment/667f3606_5f67797c PS5, Line 30: uint64_t * > Fair enough... I thought blob_address was a void* already. […] hi julius, updated type cast as (void *) -- To view, visit https://review.coreboot.org/c/coreboot/+/63026 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0d59669adaf287d0eb7b58ccb0fe3f98e3d23281 Gerrit-Change-Number: 63026 Gerrit-PatchSet: 10 Gerrit-Owner: Ravi Kumar Bokka <rbokka(a)codeaurora.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 22:25:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ravi Kumar Bokka <rbokka(a)codeaurora.org> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: commonlib/bsd: Add mem_chip_info_size() function
by Ravi Kumar Bokka (Code Review) 12 Apr '22

12 Apr '22

Attention is currently required from: Paul Menzel, Julius Werner. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63407 ) Change subject: commonlib/bsd: Add mem_chip_info_size() function ...................................................................... Patch Set 5: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63407/comment/3ffb068a_7f82ef5b PS1, Line 7: src/commonlib/bsd: Fix bugs introduced with CB:59195 > hi julius, […] updated commit message Commit Message: https://review.coreboot.org/c/coreboot/+/63407/comment/7a4a1bd5_9b7563bc PS2, Line 7: src/ > hi paul, […] corrected in latest patch set https://review.coreboot.org/c/coreboot/+/63407/comment/b9ce945e_5295c804 PS2, Line 8: > Please remember to mark comments as resolved when you resolve them. hi julius, updated all the comments with resolved state Commit Message: https://review.coreboot.org/c/coreboot/+/63407/comment/8cc7dbcf_38d3462f PS4, Line 9: strcuture > typo hi juliuse, corrected typo. https://review.coreboot.org/c/coreboot/+/63407/comment/bb4cc319_5bd66095 PS4, Line 10: using > "used" hi julius, corrected in latest patch set https://review.coreboot.org/c/coreboot/+/63407/comment/1c53e495_c98cf69a PS4, Line 13: and commit sha: 4573ca42e675fb8b61bcf05591d8d4d05f93944d > No, it doesn't. The next patch does that. hi julius, removed sha in commit message as CL is mentioned in above line -- To view, visit https://review.coreboot.org/c/coreboot/+/63407 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaada45d63b82c28495166024a9655d871ba65b20 Gerrit-Change-Number: 63407 Gerrit-PatchSet: 5 Gerrit-Owner: Ravi Kumar Bokka <rbokka(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Mon, 11 Apr 2022 22:24:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ravi Kumar Bokka <rbokka(a)codeaurora.org> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device/i2c_bus: Add detect function pointer in i2c_bus_ops
by Matt DeVillier (Code Review) 12 Apr '22

12 Apr '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Julius Werner, Karthik Ramasubramanian. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63209 ) Change subject: device/i2c_bus: Add detect function pointer in i2c_bus_ops ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > Well, okay, I guess there's not that many implementations for the devicetree.cb version (i2c_bus.h). […] I adjusted transfer() and have detect as a wrapper call which makes the setup and call from i2c/generic much simpler -- To view, visit https://review.coreboot.org/c/coreboot/+/63209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia8ddfb187eabec966c253b6cc8526491c99151fc Gerrit-Change-Number: 63209 Gerrit-PatchSet: 3 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 22:24:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/i2c/dw_i2c: Adjust to handle 0-byte transfers
by Matt DeVillier (Code Review) 12 Apr '22

12 Apr '22

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63561 ) Change subject: drivers/i2c/dw_i2c: Adjust to handle 0-byte transfers ...................................................................... drivers/i2c/dw_i2c: Adjust to handle 0-byte transfers 0-byte writes can be used as a way to probe/check presence of an i2c device, so adjust _dw_i2c_transfer() to immediately set the STOP bit and suppress TX abort messages when the segment length is zero. Tested as part of entire i2c-detect patch train. Change-Id: I518e849f4c476c264a1464886b1853af66c0b29d Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/drivers/i2c/designware/dw_i2c.c 1 file changed, 7 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/63561/1 diff --git a/src/drivers/i2c/designware/dw_i2c.c b/src/drivers/i2c/designware/dw_i2c.c index 56f3f27..1a61282 100644 --- a/src/drivers/i2c/designware/dw_i2c.c +++ b/src/drivers/i2c/designware/dw_i2c.c @@ -374,6 +374,10 @@ dw_i2c_enable(regs); + if (segments->len == 0) + /* stop immediately */ + write32(&regs->cmd_data, CMD_DATA_STOP); + /* Process each segment */ while (count--) { if (CONFIG(DRIVERS_I2C_DESIGNWARE_DEBUG)) { @@ -424,8 +428,9 @@ /* Check TX abort */ if (read32(&regs->raw_intr_stat) & INTR_STAT_TX_ABORT) { - printk(BIOS_ERR, "I2C TX abort detected (%08x)\n", - read32(&regs->tx_abort_source)); + if (segments->len != 0) + printk(BIOS_ERR, "I2C TX abort detected (%08x)\n", + read32(&regs->tx_abort_source)); /* clear INTR_STAT_TX_ABORT */ read32(&regs->clear_tx_abrt_intr); goto out; -- To view, visit https://review.coreboot.org/c/coreboot/+/63561 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I518e849f4c476c264a1464886b1853af66c0b29d Gerrit-Change-Number: 63561 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/i2c/dw_i2c: Add detect callback to i2c_bus_ops
by Matt DeVillier (Code Review) 12 Apr '22

12 Apr '22

Attention is currently required from: Raul Rangel, Subrata Banik, Matt DeVillier, Karthik Ramasubramanian. Hello build bot (Jenkins), Raul Rangel, Subrata Banik, Tim Wawrzynczak, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63210 to look at the new patch set (#3). Change subject: drivers/i2c/dw_i2c: Add detect callback to i2c_bus_ops ...................................................................... drivers/i2c/dw_i2c: Add detect callback to i2c_bus_ops This patch adds the I2C equivalent of an SMBus quick write to an I2C device, which is used by some I2C drivers as a way to probe the existence (or absence) of a certain device on the bus, based on whether or not a 0-byte write to an I2C address is ACKed or NACKed. Change-Id: I9ed410669aabf9866329c6c6e74a39168a86b73e Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/drivers/i2c/designware/dw_i2c.c 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/63210/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63210 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ed410669aabf9866329c6c6e74a39168a86b73e Gerrit-Change-Number: 63210 Gerrit-PatchSet: 3 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: intel/common/block: rename dmi folder to gpmr
by Wonkyu Kim (Code Review) 12 Apr '22

12 Apr '22

Attention is currently required from: Nico Huber, Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Angel Pons, Arthur Heymans, Nick Vaccaro, Michael Niewöhner, Tim Wawrzynczak. Hello build bot (Jenkins), Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Angel Pons, Nick Vaccaro, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63170 to look at the new patch set (#15). Change subject: intel/common/block: rename dmi folder to gpmr ...................................................................... intel/common/block: rename dmi folder to gpmr As part of gpmr driver migration, rename dmi folder to gpmr folder. TEST=build Signed-off-by: Wonkyu Kim <wonkyu.kim(a)intel.com> Change-Id: I4d57f4b8bd06e0cf6c9afa4baf4a7bed64ecb56b --- R src/soc/intel/common/block/gpmr/Kconfig R src/soc/intel/common/block/gpmr/Makefile.inc R src/soc/intel/common/block/gpmr/dmi.c 3 files changed, 0 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/63170/15 -- To view, visit https://review.coreboot.org/c/coreboot/+/63170 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d57f4b8bd06e0cf6c9afa4baf4a7bed64ecb56b Gerrit-Change-Number: 63170 Gerrit-PatchSet: 15 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: intel/common/block: Add gpmr common driver
by Wonkyu Kim (Code Review) 12 Apr '22

12 Apr '22

Attention is currently required from: Nico Huber, Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Angel Pons, Arthur Heymans, Nick Vaccaro, Michael Niewöhner, Tim Wawrzynczak. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63170 ) Change subject: intel/common/block: Add gpmr common driver ...................................................................... Patch Set 14: (8 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63170/comment/08ab9f51_f391e36a PS14, Line 9: Move GPMR(General Purpose Memory Range) APIs to gpmr driver from dmi.c : For this, 3 patches are used. : 1. Add GPMR common driver in IA common code(CB:63170) : 2. Migrate all DMI API usage to GPMR(CB:63471) : 3. Drop DMI driver (CB:63472) : > Thank you, guys! Done File src/soc/intel/common/block/gpmr/gpmr.c: https://review.coreboot.org/c/coreboot/+/63170/comment/21197886_4954feaf PS13, Line 39: printk(BIOS_ERR, "%s: No available free gpmr found\n", __func__); > Maybe also log MAX_GPMR_REGS. Done https://review.coreboot.org/c/coreboot/+/63170/comment/beeea50c_fa90bd73 PS13, Line 44: uint32_t > Why? Done https://review.coreboot.org/c/coreboot/+/63170/comment/1e0e1f14_f39455bb PS13, Line 46: int > Indeed, but maybe a better name can be found? `num` is misleading at list to me. Will rename existing dmi.c to gpmr.c File src/soc/intel/common/block/gpmr/gpmr.c: https://review.coreboot.org/c/coreboot/+/63170/comment/7b2b5ce7_16c3e02c PS14, Line 22: data32 = gpmr_read32(offset); : data32 |= ordata; : gpmr_write32(offset, data32); > pcr_or32(PID_DMI, offset, ordata) Done https://review.coreboot.org/c/coreboot/+/63170/comment/76f2326f_f2d16339 PS14, Line 46: base & ~(GPMR_BASE_MASK << GPMR_BASE_SHIFT) > use IS_ALIGNED Done https://review.coreboot.org/c/coreboot/+/63170/comment/f8e6dd82_9c1e9d92 PS14, Line 47: printk(BIOS_ERR, "base is not 64-KiB aligned!\n"); > Maybe make it more explicit (!IS_ALIGNED(base, 64 * KiB))? Done File src/soc/intel/common/block/include/intelblocks/pcr_gpmr.h: https://review.coreboot.org/c/coreboot/+/63170/comment/40142ea5_f38b9fd8 PS14, Line 21: GPMR_BASE_SHIFT > I'm not sure how common this is, but I'm much more familiar with CPP definitions of shifts being lef […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/63170 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d57f4b8bd06e0cf6c9afa4baf4a7bed64ecb56b Gerrit-Change-Number: 63170 Gerrit-PatchSet: 14 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 22:07:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: implement ioc driver
by Wonkyu Kim (Code Review) 12 Apr '22

12 Apr '22

Attention is currently required from: Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Tim Wawrzynczak, Angel Pons, Nick Vaccaro. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63198 ) Change subject: soc/intel/common: implement ioc driver ...................................................................... Patch Set 12: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63198/comment/78a39708_711a201c PS11, Line 9: IOC(I/O Cache) > Please add a space before (. Ack File src/soc/intel/common/block/gpmr/gpmr.c: https://review.coreboot.org/c/coreboot/+/63198/comment/2a3b6306_d828ca37 PS5, Line 11: if (CONFIG(SOC_INTEL_COMMON_BLOCK_IOC)) : return ioc_reg_read32(offset); : else : return pcr_read32(PID_DMI, offset); > nit: you can implement as https://review.coreboot. […] Let's use like this for now. File src/soc/intel/common/block/gpmr/gpmr.c: https://review.coreboot.org/c/coreboot/+/63198/comment/5d6c8904_57d6c4e0 PS12, Line 27: uint32_t data32; : : data32 = gpmr_read32(offset); : data32 |= ordata; : gpmr_write32(offset, data32); > this also need change for IOC isn't it ? you mean ioc_or32? File src/soc/intel/common/block/ioc/ioc.c: https://review.coreboot.org/c/coreboot/+/63198/comment/fce7d35a_5e56d78a PS11, Line 16: return value; > Is the temporary variable needed? Ack File src/soc/intel/common/block/ioc/ioc.c: https://review.coreboot.org/c/coreboot/+/63198/comment/4f90a6fc_e43baa56 PS12, Line 15: read32 > isn't I have suggested to use read32p ? Ack File src/soc/intel/common/pch/lockdown/lockdown.c: https://review.coreboot.org/c/coreboot/+/63198/comment/a2f96aca_02356379 PS12, Line 32: GPMR_DMICTL > is that because you don't have `GPMR_DMICTL` for IOC ? Yes, With IOC interface, there is no CTL register and it does not need to do it. -- To view, visit https://review.coreboot.org/c/coreboot/+/63198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I768027c2ca78310c03845f70f17df19dc8cd0982 Gerrit-Change-Number: 63198 Gerrit-PatchSet: 12 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 22:02:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86: Add a common romstage entry
by Felix Held (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Jason Glenesk, Nico Huber, Furquan Shaikh, Marshall Dawson, Angel Pons, Arthur Heymans, Fred Reitberger, Patrick Rudolph. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55067 ) Change subject: arch/x86: Add a common romstage entry ...................................................................... Patch Set 13: (1 comment) File src/cpu/intel/car/romstage.c: https://review.coreboot.org/c/coreboot/+/55067/comment/cc1fea32_af16fee7 PS13, Line 58: /* We do not return here. */ : die("failed to load postcar\n"); : > Shouldn't we move this to common code too? printing "failed to load next stage" in the new car_stage_entry function after the romstage_main call would also avoid the need for the __noreturn attribute for romstage_main. i'm ok with doing this in a follow-up, but marking as not resolved to make sure that the change won't be overlooked; nearly submitted this one even though this is sounds like a good improvement to me -- To view, visit https://review.coreboot.org/c/coreboot/+/55067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4968364a95b5c69c21d3915d302d23e6f1ca182f Gerrit-Change-Number: 55067 Gerrit-PatchSet: 13 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 11 Apr 2022 21:07:32 +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]: cpu/x86/mp_init.c: Add wait_finished_mp_run_on_all_cpus
by Arthur Heymans (Code Review) 11 Apr '22

11 Apr '22

Attention is currently required from: Arthur Heymans, Tim Wawrzynczak, Kane Chen. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63553 ) Change subject: cpu/x86/mp_init.c: Add wait_finished_mp_run_on_all_cpus ...................................................................... Patch Set 3: (1 comment) File src/cpu/x86/mp_init.c: https://review.coreboot.org/c/coreboot/+/63553/comment/bddb4a50_8daedb11 PS3, Line 1014: * running a function call but does not wait for them to finish it. : * APs only accept a new task when the the previous one is finished : * so to make sure that the post_cpus_add_romcache() is not overwritten : * by an AP thread we do a NOOP call on the APs which will ensure the : * previous function actually finished running. : */ > nit: this comment is really specific; I would say that this could be used when > synchronization of APs after work is done is required. > > Although this still isn't as "atomic", as it introduces another task. Does it make sense to explicitly introduce a synchronization point in the ap_wait_for_instruction/run_on_all_aps for after the work function is done? It probably does make sense. This is a bit hacky. -- To view, visit https://review.coreboot.org/c/coreboot/+/63553 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70244557bb384739e3bd06de3d8414ec9f4d5f62 Gerrit-Change-Number: 63553 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Comment-Date: Mon, 11 Apr 2022 20:59:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022