coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3269 discussions

[M] Change in coreboot[main]: soc/mediatek/common/dp: Add read/write APIs for DP Phy register
by Yidi Lin (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Yu-Ping Wu. Yidi Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85914?usp=email ) Change subject: soc/mediatek/common/dp: Add read/write APIs for DP Phy register ...................................................................... soc/mediatek/common/dp: Add read/write APIs for DP Phy register MT8196's eDP architecture is different from previous SoCs. DP Phy needs to be configured during the initialization. Add read/write APIs for DP Phy register configuration. Add a mock definition EDP_PHY_BASE for the SoC that do not support DP Phy configuration. BUG=b:382363408 TEST=emerge-geralt coreboot && emerge-rauru coreboot TEST=check FW screen on Navi Change-Id: I5c00d0aa7e35f03cc3c3aef6a58eadd3d334d8ed Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M src/soc/mediatek/common/dp/dptx_common.c M src/soc/mediatek/common/dp/dptx_hal_common.c M src/soc/mediatek/common/dp/include/soc/dptx_common.h M src/soc/mediatek/common/dp/include/soc/dptx_hal_common.h M src/soc/mediatek/mt8188/include/soc/dptx.h M src/soc/mediatek/mt8195/include/soc/dptx.h 6 files changed, 67 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/85914/1 diff --git a/src/soc/mediatek/common/dp/dptx_common.c b/src/soc/mediatek/common/dp/dptx_common.c index 0d078cd..b43bf51 100644 --- a/src/soc/mediatek/common/dp/dptx_common.c +++ b/src/soc/mediatek/common/dp/dptx_common.c @@ -215,6 +215,7 @@ void dptx_init_variable(struct mtk_dp *mtk_dp) { mtk_dp->regs = (void *)EDP_BASE; + mtk_dp->phy_regs = (void *)EDP_PHY_BASE; mtk_dp->train_info.sys_max_linkrate = DP_LINKRATE_HBR3; mtk_dp->train_info.linkrate = DP_LINKRATE_HBR2; mtk_dp->train_info.linklane_count = DP_LANECOUNT_4; diff --git a/src/soc/mediatek/common/dp/dptx_hal_common.c b/src/soc/mediatek/common/dp/dptx_hal_common.c index a7901e3..e8180e4 100644 --- a/src/soc/mediatek/common/dp/dptx_hal_common.c +++ b/src/soc/mediatek/common/dp/dptx_hal_common.c @@ -66,6 +66,64 @@ mtk_dp_write(mtk_dp, DP_TX_TOP_APB_WSTRB, 0x0); } +u32 mtk_dp_phy_read(struct mtk_dp *mtk_dp, u32 offset) +{ + void *addr = mtk_dp->phy_regs + offset; + + if (!mtk_dp->phy_regs) { + printk(BIOS_ERR, "%s: unsupported API\n", __func__); + return 0; + } + + if (offset % 4 != 0 || offset > REG_OFFSET_LIMIT) { + printk(BIOS_ERR, "[%s] invalid offset %#x for reg %p\n", + __func__, offset, mtk_dp->regs); + return 0; + } + + return read32(addr); +} + +void mtk_dp_phy_write(struct mtk_dp *mtk_dp, u32 offset, u32 val) +{ + void *addr = mtk_dp->phy_regs + offset; + + if (!mtk_dp->phy_regs) { + printk(BIOS_ERR, "%s: unsupported API\n", __func__); + return; + } + + if (offset % 4 != 0 || offset > REG_OFFSET_LIMIT) { + printk(BIOS_ERR, "[%s] invalid offset %#x for reg %p\n", + __func__, offset, mtk_dp->regs); + return; + } + + write32(addr, val); +} + +void mtk_dp_phy_mask(struct mtk_dp *mtk_dp, u32 offset, u32 val, u32 mask) +{ + void *addr = mtk_dp->phy_regs + offset; + + if (!mtk_dp->phy_regs) { + printk(BIOS_ERR, "%s: unsupported API\n", __func__); + return; + } + + if (offset % 4 != 0 || offset > REG_OFFSET_LIMIT) { + printk(BIOS_ERR, "[%s] invalid offset %#x for reg %p\n", + __func__, offset, mtk_dp->regs); + return; + } + + /* + * TODO: modify to clrsetbits32(addr, mask, val); + * There is asserion error when testing assert((val & mask) == val). + */ + clrsetbits32(addr, mask, val & mask); +} + void dptx_hal_verify_clock(struct mtk_dp *mtk_dp) { u32 m, n, ls_clk, pix_clk; diff --git a/src/soc/mediatek/common/dp/include/soc/dptx_common.h b/src/soc/mediatek/common/dp/include/soc/dptx_common.h index db686d2..1d7e135 100644 --- a/src/soc/mediatek/common/dp/include/soc/dptx_common.h +++ b/src/soc/mediatek/common/dp/include/soc/dptx_common.h @@ -198,6 +198,7 @@ u32 max_hdisplay; u32 max_vdisplay; void *regs; + void *phy_regs; int disp_status; bool power_on; bool audio_enable; diff --git a/src/soc/mediatek/common/dp/include/soc/dptx_hal_common.h b/src/soc/mediatek/common/dp/include/soc/dptx_hal_common.h index 778fe46..7ed41e8 100644 --- a/src/soc/mediatek/common/dp/include/soc/dptx_hal_common.h +++ b/src/soc/mediatek/common/dp/include/soc/dptx_hal_common.h @@ -65,6 +65,9 @@ bool dptx_hal_setswing_preemphasis(struct mtk_dp *mtk_dp, int lane_num, int swing_value, int preemphasis); u8 dptx_hal_get_colorbpp(struct mtk_dp *mtk_dp); +u32 mtk_dp_phy_read(struct mtk_dp *mtk_dp, u32 offset); +void mtk_dp_phy_mask(struct mtk_dp *mtk_dp, u32 offset, u32 val, u32 mask); +void mtk_dp_phy_write(struct mtk_dp *mtk_dp, u32 offset, u32 val); u32 mtk_dp_read(struct mtk_dp *mtk_dp, u32 offset); void mtk_dp_write_byte(struct mtk_dp *mtk_dp, u32 addr, u8 val, u32 mask); void mtk_dp_mask(struct mtk_dp *mtk_dp, u32 offset, u32 val, u32 mask); diff --git a/src/soc/mediatek/mt8188/include/soc/dptx.h b/src/soc/mediatek/mt8188/include/soc/dptx.h index 71a6ff6..c910bed 100644 --- a/src/soc/mediatek/mt8188/include/soc/dptx.h +++ b/src/soc/mediatek/mt8188/include/soc/dptx.h @@ -5,4 +5,6 @@ #include <soc/dptx_common.h> +#define EDP_PHY_BASE 0 + #endif /* __SOC_MEDIATEK_MT8188_INCLUDE_SOC_DPTX_H__ */ diff --git a/src/soc/mediatek/mt8195/include/soc/dptx.h b/src/soc/mediatek/mt8195/include/soc/dptx.h index 9af5b14..50b73ae 100644 --- a/src/soc/mediatek/mt8195/include/soc/dptx.h +++ b/src/soc/mediatek/mt8195/include/soc/dptx.h @@ -5,4 +5,6 @@ #include <soc/dptx_common.h> +#define EDP_PHY_BASE 0 + #endif /* __SOC_MEDIATEK_MT8195_INCLUDE_SOC_DPTX_H__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/85914?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5c00d0aa7e35f03cc3c3aef6a58eadd3d334d8ed Gerrit-Change-Number: 85914 Gerrit-PatchSet: 1 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/common/dp: Move common functions to dptx_hal_common.c
by Yidi Lin (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Yidi Lin. Hello Hung-Te Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85861?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/common/dp: Move common functions to dptx_hal_common.c ...................................................................... soc/mediatek/common/dp: Move common functions to dptx_hal_common.c Move the functions that can be shared with MT8196 to dptx_hal_common.c. BUG=b:382363408 TEST=emerge-geralt coreboot && emerge-cherry coreboot TEST=verify FW screen on Navi Change-Id: I9e151bc766c312eaf81b4220782775ef1c9d2297 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> --- M src/soc/mediatek/common/dp/dptx_hal.c A src/soc/mediatek/common/dp/dptx_hal_common.c M src/soc/mediatek/mt8188/Makefile.mk M src/soc/mediatek/mt8195/Makefile.mk 4 files changed, 486 insertions(+), 504 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/85861/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/85861?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: I9e151bc766c312eaf81b4220782775ef1c9d2297 Gerrit-Change-Number: 85861 Gerrit-PatchSet: 7 Gerrit-Owner: Yidi Lin <yidilin(a)google.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: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com>

1 0

[XL] Change in coreboot[main]: soc/mediatek/common/dp: Move common functions to dptx_hal_common.c
by Yu-Ping Wu (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Yidi Lin. Yu-Ping Wu has posted comments on this change by Yidi Lin. ( https://review.coreboot.org/c/coreboot/+/85861?usp=email ) Change subject: soc/mediatek/common/dp: Move common functions to dptx_hal_common.c ...................................................................... Patch Set 6: (1 comment) File src/soc/mediatek/common/dp/dptx_hal_common.c: https://review.coreboot.org/c/coreboot/+/85861/comment/2fe9fdf6_57704241?us… : PS3, Line 115: DP_CLRSETBITS(mtk_dp, REG_303C_DP_ENCODER0_P0 + 1, val, 0x7); > Checked with MTK, `DP_TX_TOP_APB_WSTRB` only takes effect on previous SoCs. […] Great! That makes a lot of sense. -- To view, visit https://review.coreboot.org/c/coreboot/+/85861?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: I9e151bc766c312eaf81b4220782775ef1c9d2297 Gerrit-Change-Number: 85861 Gerrit-PatchSet: 6 Gerrit-Owner: Yidi Lin <yidilin(a)google.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: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Thu, 09 Jan 2025 07:34:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[M] Change in coreboot[main]: [WIP] Docs: Add information about various debugging methods
by Nicholas Chin (Code Review) 08 Jan '25

08 Jan '25

Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85913?usp=email ) Change subject: [WIP] Docs: Add information about various debugging methods ...................................................................... [WIP] Docs: Add information about various debugging methods Add information about the different methods to debug coreboot, such as the various consoles and the tools required to use them. Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> Change-Id: I0b407fc05678a944567479a6430b7d47175b4072 --- M 3rdparty/amd_blobs M 3rdparty/fsp A Documentation/debugging/cbmem.md A Documentation/debugging/ehci.md A Documentation/debugging/em100.md A Documentation/debugging/flashconsole.md A Documentation/debugging/gdb.md A Documentation/debugging/index.md A Documentation/debugging/ne2000.md A Documentation/debugging/post_codes.md A Documentation/debugging/serial.md A Documentation/debugging/spkmodem.md M Documentation/index.md 13 files changed, 163 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/85913/1 diff --git a/3rdparty/amd_blobs b/3rdparty/amd_blobs index 26c5729..6a1e145 160000 --- a/3rdparty/amd_blobs +++ b/3rdparty/amd_blobs @@ -1 +1 @@ -Subproject commit 26c572974bcf7255930b0e9a51da3144ed0104b5 +Subproject commit 6a1e1457afddf49e53bfa4f422e7fd42f9db6e49 diff --git a/3rdparty/fsp b/3rdparty/fsp index 909cf43..3beceb0 160000 --- a/3rdparty/fsp +++ b/3rdparty/fsp @@ -1 +1 @@ -Subproject commit 909cf43ad6ccebb6adee482bc0a4f098c32c9a6d +Subproject commit 3beceb01f90dbdbe7781c7fa43ecc928cd68d2f0 diff --git a/Documentation/debugging/cbmem.md b/Documentation/debugging/cbmem.md new file mode 100644 index 0000000..c1a0bca --- /dev/null +++ b/Documentation/debugging/cbmem.md @@ -0,0 +1,34 @@ +# CBMEM console +As this can only be accessed once the system has reached the payload, +this cannot be used to debug issues that cause coreboot to lock up or +reboot in earlier stages. + +As cbmem is a core part of coreboot, this should be available on all +boards coreboot supports. + +## Enabling + +## Viewing the console +### Linux +#### cbmem utility +These require th +To view all logs in cbmem: + + sudo cbmem -c + +To view only the console log for the last boot: + + sudo cbmem -1 + +#### Linux driver +If your kernel was built with the `GOOGLE_MEMCONSOLE_COREBOOT` config, +then the console should be available under `/sys/firmware/log` + +### Payloads +#### GRUB +The GRUB2 payload provides a cbmem driver, which can be used to view the +cbmem console: + +#### coreinfo +The coreinfo secondary payload can be used to view the cbmem console, by +pressing `F2` (Firmware), and then `C` (Bootlog) diff --git a/Documentation/debugging/ehci.md b/Documentation/debugging/ehci.md new file mode 100644 index 0000000..e92ef6e --- /dev/null +++ b/Documentation/debugging/ehci.md @@ -0,0 +1,55 @@ +# EHCI debug console + +EHCI (USB 2.0) controllers may optionally implement a feature which +provides a simpler, register based interface to communicate over USB, +eliminating the need for a full USB stack that requires memory. Newer +boards will not have this feature as EHCI controllers have largely been +replaced with the xHCI (USB 3.0) controllers, which do not provide a +similar interface. + +To use this as a coreboot console output you will need both a board with +EHCI debug capability and an accessible debug port, as well as a +supported debug device. + +## Supported chips with EHCI debug capability +- ICH9 +- ICH10 +- ibex peak +- bd82x6x +- lynxpoint + +## Supported debug devices +### USB to UART adapters +- FTDI FT232H/2232H/4232H + - Must not be configured for a specific mode through an attached EEPROM +- WCH CH347T + - Supports UART 1 in modes 0, 1, and 3 + +### USB debug gadget +On some single board computers, USB ports can be used as a gadget +device, where the port acts as a USB device with functionality defined +by software. +- Raspberry Pi Zero +- Beaglebone Black + +## Finding the EHCI debug port +To determine if there is an EHCI debug capable USB controller in your +system and locate the physical USB port the debug port is routed to, you +can use the `util/find_usbdebug/find_usbdebug.sh` script. The script +requires lspci and lsusb, which can included in the pciutils and +usbutils packages. + +First, start the script with root privileges. It will report whether or not your +board has an EHCI controller with debug capabilities, and will list the +port(s) on the controller which are designated as the debug port. Then, +insert a high speed (USB 2.0 device into any USB port and press `enter`. If the +device was connected to the physical port which the debug port is routed +to, the device will show up in the output of the script. Repeat the +process of inserting the device into an untested port and pressing +`enter` until either you find the debug port or you have exhausted all +possible ports. If possible also check any unused USB 2.0 headers on +your mainboard. + +If you are unlucky the debug port on the EHCI controller may not be +physically routed to any available USB port or header, in which case you +will need to resort to alternative debugging methods. diff --git a/Documentation/debugging/em100.md b/Documentation/debugging/em100.md new file mode 100644 index 0000000..427a73c --- /dev/null +++ b/Documentation/debugging/em100.md @@ -0,0 +1,3 @@ +# Dediprog EM100 console + +I diff --git a/Documentation/debugging/flashconsole.md b/Documentation/debugging/flashconsole.md new file mode 100644 index 0000000..d2361a7 --- /dev/null +++ b/Documentation/debugging/flashconsole.md @@ -0,0 +1,13 @@ +# Flash console + +The SPI flash console offers a way + +## Usage + +## Known issues +The SPI flash console has been known to cause issues later in the boot +process (usually after romstage), and should be considered as a method +of last resort to debug early boot issues. + +- On Haswell, the flash console causes coreboot to freeze during the + jump to postcar after romstage diff --git a/Documentation/debugging/gdb.md b/Documentation/debugging/gdb.md new file mode 100644 index 0000000..aa42f24 --- /dev/null +++ b/Documentation/debugging/gdb.md @@ -0,0 +1 @@ +# GDB Stub diff --git a/Documentation/debugging/index.md b/Documentation/debugging/index.md new file mode 100644 index 0000000..25ca7ad --- /dev/null +++ b/Documentation/debugging/index.md @@ -0,0 +1,19 @@ +# Debugging + +This section describes various methods of debugging coreboot. + +## Debug Method Specific Documentation + +```{toctree} +:maxdepth: 1 + +Serial console <serial.md> +EHCI Debug console <ehci.md> +CBMEM console <cbmem.md> +SPI flash console <flashconsole.md> +NE2000 network console <ne2000.md> +Spkmodem console <spkmodem.md> +Dediprog EM100 console <em100.md> +POST codes <post_codes.md> +GDB <gdb_stub.md> +``` diff --git a/Documentation/debugging/ne2000.md b/Documentation/debugging/ne2000.md new file mode 100644 index 0000000..2bf0c5e --- /dev/null +++ b/Documentation/debugging/ne2000.md @@ -0,0 +1 @@ +# NE2000 Network Console diff --git a/Documentation/debugging/post_codes.md b/Documentation/debugging/post_codes.md new file mode 100644 index 0000000..97ee032 --- /dev/null +++ b/Documentation/debugging/post_codes.md @@ -0,0 +1,7 @@ +# POST Codes + +POST codes provide a basic way to determine overall progress of the +coreboot boot process, serving as "checkpoints" indicating a point in +the code that coreboot has passed. + +Some boards may have integrated displays diff --git a/Documentation/debugging/serial.md b/Documentation/debugging/serial.md new file mode 100644 index 0000000..b119e04 --- /dev/null +++ b/Documentation/debugging/serial.md @@ -0,0 +1 @@ +# Serial console diff --git a/Documentation/debugging/spkmodem.md b/Documentation/debugging/spkmodem.md new file mode 100644 index 0000000..905506e --- /dev/null +++ b/Documentation/debugging/spkmodem.md @@ -0,0 +1,26 @@ +# Spkmodem console +Spkmodem uses tones played over the PC speaker in order to encode the +console output, which can be received by another system and decoded. +Note that it is extremely slow (about baud), and will make the boot +process significantly slower. + +The protocol and decode utility originated from the GRUB2 project. + +## Availability +As this uses the legacy 8254 Programmable Interval Timer, this is only +supported on x86. Furthermore, the output pin on the chipset for the +speaker needs to be connected to an audio output, such as a piezo beeper +on the mainboard or routed to the audio codec. In the case that it is +routed to the codec, initialization verbs may need to be sent to do +things like unmute the speakers, route the beep signal to an output, and +set the gain. These can be specified in the `pc_beep_verbs` array in a +board's `hda_verb.c`. This also currently limits spkmodem functionality +to ramstage if the beep signal goes to a codec which requires +initialization, as the verbs are not run until ramstage. + +## Usage +On the target board, enable CONFIG_SPKMODEM (Console > spkmodem (console +on speaker) console output). + +## Protocol + diff --git a/Documentation/index.md b/Documentation/index.md index 7ba88f3..8aaf9d5 100644 --- a/Documentation/index.md +++ b/Documentation/index.md @@ -233,6 +233,7 @@ Security <security/index.md> SuperIO <superio/index.md> Vendorcode <vendorcode/index.md> +Debugging <debugging/index.md> Utilities <util.md> Software Bill of Materials <sbom/sbom.md> Project infrastructure & services <infrastructure/index.md> -- To view, visit https://review.coreboot.org/c/coreboot/+/85913?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0b407fc05678a944567479a6430b7d47175b4072 Gerrit-Change-Number: 85913 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add clk_buf drivers
by ot_song fan (Code Review) 08 Jan '25

08 Jan '25

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add srclken_rc drivers
by ot_song fan (Code Review) 08 Jan '25

08 Jan '25

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add srclken_rc drivers
by ot_song fan (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu. ot_song fan has uploaded a new patch set (#4) to the change originally created by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85842?usp=email ) Change subject: soc/mediatek/mt8196: Add srclken_rc drivers ...................................................................... soc/mediatek/mt8196: Add srclken_rc drivers MT8196 uses new RC mode with clk_buf driver, and needs srclken_rc to send PMRC_EN. PMRC_EN will collect the requirements of all users, such as MD, GPS, PCIE, NFC. TEST=Build pass. BUG=b:317009620 Signed-off-by: ot_song fan <ot_song.fan(a)mediatek.corp-partner.google.com> Change-Id: I40f8d2b12027955e6bd57b666e9f04c0116a0a93 --- M src/soc/mediatek/mt8196/Makefile.mk M src/soc/mediatek/mt8196/include/soc/addressmap.h A src/soc/mediatek/mt8196/include/soc/srclken_rc.h A src/soc/mediatek/mt8196/srclken_rc.c 4 files changed, 803 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/85842/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/85842?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: I40f8d2b12027955e6bd57b666e9f04c0116a0a93 Gerrit-Change-Number: 85842 Gerrit-PatchSet: 4 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-Reviewer: ot_song fan <ot_song.fan(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add srclken_rc drivers
by ot_song fan (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu. ot_song fan has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85842?usp=email ) Change subject: soc/mediatek/mt8196: Add srclken_rc drivers ...................................................................... Patch Set 3: (27 comments) File src/soc/mediatek/mt8196/include/soc/srclken_rc.h: https://review.coreboot.org/c/coreboot/+/85842/comment/33f0a92e_8ad19bae?us… : PS1, Line 6: /*********************************************************************************** : ** Definitions : ************************************************************************************/ > remove Done https://review.coreboot.org/c/coreboot/+/85842/comment/2151fe81_2d94c064?us… : PS1, Line 16: = 5 > Remove these. Done https://review.coreboot.org/c/coreboot/+/85842/comment/6062890a_2bcb69aa?us… : PS1, Line 126: _M > What do `_M` and `_B` mean here? If that means MASK, and we remove `_M` and `_MSK_B`? DCXO_FPM_CTRL_M means dcxo_fpm_ctrl_mode. SRCVOLTEN_FPM_MSK_B is for the HW function of srcvolten_to_fpm mask bit, just register name. https://review.coreboot.org/c/coreboot/+/85842/comment/c4aa7b3f_3365b90b?us… : PS1, Line 184: struct mtk_rc_status_regs { > Merge this into the `mtk_rc_regs` struct. Remove `RC_STATE_BASE`. Done File src/soc/mediatek/mt8196/srclken_rc.c: https://review.coreboot.org/c/coreboot/+/85842/comment/80343621_f091c430?us… : PS1, Line 32: ~= > spaces around `~=` Done https://review.coreboot.org/c/coreboot/+/85842/comment/3577b79b_a8833b47?us… : PS1, Line 135: > Remove extra blank line. Done https://review.coreboot.org/c/coreboot/+/85842/comment/17b1b694_51e4a75c?us… : PS1, Line 167: #define CHN_MASK (BIT(CHN_DCXO_L) | BIT(CHN_DCXO_H)) : #define CHN_NOT_SUPPORT(i) (BIT(i) & CHN_MASK) > For `CHN_NOT_SUPPORT`, `INIT_SUBSYS_FPM2LPM` and `INIT_SUBSYS_TO_HW`, can we change them to an confi […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/554b6665_250a94a5?us… : PS1, Line 182: * > space Done https://review.coreboot.org/c/coreboot/+/85842/comment/0d8dd9d7_9995c0c0?us… : PS1, Line 203: XO_DEFAULT_STABLE_TIME, REQ_ACK_IMD_EN > `XO_DEFAULT_STABLE_TIME, REQ_ACK_IMD_EN` can be moved to the `SUB_CTRL_CON_UNUSE` macro definition. Done https://review.coreboot.org/c/coreboot/+/85842/comment/eee71e64_86f1eaaa?us… : PS1, Line 225: RG > What does RG mean? RG means register. https://review.coreboot.org/c/coreboot/+/85842/comment/04a38988_06ab6292?us… : PS1, Line 265: code flow ASSERT already > What does this mean? This code flow will call ASSERT(). If ASSERT is ready for checking that chn is correct, it is not necessary to print RC_Mxx_REQ_STA_0. https://review.coreboot.org/c/coreboot/+/85842/comment/9541f3d0_4861bab5?us… : PS1, Line 273: ASSERT > assert […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/e3cc2b22_d271dd7c?us… : PS1, Line 276: INIT_MODE > Can we split `INIT_MODE` to a separate function `rc_ctrl_mode_init`? Then `INIT_MODE` enum value can […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/5ce7f4dc_bac08897?us… : PS1, Line 278: rc_ctrl[id] > Use a local variable (const pointer) for `rc_ctrl[id]`. Done https://review.coreboot.org/c/coreboot/+/85842/comment/61c32d97_0eea0b57?us… : PS1, Line 312: else : return; > ``` […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/8fea1719_561e209c?us… : PS1, Line 315: rc_ctrl[id].sw_fpm = mode; > Can we change `rc_ctrl` const and NOT modify it here? In line #511, we can still get the current fpm […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/3fdd641a_aa365e02?us… : PS1, Line 346: rc_ctrl_mode_switch_init > Adjust the order of these 3 functions: rc_ctrl_mode_switch_init -> rc_init_subsys_lpm -> rc_init_sub […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/7ae11179_46cde0a6?us… : PS1, Line 357: unsigned int state > `u32 mask, u32 value` […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/5a51c2ba_70416e13?us… : PS1, Line 357: unsigned int chn > `enum chn_id id` Done https://review.coreboot.org/c/coreboot/+/85842/comment/007df647_672509c2?us… : PS1, Line 360: INFO > ERROR Done https://review.coreboot.org/c/coreboot/+/85842/comment/0625c1b4_f6435e34?us… : PS1, Line 378: e > E Done https://review.coreboot.org/c/coreboot/+/85842/comment/37648e42_4feaf470?us… : PS1, Line 401: e > E Done https://review.coreboot.org/c/coreboot/+/85842/comment/4abba025_3bf450cd?us… : PS1, Line 414: _BF_VALUE > `_BF_VALUE` isn't part of the API, so it should never be used. Please use `WRITE32_BITFIELDS`. Done https://review.coreboot.org/c/coreboot/+/85842/comment/65e89c36_de5c6e87?us… : PS1, Line 477: u > U Done https://review.coreboot.org/c/coreboot/+/85842/comment/d0c4b5c0_db155c44?us… : PS1, Line 514: ret > Should return -1 at the end of function if one of the channel fails. […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/5fd64450_3b0d854d?us… : PS1, Line 515: ASSERT(!ret); > We shouldn't use assertions for run-time errors. […] Done https://review.coreboot.org/c/coreboot/+/85842/comment/d55ee210_03d480ba?us… : PS1, Line 537: w > W Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85842?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: I40f8d2b12027955e6bd57b666e9f04c0116a0a93 Gerrit-Change-Number: 85842 Gerrit-PatchSet: 3 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-Reviewer: ot_song fan <ot_song.fan(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Thu, 09 Jan 2025 06:56:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[M] Change in coreboot[main]: [WIP] Docs: Start adding mainboard porting guide
by Nicholas Chin (Code Review) 08 Jan '25

08 Jan '25

Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85912?usp=email ) Change subject: [WIP] Docs: Start adding mainboard porting guide ...................................................................... [WIP] Docs: Start adding mainboard porting guide Add some initial information about porting mainboards to coreboot to replace and extend the limited information from the guide in the old wiki. It is not intended (nor would it be practical) to be a step-by-step guide for any board, but rather a general collection of common tasks when porting and where to look for determining the values to set for board specific configurations. The techniques described are generally applicable to ports being created by setting values based on vendor firmware. Change-Id: I4680becebd8733ea7a23085c1832e9eb914138b3 Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M Documentation/getting_started/index.md A Documentation/getting_started/porting/index.md A Documentation/getting_started/porting/intel.md A Documentation/getting_started/porting/x86_mainboard_porting.md 4 files changed, 133 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/85912/1 diff --git a/Documentation/getting_started/index.md b/Documentation/getting_started/index.md index 7180c96..20cac4e 100644 --- a/Documentation/getting_started/index.md +++ b/Documentation/getting_started/index.md @@ -7,6 +7,7 @@ Build System <build_system.md> Submodules <submodules.md> Kconfig <kconfig.md> +Porting Mainboards <porting/index.md> Writing Documentation <writing_documentation.md> Setting up GPIOs <gpio.md> Adding devices to a device tree <devicetree.md> diff --git a/Documentation/getting_started/porting/index.md b/Documentation/getting_started/porting/index.md new file mode 100644 index 0000000..73f731c --- /dev/null +++ b/Documentation/getting_started/porting/index.md @@ -0,0 +1,59 @@ +# Porting coreboot to a new mainboard + +## General +The first step in porting a board to coreboot is determining whether +coreboot supports the main chips on your board. The most critical of +these are the processor, chipset, and/or SoC. Refer to the [supported +chips] page for a list of chips that coreboot supports. If one of these +isn't supported, then support for that chip would need to be added first +before a board based on it could be ported. Other chips that should be +supported if possible include SuperIOs and Embedded Controllers (EC), +but these are often less critical in terms of being able to boot. + +One method of porting a board is to find an existing board with coreboot +support and using it as a template for the new port. This is a common +method to "retrofit" coreboot onto a board that originally shipped with +a proprietary firmware implementation from the vendor. + +### Adding a new vendor +If the vendor of your board does not have any boards currently supported +by coreboot, you will need to create a new directory in `src/mainboard` +as well as Kconfig entries for it. If your vendor is "foo": + +- Create a `src/mainboard/foo` directory +- Create `src/mainboard/foo/Kconfig` +- Create `src/mainboard/foo/Kconfig.name` + +The `Kconfig` and `Kconfig.name` files can be copied from other vendors +and edited to reflect the new vendor. + +### Adding a new mainboard +A new directory in `src/mainboard/vendorname/` needs to be created for +your board. If starting with a similar existing port, copy the directory +for that board to the directory for your board's vendor. As with the +vendor directory, the board directory will need a `Kconfig` and a +`Kconfig.name` file. If starting from an existing port, edit mainboard +related Kconfigs to reflect the board you are porting. + +### Board specific configuration +Various settings in files such as hda_verb.c, Kconfig, devicetree.cb, +gpio.c and more need to be set to reflect the hardware configuration of +your board. If starting from an existing board, values + +### General Porting Hints +- If you don't know what a configuration variable means, search for its + name in the code for your board's platform. That often will lead to a + register address, which can be looked up in the output of various + utilities to determine the value that the vendor firmware sets it to, + as well as the chip datasheet which will document what each bit means. +- Don't be afraid to ask for help! There are many coreboot developers + who are willing to help on various [forums]. + +## Architecture Specific Porting Information +```{toctree} +:maxdepth: 1 + +Porting x86 systems <x86_mainboard_porting.md> +``` + +[forums]: ../../community/forums.md diff --git a/Documentation/getting_started/porting/intel.md b/Documentation/getting_started/porting/intel.md new file mode 100644 index 0000000..f07ff40 --- /dev/null +++ b/Documentation/getting_started/porting/intel.md @@ -0,0 +1,42 @@ +# Porting Intel x86 based mainboards + +## GPIO +If you have access to the schematics of your board, you should refer to +that to ensure the GPIO configurations are correct for the physical +design of the board and intended usages of each GPIO pin. + +For platforms using an ICH7 through ICH10 southbridge or Ibex Peak (1st +Gen Intel Core/Ironlake) through Lynx Point (4th Gen Intel Core/Haswell) +PCH, a [utility] exists which can generate gpio.c based on the GPIO +section in the `inteltool` output. Note that depending on the chipset, +not all GPIOs will exist. Refer to the chipset datasheet to determine +which ones exist. + +For platforms based on Skylake and newer, the `util/intelp2m` utility +can be used to generate a GPIO config based on the current configuration +set by the vendor firmware. + +## SPD Mapping +For boards with slotted memory, the raminit code needs to know how each +physical slot maps to an SMBUS address to access the SPD EEPROM on each +memory module. If you have access to the schematics or boardview, the +addresses of each slot can be determined from the pull-up/pull-down +resistors on the SA0-SA2 pins. The mapping can also be determined +experimentally by booting with one slot populated at a time, determining +which SMBUS address is active, and checking memory controller registers +to determine which channel/slot is populated. + +Many boards use the following mapping, and thus using this and seeing if +the system boots is often a viable option: +- Channel 0, slot 0: 0x50 +- Channel 0, slot 1: 0x51 +- Channel 1, slot 0: 0x52 +- Channel 1, slot 1: 0x53 + +BTX boards often use a reversed mapping: +- Channel 0, slot 0: 0x53 +- Channel 0, slot 1: 0x52 +- Channel 1, slot 0: 0x51 +- Channel 1, slot 1: 0x50 + +[utility]: https://codeberg.org/Riku_V/gpio-scripts diff --git a/Documentation/getting_started/porting/x86_mainboard_porting.md b/Documentation/getting_started/porting/x86_mainboard_porting.md new file mode 100644 index 0000000..70c5d3c --- /dev/null +++ b/Documentation/getting_started/porting/x86_mainboard_porting.md @@ -0,0 +1,31 @@ +# Porting x86 based boards to coreboot + +## HD Audio +The configuration for HD Audio codecs is found in the `hda_verb.c` file +for each board. This contains configs for the vendor IDs and pin widget +configs. + +The name, vendor/device ID, and subsystem ID of each codec can be found +in the files `/proc/asound/cardn/codec#m` + +The pin configs for each codec can be found in +`/sys/class/sound/cardn/hwCnDm/init_pin_configs`. The meanings of the +values in init_pin_configs can be found in the Intel High Definition +Audio Specification, Rev. 1.0a, Section 7.3.3.31: Configuration Default. + +## SuperIO +Useful utilities: superiotool + +## Devicetree +Useful utilities: +- inteltool +- lspci + +## Vendor Specific Porting Information +Refer to the appropriate page for vendor specific porting documentation + +```{toctree} +:maxdepth: 1 + +Porting Intel x86 Mainboards <intel.md> +``` -- To view, visit https://review.coreboot.org/c/coreboot/+/85912?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4680becebd8733ea7a23085c1832e9eb914138b3 Gerrit-Change-Number: 85912 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add clk_buf drivers
by ot_song fan (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu. ot_song fan has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85841?usp=email ) Change subject: soc/mediatek/mt8196: Add clk_buf drivers ...................................................................... Patch Set 3: (36 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/85841/comment/a1281a8a_bb8a15d4?us… : PS1, Line 9: Mt8196 use mt6685 clk_buf, and will use new RC mode with srclken_rc. > Please add more details. Done File src/soc/mediatek/mt8196/clkbuf_ctl.c: https://review.coreboot.org/c/coreboot/+/85841/comment/db7e6724_7770c9ed?us… : PS1, Line 9: > Remove 1 tab. Done https://review.coreboot.org/c/coreboot/+/85841/comment/6940bbd6_b1c0f4a5?us… : PS1, Line 15: * > space Done https://review.coreboot.org/c/coreboot/+/85841/comment/4ddcb85b_278ebb85?us… : PS1, Line 17: #define XO_IMPEDANCE_SHIFT 3 > Add a blank line below. Done https://review.coreboot.org/c/coreboot/+/85841/comment/fc2d117e_9facd531?us… : PS1, Line 20: XO_MODE_SHIFT > Remove and use `XO_EN_MODE_SHIFT` defined above. I assume they are the same thing. Done https://review.coreboot.org/c/coreboot/+/85841/comment/d08c099d_84904790?us… : PS1, Line 22: #define clk_buf_enable_xo(xo_id) \ : mt6685_write_field(ox[(xo_id)].reg, \ : XO_RC_MODE, \ : XO_EN_MODE_MASK, \ : XO_MODE_SHIFT) : : #define clk_buf_disable_xo(xo_id) \ : mt6685_write_field(ox[(xo_id)].reg, \ : XO_DISABLE_MODE, \ : XO_EN_MODE_MASK, \ : XO_MODE_SHIFT) : : #define clk_buf_impedance_config(xo_id) \ : mt6685_write_field(ox[(xo_id)].reg, \ : ox[(xo_id)].impedance, \ : XO_IMPEDANCE_MASK, \ : XO_IMPEDANCE_SHIFT) : : #define clk_buf_driving_config(xo_id) \ : mt6685_write_field(ox[(xo_id)].reg, \ : ox[(xo_id)].driving, \ : XO_DRIVING_MASK, \ : XO_DRIVING_SHIFT) > Change to static inline functions. Done https://review.coreboot.org/c/coreboot/+/85841/comment/0682eb17_c3af0fc3?us… : PS1, Line 48: /* SPMI master/slave, might change in the future */ : #define CLKBUF_SPMI_MASTER SPMI_MASTER_1 : #define CLKBUF_SPMI_SLAVE SPMI_SLAVE_9 > Remove if unused. Done https://review.coreboot.org/c/coreboot/+/85841/comment/241af7c3_c5783d81?us… : PS1, Line 74: struct ox_cfg ox[] = { > static const Done https://review.coreboot.org/c/coreboot/+/85841/comment/1fe32b17_fb4d9c07?us… : PS1, Line 148: impedance = 0, : .driving = 0, : .voter_mask = { : .low_byte = 0x0, : .high_byte = 0x0, : }, : > For `CLOCK_BUFFER_DISABLE`, fields `impedance`, `driving` and `voter_mask` are NOT used. […] Done https://review.coreboot.org/c/coreboot/+/85841/comment/4c2c870d_dc73cc58?us… : PS1, Line 207: unsigned int > Should this be u16? Done https://review.coreboot.org/c/coreboot/+/85841/comment/f7a1526b_68a800af?us… : PS1, Line 209: unsigned int out = 0; : unsigned char tmp = 0; : : tmp = mt6685_read8(MT6685_PMRC_CON0); : out = tmp; : tmp = mt6685_read8(MT6685_PMRC_CON1); : out |= tmp << 8; > Use `mt6685_read_low_high`, which I suggested in another comment. Done https://review.coreboot.org/c/coreboot/+/85841/comment/e1ac267f_87ed6d10?us… : PS1, Line 222: u8 > `int`. Please fix all `u8 i` in this file. Done https://review.coreboot.org/c/coreboot/+/85841/comment/c91c011e_fdab8ba0?us… : PS1, Line 228: \n > Print the newline at the end of the string (NOT before). […] This newline is for last loop of #226. https://review.coreboot.org/c/coreboot/+/85841/comment/d2252b30_218f82ee?us… : PS1, Line 238: > remove one blank line Done https://review.coreboot.org/c/coreboot/+/85841/comment/ace2d030_bca9eee7?us… : PS1, Line 260: clk_buf_dump_clkbuf_log > dump_clkbuf_log Done https://review.coreboot.org/c/coreboot/+/85841/comment/ad294047_1bfbd095?us… : PS1, Line 269: > Add […] Done https://review.coreboot.org/c/coreboot/+/85841/comment/d7dc9a2f_46e93a33?us… : PS1, Line 271: xo_buf_cw[i] = mt6685_read8(MT6685_DCXO_EXTBUF1_CW0 + i); : val = mt6685_read8(MT6685_XO_BUF_CTL0_L + (2 * i)); : xo_buf_vote[i] = val; : : val = mt6685_read8(MT6685_XO_BUF_CTL0_H + (2 * i)); : xo_buf_vote[i] |= (val << 8); > Add a helper function for reading L and H regs. […] Done https://review.coreboot.org/c/coreboot/+/85841/comment/f3f7bebe_7aa954fc?us… : PS1, Line 283: RG_XO_DIG26M_DIV2_MASK, > align Done https://review.coreboot.org/c/coreboot/+/85841/comment/8bc48d34_584e313a?us… : PS1, Line 286: __func__ > We don't need to print the function name so many times. […] Done https://review.coreboot.org/c/coreboot/+/85841/comment/517c48bf_58dccd56?us… : PS1, Line 286: 0x79A > Print `%#X` for `MT6685_DCXO_EXTBUF1_CW0`. Done https://review.coreboot.org/c/coreboot/+/85841/comment/5f23acd0_8244e192?us… : PS1, Line 291: 0x54C > `MT6685_XO_BUF_CTL0_L` Done https://review.coreboot.org/c/coreboot/+/85841/comment/0c174da1_a91442b0?us… : PS1, Line 333: XO_BBCK1_VOTE_L_ADDR > Write `MT6685_XO_BUF_CTL0_L` here to be consistent with `clk_buf_dump_clkbuf_log`. Done https://review.coreboot.org/c/coreboot/+/85841/comment/ce89e5ea_6f377276?us… : PS1, Line 324: for (i = 0; i < XO_NUMBER; ++i) { : if (ox[i].mode == CLOCK_BUFFER_DISABLE) { : ox[i].voter_mask.low_byte = 0; : ox[i].voter_mask.high_byte = 0; : } : } : : /* 1.4 Write RC voting table */ : for (i = 0; i < XO_NUMBER; i++) { : mt6685_write8(XO_BBCK1_VOTE_L_ADDR + (2 * i), : ox[i].voter_mask.low_byte); : mt6685_write8(XO_BBCK1_VOTE_H_ADDR + (2 * i), : ox[i].voter_mask.high_byte); : } > Combine them: […] Done https://review.coreboot.org/c/coreboot/+/85841/comment/3f78227e_b12ed956?us… : PS1, Line 339: t > T Done https://review.coreboot.org/c/coreboot/+/85841/comment/a60ee25e_04cf330c?us… : PS1, Line 341: 0x1 > align with first param Done https://review.coreboot.org/c/coreboot/+/85841/comment/5dcd5e28_c82419ef?us… : PS1, Line 355: t > T Done https://review.coreboot.org/c/coreboot/+/85841/comment/cc2bb543_77792f18?us… : PS1, Line 380: /* 1.8 RC mode setting > Wrong comment format. Done https://review.coreboot.org/c/coreboot/+/85841/comment/85a8f0be_b4be3de6?us… : PS1, Line 385: ( > remove Done https://review.coreboot.org/c/coreboot/+/85841/comment/56b3ff18_65b2b281?us… : PS1, Line 397: mt6685_write8(XO_STATIC_AUXOUT_SEL_ADDR, sel); > Should we use `mt6685_write_field` and specify `XO_STATIC_AUXOUT_SEL_MASK` and `_SHIFT`? Done https://review.coreboot.org/c/coreboot/+/85841/comment/b9b7bd5a_c1589fe1?us… : PS1, Line 399: tmp = mt6685_read8(XO_STATIC_AUXOUT_L_ADDR); : val = tmp; : : tmp = mt6685_read8(XO_STATIC_AUXOUT_H_ADDR); : val |= tmp << 8; > Use `mt6685_read_low_high`. Done https://review.coreboot.org/c/coreboot/+/85841/comment/3febb9c3_6856a0db?us… : PS1, Line 418: d > D Done File src/soc/mediatek/mt8196/include/soc/clkbuf_ctl.h: https://review.coreboot.org/c/coreboot/+/85841/comment/54e95fca_5913f240?us… : PS1, Line 78: * > Space. Same for all similar cases below. Done https://review.coreboot.org/c/coreboot/+/85841/comment/e758e4e0_921680bf?us… : PS1, Line 453: CLOCK_BUFFER_HW_CONTROL > Add `,` Done https://review.coreboot.org/c/coreboot/+/85841/comment/dc218bf5_540dda16?us… : PS1, Line 464: CLK_BUF_OUTPUT_IMPEDANCE_7 > Add `,` Done https://review.coreboot.org/c/coreboot/+/85841/comment/ed971e88_434c8198?us… : PS1, Line 536: clk_buf_set_default > Remove if unused. Done https://review.coreboot.org/c/coreboot/+/85841/comment/f5e29d75_3c3f138a?us… : PS1, Line 538: pwrap_interface_init > Remove if unused. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85841?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: Ie18bfbb2f3354ba3645799857061dc20de7f6d84 Gerrit-Change-Number: 85841 Gerrit-PatchSet: 3 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-Reviewer: ot_song fan <ot_song.fan(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Thu, 09 Jan 2025 06:52:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025