coreboot-gerrit September 2024

coreboot-gerrit@coreboot.org

1 participants
2312 discussions

[S] Change in coreboot[main]: Makefile.mk: Change dependency on static.c to static.h
by Nicholas Chin (Code Review) 16 Sep '24

16 Sep '24

Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84387?usp=email ) Change subject: Makefile.mk: Change dependency on static.c to static.h ...................................................................... Makefile.mk: Change dependency on static.c to static.h The generated static.c file output by sconfig is currently added as a prerequisite for all objects to ensure that static.h exists before compiling anything that might need it. However, this forces every single object out of date when the compiled devicetree is updated, even though not every file actually needs static.h. Only static.h actually needs to exist before compilation of other objects, since static.c is an independent compilation unit that doesn't need to exist before other objects can be built. Thus, change the prerequisite from static.c to static.h, and add a rule for static.h that depends on static.c. The recipe is a simple `true` since sconfig generates static.c and static.h at the same time. To prevent unnecessary recompiles, make static.c a an order-only prerequisite [1] using the new order only prerequisite argument for create_cc_template to ensure that static.h exists before any object might need it, but without forcing a recompile of all objects by default whenever it is updated. On a clean build, all objects will be compiled since they do not exist, and these will occur after static.h is generated due to the default order-only prerequisite. On subsequent incremental compiles, sources that do need static.h will be appropriately marked out of date due to the generated .d dependency files from the compiler, which list static.h as a normal prerequisite for each objects that do include it, which overrides the default order-only prerequisite. The dependency files generated for all other objects will not include static.h, and thus the objects will not be updated since the default order-only dependency does not force them out of date. After updating the devicetree of qemu-i440f after a clean build, comparing the build log with `make --debug=why` with the generated dependency files indicates that only objects that actually depend on static.h were rebuilt, instead of every object. Running a timeless incremental build after making a change in the devicetree yielded identical roms when performed with this patch and main, with the only difference being the number of objects that needed to be rebuilt. Also tested with the E6430. [1] https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html Change-Id: I34efc162760ae703285f3982fa16cc23a86b37f6 Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M Makefile.mk 1 file changed, 11 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/84387/1 diff --git a/Makefile.mk b/Makefile.mk index a046438..1353b05 100644 --- a/Makefile.mk +++ b/Makefile.mk @@ -776,6 +776,10 @@ DEVICETREE_FWCONFIG_H := $(obj)/static_fw_config.h SCONFIG_OPTIONS += --output_f=$(DEVICETREE_FWCONFIG_H) +# Generated at the same time as static.c +$(DEVICETREE_STATIC_H): $(DEVICETREE_STATIC_C) + true + $(DEVICETREE_STATIC_C): $(DEVICETREE_FILE) $(OVERRIDE_DEVICETREE_FILE) $(CHIPSET_DEVICETREE_FILE) $(objutil)/sconfig/sconfig @printf " SCONFIG $(subst $(src)/,,$(<))\n" mkdir -p $(dir $(DEVICETREE_STATIC_C)) @@ -788,13 +792,13 @@ postcar-y+=$(DEVICETREE_STATIC_C) smm-y+=$(DEVICETREE_STATIC_C) -# Ensure static.c and static.h are created before any objects are compiled -ramstage-c-deps+=$(DEVICETREE_STATIC_C) -romstage-c-deps+=$(DEVICETREE_STATIC_C) -verstage-c-deps+=$(DEVICETREE_STATIC_C) -bootblock-c-deps+=$(DEVICETREE_STATIC_C) -postcar-c-deps+=$(DEVICETREE_STATIC_C) -smm-c-deps+=$(DEVICETREE_STATIC_C) +# Ensure static.h is created before any objects are compiled +ramstage-c-order-deps+=$(DEVICETREE_STATIC_H) +romstage-c-order-deps+=$(DEVICETREE_STATIC_H) +verstage-c-order-deps+=$(DEVICETREE_STATIC_H) +bootblock-c-order-deps+=$(DEVICETREE_STATIC_H) +postcar-c-order-deps+=$(DEVICETREE_STATIC_H) +smm-c-order-deps+=$(DEVICETREE_STATIC_H) # Ensure fmap_config.h are created before any objects are compiled ramstage-c-deps+=$(obj)/fmap_config.h -- To view, visit https://review.coreboot.org/c/coreboot/+/84387?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: I34efc162760ae703285f3982fa16cc23a86b37f6 Gerrit-Change-Number: 84387 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[XS] Change in coreboot[main]: Makefile: Allow defining order-only prerequisites in create_cc_template
by Nicholas Chin (Code Review) 16 Sep '24

16 Sep '24

Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84386?usp=email ) Change subject: Makefile: Allow defining order-only prerequisites in create_cc_template ...................................................................... Makefile: Allow defining order-only prerequisites in create_cc_template Generated header files such as static.h are currently added as prerequisites for all compilation units to ensure that they exist and are up to date before anything that might need them is compiled. However, this has the side effect of forcing every compilation unit out of date when the headers are regenerated, even if the object has no dependency on the header. GNU make has order-only prerequisites [1] which are used to define prerequisites that must be updated before a given target, but which don't force the target out of date when the prerequisite is updated. Add a new argument to create_cc_template, similar to the "additional dependencies" argument, which allows order-only dependencies for a specified object class and source suffix to be defined. This new functionality will be utilized in subsequent commits to fix up the dependencies on generated header files. Objects that do depend on generated headers will still be handled correctly due to the .d dependency files that are generated by the compiler during the build, which declare normal prerequisites to any headers an object directly or indirectly includes. As per the GNU make documentation, normal prerequisites take precedence over order-only prerequisites, so the header dependencies declared in the .d files will override the order-only one declared through create_cc_template. [1] https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html Change-Id: I50d87b3d9012967eefb197be12b2e0f096b0b67c Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M Makefile 1 file changed, 4 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/84386/1 diff --git a/Makefile b/Makefile index cc73900..dd97d83 100644 --- a/Makefile +++ b/Makefile @@ -392,16 +392,17 @@ # $2 source suffix (c, S, ld, ...) # $3 additional compiler flags # $4 additional dependencies +# $5 additional order only dependencies ifn$(EMPTY)def $(1)-objs_$(2)_template de$(EMPTY)fine $(1)-objs_$(2)_template ifn$(EMPTY)eq ($(filter ads adb,$(2)),) -$$(call src-to-obj,$1,$$(1).$2): $$(1).$2 $$(call create_ada_deps,$1,$$(call src-to-ali,$1,$$(1).$2)) $(4) +$$(call src-to-obj,$1,$$(1).$2): $$(1).$2 $$(call create_ada_deps,$1,$$(call src-to-ali,$1,$$(1).$2)) $(4) | $(5) @printf " GCC $$$$(subst $$$$(obj)/,,$$$$(@))\n" $(GCC_$(1)) \ $$$$(ADAFLAGS_$(1)) $$$$(addprefix -I,$$$$($(1)-ada-dirs)) \ $(3) -c -o $$$$@ $$$$< el$(EMPTY)se -$$(call src-to-obj,$1,$$(1).$2): $$(1).$2 $(KCONFIG_AUTOHEADER) $(4) +$$(call src-to-obj,$1,$$(1).$2): $$(1).$2 $(KCONFIG_AUTOHEADER) $(4) | $(5) @printf " CC $$$$(subst $$$$(obj)/,,$$$$(@))\n" $(CC_$(1)) \ -MMD $$$$(CPPFLAGS_$(1)) $$$$(CFLAGS_$(1)) -MT $$$$(@) \ @@ -416,7 +417,7 @@ $(foreach type,$(call filetypes-of-class,$(class)), \ $(eval $(class)-$(type)-ccopts += $(generic-$(type)-ccopts) $($(class)-generic-ccopts)) \ $(if $(generic-objs_$(type)_template_gen),$(eval $(call generic-objs_$(type)_template_gen,$(class))),\ - $(eval $(call create_cc_template,$(class),$(type),$($(class)-$(type)-ccopts),$($(class)-$(type)-deps)))))) + $(eval $(call create_cc_template,$(class),$(type),$($(class)-$(type)-ccopts),$($(class)-$(type)-deps),$($(class)-$(type)-order-deps)))))) foreach-src=$(foreach file,$($(1)-srcs),$(eval $(call $(1)-objs_$(subst .,,$(suffix $(file)))_template,$(basename $(file))))) $(eval $(foreach class,$(classes),$(call foreach-src,$(class)))) -- To view, visit https://review.coreboot.org/c/coreboot/+/84386?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: I50d87b3d9012967eefb197be12b2e0f096b0b67c Gerrit-Change-Number: 84386 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[XS] Change in coreboot[main]: Update opensbi submodule to upstream master
by Elyes Haouas (Code Review) 15 Sep '24

15 Sep '24

Elyes Haouas has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/84358?usp=email ) Change subject: Update opensbi submodule to upstream master ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/84358?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0bc93a425040f1dab7cf1a4afe31e3caa5257775 Gerrit-Change-Number: 84358 Gerrit-PatchSet: 1 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in coreboot[main]: mb/google/dedede: Select INTEL_CRASHLOG only for ChromeOS builds
by Matt DeVillier (Code Review) 15 Sep '24

15 Sep '24

Attention is currently required from: Jakub Czapiga, Jędrzej Ciupis, Subrata Banik. Matt DeVillier has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/84359?usp=email ) Change subject: mb/google/dedede: Select INTEL_CRASHLOG only for ChromeOS builds ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/dedede/Kconfig: https://review.coreboot.org/c/coreboot/+/84359/comment/6314b66e_ac529f16?us… : PS1, Line 39: select SOC_INTEL_CRASHLOG > to the problem that you faced while booting windows/linux on jsl Chromebook. actually, it seems to have nothing to do with OS or even payload; crashlog is just broken under JSL with upstream coreboot: [DEBUG] cmd_reg from pmc_make_ipc_cmd 1052838 [ERROR] Null dereference at eip: 0x76ac8512 [ERROR] Null dereference at eip: 0x76ac85ea [ERROR] Null dereference at eip: 0x76ac610c [DEBUG] PMC crashlog size in legacy mode = 0xd00 [DEBUG] Read ID for Telemetry: 0x0 differs from expected: 0x23 [ERROR] CPU crashlog capability not found. [DEBUG] PMC crashLog size in legacy mode : 0xD00 [DEBUG] Invalid data 0x0 at offset 0x1300 from addr 0xfe010000 of PMC SRAM. [DEBUG] legacy mode PMC crashlog size adjusted to: 0x0 [DEBUG] m_cpu_crashLog_size : 0x0 bytes [DEBUG] Skipping CPU crashLog collection. Data not present. unless this is expected behavior -- To view, visit https://review.coreboot.org/c/coreboot/+/84359?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: I53be4befe9a04bdaece21f40f93af6599baa7e0b Gerrit-Change-Number: 84359 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jędrzej Ciupis <jciupis(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jędrzej Ciupis <jciupis(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Comment-Date: Sun, 15 Sep 2024 19:01:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[L] Change in coreboot[main]: mb/asus: Add PRIME H610M-K D4
by Máté Kukri (Code Review) 15 Sep '24

15 Sep '24

Attention is currently required from: Máté Kukri. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84243?usp=email to look at the new patch set (#11). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb/asus: Add PRIME H610M-K D4 ...................................................................... mb/asus: Add PRIME H610M-K D4 See `Documentation/mainboard/asus/h610m-k.md`. TODO (please don't submit before this is removed): - Ramstage code for NCT6798D - No public datasheet, but likely similar to NCT6796D - Needed for PS/2 port, UART with IRQs, PWM fans, RGB header, port 80 header, etc Notes: - GPIO table obtained from vendor FW via inteltool+intelp2m - Obtained using hackily examining registers: - PCIe clk_src and clk_req map (the ICC and FIA sideband ports, and the src_muxsel register offsets in the PCH datasheet are an outright lie) - SATA port list - USB OC pins - Confirmed enabled USB port list by plugging in things :) - HDA verbs and VBT dumped from Linux sysfs under vendor FW Change-Id: I6ab821f6799009d231de4d21219f0ac2ae89c5c9 Signed-off-by: Mate Kukri <km(a)mkukri.xyz> --- A Documentation/mainboard/asus/h610m-k.md A src/mainboard/asus/h610m-k/Kconfig A src/mainboard/asus/h610m-k/Kconfig.name A src/mainboard/asus/h610m-k/Makefile.mk A src/mainboard/asus/h610m-k/board_info.txt A src/mainboard/asus/h610m-k/bootblock.c A src/mainboard/asus/h610m-k/data.vbt A src/mainboard/asus/h610m-k/devicetree.cb A src/mainboard/asus/h610m-k/dsdt.asl A src/mainboard/asus/h610m-k/gpio.h A src/mainboard/asus/h610m-k/hda_verb.c A src/mainboard/asus/h610m-k/ramstage.c A src/mainboard/asus/h610m-k/romstage.c 13 files changed, 759 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/84243/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/84243?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: I6ab821f6799009d231de4d21219f0ac2ae89c5c9 Gerrit-Change-Number: 84243 Gerrit-PatchSet: 11 Gerrit-Owner: Máté Kukri <km(a)mkukri.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Máté Kukri <km(a)mkukri.xyz>

1 0

[XS] Change in coreboot[main]: Makefile: Fix no-op incremental build
by Nicholas Chin (Code Review) 15 Sep '24

15 Sep '24

Attention is currently required from: Martin L Roth. Nicholas Chin has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/84385?usp=email ) Change subject: Makefile: Fix no-op incremental build ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: Personally I'd rather try and move away from all the quirks of GNU Make than introduce more GNUisms to fix other GNUisms, but that's probably a more involved process. This seemed like the simplest fix that minimally changes the current behaviour of the build process. -- To view, visit https://review.coreboot.org/c/coreboot/+/84385?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: I98c49d47f811e5cceebce7b7d54b282c773734e3 Gerrit-Change-Number: 84385 Gerrit-PatchSet: 2 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Sun, 15 Sep 2024 18:19:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: Makefile: Fix no-op incremental build
by Nicholas Chin (Code Review) 15 Sep '24

15 Sep '24

Attention is currently required from: Martin L Roth. Hello Martin L Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84385?usp=email to look at the new patch set (#2). Change subject: Makefile: Fix no-op incremental build ...................................................................... Makefile: Fix no-op incremental build If make is ran a second time after an initial clean compile, the entire rom will be rebuilt. Subsequent calls to make will not rebuild the rom. This initial rebuild was due to build/util/kconfig/conf being newer than config.h, and the subsequent rebuild of the header marked everything else as out of date. The reason conf was newer than config.h is because it was being treated as an intermediate file [1]. In the rule for config.h, conf is a prerequisite, but since it is treated as an intermediate, its rule will not be run if config.h exists and all the prerequisites of conf (i.e. its source files) are also up to date. On a clean build after a make menuconfig, config.h exists, satisfying these conditions. In this case, config.h is treated as being up to date even though conf does not exist. However, if another target does not exist and also has conf as a prerequisite, conf will then be built so that the target can be built. This caused conf to be newer than config.h, but by default GNU Make deletes intermediate files after a build which would prevent conf from affecting config.h on subsequent rebuilds. However, commit dd6efce934fb ("Makefile: Add .SECONDARY") adds the .SECONDARY special target, which prevents intermediate files from being deleted after the build [2]. Thus, conf persists to the first no-op build, making config.h out of date. Since config.h is updated during this first rebuild, conf is no longer newer than it, and thus subsequent no-op builds behave as expected. Fix this by preventing conf from being treated as an intermediate file by adding it as a prerequisite of the .NOTINTERMEDIATE special target, which causes conf to always be rebuilt if it does not exist. Thus, on the initial clean compile, config.h will be updated after building conf as a prerequisite, preventing config.h from being marked out of date on a subsequent rebuild. [1] https://www.gnu.org/software/make/manual/html_node/Chained-Rules.html [2] https://www.gnu.org/software/make/manual/html_node/Special-Targets.html Change-Id: I98c49d47f811e5cceebce7b7d54b282c773734e3 Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M Makefile 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/84385/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84385?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: I98c49d47f811e5cceebce7b7d54b282c773734e3 Gerrit-Change-Number: 84385 Gerrit-PatchSet: 2 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>

1 0

[XS] Change in coreboot[main]: soc/intel/alderlake: Enable CRASHLOG for Chrome OS
by Matt DeVillier (Code Review) 15 Sep '24

15 Sep '24

Attention is currently required from: Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro, Rishika Raj, Subrata Banik. Matt DeVillier has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84365?usp=email ) Change subject: soc/intel/alderlake: Enable CRASHLOG for Chrome OS ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/alderlake/Kconfig: https://review.coreboot.org/c/coreboot/+/84365/comment/de21dad6_3fedd1e5?us… : PS1, Line 87: MAINBOARD_HAS_CHROMEOS > > while this is consistent with the previous behavior, I'm not sure why you want to move it to the S […] Fair enough. But currently there is no mechanism to disable the feature via Kconfig, and it doesn't appear to have been tested for the non-CHROMEOS use case. -- To view, visit https://review.coreboot.org/c/coreboot/+/84365?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: Ia23ef1cbebdba9a3b724204eb25ee788afa3e8fd Gerrit-Change-Number: 84365 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Sun, 15 Sep 2024 18:06:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[XS] Change in coreboot[main]: Makefile: Fix no-op incremental build
by Nicholas Chin (Code Review) 15 Sep '24

15 Sep '24

Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84385?usp=email ) Change subject: Makefile: Fix no-op incremental build ...................................................................... Makefile: Fix no-op incremental build If make is ran a second time after an initial clean compile, the entire rom will be rebuilt. Subsequent calls to make will not rebuild the rom. This initial rebuild was due to build/util/kconfig/conf being newer than config.h, and the subsequent rebuild of the header marked everything else as out of date. The reason conf was newer than config.h is because it was being treated as an intermediate file [1]. In the rule for config.h, conf is a prerequisite, but since it is treated as an intermediate, its rule will not be run if config.h exists and all the prerequisites of conf (i.e. its source files) are also up to date. On a clean build after a make menuconfig, config.h exists, satisfying these conditions. In this case, config.h is treated as being up to date even though conf does not exist. However, if another target does not exist and also has conf as a prerequisite, conf will then be built so that the target can be built. This caused conf to be newer than config.h, but by default GNU Make deletes intermediate files after a build which would prevent conf from affecting config.h on subsequent rebuilds. However, commit dd6efce934fb ("Makefile: Add .SECONDARY") adds the .SECONDARY special target, which prevents intermediate files from being deleted after the build [2]. Thus, conf persists to the first no-op build, making config.h out of date. Since config.h is updated during this first rebuild, conf is no longer newer than it, and thus subsequent no-op builds behave as expected. Fix this by preventing conf from being treated as an intermediate file by adding it as a prerequisite of the .NOTINTERMEDIATE special target, which causes conf to always be rebuilt if it does not exist. Thus, on the initial clean compile, config.h will be updated after building conf as a prerequisite, preventing config.h from being marked out of date on a subsequent rebuild. [1] https://www.gnu.org/software/make/manual/html_node/Chained-Rules.html [2] https://www.gnu.org/software/make/manual/html_node/Special-Targets.html Change-Id: I98c49d47f811e5cceebce7b7d54b282c773734e3 Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M Makefile 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/84385/1 diff --git a/Makefile b/Makefile index cc73900..2dce0a1 100644 --- a/Makefile +++ b/Makefile @@ -229,6 +229,7 @@ .SECONDEXPANSION: .DELETE_ON_ERROR: +.NOTINTERMEDIATE: $(objutil)/kconfig/conf $(KCONFIG_AUTOHEADER): $(KCONFIG_CONFIG) $(objutil)/kconfig/conf $(MAKE) olddefconfig $(MAKE) syncconfig -- To view, visit https://review.coreboot.org/c/coreboot/+/84385?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: I98c49d47f811e5cceebce7b7d54b282c773734e3 Gerrit-Change-Number: 84385 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[XS] Change in coreboot[main]: mb/google/brya/var/trulo: Update ISH GPIO config for tablet mode switch
by Subrata Banik (Code Review) 15 Sep '24

15 Sep '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Paul Menzel, Varun Upadhyay. Subrata Banik has posted comments on this change by Varun Upadhyay. ( https://review.coreboot.org/c/coreboot/+/84351?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/google/brya/var/trulo: Update ISH GPIO config for tablet mode switch ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/84351/comment/49ba2f77_9ad20b5e?us… : PS2, Line 13: TEST=Builds successfully for google/trulo. > yes, Its been tested with ISH changes CL-5849310 Please mention the specific tests that were performed to ensure this CL fixes the intended issue. Booting to the OS is a general statement. -- To view, visit https://review.coreboot.org/c/coreboot/+/84351?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: Id22e397e46b522428ffdabe34a445ed7e4fb6fc5 Gerrit-Change-Number: 84351 Gerrit-PatchSet: 2 Gerrit-Owner: Varun Upadhyay <varun.upadhyay(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jayvik Desai <jayvik(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Varun Upadhyay <varun.upadhyay(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Sun, 15 Sep 2024 17:18:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Varun Upadhyay <varun.upadhyay(a)intel.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2024