Attention is currently required from: Alexander Couzens, Evgeny Zinoviev.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80437?usp=email )
Change subject: ec/lenovo/h8/acpi: Support pulsing LEDLOGO on Haswell ThinkPads
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/lenovo/haswell/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/80437/comment/f7d94659_061a9912 :
PS1, Line 3: #define H8_HAS_LEDLOGO
It might be be better to implement this as a Kconfig and use something like `#if CONFIG(H8_HAS_LEDLOGO)` in `systemstatus.asl` instead of the `#if defined`
--
To view, visit https://review.coreboot.org/c/coreboot/+/80437?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: I85fb69c8c1bed8635a1b31e9b8385c7036bb46dd
Gerrit-Change-Number: 80437
Gerrit-PatchSet: 1
Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Sat, 10 Feb 2024 06:26:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Fred Reitberger, Jason Glenesk.
Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80413?usp=email )
Change subject: soc/amd/picasso: Use pcie_gpp_dxio_update_clk_req_config
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80413/comment/ec51ef94_f459f624 :
PS2, Line 11: inline
> in line
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80413?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: Ice2e3a5a78359da9a438434c7d4aa1eca878d396
Gerrit-Change-Number: 80413
Gerrit-PatchSet: 3
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Sat, 10 Feb 2024 02:19:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Fred Reitberger, Jason Glenesk, Varshit Pandya.
Hello Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80413?usp=email
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Use pcie_gpp_dxio_update_clk_req_config
......................................................................
soc/amd/picasso: Use pcie_gpp_dxio_update_clk_req_config
This function turns off gpp_clk for the devices which are disabled, and
adds the code to fix up the clock configuration depending on dxio
descriptors. Also this brings picasso in line with cezanne, mendocino
and phoenix. This also prepares picasso to use the common function
gpp_clk_setup_common.
Change-Id: Ice2e3a5a78359da9a438434c7d4aa1eca878d396
Signed-off-by: Varshit Pandya <pandyavarshit(a)gmail.com>
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/chip.h
M src/soc/amd/picasso/fch.c
3 files changed, 7 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/80413/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80413?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: Ice2e3a5a78359da9a438434c7d4aa1eca878d396
Gerrit-Change-Number: 80413
Gerrit-PatchSet: 3
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jason Glenesk.
Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80412?usp=email )
Change subject: vc/amd/fsp/picasso: Bring picasso inline with other AMD SoC
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80412/comment/010f1a4f_bce94c28 :
PS2, Line 7: inline
> in line […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80412?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: I9753acdff15921c84516ec873c925f36afdd2aa3
Gerrit-Change-Number: 80412
Gerrit-PatchSet: 3
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Comment-Date: Sat, 10 Feb 2024 02:18:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Varshit Pandya.
Hello Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80412?usp=email
to look at the new patch set (#3).
Change subject: vc/amd/fsp/picasso: Bring picasso inline with other AMD SoC
......................................................................
vc/amd/fsp/picasso: Bring picasso inline with other AMD SoC
In preparation to using gpp_clk_setup_common for picasso, bring enum
defined in picasso more in line with other AMD SoC.
Change-Id: I9753acdff15921c84516ec873c925f36afdd2aa3
Signed-off-by: Varshit Pandya <pandyavarshit(a)gmail.com>
---
M src/vendorcode/amd/fsp/picasso/platform_descriptors.h
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/80412/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80412?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: I9753acdff15921c84516ec873c925f36afdd2aa3
Gerrit-Change-Number: 80412
Gerrit-PatchSet: 3
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Alper Nebi Yasak.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80364?usp=email )
Change subject: mainboard/qemu-aarch64: Get top of memory from device-tree blob
......................................................................
Patch Set 2:
(1 comment)
File src/mainboard/emulation/qemu-aarch64/cbmem.c:
https://review.coreboot.org/c/coreboot/+/80364/comment/1e70ab06_fb71241f :
PS2, Line 12: top = fdt_get_memory_top((void *)_dram);
On all the other Arm devices we've followed the example from x86 to use `((uintptr_t)4 * GiB)` as cbmem_top if the physical DRAM goes beyond that point. There's no real reason to put CBMEM exactly at the end, it just needs to be high enough to have room to grow downwards. Since most of coreboot was originally designed for x86_32 we thought it would be better this way so that we'll never have any pointers go over the 32-bit limit, just in case there still is some old code that makes incorrect pointer size assumptions somewhere that we have overlooked.
--
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
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4cc888b57cf98e0797ce7f9ddfa2eb34d14cd9c1
Gerrit-Change-Number: 80364
Gerrit-PatchSet: 2
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Sat, 10 Feb 2024 01:05:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Alper Nebi Yasak, Julius Werner.
Julius Werner has posted comments on this change. ( 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 3:
(5 comments)
File src/lib/device_tree.c:
https://review.coreboot.org/c/coreboot/+/80322/comment/caf805ca_e1a01432 :
PS3, Line 181: if (be32_to_cpu(header->magic) != FDT_HEADER_MAGIC)
I assume that you're going to be needing more from that FDT blob in your boot model and this won't remain the only bit of code you need to access that wants to extract properties from a flattened DT blob? In that case I think we should start right away with adding more high-level access routines for use on FDT blobs. For the header check here it could be an `fdt_is_valid(void *blob)` that checks the magic number and maybe some of the other things `fdt_unflatten()` currently does as well (like compatible version). To read a node could have an `fdt_find_node()` analogous to our existing `dt_find_node()`... or, if you're worried about the inefficiencies of walking the tree multiple times, you could come up with some kind of API that finds multiple nodes at once:
```
struct fdt_lookup {
// fill out before passing into fdt_find_nodes()
const char *path;
// filled out by fdt_find_nodes()
void *fdt_node;
uint32_t addr_cells;
uint32_t size_cells;
}
enum cb_err fdt_find_nodes(struct fdt_lookup *lookups, size_t num_lookups);
```
Anyway, point being I think we should factor out and reuse the things we're going to need multiple times.
https://review.coreboot.org/c/coreboot/+/80322/comment/2ba0e5d2_c45f5a15 :
PS3, Line 203: offset
Aren't you hardcoding an assumption here that the `memory` node will come after the `#address-cells` and `#size-cells` nodes if you don't reset `offset` to 0?
https://review.coreboot.org/c/coreboot/+/80322/comment/0f5e9af6_6ab7bd5c :
PS3, Line 205: if (!strncmp(name, "memory", sizeof("memory")) ||
`strncmp(a, "literal", sizeof("literal"))` is just a more complicated way to write `strcmp(a, "literal")`.
https://review.coreboot.org/c/coreboot/+/80322/comment/8efd5c10_0a16ba19 :
PS3, Line 220: while ((size = fdt_next_property(blob, offset, &prop))) {
Same for properties, we should probably write an `fdt_find_prop()` helper rather than having to code this while loop again every time.
https://review.coreboot.org/c/coreboot/+/80322/comment/5f50e0e7_2808bf27 :
PS3, Line 234: uintptr_t mem_size = 0;
I would recommend using uint64_t when dealing with physical addresses and sizes. (For arm64 it doesn't make a difference, but e.g. arm32 systems can have more memory than they can physically address with a uintptr_t.)
--
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
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8bef09bc1bc4e324ebeaa37f78d67d3aa315f52c
Gerrit-Change-Number: 80322
Gerrit-PatchSet: 3
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Sat, 10 Feb 2024 00:57:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Varshit Pandya.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80412?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: vc/amd/fsp/picasso: Bring picasso inline with other AMD SoC
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80412/comment/6579b633_3f846a75 :
PS2, Line 7: inline
in line
same below
--
To view, visit https://review.coreboot.org/c/coreboot/+/80412?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: I9753acdff15921c84516ec873c925f36afdd2aa3
Gerrit-Change-Number: 80412
Gerrit-PatchSet: 2
Gerrit-Owner: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Comment-Date: Fri, 09 Feb 2024 19:50:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Stefan Reinauer.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80435?usp=email )
Change subject: crossgcc: Add buildgcc support for Apple M1/M2 devices
......................................................................
Patch Set 4: -Code-Review
(1 comment)
Patchset:
PS4:
Rebased just to be sure, since the coreboot-toolchain builder wasn't happy with patchset 2. Somehow Jenkins improved patchset 3 anyway.
There is an issue while building the aarch64 toolchain.
```
Building GCC v13.2.0 for target ... failed. Check 'build-aarch64-elf-GCC/build.log'.
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/80435?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: If4cca9d3e55051a6121d992e5320bee1df17af9f
Gerrit-Change-Number: 80435
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Fri, 09 Feb 2024 19:23:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment