coreboot-gerrit January 2020

coreboot-gerrit@coreboot.org

1 participants
1728 discussions

Change in coreboot[master]: mb/intel/tglrvp: Add initial mainboard code
by Wonkyu Kim (Code Review) 03 Jan '20

03 Jan '20

Wonkyu Kim has uploaded a new patch set (#9) to the change originally created by Ravishankar Sarawadi. ( https://review.coreboot.org/c/coreboot/+/37868 ) Change subject: mb/intel/tglrvp: Add initial mainboard code ...................................................................... mb/intel/tglrvp: Add initial mainboard code This is a initial mainboard code aimed to serve as base for further mainboard check-ins. This is a copy patch from icelake_rvp as on commit ID: I64db2460115f5fb35ca197b83440f8ee47470761 Below are the changes done over the copy patch: 1. Rename "Icelake" with "Tigerlake". 2. Replace "icelake_rvp" with "tglrvp". 3. Rename "icl" with "tgl". 4. Remove unwanted SPD file, add empty SPD as placeholder. 5. Replace "soc/intel/icelake" with "soc/intel/tigerlake". 6. Empty romstage_fsp_params.c, to fill it later with SOC specific config. 7. Empty GPIO configuration, to be filled as per board. 8. Change copyright year to 2019. 9. Add board support namely BOARD_INTEL_TGLRVP_UP3 10. Replace icl_u and icl_y variant with tglrvp variant. 11. Remove basebord gpio.c and rely on variant override. 12. Remove HDA verb table and config support. Changes to follow on top of this: 1. Add correct memory parameters, add SPDs. 2. Clean up devicetree as per tigerlake SOC. 3. Add GPIO support. 4. Update chromeos.fmd to make 32MB BIOS region. TEST=Build tigerlake rvp board Signed-off-by: Ravi Sarawadi <ravishankar.sarawadi(a)intel.com> Change-Id: I86ada611de1cf28a1b872eea35cf41c0dc1c57f1 --- A src/mainboard/intel/tglrvp/Kconfig A src/mainboard/intel/tglrvp/Kconfig.name A src/mainboard/intel/tglrvp/Makefile.inc A src/mainboard/intel/tglrvp/acpi/mainboard.asl A src/mainboard/intel/tglrvp/board_id.c A src/mainboard/intel/tglrvp/board_id.h A src/mainboard/intel/tglrvp/board_info.txt A src/mainboard/intel/tglrvp/bootblock.c A src/mainboard/intel/tglrvp/chromeos.c A src/mainboard/intel/tglrvp/chromeos.fmd A src/mainboard/intel/tglrvp/dsdt.asl A src/mainboard/intel/tglrvp/mainboard.c A src/mainboard/intel/tglrvp/romstage_fsp_params.c A src/mainboard/intel/tglrvp/spd/Makefile.inc A src/mainboard/intel/tglrvp/spd/empty.spd.hex A src/mainboard/intel/tglrvp/spd/spd.h A src/mainboard/intel/tglrvp/spd/spd_util.c A src/mainboard/intel/tglrvp/variants/baseboard/include/baseboard/ec.h A src/mainboard/intel/tglrvp/variants/baseboard/include/baseboard/gpio.h A src/mainboard/intel/tglrvp/variants/baseboard/include/baseboard/variants.h A src/mainboard/intel/tglrvp/variants/tglrvp_up3/Makefile.inc A src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb A src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c 23 files changed, 1,212 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/37868/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/37868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86ada611de1cf28a1b872eea35cf41c0dc1c57f1 Gerrit-Change-Number: 37868 Gerrit-PatchSet: 9 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: device/Kconfig: make sure display can't be selected by accident
by Michael Niewöhner (Code Review) 02 Jan '20

02 Jan '20

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Nico Huber, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/36444 to look at the new patch set (#9). Change subject: device/Kconfig: make sure display can't be selected by accident ...................................................................... device/Kconfig: make sure display can't be selected by accident Make sure display can't be selected by accident when NO_GFX_INIT is selected. Change-Id: Iec5a47f84b8c776a45edc6f4b31a03b9ac714b4e Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/device/Kconfig 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36444/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/36444 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec5a47f84b8c776a45edc6f4b31a03b9ac714b4e Gerrit-Change-Number: 36444 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: device/Kconfig: make sure display can't be selected by accident
by Michael Niewöhner (Code Review) 02 Jan '20

02 Jan '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36444 ) Change subject: device/Kconfig: make sure display can't be selected by accident ...................................................................... Uploaded patch set 9: Commit message was updated. -- To view, visit https://review.coreboot.org/c/coreboot/+/36444 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec5a47f84b8c776a45edc6f4b31a03b9ac714b4e Gerrit-Change-Number: 36444 Gerrit-PatchSet: 9 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 02 Jan 2020 18:52:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device/Kconfig: hide display menu when NO_GFX_INIT is selected
by Michael Niewöhner (Code Review) 02 Jan '20

02 Jan '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36444 ) Change subject: device/Kconfig: hide display menu when NO_GFX_INIT is selected ...................................................................... Patch Set 8: > Patch Set 8: > > Have been away too long from this and now I'm totally puzzled. Maybe > it's just a stale commit message? It seems this patch doesn't hide > the menu because that is already done. It just makes sure that one > can't select HAVE_* by accident, right? Hmm, I just tested this again without this patch and indeed, the display menu is hidden. That wasn't the case before cb:35726. I updated the commit msg since this is still useful -- To view, visit https://review.coreboot.org/c/coreboot/+/36444 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec5a47f84b8c776a45edc6f4b31a03b9ac714b4e Gerrit-Change-Number: 36444 Gerrit-PatchSet: 8 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 02 Jan 2020 18:52:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/malloc: Implement a simple free() only for last malloc()
by Aaron Durbin (Code Review) 02 Jan '20

02 Jan '20

Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37919 ) Change subject: lib/malloc: Implement a simple free() only for last malloc() ...................................................................... lib/malloc: Implement a simple free() only for last malloc() Implement a free() that supports only the last malloc(). Rewind the heap to the last allocation point if the ptr to be freed is matching the end of heap before last malloc(). With current situation, since free() is no-op, every call to malloc() is a memory leak. BUG=b:140124451 TEST=Wrote a test function to do malloc and free operations. Change-Id: I6d43cf54b79e6897cf6882335730b2310e4eae45 Signed-off-by: Bora Guvendik <bora.guvendik(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37919 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> --- M src/include/stdlib.h M src/lib/malloc.c 2 files changed, 30 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved diff --git a/src/include/stdlib.h b/src/include/stdlib.h index a8297f8..f6369bf 100644 --- a/src/include/stdlib.h +++ b/src/include/stdlib.h @@ -5,7 +5,6 @@ void *memalign(size_t boundary, size_t size); void *malloc(size_t size); -/* We never free memory */ -static inline void free(void *ptr) {} +void free(void *ptr); #endif /* STDLIB_H */ diff --git a/src/lib/malloc.c b/src/lib/malloc.c index aa266b4..ddc7ea5 100644 --- a/src/lib/malloc.c +++ b/src/lib/malloc.c @@ -11,6 +11,8 @@ extern unsigned char _heap, _eheap; static void *free_mem_ptr = &_heap; /* Start of heap */ static void *free_mem_end_ptr = &_eheap; /* End of heap */ +static void *free_last_alloc_ptr = &_heap; /* End of heap before + last allocation */ /* We don't restrict the boundary. This is firmware, * you are supposed to know what you are doing. @@ -26,6 +28,12 @@ p = free_mem_ptr; free_mem_ptr += size; + /* + * Store last allocation pointer after ALIGN, as malloc() will + * return it. This may cause n bytes of gap between allocations + * where n < boundary. + */ + free_last_alloc_ptr = p; if (free_mem_ptr >= free_mem_end_ptr) { printk(BIOS_ERR, "memalign(boundary=%zu, size=%zu): failed: ", @@ -46,3 +54,24 @@ { return memalign(sizeof(u64), size); } + +void free(void *ptr) +{ + if (ptr == NULL) + return; + + if (ptr < (void *)&_heap || ptr >= free_mem_end_ptr) { + printk(BIOS_WARNING, "Warning - Pointer passed to %s is not " + "pointing to the heap\n", __func__); + return; + } + + /* + * Rewind the heap pointer to the end of heap + * before the last successful malloc(). + */ + if (ptr == free_last_alloc_ptr) { + free_mem_ptr = free_last_alloc_ptr; + free_last_alloc_ptr = NULL; + } +} -- To view, visit https://review.coreboot.org/c/coreboot/+/37919 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6d43cf54b79e6897cf6882335730b2310e4eae45 Gerrit-Change-Number: 37919 Gerrit-PatchSet: 11 Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Thejaswani Putta <thejaswani.putta(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: lib/malloc: Implement a simple free() only for last malloc()
by Bora Guvendik (Code Review) 02 Jan '20

02 Jan '20

Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37919 ) Change subject: lib/malloc: Implement a simple free() only for last malloc() ...................................................................... Patch Set 10: > Patch Set 10: > > > Patch Set 10: > > > > Do you have statistics on different platforms how much memory is successfully freed? > > Is there a static code analyser to find missing free calls, now that it's doing something useful? > > This allows the code in src/commonlib/storage/storage_write.c to not exhaust the heap. With a simple alloc and free pattern. As for finding leaks of the heap w.r.t. missing free() calls that's a different issue all together. Most of the malloc() calls in coreboot are for longer lifetimes, however when there's a succession of malloc() and free() calls it's easy to exhaust the heap. No, I don't have statistics information, as Aaron mentioned, this is needed so that we don't run out of heap while using storage_block_fill_write(). -- To view, visit https://review.coreboot.org/c/coreboot/+/37919 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6d43cf54b79e6897cf6882335730b2310e4eae45 Gerrit-Change-Number: 37919 Gerrit-PatchSet: 10 Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Thejaswani Putta <thejaswani.putta(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 02 Jan 2020 18:21:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables: pass the early_mmc_wake_hw status to payload
by Bora Guvendik (Code Review) 02 Jan '20

02 Jan '20

Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25464 ) Change subject: coreboot_tables: pass the early_mmc_wake_hw status to payload ...................................................................... Patch Set 29: (1 comment) https://review.coreboot.org/c/coreboot/+/25464/29/payloads/libpayload/inclu… File payloads/libpayload/include/coreboot_tables.h: https://review.coreboot.org/c/coreboot/+/25464/29/payloads/libpayload/inclu… PS29, Line 299: Jedec Standard JESD83-B1 > are you sure ? […] do you mean this document is old and can't find it? I see it in newer JESD84-B51 (5.1 spec) as well. This is what I was referring to "The busy bit in the CMD1 response can be used by a device to tell the host that it is still working on its power-up/reset procedure (e.g., downloading the register information from memory field) and is not ready yet for communication. In this case the host must repeat CMD1 until the busy bit is cleared." So instead of payload sending: CMD0 -> CMD1 -> CMD1 ... ->CMD1 This patch allows coreboot to send CMD0. Payload will send only CMD1s until device it ready. This saves time since by the time we are in payload, device is ready or closer to being ready. -- To view, visit https://review.coreboot.org/c/coreboot/+/25464 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia4c57d05433c3966118c3642913d7017958cce55 Gerrit-Change-Number: 25464 Gerrit-PatchSet: 29 Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com> 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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Comment-Date: Thu, 02 Jan 2020 18:06:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/kconfig/nconf.c: Use strnlen vs. strlen and int vs. size_t
by Jude Rich (Code Review) 02 Jan '20

02 Jan '20

Jude Rich has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38140 ) Change subject: util/kconfig/nconf.c: Use strnlen vs. strlen and int vs. size_t ...................................................................... util/kconfig/nconf.c: Use strnlen vs. strlen and int vs. size_t strnlen helps prevent errors from no null termination. Also, sizeof/strnlen both return size_t, so we should use that rather than int. Additionally, formatting has been changed to fit with the rest of the file. Signed-off-by: Jude A Rich <juder11(a)gmail.com> Change-Id: I1c62b119bfd9953a7296ccd058d562c35cc9c75f --- M util/kconfig/nconf.c 1 file changed, 8 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/38140/1 diff --git a/util/kconfig/nconf.c b/util/kconfig/nconf.c index 4181c41..afbeb52 100644 --- a/util/kconfig/nconf.c +++ b/util/kconfig/nconf.c @@ -567,14 +567,16 @@ va_list ap; char *append_ptr; /* pointer to the free space in the target string */ int index = items_num-1; /* index of item to append string */ - int kmi_str_len; /* length of initial string */ - int str_space; /* length of unused space in string */ + size_t kmi_str_len; /* length of initial string */ + size_t str_space; /* length of unused space in string */ /* return if there's no item to work with */ if (index < 0) return; - kmi_str_len = strlen(k_menu_items[index].str); + kmi_str_len = strnlen( + k_menu_items[index].str, + sizeof(k_menu_items[index].str)); str_space = sizeof(k_menu_items[index].str) - kmi_str_len; append_ptr = k_menu_items[index].str + kmi_str_len; @@ -585,8 +587,9 @@ /* set menu item to new item string */ free_item(curses_menu_items[index]); - curses_menu_items[index] = new_item(k_menu_items[index].str, - k_menu_items[index].str); + curses_menu_items[index] = new_item( + k_menu_items[index].str, + k_menu_items[index].str); set_item_userptr(curses_menu_items[index], &k_menu_items[index]); } -- To view, visit https://review.coreboot.org/c/coreboot/+/38140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1c62b119bfd9953a7296ccd058d562c35cc9c75f Gerrit-Change-Number: 38140 Gerrit-PatchSet: 1 Gerrit-Owner: Jude Rich <juder11(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: util/kconfig/nconf.c: change strlen to strnlen and int to size_t
by Jude Rich (Code Review) 02 Jan '20

02 Jan '20

Jude Rich has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38139 ) Change subject: util/kconfig/nconf.c: change strlen to strnlen and int to size_t ...................................................................... util/kconfig/nconf.c: change strlen to strnlen and int to size_t size_t is a more appropriate data type for strings, and strnlen is safer than strlen when faced with no null termination. Function argument spacing has also been changed to be more consistent with the rest of the file. Signed-off-by: Jude A Rich <juder11(a)gmail.com> Change-Id: I950a65a8a2051e296c4e72afc4a72f789eb5be22 --- M util/kconfig/nconf.c 1 file changed, 8 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/38139/1 diff --git a/util/kconfig/nconf.c b/util/kconfig/nconf.c index 4181c41..afbeb52 100644 --- a/util/kconfig/nconf.c +++ b/util/kconfig/nconf.c @@ -567,14 +567,16 @@ va_list ap; char *append_ptr; /* pointer to the free space in the target string */ int index = items_num-1; /* index of item to append string */ - int kmi_str_len; /* length of initial string */ - int str_space; /* length of unused space in string */ + size_t kmi_str_len; /* length of initial string */ + size_t str_space; /* length of unused space in string */ /* return if there's no item to work with */ if (index < 0) return; - kmi_str_len = strlen(k_menu_items[index].str); + kmi_str_len = strnlen( + k_menu_items[index].str, + sizeof(k_menu_items[index].str)); str_space = sizeof(k_menu_items[index].str) - kmi_str_len; append_ptr = k_menu_items[index].str + kmi_str_len; @@ -585,8 +587,9 @@ /* set menu item to new item string */ free_item(curses_menu_items[index]); - curses_menu_items[index] = new_item(k_menu_items[index].str, - k_menu_items[index].str); + curses_menu_items[index] = new_item( + k_menu_items[index].str, + k_menu_items[index].str); set_item_userptr(curses_menu_items[index], &k_menu_items[index]); } -- To view, visit https://review.coreboot.org/c/coreboot/+/38139 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I950a65a8a2051e296c4e72afc4a72f789eb5be22 Gerrit-Change-Number: 38139 Gerrit-PatchSet: 1 Gerrit-Owner: Jude Rich <juder11(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mainboard/google: Allow Hatch variants to read SPD data over SMBus
by Edward O'Callaghan (Code Review) 02 Jan '20

02 Jan '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36449 ) Change subject: mainboard/google: Allow Hatch variants to read SPD data over SMBus ...................................................................... mainboard/google: Allow Hatch variants to read SPD data over SMBus All Hatch variants so far embed static SPD data encoded within the firmware image. However we wish the flexibility for romstage implementations that allow for reading the SPD data dynamically over SMBus. This romstage variant allows for reading the SPD data over SMBus. BRANCH=none BUG=b:143134702 TEST=./util/abuild/abuild -p none -t google/hatch -x -a Change-Id: I3516a46b91840a9f6d1f4cffb2553d939d79cda2 Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M src/mainboard/google/hatch/Kconfig M src/mainboard/google/hatch/Makefile.inc A src/mainboard/google/hatch/romstage_spd_smbus.c 3 files changed, 63 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/36449/1 diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 219be22..e339693 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -60,7 +60,11 @@ config ROMSTAGE_SPD_CBFS bool - default y + default y if !ROMSTAGE_SPD_SMBUS + +config ROMSTAGE_SPD_SMBUS + bool + default n config DRIVER_TPM_SPI_BUS default 0x1 diff --git a/src/mainboard/google/hatch/Makefile.inc b/src/mainboard/google/hatch/Makefile.inc index 3ed82e7..0740c08 100644 --- a/src/mainboard/google/hatch/Makefile.inc +++ b/src/mainboard/google/hatch/Makefile.inc @@ -21,6 +21,7 @@ ramstage-$(CONFIG_EC_GOOGLE_CHROMEEC) += ec.c romstage-$(CONFIG_ROMSTAGE_SPD_CBFS) += romstage_spd_cbfs.c +romstage-$(CONFIG_ROMSTAGE_SPD_SMBUS) += romstage_spd_smbus.c romstage-$(CONFIG_CHROMEOS) += chromeos.c verstage-$(CONFIG_CHROMEOS) += chromeos.c diff --git a/src/mainboard/google/hatch/romstage_spd_smbus.c b/src/mainboard/google/hatch/romstage_spd_smbus.c new file mode 100644 index 0000000..245b61d --- /dev/null +++ b/src/mainboard/google/hatch/romstage_spd_smbus.c @@ -0,0 +1,57 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <baseboard/variants.h> +#include <soc/cnl_memcfg_init.h> +#include <soc/romstage.h> +#include <variant/gpio.h> +#include <spd_bin.h> +#include <gpio.h> + +/* + * GPIO_MEM_CH_SEL is set to 1 for single channel skus + * and 0 for dual channel skus. + */ +#define GPIO_MEM_CH_SEL GPP_F2 + +void mainboard_memory_init_params(FSPM_UPD *memupd) +{ + int is_single_ch_mem; + struct cnl_mb_cfg memcfg; + variant_memory_params(&memcfg); + + /* + * GPP_F2 is the MEM_CH_SEL gpio, which is set to 1 for single + * channel skus and 0 for dual channel skus. + */ + is_single_ch_mem = gpio_get(GPIO_MEM_CH_SEL); + + /* Read spd block to get memory config */ + struct spd_block blk = { + .addr_map = { 0x50, 0x52 }, + }; + + memcfg.dq_pins_interleaved = 1; + get_spd_smbus(&blk); + memcfg.spd[0].read_type = READ_SMBUS; + memcfg.spd[0].spd_spec.spd_smbus_address = (uintptr_t)blk.spd_array[0]; + if (!is_single_ch_mem) { + memcfg.spd[1].read_type = READ_SMBUS; + memcfg.spd[1].spd_spec.spd_smbus_address = (uintptr_t)blk.spd_array[1]; + } + dump_spd_info(&blk); + + cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/36449 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3516a46b91840a9f6d1f4cffb2553d939d79cda2 Gerrit-Change-Number: 36449 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 25

Jump to page:

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2020