coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3044 discussions

[XS] Change in coreboot[main]: soc/amd/common/include/acpi: add missing device/device.h include
by Felix Held (Code Review) 26 Jan '24

26 Jan '24

1 0

[XS] Change in coreboot[main]: soc/amd/common/fsp: use expected types for add_agesa_fsp_acpi_table
by Felix Held (Code Review) 26 Jan '24

26 Jan '24

1 0

[S] Change in coreboot[main]: soc/amd: use expected function signature for soc_acpi_write_tables
by Felix Held (Code Review) 26 Jan '24

26 Jan '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80218?usp=email ) Change subject: soc/amd: use expected function signature for soc_acpi_write_tables ...................................................................... soc/amd: use expected function signature for soc_acpi_write_tables A pointer to soc_acpi_write_tables gets assigned to the write_acpi_tables element of the device_operations struct, so make sure that teh function has the expected function signature which in this case means using unsigned long as type for both the 'current' parameter and the return value. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Iee45badb904fa20c6db146edbc00c40ca09361d1 --- M src/soc/amd/cezanne/agesa_acpi.c M src/soc/amd/cezanne/include/soc/acpi.h M src/soc/amd/glinda/agesa_acpi.c M src/soc/amd/glinda/include/soc/acpi.h M src/soc/amd/mendocino/agesa_acpi.c M src/soc/amd/mendocino/include/soc/acpi.h M src/soc/amd/phoenix/agesa_acpi.c M src/soc/amd/phoenix/include/soc/acpi.h M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/include/soc/acpi.h 10 files changed, 20 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/80218/1 diff --git a/src/soc/amd/cezanne/agesa_acpi.c b/src/soc/amd/cezanne/agesa_acpi.c index 5dfc59d..e22cc08 100644 --- a/src/soc/amd/cezanne/agesa_acpi.c +++ b/src/soc/amd/cezanne/agesa_acpi.c @@ -7,8 +7,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/cezanne/include/soc/acpi.h b/src/soc/amd/cezanne/include/soc/acpi.h index 5c8b5d2..37655a9 100644 --- a/src/soc/amd/cezanne/include/soc/acpi.h +++ b/src/soc/amd/cezanne/include/soc/acpi.h @@ -10,7 +10,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp); #endif /* AMD_CEZANNE_ACPI_H */ diff --git a/src/soc/amd/glinda/agesa_acpi.c b/src/soc/amd/glinda/agesa_acpi.c index e55431b..e04f7cb 100644 --- a/src/soc/amd/glinda/agesa_acpi.c +++ b/src/soc/amd/glinda/agesa_acpi.c @@ -9,8 +9,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/glinda/include/soc/acpi.h b/src/soc/amd/glinda/include/soc/acpi.h index 6f8e06c..d47ff002 100644 --- a/src/soc/amd/glinda/include/soc/acpi.h +++ b/src/soc/amd/glinda/include/soc/acpi.h @@ -12,7 +12,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp); #endif /* AMD_GLINDA_ACPI_H */ diff --git a/src/soc/amd/mendocino/agesa_acpi.c b/src/soc/amd/mendocino/agesa_acpi.c index 55267ec..8866fc4 100644 --- a/src/soc/amd/mendocino/agesa_acpi.c +++ b/src/soc/amd/mendocino/agesa_acpi.c @@ -9,8 +9,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/mendocino/include/soc/acpi.h b/src/soc/amd/mendocino/include/soc/acpi.h index e1994aa..5b72492 100644 --- a/src/soc/amd/mendocino/include/soc/acpi.h +++ b/src/soc/amd/mendocino/include/soc/acpi.h @@ -10,7 +10,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp); #endif /* AMD_MENDOCINO_ACPI_H */ diff --git a/src/soc/amd/phoenix/agesa_acpi.c b/src/soc/amd/phoenix/agesa_acpi.c index 98e25f6..e4fa2bf 100644 --- a/src/soc/amd/phoenix/agesa_acpi.c +++ b/src/soc/amd/phoenix/agesa_acpi.c @@ -10,8 +10,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/phoenix/include/soc/acpi.h b/src/soc/amd/phoenix/include/soc/acpi.h index 39687219..b3f7adc 100644 --- a/src/soc/amd/phoenix/include/soc/acpi.h +++ b/src/soc/amd/phoenix/include/soc/acpi.h @@ -12,7 +12,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp); #endif /* AMD_PHOENIX_ACPI_H */ diff --git a/src/soc/amd/picasso/agesa_acpi.c b/src/soc/amd/picasso/agesa_acpi.c index 5604faa..bde9543 100644 --- a/src/soc/amd/picasso/agesa_acpi.c +++ b/src/soc/amd/picasso/agesa_acpi.c @@ -539,8 +539,8 @@ return current; } -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp) { struct acpi_crat_header *crat; diff --git a/src/soc/amd/picasso/include/soc/acpi.h b/src/soc/amd/picasso/include/soc/acpi.h index 7a0f749..5f7e748 100644 --- a/src/soc/amd/picasso/include/soc/acpi.h +++ b/src/soc/amd/picasso/include/soc/acpi.h @@ -10,7 +10,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +unsigned long soc_acpi_write_tables(const struct device *device, unsigned long current, + acpi_rsdp_t *rsdp); #endif /* AMD_PICASSO_ACPI_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/80218?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: Iee45badb904fa20c6db146edbc00c40ca09361d1 Gerrit-Change-Number: 80218 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: soc/amd: rename agesa_write_acpi_tables to soc_acpi_write_tables
by Felix Held (Code Review) 26 Jan '24

26 Jan '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80217?usp=email ) Change subject: soc/amd: rename agesa_write_acpi_tables to soc_acpi_write_tables ...................................................................... soc/amd: rename agesa_write_acpi_tables to soc_acpi_write_tables It's not the the AGESA code that generates most of the ACPI tables, so rename the function. This also aligns the other SoCs more with Genoa. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I6b2e6c4cb7139c8bde01b4440ab2e923a1086827 --- M src/soc/amd/cezanne/agesa_acpi.c M src/soc/amd/cezanne/chip.c M src/soc/amd/cezanne/include/soc/acpi.h M src/soc/amd/glinda/agesa_acpi.c M src/soc/amd/glinda/chip.c M src/soc/amd/glinda/include/soc/acpi.h M src/soc/amd/mendocino/agesa_acpi.c M src/soc/amd/mendocino/chip.c M src/soc/amd/mendocino/include/soc/acpi.h M src/soc/amd/phoenix/agesa_acpi.c M src/soc/amd/phoenix/chip.c M src/soc/amd/phoenix/include/soc/acpi.h M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/chip.c M src/soc/amd/picasso/include/soc/acpi.h M src/soc/amd/stoneyridge/northbridge.c 16 files changed, 29 insertions(+), 29 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/80217/1 diff --git a/src/soc/amd/cezanne/agesa_acpi.c b/src/soc/amd/cezanne/agesa_acpi.c index e6ccd9c..5dfc59d 100644 --- a/src/soc/amd/cezanne/agesa_acpi.c +++ b/src/soc/amd/cezanne/agesa_acpi.c @@ -7,8 +7,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/cezanne/chip.c b/src/soc/amd/cezanne/chip.c index 4723321..1ef8fcc 100644 --- a/src/soc/amd/cezanne/chip.c +++ b/src/soc/amd/cezanne/chip.c @@ -35,7 +35,7 @@ static void soc_init(void *chip_info) { - default_dev_ops_root.write_acpi_tables = agesa_write_acpi_tables; + default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; amd_fsp_silicon_init(); diff --git a/src/soc/amd/cezanne/include/soc/acpi.h b/src/soc/amd/cezanne/include/soc/acpi.h index 1a0e2a1..5c8b5d2 100644 --- a/src/soc/amd/cezanne/include/soc/acpi.h +++ b/src/soc/amd/cezanne/include/soc/acpi.h @@ -10,7 +10,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp); #endif /* AMD_CEZANNE_ACPI_H */ diff --git a/src/soc/amd/glinda/agesa_acpi.c b/src/soc/amd/glinda/agesa_acpi.c index f6c3c98..e55431b 100644 --- a/src/soc/amd/glinda/agesa_acpi.c +++ b/src/soc/amd/glinda/agesa_acpi.c @@ -9,8 +9,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/glinda/chip.c b/src/soc/amd/glinda/chip.c index 32d3ce5..ff10b62 100644 --- a/src/soc/amd/glinda/chip.c +++ b/src/soc/amd/glinda/chip.c @@ -37,7 +37,7 @@ static void soc_init(void *chip_info) { - default_dev_ops_root.write_acpi_tables = agesa_write_acpi_tables; + default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; amd_fsp_silicon_init(); diff --git a/src/soc/amd/glinda/include/soc/acpi.h b/src/soc/amd/glinda/include/soc/acpi.h index 138086e..6f8e06c 100644 --- a/src/soc/amd/glinda/include/soc/acpi.h +++ b/src/soc/amd/glinda/include/soc/acpi.h @@ -12,7 +12,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp); #endif /* AMD_GLINDA_ACPI_H */ diff --git a/src/soc/amd/mendocino/agesa_acpi.c b/src/soc/amd/mendocino/agesa_acpi.c index 1e4d600..55267ec 100644 --- a/src/soc/amd/mendocino/agesa_acpi.c +++ b/src/soc/amd/mendocino/agesa_acpi.c @@ -9,8 +9,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/mendocino/chip.c b/src/soc/amd/mendocino/chip.c index 840d29a..50a1edf 100644 --- a/src/soc/amd/mendocino/chip.c +++ b/src/soc/amd/mendocino/chip.c @@ -35,7 +35,7 @@ static void soc_init(void *chip_info) { - default_dev_ops_root.write_acpi_tables = agesa_write_acpi_tables; + default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; amd_fsp_silicon_init(); diff --git a/src/soc/amd/mendocino/include/soc/acpi.h b/src/soc/amd/mendocino/include/soc/acpi.h index ac3c490..e1994aa 100644 --- a/src/soc/amd/mendocino/include/soc/acpi.h +++ b/src/soc/amd/mendocino/include/soc/acpi.h @@ -10,7 +10,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp); #endif /* AMD_MENDOCINO_ACPI_H */ diff --git a/src/soc/amd/phoenix/agesa_acpi.c b/src/soc/amd/phoenix/agesa_acpi.c index 598ae81..98e25f6 100644 --- a/src/soc/amd/phoenix/agesa_acpi.c +++ b/src/soc/amd/phoenix/agesa_acpi.c @@ -10,8 +10,8 @@ #include <soc/acpi.h> #include <types.h> -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp) { /* TODO: look into adding CRAT */ diff --git a/src/soc/amd/phoenix/chip.c b/src/soc/amd/phoenix/chip.c index ebc3c7c..970ac6f 100644 --- a/src/soc/amd/phoenix/chip.c +++ b/src/soc/amd/phoenix/chip.c @@ -37,7 +37,7 @@ static void soc_init(void *chip_info) { - default_dev_ops_root.write_acpi_tables = agesa_write_acpi_tables; + default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; amd_fsp_silicon_init(); diff --git a/src/soc/amd/phoenix/include/soc/acpi.h b/src/soc/amd/phoenix/include/soc/acpi.h index 823b9ce8..39687219 100644 --- a/src/soc/amd/phoenix/include/soc/acpi.h +++ b/src/soc/amd/phoenix/include/soc/acpi.h @@ -12,7 +12,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp); #endif /* AMD_PHOENIX_ACPI_H */ diff --git a/src/soc/amd/picasso/agesa_acpi.c b/src/soc/amd/picasso/agesa_acpi.c index 7a32779..5604faa 100644 --- a/src/soc/amd/picasso/agesa_acpi.c +++ b/src/soc/amd/picasso/agesa_acpi.c @@ -539,8 +539,8 @@ return current; } -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp) +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp) { struct acpi_crat_header *crat; diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c index 4ddfa56..45b4ca2 100644 --- a/src/soc/amd/picasso/chip.c +++ b/src/soc/amd/picasso/chip.c @@ -36,7 +36,7 @@ static void soc_init(void *chip_info) { - default_dev_ops_root.write_acpi_tables = agesa_write_acpi_tables; + default_dev_ops_root.write_acpi_tables = soc_acpi_write_tables; amd_fsp_silicon_init(); diff --git a/src/soc/amd/picasso/include/soc/acpi.h b/src/soc/amd/picasso/include/soc/acpi.h index 3b83123..7a0f749 100644 --- a/src/soc/amd/picasso/include/soc/acpi.h +++ b/src/soc/amd/picasso/include/soc/acpi.h @@ -10,7 +10,7 @@ #define ACPI_SCI_IRQ 9 -uintptr_t agesa_write_acpi_tables(const struct device *device, uintptr_t current, - acpi_rsdp_t *rsdp); +uintptr_t soc_acpi_write_tables(const struct device *device, uintptr_t current, + acpi_rsdp_t *rsdp); #endif /* AMD_PICASSO_ACPI_H */ diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 2b6ba45..5ac6909 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -131,9 +131,9 @@ return (unsigned long)current; } -static unsigned long agesa_write_acpi_tables(const struct device *device, - unsigned long current, - acpi_rsdp_t *rsdp) +static unsigned long soc_acpi_write_tables(const struct device *device, + unsigned long current, + acpi_rsdp_t *rsdp) { acpi_srat_t *srat; acpi_slit_t *slit; @@ -210,7 +210,7 @@ .enable_resources = pci_dev_enable_resources, .init = northbridge_init, .acpi_fill_ssdt = acpi_fill_root_complex_tom, - .write_acpi_tables = agesa_write_acpi_tables, + .write_acpi_tables = soc_acpi_write_tables, }; /* -- To view, visit https://review.coreboot.org/c/coreboot/+/80217?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: I6b2e6c4cb7139c8bde01b4440ab2e923a1086827 Gerrit-Change-Number: 80217 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: device: Add a helper function to add a downstream bus
by Nico Huber (Code Review) 26 Jan '24

26 Jan '24

Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Jincheng Li, Johnny Lin, Patrick Rudolph, Shuo Liu, Tim Chu. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80210?usp=email ) Change subject: device: Add a helper function to add a downstream bus ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/80210?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: Ic898189b92997b93304fcbf47c73e2bb5ec09023 Gerrit-Change-Number: 80210 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 26 Jan 2024 14:42:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: Add MTCL function to ACPI SSDT tables
by Arthur Heymans (Code Review) 26 Jan '24

26 Jan '24

Attention is currently required from: Cliff Huang, David Ruth, Lance Zhao, Paul Menzel, Subrata Banik, Subrata Banik, Tim Wawrzynczak. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email ) Change subject: Add MTCL function to ACPI SSDT tables ...................................................................... Patch Set 8: Code-Review+1 (2 comments) File src/drivers/wifi/generic/mtcl.c: https://review.coreboot.org/c/coreboot/+/80170/comment/d04e70f2_0d47f494 : PS8, Line 52: cbfs_map(WIFI_MTCL_CBFS_DEFAULT_FILENAME, &mtcl_bin_len); Why not use this pointer instead of having the caller provide a buffer (which may be too small btw). You'd have to unmap later after ACPI generation, but can skip the copying. File src/include/mtcl.h: https://review.coreboot.org/c/coreboot/+/80170/comment/18f0665c_3d82997b : PS8, Line 21: __packed Not necessary as all entries are u8? -- To view, visit https://review.coreboot.org/c/coreboot/+/80170?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: I9b5e7312a44e114270e664b983626faa6cfee350 Gerrit-Change-Number: 80170 Gerrit-PatchSet: 8 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Comment-Date: Fri, 26 Jan 2024 14:36:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: Add MTCL function to ACPI SSDT tables
by Nico Huber (Code Review) 26 Jan '24

26 Jan '24

Attention is currently required from: Cliff Huang, David Ruth, Lance Zhao, Paul Menzel, Subrata Banik, Subrata Banik, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email ) Change subject: Add MTCL function to ACPI SSDT tables ...................................................................... Patch Set 8: Code-Review+1 (5 comments) Patchset: PS6: > My answer applied to the data that is being stored in coreboot. […] First of all, don't worry about this comment. I'm merely curious. If you think it's taking too much of your time, please ignore it. The reason I ask is because I looked into the kernel driver and know too much already: There are three things coming together when the driver gets loaded: 1. driver code itself, obviously, 2. the information queried from ACPI, which your patch adds, 3. a firmware binary, I assume that gets loaded into the wifi chip. So there are three places where this country list could be stored, all available at the same time. But somebody picked ACPI for it and that usually has a reason. Thought you might know it. ACPI usually describes hardware, not countries. So this would only make sense if the country list is actually derived from some hardware setting (e.g. straps or registers configured by coreboot). Do you happen to know if the CBFS files you're going to add are specific to the board design or is it just "the file Mediatek provides us"? File src/acpi/acpigen.c: https://review.coreboot.org/c/coreboot/+/80170/comment/2723e204_b1ad1812 : PS8, Line 1940: * Generate ACPI AML code for MTCL method. Looking around, it seems there really are only standardized things in this file. If MTCL means what I think, in particular MT = Mediatek, it might be better to move it or maybe write a more generalized version, e.g. ``` void acpigen_write_buffer_as_int_package(const char *name, uint8_t *, size_t) ``` Don't know if this would be useful for any other use-case, though. It looks much like somebody confused `Buffer()` and `Package()` and now Mediatek is stuck with it. https://review.coreboot.org/c/coreboot/+/80170/comment/770e28a2_89458e92 : PS8, Line 1953: * } You could achieve the same with ``` Name (MTCL, Package() { // data } ``` i.e. without a method. In ACPI it's all just objects that evaluate to something. And that something has to be of the correct type. But what the actual object (named or method) is, doesn't really matter. File src/drivers/wifi/generic/acpi.c: https://review.coreboot.org/c/coreboot/+/80170/comment/027f36d8_3dba4596 : PS6, Line 592: uint8_t mtcl_package[sizeof(struct wifi_mtcl)]; > Addressing the organization of the code (and updated the commit message to hopefully make what I'm t […] Thanks for moving. I think everything is in a good place now. File src/vendorcode/google/chromeos/mtcl.c: https://review.coreboot.org/c/coreboot/+/80170/comment/98597b9d_cb864cac : PS6, Line 59: if (mtcl_bin_len != sizeof(struct wifi_mtcl)) { > and keep them localized to this file. You didn't, though. There's a `struct wifi_mtcl` at the calling site, even if it's just for a `sizeof()`. And that can make readers stumble. Upstream code is more often read than changed. I don't want to bikeshed this, though. It just looked odd that both sites knew the struct but it wasn't used in the signature, hence I asked. -- To view, visit https://review.coreboot.org/c/coreboot/+/80170?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: I9b5e7312a44e114270e664b983626faa6cfee350 Gerrit-Change-Number: 80170 Gerrit-PatchSet: 8 Gerrit-Owner: David Ruth <druth(a)chromium.org> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: David Ruth <druth(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Comment-Date: Fri, 26 Jan 2024 14:15:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: David Ruth <druth(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: util/ifdtool: Refactor GPR0 Unlock Implemetation
by Subrata Banik (Code Review) 26 Jan '24

26 Jan '24

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80216?usp=email ) Change subject: util/ifdtool: Refactor GPR0 Unlock Implemetation ...................................................................... util/ifdtool: Refactor GPR0 Unlock Implemetation This patch refactors GPR0 unlock function to add few important logic as below 1. Perform GPR0 unlock if GPR0 is locked. 2. While unlocking dump the GPRD PCH strap details 3. Additionally, print the GPR start and end range if GPR0 protection is enabled. TEST=Able to test GPR0 protection on google/rex and google/yahiko. Exp 1: Trying to unlock GPR0 protection for a locked image > ifdtool -p mtl -g image.bin -O image.bin_unlock File image.bin is 33554432 bytes Value at GPRD offset (64) is 0x83220004 --------- GPR0 Protected Range -------------- Start address = 0x00004000 End address = 0x00322fff Writing new image to image.bin_unlock Exp 2: Trying to unlock GPR0 protection for a unlocked image > ifdtool -p mtl -g image.bin_unlock -O image.bin_unlock File image.bin_unlock is 33554432 bytes GPR0 protection is already disabled Change-Id: Id35ebdefe83182ad7a3e735bdd2998baa0ec3ed7 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M util/ifdtool/ifdtool.c 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/80216/1 diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c index 191b321..07cc268 100644 --- a/util/ifdtool/ifdtool.c +++ b/util/ifdtool/ifdtool.c @@ -1577,10 +1577,28 @@ fprintf(stderr, "Disabling GPR0 not supported on this platform\n"); exit(EXIT_FAILURE); } + /* If bit 31 is set then GPR0 protection is enable */ + bool gpr0_status = fpsba->pchstrp[gpr0_offset] & 0x80000000; + if (!gpr0_status) { + printf("GPR0 protection is already disabled\n"); + return; + } + printf("Value at GPRD offset (%d) is 0x%08x\n", gpr0_offset, fpsba->pchstrp[gpr0_offset]); + printf("--------- GPR0 Protected Range --------------\n"); + /* + * Start Address: bit 0-15 of the GPRD represents the protected region start address, + * where bit 0-11 of the start address are assumed to be zero. + */ + printf("Start address = 0x%08x\n", (fpsba->pchstrp[gpr0_offset] & 0xffff) << 12); + /* + * End Address: bit 16-30 of the GPRD represents the protected region end address, + * where bit 0-11 of the end address are assumed to be 0xfff. + */ + printf("End address = 0x%08x\n", + ((fpsba->pchstrp[gpr0_offset] >> 16) & 0x7fff) << 12 | 0xfff); /* 0 means GPR0 protection is disabled */ fpsba->pchstrp[gpr0_offset] = 0; - write_image(filename, image, size); } -- To view, visit https://review.coreboot.org/c/coreboot/+/80216?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: Id35ebdefe83182ad7a3e735bdd2998baa0ec3ed7 Gerrit-Change-Number: 80216 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: src, util: Update toolchain.inc references to .mk
by Felix Singer (Code Review) 26 Jan '24

26 Jan '24

Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80129?usp=email ) Change subject: src, util: Update toolchain.inc references to .mk ...................................................................... src, util: Update toolchain.inc references to .mk Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: Ieaf7894f49a90f562b164924cc025e3eab5a3f7f Reviewed-on: https://review.coreboot.org/c/coreboot/+/80129 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> --- M src/arch/arm/include/arch/memlayout.h M src/arch/arm64/include/arch/memlayout.h M src/include/rules.h M util/lint/lint-stable-003-whitespace 4 files changed, 4 insertions(+), 4 deletions(-) Approvals: Felix Held: Looks good to me, approved build bot (Jenkins): Verified Felix Singer: Looks good to me, approved diff --git a/src/arch/arm/include/arch/memlayout.h b/src/arch/arm/include/arch/memlayout.h index 1395e82..408443f 100644 --- a/src/arch/arm/include/arch/memlayout.h +++ b/src/arch/arm/include/arch/memlayout.h @@ -20,7 +20,7 @@ /* ARM stacks need 8-byte alignment and stay in one place through ramstage. */ #define STACK(addr, size) \ REGION(stack, addr, size, 8) \ - _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.inc"); + _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.mk"); #define DMA_COHERENT(addr, size) \ REGION(dma_coherent, addr, size, SUPERPAGE_SIZE) \ diff --git a/src/arch/arm64/include/arch/memlayout.h b/src/arch/arm64/include/arch/memlayout.h index a04033d..69e4731 100644 --- a/src/arch/arm64/include/arch/memlayout.h +++ b/src/arch/arm64/include/arch/memlayout.h @@ -22,7 +22,7 @@ /* ARM64 stacks need 16-byte alignment. */ #define STACK(addr, size) \ REGION(stack, addr, size, 16) \ - _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.inc"); + _ = ASSERT(size >= 2K, "stack should be >= 2K, see toolchain.mk"); #define BL31(addr, size) \ REGION(bl31, addr, size, 4K) \ diff --git a/src/include/rules.h b/src/include/rules.h index 17ef198..04b0a82 100644 --- a/src/include/rules.h +++ b/src/include/rules.h @@ -156,7 +156,7 @@ #define ENV_STRING "UNKNOWN" #endif -/* Define helpers about the current architecture, based on toolchain.inc. */ +/* Define helpers about the current architecture, based on toolchain.mk. */ #if defined(__ARCH_arm__) #define ENV_ARM 1 diff --git a/util/lint/lint-stable-003-whitespace b/util/lint/lint-stable-003-whitespace index 4dd0260..b4e50f0 100755 --- a/util/lint/lint-stable-003-whitespace +++ b/util/lint/lint-stable-003-whitespace @@ -12,7 +12,7 @@ . "${LINTDIR}/helper_functions.sh" EXCLUDELIST='^src/vendorcode/|^util/kconfig/|^util/nvidia/cbootimage|^util/goswid|__pycache__|COPYING|LICENSE|README|_shipped$|\.patch$|\.bin$|\.hex$|\.jpg$|\.gif$|\.ttf$|\.woff$|\.png$|\.eot$|\.vbt$|\.ico$|\.md$|\.apcb$' -INCLUDELIST="src util payloads Makefile* toolchain.inc tests" +INCLUDELIST="src util payloads Makefile* toolchain.mk tests" # shellcheck disable=SC2086,SC2046 if uname | grep -qi "linux"; then -- To view, visit https://review.coreboot.org/c/coreboot/+/80129?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: Ieaf7894f49a90f562b164924cc025e3eab5a3f7f Gerrit-Change-Number: 80129 Gerrit-PatchSet: 5 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: Documentation: Update Makefile .inc references to .mk
by Felix Singer (Code Review) 26 Jan '24

26 Jan '24

Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80128?usp=email ) Change subject: Documentation: Update Makefile .inc references to .mk ...................................................................... Documentation: Update Makefile .inc references to .mk Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I464170e60a22f39225044c6794d091455d931e9c Reviewed-on: https://review.coreboot.org/c/coreboot/+/80128 Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M Documentation/getting_started/build_system.md M Documentation/getting_started/kconfig.md M Documentation/sbom/sbom.md M Documentation/technotes/2020-03-unit-testing-coreboot.md M Documentation/tutorial/part3.md 5 files changed, 13 insertions(+), 13 deletions(-) Approvals: Felix Singer: Looks good to me, approved build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/Documentation/getting_started/build_system.md b/Documentation/getting_started/build_system.md index 4013bc6..35ed187 100644 --- a/Documentation/getting_started/build_system.md +++ b/Documentation/getting_started/build_system.md @@ -7,10 +7,10 @@ The overhead of learning this new syntax is (hopefully) offset by its lower complexity. -The build system is defined in the toplevel `Makefile` and `toolchain.inc` +The build system is defined in the toplevel `Makefile` and `toolchain.mk` and is supposed to be generic (and is in fact used with a number of other projects). Project specific configuration should reside in files called -`Makefile.inc`. +`Makefile.mk`. In general, the build system provides a number of "classes" that describe various parts of the build. These cover the various build targets in coreboot @@ -36,7 +36,7 @@ ### subdirs `subdirs` contains subdirectories (relative to the current directory) that should also be handled by the build system. The build system expects these -directories to contain a file called `Makefile.inc`. +directories to contain a file called `Makefile.mk`. Subdirectories are not read at the point where the `subdirs` statement resides but later, after the current directory is handled (and potentially @@ -66,7 +66,7 @@ You can use the `add_intermediate` helper to add new post-processing steps for the final `coreboot.rom` image. For example you can add new files to CBFS by -adding something like this to `site-local/Makefile.inc` +adding something like this to `site-local/Makefile.mk` ``` $(call add_intermediate, add_mrc_data) @@ -100,4 +100,4 @@ all files. vboot provides its own implementation of `regions-for-file` that can be used -as reference in `src/vboot/Makefile.inc`. +as reference in `src/vboot/Makefile.mk`. diff --git a/Documentation/getting_started/kconfig.md b/Documentation/getting_started/kconfig.md index d40a4cc..ddd3276 100644 --- a/Documentation/getting_started/kconfig.md +++ b/Documentation/getting_started/kconfig.md @@ -963,7 +963,7 @@ for all of its symbols. The coreboot makefile forces the config.h file to be included into all coreboot -C files. This is done in Makefile.inc on the compiler command line using the +C files. This is done in Makefile.mk on the compiler command line using the “-include $(obj)/config.h” command line option. Example of various symbol types in the config.h file: diff --git a/Documentation/sbom/sbom.md b/Documentation/sbom/sbom.md index c4984f9..654fe55 100644 --- a/Documentation/sbom/sbom.md +++ b/Documentation/sbom/sbom.md @@ -114,7 +114,7 @@ as part of your software's build process. For example in form of a Makefile target. -2. Change src/sbom/Makefile.inc (in order to know where to find the +2. Change src/sbom/Makefile.mk (in order to know where to find the CoSWID/SWID/uSWID file) as well as the Makefile in coreboot which builds said software. For example for GRUB2 that could mean to add a Makefile target in payloads/external/GRUB2/Makefile. diff --git a/Documentation/technotes/2020-03-unit-testing-coreboot.md b/Documentation/technotes/2020-03-unit-testing-coreboot.md index a4d283f..d95b1a0 100644 --- a/Documentation/technotes/2020-03-unit-testing-coreboot.md +++ b/Documentation/technotes/2020-03-unit-testing-coreboot.md @@ -247,13 +247,13 @@ │ ├── include │ │ ├── mocks <- mock headers, which replace original headers │ │ -│ ├── Makefile.inc <- top Makefile for unit tests subsystem +│ ├── Makefile.mk <- top Makefile for unit tests subsystem │ ├── lib -│ │ ├── Makefile.inc +│ │ ├── Makefile.mk │ │ ├── string-test.c <- test code for src/lib/string.c │ │ │ │ ├── device -│ │ ├── Makefile.inc +│ │ ├── Makefile.mk │ ├── i2c-test.c │ ├── build diff --git a/Documentation/tutorial/part3.md b/Documentation/tutorial/part3.md index 39d6ed5..ec49637 100644 --- a/Documentation/tutorial/part3.md +++ b/Documentation/tutorial/part3.md @@ -96,8 +96,8 @@ be registered with the coreboot unit testing infrastructure. ``` -Every directory under `tests/` should contain a Makefile.inc, similar to -what can be seen under the `src/`. Register a new test in Makefile.inc, +Every directory under `tests/` should contain a Makefile.mk, similar to +what can be seen under the `src/`. Register a new test in Makefile.mk, by __appending__ test name to the `tests-y` variable. ```eval_rst @@ -285,7 +285,7 @@ coreboot unit test infrastructure supports overriding of functions at link time. This is as simple as adding a `name_of_function` to be -mocked into <test_name>-mocks variable in Makefile.inc. The result is +mocked into <test_name>-mocks variable in Makefile.mk. The result is that the test's implementation of that function is called instead of coreboot's. -- To view, visit https://review.coreboot.org/c/coreboot/+/80128?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: I464170e60a22f39225044c6794d091455d931e9c Gerrit-Change-Number: 80128 Gerrit-PatchSet: 5 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024