coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: soc/mediatek/mt8195: Add APU device apc driver
by Yu-Ping Wu (Code Review) 09 Nov '21

09 Nov '21

Attention is currently required from: Hung-Te Lin, Paul Menzel, flora.fu(a)mediatek.com. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58969 ) Change subject: soc/mediatek/mt8195: Add APU device apc driver ...................................................................... Patch Set 6: (10 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/58969/comment/fb4216a5_5a6327a3 PS6, Line 12: are is https://review.coreboot.org/c/coreboot/+/58969/comment/dd1109a1_9741fbf1 PS6, Line 19: others other File src/soc/mediatek/mt8195/apusys_devapc.c: https://review.coreboot.org/c/coreboot/+/58969/comment/e5838a29_4eeb7fe7 PS6, Line 167: DEBUG ERR https://review.coreboot.org/c/coreboot/+/58969/comment/45b9b0a8_7ca3194a PS6, Line 175: DEBUG ERR https://review.coreboot.org/c/coreboot/+/58969/comment/3dc601ed_ae3fdd76 PS6, Line 196: ARRAY_SIZE(apusys_noc_dapc) Does this have the same meaning as APUSYS_NOC_DAPC_AO_SLAVE_NUM? If so, do we still need APUSYS_NOC_DAPC_AO_SLAVE_NUM? https://review.coreboot.org/c/coreboot/+/58969/comment/187bdbb7_c288278b PS6, Line 201: i, Can fit into previous line. Line length limit is now 96 chars in coreboot. https://review.coreboot.org/c/coreboot/+/58969/comment/29596df2_d876cdca PS6, Line 217: DEBUG ERR https://review.coreboot.org/c/coreboot/+/58969/comment/f9fc7d3f_da5cc608 PS6, Line 224: DEBUG ERR https://review.coreboot.org/c/coreboot/+/58969/comment/29439967_bb506863 PS6, Line 265: += Does it make sense to add all the return values? File src/soc/mediatek/mt8195/include/soc/apusys_devapc.h: https://review.coreboot.org/c/coreboot/+/58969/comment/9abf108c_b050b9b6 PS6, Line 6: APUSYS_APC_ERR_STATUS Lower case. -- To view, visit https://review.coreboot.org/c/coreboot/+/58969 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If92d3b02ac4966332315b85d68e0f48c6a9fce85 Gerrit-Change-Number: 58969 Gerrit-PatchSet: 6 Gerrit-Owner: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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: Rex-BC Chen <rex-bc.chen(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-Comment-Date: Tue, 09 Nov 2021 10:01:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/mediatek/mt8195: fix apusys coding defects
by flora.fu＠mediatek.com (Code Review) 09 Nov '21

09 Nov '21

1 0

Change in coreboot[master]: soc/mediatek/mt8195: fix apusys coding defects
by flora.fu＠mediatek.com (Code Review) 09 Nov '21

09 Nov '21

Attention is currently required from: Hung-Te Lin, Paul Menzel. flora.fu(a)mediatek.com has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59038 ) Change subject: soc/mediatek/mt8195: fix apusys coding defects ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/59038 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic03f71b7a9038edb5877ebd9b6aed5e9bd63c918 Gerrit-Change-Number: 59038 Gerrit-PatchSet: 2 Gerrit-Owner: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 09 Nov 2021 09:53:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/mediatek/mt8195: fix apusys coding defects
by flora.fu＠mediatek.com (Code Review) 09 Nov '21

09 Nov '21

Attention is currently required from: Hung-Te Lin, Paul Menzel, flora.fu(a)mediatek.com. Hello Hung-Te Lin, Paul Menzel, Yu-Ping Wu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59038 to look at the new patch set (#2). Change subject: soc/mediatek/mt8195: fix apusys coding defects ...................................................................... soc/mediatek/mt8195: fix apusys coding defects Use size_t for count variables. Reduce debug log level and fix typo. Fix commit: https://review.coreboot.org/c/coreboot/+/58794 BUG=b:203145462 BRANCH=cherry TEST=boot cherry correctly Signed-off-by: Flora Fu <flora.fu(a)mediatek.com> Change-Id: Ic03f71b7a9038edb5877ebd9b6aed5e9bd63c918 --- M src/soc/mediatek/mt8195/apusys.c 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/59038/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59038 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic03f71b7a9038edb5877ebd9b6aed5e9bd63c918 Gerrit-Change-Number: 59038 Gerrit-PatchSet: 2 Gerrit-Owner: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/mediatek/mt8195: Add APU device apc driver
by flora.fu＠mediatek.com (Code Review) 09 Nov '21

09 Nov '21

1 0

Change in coreboot[master]: soc/mediatek/mt8195: fix apusys coding defects
by Yu-Ping Wu (Code Review) 09 Nov '21

09 Nov '21

1 0

Change in coreboot[master]: arch/x86: Init firmware pointer for EC SMSC KBC1098/KBC1126 at build ...
by Bill XIE (Code Review) 09 Nov '21

09 Nov '21

Attention is currently required from: Nico Huber, Patrick Rudolph, Angel Pons, Julius Werner, Arthur Heymans, Michael Niewöhner, Kyösti Mälkki, Aaron Durbin. Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51671 ) Change subject: arch/x86: Init firmware pointer for EC SMSC KBC1098/KBC1126 at build time ...................................................................... Patch Set 24: (1 comment) File src/arch/x86/bootblock.ld: https://review.coreboot.org/c/coreboot/+/51671/comment/044b9276_f7da1d72 PS22, Line 57: _ID_SECTION_END - _ECFW_PTR > > It can only happen when someone messes up the code. […] ECFW_PTR_ADDR is not a user configurable symbol, and should be a fixed value according to the EC model, so it makes little sense to restrict its value, for it may even lie in the range of another stage for a different EC. As Nico said, it should be declared in arch/x86/Kconfig, and assigned in EC Kconfig. -- To view, visit https://review.coreboot.org/c/coreboot/+/51671 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4f0de0c4d7283e630242fbe84a46e0547783c49e Gerrit-Change-Number: 51671 Gerrit-PatchSet: 24 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Tue, 09 Nov 2021 09:41:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/mediatek/mt8195: Add APU device apc driver
by flora.fu＠mediatek.com (Code Review) 09 Nov '21

09 Nov '21

Attention is currently required from: Hung-Te Lin, Paul Menzel, Yu-Ping Wu. flora.fu(a)mediatek.com has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58969 ) Change subject: soc/mediatek/mt8195: Add APU device apc driver ...................................................................... Patch Set 6: (17 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/58969/comment/f5672ddf_a12bb47c PS2, Line 9: setup > set up Done https://review.coreboot.org/c/coreboot/+/58969/comment/86dcccd4_7fe6080b PS2, Line 9: Add APU device apc driver and setup permissions. > Please elaborate, what the APU device is, and what kind of permissions you mean. Done File src/soc/mediatek/mt8195/apusys_devapc.c: https://review.coreboot.org/c/coreboot/+/58969/comment/b69fd70a_2f783f97 PS2, Line 58: APUSYS_NOC_DAPC_AO > Lower case name. Done https://review.coreboot.org/c/coreboot/+/58969/comment/d791129e_d2af847c PS2, Line 78: APUSYS_AO_Devices > Lower case name. Done https://review.coreboot.org/c/coreboot/+/58969/comment/2714bbf8_b9d9357f PS2, Line 166: > _Static_assert(ARRAY_SIZE(APUSYS_AO_Devices) == 71); Done https://review.coreboot.org/c/coreboot/+/58969/comment/f9d0a6a2_38b87eb0 PS2, Line 179: 0x%x > %#x […] Done https://review.coreboot.org/c/coreboot/+/58969/comment/6d32d9b0_4a5c0d22 PS2, Line 181: ret = APUSYS_APC_ERR_PERMISSION_NOT_SUPPORTED; : goto exit; > return APUSYS_APC_ERR_PERMISSION_NOT_SUPPORTED; Done https://review.coreboot.org/c/coreboot/+/58969/comment/2ece2b00_8c394b59 PS2, Line 191: if (slave < APUSYS_NOC_DAPC_AO_SLAVE_NUM && : domain_id < APUSYS_NOC_DAPC_AO_DOM_NUM) { : base = (u32 *)((size_t)APUSYS_NOC_DAPC_AO_BASE + : domain_id * 0x40 + apc_register_index * 4); : apuapc_writel(apuapc_readl(base) & clr_bit, base); : apuapc_writel(apuapc_readl(base) | set_bit, base); : ret = APUSYS_APC_OK; : } else { : printk(BIOS_DEBUG, "[NOC_DAPC] %s: %s, %s:0x%x, %s:0x%x\n", : __func__, "out of boundary", "slave", slave, "domain_id", : domain_id); : ret = APUSYS_APC_ERR_OUT_OF_BOUNDARY; : } > Rewrite as […] Done https://review.coreboot.org/c/coreboot/+/58969/comment/a9371e55_f7feff66 PS2, Line 205: exit: : return ret; > Remove Done https://review.coreboot.org/c/coreboot/+/58969/comment/1fb0068b_1d655ac4 PS2, Line 212: u32 d, i; > Please use native types, for example, `size_t` or `unsigned int`, for count variables. Done https://review.coreboot.org/c/coreboot/+/58969/comment/46d2e0c1_803a4f9d PS2, Line 217: <= > Why is this "<=" instead of "<"? Should we define reg_num as […] Done https://review.coreboot.org/c/coreboot/+/58969/comment/ba36b382_42a512a8 PS2, Line 231: = NULL > No need to initialize. Done https://review.coreboot.org/c/coreboot/+/58969/comment/28d9d7f1_12aba3b0 PS2, Line 239: ret = APUSYS_APC_ERR_PERMISSION_NOT_SUPPORTED; : goto exit; > return APUSYS_APC_ERR_PERMISSION_NOT_SUPPORTED Done https://review.coreboot.org/c/coreboot/+/58969/comment/4a4a16b9_d1e0af59 PS2, Line 249: if (slave < APUSYS_APC_SYS0_AO_SLAVE_NUM && : domain_id < APUSYS_APC_SYS0_AO_DOM_NUM) { : base = (u32 *)((size_t)APUSYS_APC_AO_BASE + domain_id * 0x40 + : apc_register_index * 4); : apuapc_writel(apuapc_readl(base) & clr_bit, base); : apuapc_writel(apuapc_readl(base) | set_bit, base); : ret = APUSYS_APC_OK; : } else { : printk(BIOS_DEBUG, "[APUAPC] %s: %s, %s:0x%x, %s:0x%x\n", : __func__, "out of boundary", "slave", slave, "domain_id", : domain_id); : ret = APUSYS_APC_ERR_OUT_OF_BOUNDARY; : } : : exit: : return ret; > Same as set_slave_noc_dapc(). Please rewrite. Done https://review.coreboot.org/c/coreboot/+/58969/comment/184d4e56_2f9d66c2 PS2, Line 275: <= > Same. Done https://review.coreboot.org/c/coreboot/+/58969/comment/da629e93_f3331607 PS2, Line 377: set_bit > Write the value inline: […] Done. Thanks for your review. File src/soc/mediatek/mt8195/devapc.c: https://review.coreboot.org/c/coreboot/+/58969/comment/8cd5d4c7_46287a91 PS2, Line 1889: Setup > Set up Done -- To view, visit https://review.coreboot.org/c/coreboot/+/58969 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If92d3b02ac4966332315b85d68e0f48c6a9fce85 Gerrit-Change-Number: 58969 Gerrit-PatchSet: 6 Gerrit-Owner: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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: Rex-BC Chen <rex-bc.chen(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Tue, 09 Nov 2021 09:39:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/mediatek/mt8195: fix apusys coding defects
by flora.fu＠mediatek.com (Code Review) 09 Nov '21

09 Nov '21

1 0

Change in coreboot[master]: soc/mediatek/mt8195: Add APU device apc driver
by flora.fu＠mediatek.com (Code Review) 09 Nov '21

09 Nov '21

Attention is currently required from: Hung-Te Lin. Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/58969 to look at the new patch set (#6). Change subject: soc/mediatek/mt8195: Add APU device apc driver ...................................................................... soc/mediatek/mt8195: Add APU device apc driver Add APU device apc driver and set up permissions. APU has its own device apc for control access by domains. For Domain 0, the access to the following slave are restricted to security read and write: apusys_ao-2, apusys_ao-4, apusys_ao-5, apu_sctrl_reviser, apu_iommu0_r1 apu_iommu0_r2, apu_iommu0_r3, apu_iommu0_r4 apu_iommu1_r1, apu_iommu1_r2, apu_iommu1_r3,apu_iommu1_r4 For VPU, D0/D5 are set as no protection, other domains are forbidden. For other slaves, the D0 is no protection, others domains are forbidden. BUG=b:203145462 BRANCH=cherry TEST=boot cherry, check dump log and test permissions Signed-off-by: Flora Fu <flora.fu(a)mediatek.com> Change-Id: If92d3b02ac4966332315b85d68e0f48c6a9fce85 --- M src/soc/mediatek/mt8195/Makefile.inc A src/soc/mediatek/mt8195/apusys_devapc.c M src/soc/mediatek/mt8195/devapc.c M src/soc/mediatek/mt8195/include/soc/addressmap.h A src/soc/mediatek/mt8195/include/soc/apusys_devapc.h 5 files changed, 358 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/58969/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/58969 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If92d3b02ac4966332315b85d68e0f48c6a9fce85 Gerrit-Change-Number: 58969 Gerrit-PatchSet: 6 Gerrit-Owner: flora.fu(a)mediatek.com <flora.fu(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> 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: Rex-BC Chen <rex-bc.chen(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021