coreboot-gerrit May 2017

coreboot-gerrit@coreboot.org

1 participants
3888 discussions

Change in coreboot[master]: Kconfig: Introduce HAVE_(VBE_)LINEAR_FRAMEBUFFER
by build bot (Jenkins) (Code Review) 20 May '17

20 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19800 ) Change subject: Kconfig: Introduce HAVE_(VBE_)LINEAR_FRAMEBUFFER ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/9783/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54046/ : SUCCESS -- To view, visit https://review.coreboot.org/19800 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff6dac5a5f61af49456bc6312e7a376def02ab00 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: Kconfig: Rework MAINBOARD_HAS_NATIVE_VGA_INIT_TEXTMODECFG
by build bot (Jenkins) (Code Review) 20 May '17

20 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19791 ) Change subject: Kconfig: Rework MAINBOARD_HAS_NATIVE_VGA_INIT_TEXTMODECFG ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/9782/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54045/ : SUCCESS -- To view, visit https://review.coreboot.org/19791 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2d9d09be9aa6d019e77460e69a245ad2d8cda4ea Gerrit-PatchSet: 3 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: lib/edid: Split out fill_lb_framebuffer()
by build bot (Jenkins) (Code Review) 20 May '17

20 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19764 ) Change subject: lib/edid: Split out fill_lb_framebuffer() ...................................................................... Patch Set 4: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/9781/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54044/ : SUCCESS -- To view, visit https://review.coreboot.org/19764 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I90634b835bd8e2d150b1c714328a5b2774d891bd Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: arch: Unify basic cache clearing API
by Julius Werner (Code Review) 20 May '17

20 May '17

Julius Werner has posted comments on this change. ( https://review.coreboot.org/19788 ) Change subject: arch: Unify basic cache clearing API ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/19788/1/src/arch/mips/include/arch/cache.h File src/arch/mips/include/arch/cache.h: PS1, Line 44: /* TODO: Global cache API. Implement properly once we finally have a MIPS board : again where we can figure out what exactly these should be doing. */ > I thought the concise multi-line comments should only be used inside functi It's my understanding that you should in any situation chose the comment style that fits best with the existing code and gives the comment the vertical weight that is appropriate for its content (roughly guided by length). There is no hard "in/out of functions" rule. https://review.coreboot.org/#/c/19788/1/src/arch/x86/include/arch/cache.h File src/arch/x86/include/arch/cache.h: Line 59: wbinvd(); > Just call dcache_clean_all()? Uhh... sure? Doesn't really make a difference? I guess I can make dcache_clean_all() call dcache_clean_invalidate_all() instead, that would make a little more sense (because WBINVD both cleans and invalidates). -- To view, visit https://review.coreboot.org/19788 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c1723a287f76cd4118ef38a445339840601aeea Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: arch/x86: Add function to determine if we're currently runni...
by Julius Werner (Code Review) 20 May '17

20 May '17

Julius Werner has posted comments on this change. ( https://review.coreboot.org/19787 ) Change subject: arch/x86: Add function to determine if we're currently running from CAR ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/19787/1/src/arch/x86/include/arch/early_var… File src/arch/x86/include/arch/early_variables.h: Line 50: return 1; > Hmm.. I thought latest intel's don't use cache-as-ram at all? Uhh... I'm not really the expert on that tbh. I think they still use CAR but they don't do CAR migration anymore, or something? (Looks like NO_CAR_GLOBAL_MIGRATION is selected by default for all boards with POSTCAR_STAGE, which would substantiate this. That would mean those boards always return 1 for bootblock/verstage/romstage and 0 for postcar/ramstage in this function, which would be working as intended.) https://review.coreboot.org/#/c/19787/1/src/cpu/x86/car.c File src/cpu/x86/car.c: Line 118: return !!car_migrated; > Yes, it should be. ~0 is migrated. 0 is not-migrated. Whoops, right, sorry, will fix. -- To view, visit https://review.coreboot.org/19787 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib7ad0896a691ef6e89e622b985417fedc43579c1 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: arm64: Align cache maintenance code with libpayload and ARM32
by Julius Werner (Code Review) 20 May '17

20 May '17

Julius Werner has posted comments on this change. ( https://review.coreboot.org/19785 ) Change subject: arm64: Align cache maintenance code with libpayload and ARM32 ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/19785/1/src/arch/arm64/armv8/cache.c File src/arch/arm64/armv8/cache.c: Line 126: else if (sctlr & SCTLR_I) > Are _C and _I mutually exclusive? No. If _C is set, we want to run clean_all() (regardless of whether _I is set as well... usually it would be). The reason ARM32 has it this way goes back to a weird edge case discussed in https://chromium-review.googlesource.com/c/236422/ : if you have the instruction cache enabled (_I) but don't have the data cache enabled (_C), instruction fetches will still fetch instructions into the unified L2 cache. Then if you have a situation like OVERLAP_VERSTAGE_ROMSTAGE where you load different code to the same addresses, running a clean_all() will just flush those old instructions from L2 back to RAM and overwrite the new instructions you just loaded. They figured out that they needed this to get that Qualcomm board to boot reliably. For all our current boards this should make no difference since they will always run with _C and _I right from the bootblock. With their data caches enabled they will just write the new stage code to the L2 cache at load time and a flush without invalidate is enough to get back in sync with RAM. -- To view, visit https://review.coreboot.org/19785 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e05b425eeeaa27a447b37f98c0928fed3f74340 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: soc/mediatek/mt8173: Move spi driver to use spi_bus_map
by Julius Werner (Code Review) 20 May '17

20 May '17

Julius Werner has posted comments on this change. ( https://review.coreboot.org/19770 ) Change subject: soc/mediatek/mt8173: Move spi driver to use spi_bus_map ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/19770/1/src/soc/mediatek/mt8173/spi.c File src/soc/mediatek/mt8173/spi.c: Line 333: .bus_end = CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS, > Yeah, I just tried to keep the changes as minimal as possible from the orig I'm just suggesting to replace this with a literal 0 for now. That will come out to the same value (for Oak, which is the only board using this right now). This would make more sense if a future board might not want to use an EC and connect something different here instead. For the other one, it's a bit more debatable I guess since that controller can only be used for flash anyway. The number Oak currently uses there is 9. I guess we could just put that literal here too, and say that 9 is our NOR flash controller number... or we could leave this one as it is since it's very likely that all board will want to use it for boot flash. (I would maybe not change it to #1 since it's possible that a future Mediatek chip with more generic SPI controllers might want to reuse this code, and then we'd want all the generic ones to be contiguous e.g. from 0 to 3.) -- To view, visit https://review.coreboot.org/19770 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib0d6e4e8185ce1285b671af5ebcead1d42e049bc Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: soc/nvidia/tegra*: Move spi driver to use spi_bus_map
by Furquan Shaikh (Code Review) 20 May '17

20 May '17

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/19769 ) Change subject: soc/nvidia/tegra*: Move spi driver to use spi_bus_map ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/19769/1/src/soc/nvidia/tegra124/spi.c File src/soc/nvidia/tegra124/spi.c: Line 800: return -1; > nit: This check seems to have always been pretty pointless. It'll never act Done -- To view, visit https://review.coreboot.org/19769 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I873b96d286655a814554bfd89f899ee87302b06d Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: soc/mediatek/mt8173: Move spi driver to use spi_bus_map
by Furquan Shaikh (Code Review) 20 May '17

20 May '17

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/19770 ) Change subject: soc/mediatek/mt8173: Move spi driver to use spi_bus_map ...................................................................... Patch Set 1: (4 comments) https://review.coreboot.org/#/c/19770/1/src/soc/mediatek/mt8173/spi.c File src/soc/mediatek/mt8173/spi.c: Line 304: default: > Can take out the flash and the "wrong number" case now because generic code Yeah, that makes sense. PS1, Line 311: spi_ctrlr_setup > Should be NULL? Yeah, then we can get rid of the case for SPI_FLASH_BUS above. PS1, Line 331: ec > Shouldn't have "ec" in the name in this file. Just spi_ctrlr. Sounds good. Line 333: .bus_end = CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS, > I guess it was never really a great idea to put this Kconfig in here becaus Yeah, I just tried to keep the changes as minimal as possible from the original logic. Are you suggesting that spi_ctrlr have bus #0 and flash bus #1? But then would we still be providing boards to select bus #? The reason I used the config here is to make sure the expectations matches what the board selected. -- To view, visit https://review.coreboot.org/19770 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib0d6e4e8185ce1285b671af5ebcead1d42e049bc Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: soc/rockchip: Move spi driver to use spi_bus_map
by Julius Werner (Code Review) 19 May '17

19 May '17

Julius Werner has posted comments on this change. ( https://review.coreboot.org/19771 ) Change subject: soc/rockchip: Move spi driver to use spi_bus_map ...................................................................... Patch Set 1: Code-Review+2 (1 comment) https://review.coreboot.org/#/c/19771/1/src/soc/rockchip/common/spi.c File src/soc/rockchip/common/spi.c: Line 328: assert(slave->bus < ARRAY_SIZE(rockchip_spi_slaves)); nit: unnecessary, already guaranteed by framework now? -- To view, visit https://review.coreboot.org/19771 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I66b1b9635ece2381f62f2a9d6f5744d639d59163 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2017