Attention is currently required from: Felix Singer, Kapil Porwal, Nick Vaccaro.
Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81631?usp=email )
Change subject: mb/google/brya: Sort Kconfig option alphabetically
......................................................................
Patch Set 3:
(1 comment)
File src/mainboard/google/brya/Kconfig:
https://review.coreboot.org/c/coreboot/+/81631/comment/62517087_979b344e :
PS3, Line 574: config DRIVER_TPM_I2C_BUS
> How about combining multiple mainboards with conditions here? […]
that would result in quite long line,
but could be done in a follow up patch?
--
To view, visit https://review.coreboot.org/c/coreboot/+/81631?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I878c14058e1edc0f64e37c2fc16b8dcf75b90192
Gerrit-Change-Number: 81631
Gerrit-PatchSet: 3
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Fri, 05 Apr 2024 04:49:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-MessageType: comment
Attention is currently required from: Jakub Czapiga, Martin L Roth, Maximilian Brune.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81081?usp=email )
Change subject: lib/device_tree: Add some FDT helper functions
......................................................................
Patch Set 18:
(2 comments)
File src/include/device_tree.h:
https://review.coreboot.org/c/coreboot/+/81081/comment/920b5f7d_7326afa7 :
PS18, Line 203:
> To all functions?
Yes. I mean I'm not saying you need to do strict full doxygen style everywhere, but it should be possible to read the header and understand roughly what each function does and how to use it without digging through the code. In many cases that may be obvious already but for example for most of the return values here I don't think it is.
File src/lib/device_tree.c:
https://review.coreboot.org/c/coreboot/+/81081/comment/069740eb_9cf7f5c0 :
PS18, Line 175: ""
> I disagree. The root node is the same as any other node. Its name is an empty string. […]
Look at `dt_find_node()` which really does exactly the same thing, I think it works very cleanly. For the `/` case you will pass the path `{ NULL }` and then it just hits the `if (!*path)` base case of the recursion immediately.
In `fdt_find_node_by_path()`, if you use the `strtok_r()` solution as I suggested, it will automatically do the right thing for "/" (set `path_array[0]` to `NULL`), no special case needed.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81081?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2fb1d93c5b3e1cb2f7d9584db52bbce3767b63d8
Gerrit-Change-Number: 81081
Gerrit-PatchSet: 18
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Fri, 05 Apr 2024 01:09:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Paul Menzel.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69747?usp=email )
Change subject: arch/arm: Build test all arm targets with clang
......................................................................
Patch Set 22:
(1 comment)
File src/soc/nvidia/tegra124/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/69747/comment/3ae6ee98_68345737 :
PS22, Line 25: endif
I feel like it would be better to set CLANG_UNSUPPORTED than to do this tbh. When clang is supported people will assume that there's no practical difference between the two images, but changing optimization settings can make a pretty serious difference in boot speed.
--
To view, visit https://review.coreboot.org/c/coreboot/+/69747?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I88cf8ce16fb6c61c19d615e396f5871179b06fc8
Gerrit-Change-Number: 69747
Gerrit-PatchSet: 22
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 05 Apr 2024 00:35:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Jérémy Compostella, Philipp Hug, ron minnich.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81659?usp=email )
Change subject: Kconfig: Reverse ARCH_SUPPORTS_CLANG
......................................................................
Patch Set 1:
(1 comment)
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/81659/comment/530b5842_16086a0a :
PS1, Line 91: Opt-in flag for architectures that generally work well with CLANG.
Needs rewrite
--
To view, visit https://review.coreboot.org/c/coreboot/+/81659?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib28e7a4cb286b9f8b05be94dae3947179f43c746
Gerrit-Change-Number: 81659
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Fri, 05 Apr 2024 00:32:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Jon Murphy.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74501?usp=email )
Change subject: arch/arm64: Add Clang as supported target
......................................................................
Patch Set 27:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74501/comment/e110dc24_a40a303d :
PS26, Line 13: config files to achieve the same level of build testing.
> > Can you just change the sc7180 memlayout to make a bit more space instead? You can steal a few KB […]
Yes, in this case you can freely increase the bootblock size and reduce that other section by a bit.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74501?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I940a1ccf5cc4ec7bed5b6c8be92fc47922e1e747
Gerrit-Change-Number: 74501
Gerrit-PatchSet: 27
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jon Murphy <jpmurphy(a)google.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 05 Apr 2024 00:30:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81658?usp=email )
Change subject: arch/arm64: Use -mno-implicit-float with clang
......................................................................
Patch Set 1:
(1 comment)
File toolchain.mk:
https://review.coreboot.org/c/coreboot/+/81658/comment/0c91aaf4_56d6548a :
PS1, Line 103: endif
Wouldn't it make more sense to do this in xcompile?
--
To view, visit https://review.coreboot.org/c/coreboot/+/81658?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I24fa9d9f81430ea3ecd40de4304a10c6e235fece
Gerrit-Change-Number: 81658
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 05 Apr 2024 00:29:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80639?usp=email )
Change subject: soc/qualcomm/sc7{1,2}80: Increase romstage/verstage section for clang
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80639?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ide01233f209613678c5408f1afab19415c1071be
Gerrit-Change-Number: 80639
Gerrit-PatchSet: 6
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 05 Apr 2024 00:27:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment