coreboot-gerrit May 2024

coreboot-gerrit@coreboot.org

1 participants
3267 discussions

[XS] Change in coreboot[main]: libpayload: Include libpayload-config.h in lib target
by Subrata Banik (Code Review) 29 May '24

29 May '24

Attention is currently required from: Subrata Banik. Hello Julius Werner, Raul Rangel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82687?usp=email to look at the new patch set (#4). Change subject: libpayload: Include libpayload-config.h in lib target ...................................................................... libpayload: Include libpayload-config.h in lib target - Added `$(obj)/libpayload-config.h` as a dependency for the `lib` target. - This ensures the config header is up-to-date before building the library. TEST=Able to build google/rex. Change-Id: If26336f6261aadf611fa5338c4300873156cc3da Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M payloads/libpayload/Makefile.mk 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/82687/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/82687?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If26336f6261aadf611fa5338c4300873156cc3da Gerrit-Change-Number: 82687 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>

1 0

[S] Change in coreboot[main]: tree: Remove unused <stddef.h>
by Elyes Haouas (Code Review) 28 May '24

28 May '24

Elyes Haouas has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82669?usp=email ) Change subject: tree: Remove unused <stddef.h> ...................................................................... tree: Remove unused <stddef.h> Change-Id: I7d7ad562eeff7247b7377b6570d489faee0aeda0 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82669 Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yidi Lin <yidilin(a)google.com> --- M src/drivers/uart/oxpcie_early.c M src/ec/google/chromeec/ec_skuid.c M src/ec/google/chromeec/ec_smbios.c M src/ec/purism/librem-ec/librem_ec.c M src/mainboard/intel/glkrvp/boardid.c M src/mainboard/intel/kblrvp/board_id.c M src/soc/amd/cezanne/psp_verstage/svc.c M src/soc/amd/glinda/psp_verstage/svc.c M src/soc/amd/mendocino/psp_verstage/svc.c M src/soc/amd/phoenix/psp_verstage/svc.c M src/soc/amd/picasso/psp_verstage/svc.c M src/soc/mediatek/mt8186/pll.c M src/soc/mediatek/mt8188/pll.c M src/soc/mediatek/mt8195/pll.c 14 files changed, 1 insertion(+), 14 deletions(-) Approvals: Yidi Lin: Looks good to me, approved build bot (Jenkins): Verified Yu-Ping Wu: Looks good to me, approved diff --git a/src/drivers/uart/oxpcie_early.c b/src/drivers/uart/oxpcie_early.c index 206f54d..7cc908c 100644 --- a/src/drivers/uart/oxpcie_early.c +++ b/src/drivers/uart/oxpcie_early.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <stdint.h> -#include <stddef.h> #include <device/pci_ops.h> #include <console/uart.h> #include <device/pci.h> diff --git a/src/ec/google/chromeec/ec_skuid.c b/src/ec/google/chromeec/ec_skuid.c index 259ce44..c0e079c 100644 --- a/src/ec/google/chromeec/ec_skuid.c +++ b/src/ec/google/chromeec/ec_skuid.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ -#include <stddef.h> #include <ec/google/chromeec/ec.h> uint32_t google_chromeec_get_board_sku(void) diff --git a/src/ec/google/chromeec/ec_smbios.c b/src/ec/google/chromeec/ec_smbios.c index d804f64..cd7ac2e 100644 --- a/src/ec/google/chromeec/ec_smbios.c +++ b/src/ec/google/chromeec/ec_smbios.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ -#include <stddef.h> #include <ec/google/chromeec/ec.h> #include <console/console.h> #include <string.h> diff --git a/src/ec/purism/librem-ec/librem_ec.c b/src/ec/purism/librem-ec/librem_ec.c index 8298b73..8cc7382 100644 --- a/src/ec/purism/librem-ec/librem_ec.c +++ b/src/ec/purism/librem-ec/librem_ec.c @@ -1,8 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <commonlib/bsd/helpers.h> #include "librem_ec.h" #include "../../system76/ec/system76_ec.h" -#include <stddef.h> #define CMD_PROBE 1 diff --git a/src/mainboard/intel/glkrvp/boardid.c b/src/mainboard/intel/glkrvp/boardid.c index 35dcc4b..ba2f879 100644 --- a/src/mainboard/intel/glkrvp/boardid.c +++ b/src/mainboard/intel/glkrvp/boardid.c @@ -2,7 +2,6 @@ #include <baseboard/variants.h> #include <boardid.h> -#include <stddef.h> #include <ec/acpi/ec.h> #define BOARD_ID_GLK_RVP1_DDR4 0x5 /* RVP1 - DDR4 */ diff --git a/src/mainboard/intel/kblrvp/board_id.c b/src/mainboard/intel/kblrvp/board_id.c index 4305295..23c9335 100644 --- a/src/mainboard/intel/kblrvp/board_id.c +++ b/src/mainboard/intel/kblrvp/board_id.c @@ -2,7 +2,6 @@ #include "board_id.h" #include <ec/acpi/ec.h> #include <stdint.h> -#include <stddef.h> /* * Get Board info via EC I/O port write/read diff --git a/src/soc/amd/cezanne/psp_verstage/svc.c b/src/soc/amd/cezanne/psp_verstage/svc.c index 17f9d51..b59b86f 100644 --- a/src/soc/amd/cezanne/psp_verstage/svc.c +++ b/src/soc/amd/cezanne/psp_verstage/svc.c @@ -3,7 +3,6 @@ #include <assert.h> #include <bl_uapp/bl_syscall_public.h> #include <psp_verstage.h> -#include <stddef.h> #include <svc_call.h> void svc_exit(uint32_t status) diff --git a/src/soc/amd/glinda/psp_verstage/svc.c b/src/soc/amd/glinda/psp_verstage/svc.c index d533d8f..531e3d5 100644 --- a/src/soc/amd/glinda/psp_verstage/svc.c +++ b/src/soc/amd/glinda/psp_verstage/svc.c @@ -7,7 +7,6 @@ #include <assert.h> #include <bl_uapp/bl_syscall_public.h> #include <psp_verstage.h> -#include <stddef.h> #include <string.h> #include <svc_call.h> diff --git a/src/soc/amd/mendocino/psp_verstage/svc.c b/src/soc/amd/mendocino/psp_verstage/svc.c index 0c6fb5e..ba35fa7 100644 --- a/src/soc/amd/mendocino/psp_verstage/svc.c +++ b/src/soc/amd/mendocino/psp_verstage/svc.c @@ -5,7 +5,6 @@ #include <assert.h> #include <bl_uapp/bl_syscall_public.h> #include <psp_verstage.h> -#include <stddef.h> #include <string.h> #include <svc_call.h> diff --git a/src/soc/amd/phoenix/psp_verstage/svc.c b/src/soc/amd/phoenix/psp_verstage/svc.c index 6f08678..1bea8fc 100644 --- a/src/soc/amd/phoenix/psp_verstage/svc.c +++ b/src/soc/amd/phoenix/psp_verstage/svc.c @@ -5,7 +5,6 @@ #include <assert.h> #include <bl_uapp/bl_syscall_public.h> #include <psp_verstage.h> -#include <stddef.h> #include <string.h> #include <svc_call.h> diff --git a/src/soc/amd/picasso/psp_verstage/svc.c b/src/soc/amd/picasso/psp_verstage/svc.c index c1af525..d23ea3b 100644 --- a/src/soc/amd/picasso/psp_verstage/svc.c +++ b/src/soc/amd/picasso/psp_verstage/svc.c @@ -3,7 +3,6 @@ #include <assert.h> #include <bl_uapp/bl_syscall_public.h> #include <psp_verstage.h> -#include <stddef.h> #include <svc_call.h> void svc_exit(uint32_t status) diff --git a/src/soc/mediatek/mt8186/pll.c b/src/soc/mediatek/mt8186/pll.c index a6165ef..6b9eae6 100644 --- a/src/soc/mediatek/mt8186/pll.c +++ b/src/soc/mediatek/mt8186/pll.c @@ -8,7 +8,6 @@ #include <console/console.h> #include <device/mmio.h> #include <delay.h> -#include <stddef.h> #include <timer.h> #include <soc/addressmap.h> diff --git a/src/soc/mediatek/mt8188/pll.c b/src/soc/mediatek/mt8188/pll.c index d892e1c..3dfaac9 100644 --- a/src/soc/mediatek/mt8188/pll.c +++ b/src/soc/mediatek/mt8188/pll.c @@ -3,7 +3,6 @@ #include <console/console.h> #include <device/mmio.h> #include <delay.h> -#include <stddef.h> #include <timer.h> #include <soc/addressmap.h> diff --git a/src/soc/mediatek/mt8195/pll.c b/src/soc/mediatek/mt8195/pll.c index 69b9c1e..397ff21 100644 --- a/src/soc/mediatek/mt8195/pll.c +++ b/src/soc/mediatek/mt8195/pll.c @@ -3,7 +3,6 @@ #include <console/console.h> #include <device/mmio.h> #include <delay.h> -#include <stddef.h> #include <timer.h> #include <soc/addressmap.h> -- To view, visit https://review.coreboot.org/c/coreboot/+/82669?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I7d7ad562eeff7247b7377b6570d489faee0aeda0 Gerrit-Change-Number: 82669 Gerrit-PatchSet: 5 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: mb/google/nissa/var/pujjoga: Tune SX9324 register for pujjoga
by Leo Chou (Code Review) 28 May '24

28 May '24

Leo Chou has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82689?usp=email ) Change subject: mb/google/nissa/var/pujjoga: Tune SX9324 register for pujjoga ...................................................................... mb/google/nissa/var/pujjoga: Tune SX9324 register for pujjoga Update SX9324 register settings based on tuning value from SEMTECH. BUG=b:340749850 TEST=Check i2c register settings on Pujjoga and confirm P sensor function can work. Change-Id: I205c1f5228d792afc763a06f74a8744918e2da75 Signed-off-by: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> --- M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/variants/pujjoga/overridetree.cb 2 files changed, 71 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/82689/1 diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index fcc2e63..c404060 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -417,6 +417,8 @@ config BOARD_GOOGLE_PUJJOGA select BOARD_GOOGLE_BASEBOARD_NISSA select DRIVERS_GENERIC_GPIO_KEYS + select DRIVERS_I2C_SX9324 + select DRIVERS_I2C_SX9324_SUPPORT_LEGACY_LINUX_DRIVER config BOARD_GOOGLE_QUANDISO select BOARD_GOOGLE_BASEBOARD_NISSA diff --git a/src/mainboard/google/brya/variants/pujjoga/overridetree.cb b/src/mainboard/google/brya/variants/pujjoga/overridetree.cb index 2804e16..847dccf 100644 --- a/src/mainboard/google/brya/variants/pujjoga/overridetree.cb +++ b/src/mainboard/google/brya/variants/pujjoga/overridetree.cb @@ -207,6 +207,75 @@ device generic 0 on end end end + device ref i2c2 on + chip drivers/i2c/sx9324 + register "desc" = ""SAR Proximity Sensor"" + register "irq" = "ACPI_IRQ_LEVEL_LOW(GPP_H19_IRQ)" + register "speed" = "I2C_SPEED_FAST" + register "uid" = "1" + register "reg_gnrl_ctrl0" = "0x16" + register "reg_gnrl_ctrl1" = "0x21" + register "reg_afe_ctrl0" = "0x00" + register "reg_afe_ctrl1" = "0x10" + register "reg_afe_ctrl2" = "0x00" + register "reg_afe_ctrl3" = "0x00" + register "reg_afe_ctrl4" = "0x47" + register "reg_afe_ctrl5" = "0x00" + register "reg_afe_ctrl6" = "0x00" + register "reg_afe_ctrl7" = "0x47" + register "reg_afe_ctrl8" = "0x12" + register "reg_afe_ctrl9" = "0x08" + register "reg_afe_ph0" = "0x3d" + register "reg_afe_ph1" = "0x1b" + register "reg_afe_ph2" = "0x1f" + register "reg_afe_ph3" = "0x3d" + register "reg_prox_ctrl0" = "0x0b" + register "reg_prox_ctrl1" = "0x0a" + register "reg_prox_ctrl2" = "0x90" + register "reg_prox_ctrl3" = "0x60" + register "reg_prox_ctrl4" = "0x0c" + register "reg_prox_ctrl5" = "0x00" + register "reg_prox_ctrl6" = "0x19" + register "reg_prox_ctrl7" = "0x58" + register "reg_adv_ctrl0" = "0x00" + register "reg_adv_ctrl1" = "0x00" + register "reg_adv_ctrl2" = "0x00" + register "reg_adv_ctrl3" = "0x00" + register "reg_adv_ctrl4" = "0x00" + register "reg_adv_ctrl5" = "0x05" + register "reg_adv_ctrl6" = "0x00" + register "reg_adv_ctrl7" = "0x00" + register "reg_adv_ctrl8" = "0x00" + register "reg_adv_ctrl9" = "0x00" + register "reg_adv_ctrl10" = "0x00" + register "reg_adv_ctrl11" = "0x00" + register "reg_adv_ctrl12" = "0x00" + register "reg_adv_ctrl13" = "0x00" + register "reg_adv_ctrl14" = "0x80" + register "reg_adv_ctrl15" = "0x0c" + register "reg_adv_ctrl16" = "0x08" + register "reg_adv_ctrl17" = "0x56" + register "reg_adv_ctrl18" = "0x33" + register "reg_adv_ctrl19" = "0x00" + register "reg_adv_ctrl20" = "0x00" + + register "ph0_pin" = "{1, 3, 3}" + register "ph1_pin" = "{3, 2, 1}" + register "ph2_pin" = "{3, 3, 1}" + register "ph3_pin" = "{1, 3, 3}" + register "ph01_resolution" = "1024" + register "ph23_resolution" = "1024" + register "startup_sensor" = "1" + register "ph01_proxraw_strength" = "3" + register "ph23_proxraw_strength" = "2" + register "avg_pos_strength" = "256" + register "cs_idle_sleep" = ""hi-z"" + register "int_comp_resistor" = ""lowest"" + register "input_precharge_resistor_ohms" = "4000" + register "input_analog_gain" = "1" + device i2c 28 on end + end + end device ref i2c3 on chip drivers/i2c/generic register "hid" = ""RTL5682"" -- To view, visit https://review.coreboot.org/c/coreboot/+/82689?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I205c1f5228d792afc763a06f74a8744918e2da75 Gerrit-Change-Number: 82689 Gerrit-PatchSet: 1 Gerrit-Owner: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com>

1 0

[XS] Change in coreboot[main]: libpayload: Include libpayload-config.h in lib target
by Subrata Banik (Code Review) 28 May '24

28 May '24

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82688?usp=email ) Change subject: libpayload: Include libpayload-config.h in lib target ...................................................................... libpayload: Include libpayload-config.h in lib target * Added `$(obj)/libpayload-config.h` as a dependency for the `lib` target. * This ensures the config header is up-to-date before building the library. Change-Id: If20acf05728a8606db49fb2c7bad06a1e79ea123 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M payloads/libpayload/Makefile.mk 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/82688/1 diff --git a/payloads/libpayload/Makefile.mk b/payloads/libpayload/Makefile.mk index d5a9a81..15110f5 100644 --- a/payloads/libpayload/Makefile.mk +++ b/payloads/libpayload/Makefile.mk @@ -96,7 +96,7 @@ cmp $@ $< 2>/dev/null || cp $< $@ library-targets = $(addsuffix .a,$(addprefix $(obj)/,$(libraries))) $(obj)/libpayload.a -lib: $$(library-targets) $(obj)/libpayload.ldscript +lib: $$(library-targets) $(obj)/libpayload-config.h $(obj)/libpayload.ldscript extract_nth=$(word $(1), $(subst |, ,$(2))) -- To view, visit https://review.coreboot.org/c/coreboot/+/82688?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If20acf05728a8606db49fb2c7bad06a1e79ea123 Gerrit-Change-Number: 82688 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>

1 0

[XS] Change in coreboot[main]: soc/intel/meteorlake: Hook up public microcode
by Subrata Banik (Code Review) 28 May '24

28 May '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Michał Kopeć, Michał Żygowski, Tarun. Subrata Banik has posted comments on this change by Michał Kopeć. ( https://review.coreboot.org/c/coreboot/+/82686?usp=email ) Change subject: soc/intel/meteorlake: Hook up public microcode ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/meteorlake/Kconfig: https://review.coreboot.org/c/coreboot/+/82686/comment/71d4e4a9_4b93da2b?us… : PS1, Line 39: select MICROCODE_BLOB_UNDISCLOSED > I assume you are using CPU_MICROCODE_CBFS_EXTERNAL_BINS then? So this change shouldn't have an effect on you, I think. these are my ucode related configs while building Google/rex. I think we still need `CONFIG_MICROCODE_BLOB_UNDISCLOSED` to be set for chromeos build. ``` CONFIG_CPU_INTEL_MICROCODE_CBFS_SPLIT_BINS=y CONFIG_CPU_INTEL_UCODE_SPLIT_BINARIES="3rdparty/blobs/mainboard/google/rex/microcode_inputs/rex0" CONFIG_SUPPORT_CPU_UCODE_IN_CBFS=y CONFIG_MICROCODE_BLOB_UNDISCLOSED=y ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/82686?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I16f20956a1490da02acc24156360aef235111494 Gerrit-Change-Number: 82686 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Wed, 29 May 2024 00:56:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[XS] Change in coreboot[main]: mainboard/qemu-aarch64: Get top of memory from device-tree blob
by Julius Werner (Code Review) 28 May '24

28 May '24

Attention is currently required from: Alper Nebi Yasak, Nico Huber. Julius Werner has posted comments on this change by Alper Nebi Yasak. ( https://review.coreboot.org/c/coreboot/+/80364?usp=email ) Change subject: mainboard/qemu-aarch64: Get top of memory from device-tree blob ...................................................................... Patch Set 4: (1 comment) File src/mainboard/emulation/qemu-aarch64/cbmem.c: https://review.coreboot.org/c/coreboot/+/80364/comment/1872cb3c_91d35b33?us… : PS2, Line 12: top = fdt_get_memory_top((void *)_dram); > Thanks for the explanation. […] For 64-bit platforms like this, you shouldn't need to worry about that... we've had platforms that returned exactly `0x100000000` for years now. The CBMEM code itself is robust about that, the worry is just about code using CBMEM areas for various things. For 32-bit platforms, using -1 should be fine (it would be more correct to say `-4*KiB` but the CBMEM/IMD code will do that for you). And yes, putting the -1 outside is not right, if you have less than 4GB RAM you probably want it right at the end or the remaining 4KB would likely be wasted. You should write it as `MIN(top, (uint64_t)4 * GiB - 1)`. -- To view, visit https://review.coreboot.org/c/coreboot/+/80364?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4cc888b57cf98e0797ce7f9ddfa2eb34d14cd9c1 Gerrit-Change-Number: 80364 Gerrit-PatchSet: 4 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Comment-Date: Wed, 29 May 2024 00:28:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>

1 0

[M] Change in coreboot[main]: device_tree: Add function to get top of memory from a FDT blob
by Julius Werner (Code Review) 28 May '24

28 May '24

Attention is currently required from: Alper Nebi Yasak, Jakub Czapiga, Maximilian Brune, Nico Huber. Julius Werner has posted comments on this change by Alper Nebi Yasak. ( https://review.coreboot.org/c/coreboot/+/80322?usp=email ) Change subject: device_tree: Add function to get top of memory from a FDT blob ...................................................................... Patch Set 4: (2 comments) File src/lib/device_tree.c: https://review.coreboot.org/c/coreboot/+/80322/comment/aa8e3478_93d13c9e?us… : PS3, Line 181: if (be32_to_cpu(header->magic) != FDT_HEADER_MAGIC) > Sorry for being very late with all this, I've been going through execution dysfunction hell for a lo […] At the end of the day some of these things will always remain a matter of taste, but now that CB:81081 has landed I hope we can stick to those APIs? File src/lib/device_tree.c: https://review.coreboot.org/c/coreboot/+/80322/comment/fe88c0f8_61647400?us… : PS4, Line 541: addrcp/sizecp should be reset to 2/1 here. (Maybe it would be cleaner to just call `fdt_find_node_by_path("/")`?) -- To view, visit https://review.coreboot.org/c/coreboot/+/80322?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8bef09bc1bc4e324ebeaa37f78d67d3aa315f52c Gerrit-Change-Number: 80322 Gerrit-PatchSet: 4 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Comment-Date: Wed, 29 May 2024 00:19:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>

1 0

[L] Change in coreboot[main]: arch/io.h: Add port I/O functions to other architectures
by Julius Werner (Code Review) 28 May '24

28 May '24

Attention is currently required from: Alper Nebi Yasak, Arthur Heymans, Jianjun Wang, Nico Huber, Ron Minnich, Shelley Chen, Yu-Ping Wu. Julius Werner has posted comments on this change by Alper Nebi Yasak. ( https://review.coreboot.org/c/coreboot/+/80372?usp=email ) Change subject: arch/io.h: Add port I/O functions to other architectures ...................................................................... Patch Set 5: (2 comments) File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/80372/comment/dcf87bc0_2fa5f5a2?us… : PS5, Line 532: CPU communicate with peripheral devices over PCI I/O space. > > Sorry, I have to admit I know basically nothing about PCI […] Hmm... well, to be honest the more I read about this the more I feel that this shouldn't be handled at a global level like this. Yes, it looks like Linux does it this way but that doesn't necessarily mean it's the best way. coreboot already has a `resource_t` system that's able to model both I/O port and MMIO resources with the same data type, and it seems like that's the best way to cleanly implement drivers that can work with either here. I think your problem may be(?) that you're trying to use the VGA driver and VGA is "special" in that it has "well-known" I/O ports (because it's a legacy PS/2 device, older than PCI), as opposed to PCI-enumerated I/O ports whose drivers already use the resource system anyway. Maybe for that it would be easiest to create a `CONFIG_VGA_MMIO_ADDRESS` Kconfig within that driver that specifically for that driver contains the correct base address for MMIO translation (and implement that in the fundamental accessor functions in `vga_io.c`). Other device drivers can either implement their own MMIO base address Kconfigs or use the resource system to take the right base address from `devicetree.cb`. https://review.coreboot.org/c/coreboot/+/80372/comment/c1eb62e2_7aa61c9a?us… : PS5, Line 536: default 0xffff Not sure what the point of this is? Just masking out the higher bits seems just as incorrect as accessing an invalid address. I'd say either leave this out completely or turn accesses beyond it into an error. -- To view, visit https://review.coreboot.org/c/coreboot/+/80372?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If7d9177283e8c692088ba8e30d6dfe52623c8cb9 Gerrit-Change-Number: 80372 Gerrit-PatchSet: 5 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Tue, 28 May 2024 23:45:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[L] Change in coreboot[main]: arch/io.h: Add port I/O functions to other architectures
by Alper Nebi Yasak (Code Review) 28 May '24

28 May '24

Attention is currently required from: Arthur Heymans, Jianjun Wang, Julius Werner, Nico Huber, Ron Minnich, Shelley Chen, Yu-Ping Wu. Alper Nebi Yasak has posted comments on this change by Alper Nebi Yasak. ( https://review.coreboot.org/c/coreboot/+/80372?usp=email ) Change subject: arch/io.h: Add port I/O functions to other architectures ...................................................................... Patch Set 5: (1 comment) File src/device/Kconfig: https://review.coreboot.org/c/coreboot/+/80372/comment/a1f5d3d2_5d8b90a0?us… : PS5, Line 532: CPU communicate with peripheral devices over PCI I/O space. > Sorry, I have to admit I know basically nothing about PCI Honestly, me too -- all I think I know is thanks to this endeavor. I don't really have solid answers for most of what you are asking. > So if we do translate this by just adding CONFIG_PCI_IOBASE to everything, where does that IOBASE come from in the hardware? Looks to me like the translation is handled by the PCIe Controller (a.k.a. Host Bridge? a.k.a. Root Complex?) hardware inside the SoC. I'd assume it's statically set up, and the address space details would be originally listed in each SoC's TRMs, but don't actually know. Maybe drivers for some hardware can set it up dynamically? > I mostly want to make sure that we can really have this thing in a Kconfig It's a simplification, in part due to me not fully understanding things. I found the [first article I linked](https://www.thegoodpenguin.co.uk/blog/outb-iowrite-and-friends/) while writing that comment here, which helped a lot, but I'm still deciphering things. Linux seems to be doing some virtual addressing and remapping stuff, and the proper addresses are usually from device-tree, and there's weird boards that don't use MMIO for it but run custom code? I checked some device-tree files in Linux, and some of them have multiple PCIe sections like that ([`sc8280xp-lenovo-thinkpad-x13s.dts`](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts?h=v6.10-rc1#n721) and [`sc8280xp.dtsi`](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sc8280xp.dtsi?h=v6.10-rc1#n1734) for example) so having multiple controllers appears to be possible. Some of these have what looks to me like I/O space ranges, so it's not a single global window, but per controller? There are a number of files with ranges commented as I/O if you want to check, try `git grep "^\s*ranges = <0x[0-9]1[0-9]*\s\+[0x]\+\s\+[0x]\+\s"` in kernel tree. --- I see there's a device-tree entry for PCIe host bridge in the QEMU ARM64 device-tree. Ideally, we should parse it and use mappings from that, e.g.: ``` pcie@10000000 { interrupt-map-mask = <0x1800 0x00 0x00 0x07>; interrupt-map = [...]; #interrupt-cells = <0x01>; ranges = <0x1000000 0x00 0x00 0x00 0x3eff0000 0x00 0x10000 0x2000000 0x00 0x10000000 0x00 0x10000000 0x00 0x2eff0000 0x3000000 0x80 0x00 0x80 0x00 0x80 0x00>; reg = <0x40 0x10000000 0x00 0x10000000>; msi-map = <0x00 0x8003 0x00 0x10000>; dma-coherent; bus-range = <0x00 0xff>; linux,pci-domain = <0x00>; #size-cells = <0x02>; #address-cells = <0x03>; device_type = "pci"; compatible = "pci-host-ecam-generic"; }; ``` I've formatted the ranges field to my best understanding. The first three cells are a PCI address, where the very first is [some weird bitfield](https://elinux.org/Device_Tree_Usage#PCI_Address_Translation). Next `0x00 0x3eff0000` is the `CONFIG_PCI_IOBASE` address that I've used and works. Last two `0x00 0x10000` is a 64K size. Also, other ranges look like `VIRT_PCIE_LOW/HIGH_MMIO_BASE` from mainboard `addressmap.h`, and the `reg` property looks like `CONFIG_ECAM_MMCONF_BASE_ADDRESS`. So probably all of `mainboard.c` `read_resources()` can be replaced by parsing the device-tree. But I probably won't be able to do that much work soon. -- To view, visit https://review.coreboot.org/c/coreboot/+/80372?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If7d9177283e8c692088ba8e30d6dfe52623c8cb9 Gerrit-Change-Number: 80372 Gerrit-PatchSet: 5 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.com> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Tue, 28 May 2024 21:20:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>

1 0

[XS] Change in coreboot[main]: include/device: Fix IO resource handling covering 0xFFFF
by Felix Held (Code Review) 28 May '24

28 May '24

Felix Held has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/82192?usp=email ) Change subject: include/device: Fix IO resource handling covering 0xFFFF ...................................................................... Patch Set 3: (1 comment) File src/include/device/device.h: https://review.coreboot.org/c/coreboot/+/82192/comment/d1a514bf_b6c72a28?us… : PS1, Line 377: end - base > Yes ... For the end-base I kept it as uint16_t due to that will not be exceeded in actual cases. eventually we should change the whole resource api so that the end parameter of the *_from_to functions is the last location instead of the location after the region. the current behavior has bugged me for a while, but haven't gotten around to look into changing this. might be that Kyosti's patch train that has been abandoned long time ago did that in one of the later patches after the not-for-merge patch that didn't end up getting submitted -- To view, visit https://review.coreboot.org/c/coreboot/+/82192?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie83045683094d6330c1676809f83acf30175cc90 Gerrit-Change-Number: 82192 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 28 May 2024 21:09:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2024