Attention is currently required from: Bao Zheng, Felix Held, Fred Reitberger, Jason Glenesk, Julius Werner, Matt DeVillier, Zheng Bao.
Paul Menzel has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/85650?usp=email )
Change subject: soc/amd: Add some PSP commands
......................................................................
Patch Set 12:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85650/comment/8f52d6a2_00d66cec?us… :
PS12, Line 7: Add some PSP commands
Add PSP commands related to boot partition
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/85650/comment/7b4e89e1_4a04eb36?us… :
PS12, Line 180: // printk("toggle_bootpartition\n");
Uneeded?
--
To view, visit https://review.coreboot.org/c/coreboot/+/85650?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: Ia7f2eedae5b277745cb34a0761bd1a8b61441695
Gerrit-Change-Number: 85650
Gerrit-PatchSet: 12
Gerrit-Owner: Bao Zheng <fishbaozi(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: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 14 Jan 2025 21:10:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu.
Paul Menzel has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85978?usp=email )
Change subject: soc/mediatek/mt8196: Add RTC driver
......................................................................
Patch Set 1:
(5 comments)
File src/soc/mediatek/common/rtc_osc_init.c:
https://review.coreboot.org/c/coreboot/+/85978/comment/4a50d7b2_c396cfad?us… :
PS1, Line 7: u16
Why limit the length?
File src/soc/mediatek/mt8196/include/soc/rtc.h:
https://review.coreboot.org/c/coreboot/+/85978/comment/13b2c8c1_e9fd6690?us… :
PS1, Line 68: //void rtc_boot(void);
Remove?
File src/soc/mediatek/mt8196/mt6685_rtc.c:
https://review.coreboot.org/c/coreboot/+/85978/comment/40bf9107_0a95d1d1?us… :
PS1, Line 167: printk(BIOS_INFO, "EOSC cali val = %#x\n", val);
Sounds more like a debug message?
https://review.coreboot.org/c/coreboot/+/85978/comment/19739ab7_bb2327db?us… :
PS1, Line 221: printk(BIOS_INFO, "%s time out!\n", __func__);
Is this an error?
https://review.coreboot.org/c/coreboot/+/85978/comment/a06be23a_9db2c010?us… :
PS1, Line 497: void rtc_boot(void)
It looks like it’s used in other drivers too, but I find the name non-descriptive.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85978?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: I3dd337eaa3eed3012ddea300f7e04f2b63fb2daa
Gerrit-Change-Number: 85978
Gerrit-PatchSet: 1
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Comment-Date: Tue, 14 Jan 2025 21:08:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Hung-Te Lin, Jarried Lin, agogo.
Paul Menzel has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85888?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: soc/mediatek/mt8196: Initialize MCUPM
......................................................................
Patch Set 6:
(9 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85888/comment/96ba839d_e2acb043?us… :
PS6, Line 17: TEST=Build pass and we can see the mcupm logs after reset releases.
Please paste the new logs.
File src/soc/mediatek/mt8196/Kconfig:
https://review.coreboot.org/c/coreboot/+/85888/comment/18ff0ec3_bacf3f18?us… :
PS5, Line 64: The file name of the MediaTek MCUPM firmware.
> Paul, please see the MCUPM README in https://review.coreboot. […]
Thank you for the pointer. In my opinion it still should go into the commit message.
Additionally the commit also has the init sequence and it says something about secure master and so on. In my opinion this should be also described.
File src/soc/mediatek/mt8196/include/soc/mcupm_plat.h:
https://review.coreboot.org/c/coreboot/+/85888/comment/4530da61_914762fb?us… :
PS6, Line 56: MCUPM_SRAM_SIZE
I’d append _KB.
https://review.coreboot.org/c/coreboot/+/85888/comment/efaed8ec_d940ae4e?us… :
PS6, Line 69: POLLING_MCU_TIME
Please append the unit to the name.
https://review.coreboot.org/c/coreboot/+/85888/comment/2de02e7b_50860a2a?us… :
PS6, Line 79: 0x4
Hex notation not necessary?
File src/soc/mediatek/mt8196/mcupm.c:
https://review.coreboot.org/c/coreboot/+/85888/comment/1e9ff732_152b767a?us… :
PS6, Line 17: POLLING_MCU_TIME
retry takes number of attempts as first argument. Maybe use `wait_…()` from the stopwatch() framework `src/include/timer.h`.
https://review.coreboot.org/c/coreboot/+/85888/comment/9c2d8dc0_84427239?us… :
PS6, Line 21: "[EB_SPMC] Polling MCU_PORT_SET_R0_0 timeout, %#x\n",
Please also print the timeout value.
https://review.coreboot.org/c/coreboot/+/85888/comment/69919cbc_efa5fb7b?us… :
PS6, Line 72: int ret = 0;
Does not need to be initialized.
https://review.coreboot.org/c/coreboot/+/85888/comment/2b73118c_4b989120?us… :
PS6, Line 80: ret = eb_spmc_spm();
Return right away?
--
To view, visit https://review.coreboot.org/c/coreboot/+/85888?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: I223f245d384f32d54f6170a28b29573638f77296
Gerrit-Change-Number: 85888
Gerrit-PatchSet: 6
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: agogo <agogo.huang(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: agogo <agogo.huang(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Tue, 14 Jan 2025 21:02:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: agogo <agogo.huang(a)mediatek.corp-partner.google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Comment-In-Reply-To: Jarried Lin <jarried.lin(a)mediatek.com>
Attention is currently required from: Sean Rhodes.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/85715?usp=email
to look at the new patch set (#20).
Change subject: mb/starlabs/starbook: Add Meteor Lake (165H) variant
......................................................................
mb/starlabs/starbook: Add Meteor Lake (165H) variant
Tested using `edk2` from
`https://github.com/starlabsltd/edk2/tree/uefipayload_vs`:
* Ubuntu 24.04
* Manjaro 24
No known issues.
https://starlabs.systems/pages/starbook-specification
Change-Id: I6621585086c58d19574841314796ed9db779036e
Signed-off-by: Sean Rhodes <sean(a)starlabs.systems>
---
M Documentation/mainboard/index.md
A Documentation/mainboard/starlabs/starbook_mtl.md
M src/mainboard/starlabs/starbook/Kconfig
M src/mainboard/starlabs/starbook/Kconfig.name
M src/mainboard/starlabs/starbook/dsdt.asl
A src/mainboard/starlabs/starbook/variants/mtl/Makefile.mk
A src/mainboard/starlabs/starbook/variants/mtl/board.fmd
A src/mainboard/starlabs/starbook/variants/mtl/data.vbt
A src/mainboard/starlabs/starbook/variants/mtl/devicetree.cb
A src/mainboard/starlabs/starbook/variants/mtl/devtree.c
A src/mainboard/starlabs/starbook/variants/mtl/gpio.c
A src/mainboard/starlabs/starbook/variants/mtl/hda_verb.c
A src/mainboard/starlabs/starbook/variants/mtl/ramstage.c
A src/mainboard/starlabs/starbook/variants/mtl/romstage.c
14 files changed, 830 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/85715/20
--
To view, visit https://review.coreboot.org/c/coreboot/+/85715?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: I6621585086c58d19574841314796ed9db779036e
Gerrit-Change-Number: 85715
Gerrit-PatchSet: 20
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Attention is currently required from: Matt DeVillier, Paul Menzel.
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/85714?usp=email )
Change subject: mb/starlabs/starbook: Add Alder Lake-N (N200) variant
......................................................................
Patch Set 23:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85714/comment/f7ee84b5_8548012a?us… :
PS22, Line 7: Alder Lake N
> Thank you, but the hyphen/minus should stay though: Alder Lake-N.
Done
https://review.coreboot.org/c/coreboot/+/85714/comment/ca51169d_66d9051d?us… :
PS22, Line 10: `github.com/starlabsltd/edk2/tree/uefipayload_vs`:
> Make it a URL by prepending https://?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/85714?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: Id45e31b61046748a57c8104081f689057621bb04
Gerrit-Change-Number: 85714
Gerrit-PatchSet: 23
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 14 Jan 2025 20:48:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Matt DeVillier, Sean Rhodes.
Hello Matt DeVillier, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/85714?usp=email
to look at the new patch set (#23).
Change subject: mb/starlabs/starbook: Add Alder Lake-N (N200) variant
......................................................................
mb/starlabs/starbook: Add Alder Lake-N (N200) variant
Tested using `edk2` from
`https://github.com/starlabsltd/edk2/tree/uefipayload_vs`:
* Ubuntu 24.04
* Manjaro 24
No known issues.
https://starlabs.systems/pages/starbook-specification
Change-Id: Id45e31b61046748a57c8104081f689057621bb04
Signed-off-by: Sean Rhodes <sean(a)starlabs.systems>
---
M Documentation/mainboard/index.md
A Documentation/mainboard/starlabs/starbook_adl_n.md
M src/mainboard/starlabs/starbook/Kconfig
M src/mainboard/starlabs/starbook/Kconfig.name
A src/mainboard/starlabs/starbook/variants/adl_n/Makefile.mk
A src/mainboard/starlabs/starbook/variants/adl_n/board.fmd
A src/mainboard/starlabs/starbook/variants/adl_n/data.vbt
A src/mainboard/starlabs/starbook/variants/adl_n/devicetree.cb
A src/mainboard/starlabs/starbook/variants/adl_n/devtree.c
A src/mainboard/starlabs/starbook/variants/adl_n/gpio.c
A src/mainboard/starlabs/starbook/variants/adl_n/hda_verb.c
A src/mainboard/starlabs/starbook/variants/adl_n/ramstage.c
A src/mainboard/starlabs/starbook/variants/adl_n/romstage.c
A src/mainboard/starlabs/starbook/variants/adl_n/vboot.fmd
14 files changed, 1,119 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/85714/23
--
To view, visit https://review.coreboot.org/c/coreboot/+/85714?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: Id45e31b61046748a57c8104081f689057621bb04
Gerrit-Change-Number: 85714
Gerrit-PatchSet: 23
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Attention is currently required from: Ana Carolina Cabral.
Paul Menzel has posted comments on this change by Ana Carolina Cabral. ( https://review.coreboot.org/c/coreboot/+/85493?usp=email )
Change subject: mb/amd/birman_plus: Kconfig
......................................................................
Patch Set 12:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85493/comment/464f5214_bde6597b?us… :
PS12, Line 7: mb/amd/birman_plus: Kconfig
Please make it a statement by adding a verb (in imperative mood).
--
To view, visit https://review.coreboot.org/c/coreboot/+/85493?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: Ia310ea616006479b9a052afb99d08df6a11431f4
Gerrit-Change-Number: 85493
Gerrit-PatchSet: 12
Gerrit-Owner: Ana Carolina Cabral
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ana Carolina Cabral
Gerrit-Comment-Date: Tue, 14 Jan 2025 20:48:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Matt DeVillier, Sean Rhodes.
Paul Menzel has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/85714?usp=email )
Change subject: mb/starlabs/starbook: Add Alder Lake N (N200) variant
......................................................................
Patch Set 22: Code-Review+1
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85714/comment/8640e694_409432df?us… :
PS22, Line 7: Alder Lake N
Thank you, but the hyphen/minus should stay though: Alder Lake-N.
https://review.coreboot.org/c/coreboot/+/85714/comment/1a0b548d_be409f27?us… :
PS22, Line 10: `github.com/starlabsltd/edk2/tree/uefipayload_vs`:
Make it a URL by prepending https://?
--
To view, visit https://review.coreboot.org/c/coreboot/+/85714?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: Id45e31b61046748a57c8104081f689057621bb04
Gerrit-Change-Number: 85714
Gerrit-PatchSet: 22
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Tue, 14 Jan 2025 20:47:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Bao Zheng, Julius Werner, Marshall Dawson, Zheng Bao, ritul guru.
Felix Held has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/84530?usp=email )
Change subject: amdfwtool: Set address mode as relative table
......................................................................
Patch Set 8:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84530/comment/66316c75_4e14bba1?us… :
PS8, Line 12: the entry address mode is not set. That is not correct.
maybe i misread the code, but isn't the entry address mode set accordingly if the table address mode is set to relative table both before and after? looking at the spec, only the proper handling of the case of relative slot seems to be missing, since it says that this attribute will be ignored when the directory's address mode isn't 2 (AMD_ADDR_REL_TAB) or 3 (AMD_ADDR_REL_SLOT)
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/84530/comment/63af7d1a_ea288d93?us… :
PS8, Line 388: AMD_ADDR_REL_BIOS
shouldn't this be AMD_ADDR_REL_SLOT (which is 3) instead of AMD_ADDR_REL_BIOS (which is 1)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84530?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: I156b315d350d9e7217afc7442ca80277bb7f9095
Gerrit-Change-Number: 84530
Gerrit-PatchSet: 8
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Zheng Bao
Gerrit-Comment-Date: Tue, 14 Jan 2025 20:36:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No