Attention is currently required from: Paul Menzel, Shelley Chen, Vamshi Krishna Gopal.
Terry Cheong has posted comments on this change by Terry Cheong. ( https://review.coreboot.org/c/coreboot/+/82794?usp=email )
Change subject: mb/google/brox: Enable Class-D calibration
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/82794/comment/0c147957_c5b56104?us… :
PS2, Line 10: based
: on the updated verb table provided by Realtek
> Yes. […]
Included the bug and comment number as well.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82794?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: I776f5c24ce3c829cbd64840957c1431608cf2b85
Gerrit-Change-Number: 82794
Gerrit-PatchSet: 5
Gerrit-Owner: Terry Cheong <htcheong(a)chromium.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Vamshi Krishna Gopal <vamshi.krishna.gopal(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Vamshi Krishna Gopal <vamshi.krishna.gopal(a)intel.com>
Gerrit-Comment-Date: Mon, 22 Jul 2024 04:18:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Terry Cheong <htcheong(a)chromium.org>
Attention is currently required from: Paul Menzel, Vamshi Krishna Gopal.
Terry Cheong has posted comments on this change by Terry Cheong. ( https://review.coreboot.org/c/coreboot/+/82794?usp=email )
Change subject: mb/google/brox: Enable Class-D calibration
......................................................................
Patch Set 5:
(3 comments)
This change is ready for review.
Commit Message:
https://review.coreboot.org/c/coreboot/+/82794/comment/28265e77_503a4e50?us… :
PS2, Line 9: DC offset of class-D amplifier is larger than expectation in Brox.
> Please add concrete values.
Added measured values in commit message.
https://review.coreboot.org/c/coreboot/+/82794/comment/bc4210f6_71a83dab?us… :
PS2, Line 10: based
: on the updated verb table provided by Realtek
> Is that a comment in the bug report?
Yes. Should I include the comment number in the bug report?
File src/mainboard/google/brox/variants/brox/include/variant/hda_verb.h:
https://review.coreboot.org/c/coreboot/+/82794/comment/e681efa5_38789aa8?us… :
PS3, Line 116: Class D
> In the commit message summary you use a hyphen: Class-D. Please find out the official spelling.
Done. From the datasheet, it should be hyphenated.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82794?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: I776f5c24ce3c829cbd64840957c1431608cf2b85
Gerrit-Change-Number: 82794
Gerrit-PatchSet: 5
Gerrit-Owner: Terry Cheong <htcheong(a)chromium.org>
Gerrit-Reviewer: Vamshi Krishna Gopal <vamshi.krishna.gopal(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Vamshi Krishna Gopal <vamshi.krishna.gopal(a)intel.com>
Gerrit-Comment-Date: Mon, 22 Jul 2024 04:17:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Hung-Te Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu.
Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83572?usp=email )
Change subject: soc/mediatek/mt8196: Add a stub implementation of the MT8196 SoC
......................................................................
Patch Set 7:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83572/comment/4eeecd55_1d7d7d57?us… :
PS4, Line 9: Add new folder and basic drivers for Mediatek SoC 'MT8196'.
> Previous discussed in https://review.coreboot. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83572?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: I8190253ed000db879b04a806ca0bdf29c14be806
Gerrit-Change-Number: 83572
Gerrit-PatchSet: 7
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Comment-Date: Mon, 22 Jul 2024 04:07:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Jarried Lin <jarried.lin(a)mediatek.com>
Attention is currently required from: Jarried Lin, Yu-Ping Wu.
Hello Yidi Lin, Yu-Ping Wu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83573?usp=email
to look at the new patch set (#9).
Change subject: mb/google/rauru: Add MediaTek MT8196 reference board
......................................................................
mb/google/rauru: Add MediaTek MT8196 reference board
Add mainboard folder and drivers for new reference board 'Rauru'.
TEST=saw the coreboot uart log to bootblock
BUG=b:317009620
Signed-off-by: Jarried Lin <jarried.lin(a)mediatek.corp-partner.google.com>
Change-Id: I789b622dcda999635f7aa2ce40adea6db28afa0e
---
A src/mainboard/google/rauru/Kconfig
A src/mainboard/google/rauru/Kconfig.name
A src/mainboard/google/rauru/Makefile.mk
A src/mainboard/google/rauru/board_info.txt
A src/mainboard/google/rauru/bootblock.c
A src/mainboard/google/rauru/chromeos.c
A src/mainboard/google/rauru/chromeos.fmd
A src/mainboard/google/rauru/devicetree.cb
A src/mainboard/google/rauru/mainboard.c
A src/mainboard/google/rauru/memlayout.ld
A src/mainboard/google/rauru/reset.c
A src/mainboard/google/rauru/romstage.c
12 files changed, 157 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/83573/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/83573?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: I789b622dcda999635f7aa2ce40adea6db28afa0e
Gerrit-Change-Number: 83573
Gerrit-PatchSet: 9
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
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-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Attention is currently required from: Jarried Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu.
Hello Hung-Te Lin, Yidi Lin, Yu-Ping Wu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83572?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/mediatek/mt8196: Add a stub implementation of the MT8196 SoC
......................................................................
soc/mediatek/mt8196: Add a stub implementation of the MT8196 SoC
Add new folder and basic drivers for Mediatek SoC 'MT8196'.
Refer to MT8196_Chromebook_Application_Processor_Datasheet_V1.0 for
MT8196 SPEC detail.
This patch also enables UART and ARM arch timer.
TEST=saw the coreboot uart log to bootblock
BUG=b:317009620
Change-Id: I8190253ed000db879b04a806ca0bdf29c14be806
Signed-off-by: Jarried Lin <jarried.lin(a)mediatek.corp-partner.google.com>
---
A src/soc/mediatek/mt8196/Kconfig
A src/soc/mediatek/mt8196/Makefile.mk
A src/soc/mediatek/mt8196/bootblock.c
A src/soc/mediatek/mt8196/emi.c
A src/soc/mediatek/mt8196/include/soc/addressmap.h
A src/soc/mediatek/mt8196/include/soc/emi.h
A src/soc/mediatek/mt8196/include/soc/memlayout.ld
A src/soc/mediatek/mt8196/include/soc/pll.h
A src/soc/mediatek/mt8196/include/soc/spi.h
A src/soc/mediatek/mt8196/include/soc/timer.h
A src/soc/mediatek/mt8196/soc.c
A src/soc/mediatek/mt8196/spi.c
A src/soc/mediatek/mt8196/timer.c
13 files changed, 334 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/83572/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/83572?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: I8190253ed000db879b04a806ca0bdf29c14be806
Gerrit-Change-Number: 83572
Gerrit-PatchSet: 7
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: Paul Menzel <paulepanter(a)mailbox.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>
Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Paul Menzel, SH Kim, Subrata Banik, Sumeet R Pawnikar, YH Lin, YH Lin.
Eric Lai has posted comments on this change by SH Kim. ( https://review.coreboot.org/c/coreboot/+/83479?usp=email )
Change subject: mb/google/brya/var/xol: Limit power limits for low/no battery case
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/83479?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: Ic19119042ffdcc15c72764d8c27bcdce9f229438
Gerrit-Change-Number: 83479
Gerrit-PatchSet: 7
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: YH Lin <yueherngl(a)chromium.org>
Gerrit-Comment-Date: Mon, 22 Jul 2024 02:14:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83320?usp=email )
Change subject: soc/intel/common/block/imc: Add Integrated Memory Controller driver
......................................................................
Patch Set 7:
(2 comments)
File src/soc/intel/common/block/imc/Kconfig:
https://review.coreboot.org/c/coreboot/+/83320/comment/65e419e0_187bbc18?us… :
PS3, Line 4: bool
> I guess you are using an IMC with an SMBUS in it? Would it be possible to be more explicit on naming […]
There was an imc.h, but seems no one is using it. soc/intel/common/block/include/intelblocks/imc.h.
I'm not sure if all intel imc is with a smbus controller inside it, or this is indicating a special kind of imc with smbus controller inside. If the former case, maybe to name it generally as SOC_INTEL_COMMON_BLOCK_IMC should be okay, but it would be great to add some comment message to indicate its functionality.
File src/soc/intel/common/block/smbus/smbuslib.c:
https://review.coreboot.org/c/coreboot/+/83320/comment/addb290b_1381597d?us… :
PS7, Line 38: static void spd_read(u8 *spd, u8 addr)
maybe this can be moved to a generic spd.c, which is implemented either by smbuslib, or the imc/spd.c
--
To view, visit https://review.coreboot.org/c/coreboot/+/83320?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: I3f47ddeda94d3882852d64c0052f8fb42b6b7ad2
Gerrit-Change-Number: 83320
Gerrit-PatchSet: 7
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Comment-Date: Mon, 22 Jul 2024 02:01:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Jérémy Compostella, yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83318?usp=email )
Change subject: soc/intel/common/systemagent: Improve systemagent
......................................................................
Patch Set 7:
(7 comments)
File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/83318/comment/5d42ca0e_97c2b477?us… :
PS3, Line 78: return;
> Yes because the remaining logics are all depends on the value of CAPID_A register.
Done
https://review.coreboot.org/c/coreboot/+/83318/comment/fa7dfcd0_cd140e1f?us… :
PS3, Line 133: .is_limit = CONFIG(TOUUD_LIMIT),
> I think there is no hints, it's specified in EDS. […]
Done
https://review.coreboot.org/c/coreboot/+/83318/comment/bb2742c1_4b4016cb?us… :
PS3, Line 176: value = ALIGN_DOWN(value + entry->align, entry->align);
> The lower bits of the registers are read as 0s but should be treated as 1s, thus aligning up works.
Done
File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/83318/comment/fad1118a_33561ef6?us… :
PS7, Line 308: return;
maybe no need HAVE_MULTIPLE_DOMAINS and the below logic will work.
if (!is_domain0(dev->upstream)) return.
File src/soc/intel/common/block/systemagent/systemagent_def.h:
https://review.coreboot.org/c/coreboot/+/83318/comment/6d5d15ac_390b3438?us… :
PS3, Line 73: * IS_LIMIT = If registers/offset indicates address limit or address limit plus 1.
> IS_LIMIT indicates whether the lower bits should be treated as 1s or 0s, although they are read as 0 […]
Not sure if below pattern fits for all limit cases or not.
value | (align - 1)
If yes, we can leave 'IS_LIMIT/is_limit'.
Otherwise, maybe a renaming will work, e.g. filling_low_bits
P.S. limit will not usually indicate address limit + 1 so I think we could be safe to remove that part in the comment block.
File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/83318/comment/cb2f33b0_4495d68d?us… :
PS3, Line 133: uint32_t tolud = pci_read_config32(SA_DEV_ROOT, TOLUD);
> If calling sa_read_map_entry(), we should pass a device to it, but this function may be called befor […]
Done
https://review.coreboot.org/c/coreboot/+/83318/comment/4eac69ef_523ba9c5?us… :
PS3, Line 161: return ALIGN_DOWN((pci_read_config32(SA_DEV_ROOT, TSEG + 4) +
> Yes because the lower bits are read as 0s but should be treated as 1s.
Can the process is simplified & clarified as:
ALIGN_DOWN(tolud, CONFIG_TOLUD_ALIGNMENT) | (CONFIG_TOLUD_ALIGNMENT - 1)
--
To view, visit https://review.coreboot.org/c/coreboot/+/83318?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: If32c2a6524c9d55ce7f9c3dd203bcf85cab76c2c
Gerrit-Change-Number: 83318
Gerrit-PatchSet: 7
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Mon, 22 Jul 2024 01:45:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>