Frank Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/51171 )
Change subject: mb/google/zork/var/vilboz: Update WiFi SAR for Vilboz
......................................................................
mb/google/zork/var/vilboz: Update WiFi SAR for Vilboz
Loading wifi_sar-vilboz-1.hex for vilboz360 LTE sku for the present.
BUG=b:177684735, b:176168400
BRANCH=zork
TEST=emerge-zork coreboot chromeos-bootimage, then verify that tables are
in CBFS and loaded by iwlwifi driver.
Signed-off-by: Frank Wu <frank_wu(a)compal.corp-partner.google.com>
Change-Id: I477b55d64fd9d33d753b10b2de443041a12d13e2
---
M src/mainboard/google/zork/variants/vilboz/variant.c
1 file changed, 1 insertion(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/51171/1
diff --git a/src/mainboard/google/zork/variants/vilboz/variant.c b/src/mainboard/google/zork/variants/vilboz/variant.c
index 3816aac..e75d8fc 100644
--- a/src/mainboard/google/zork/variants/vilboz/variant.c
+++ b/src/mainboard/google/zork/variants/vilboz/variant.c
@@ -80,11 +80,7 @@
filename = "wifi_sar-vilboz-1.hex";
break;
case 7:
- /*
- TODO: Set default first. It will be replaced after the
- new table is generated.
- */
- filename = "wifi_sar_defaults.hex";
+ filename = "wifi_sar-vilboz-1.hex";
break;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/51171
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I477b55d64fd9d33d753b10b2de443041a12d13e2
Gerrit-Change-Number: 51171
Gerrit-PatchSet: 1
Gerrit-Owner: Frank Wu <frank_wu(a)compal.corp-partner.google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50857 )
Change subject: sb/intel/common: Refactor _PRT generation to support GSI-based tables
......................................................................
Patch Set 7: Code-Review+1
(5 comments)
File src/southbridge/intel/common/acpi_pirq_gen.h:
https://review.coreboot.org/c/coreboot/+/50857/comment/d2235053_80a15281
PS7, Line 55: [32][4]
Do we have names for the `32` and `4` magic numbers?
File src/southbridge/intel/common/acpi_pirq_gen.c:
https://review.coreboot.org/c/coreboot/+/50857/comment/d782558c_4afcd9fb
PS7, Line 45: pirq_map->gsi[pirq - PIRQ_A]);
nit: Even though braces aren't strictly necessary for either branch of this conditional block, I'd still add them since the statements are broken into multiple lines.
https://review.coreboot.org/c/coreboot/+/50857/comment/e47ac628_1cf7ff4c
PS7, Line 75: /* pop scope */
Huh?
https://review.coreboot.org/c/coreboot/+/50857/comment/7a0c20e8_9ced3893
PS7, Line 85: char matrix[32][4];
Since `intel_create_pirq_matrix()` may not always initialise all elements, I'd add an explicit zero initialiser to be safe.
File src/southbridge/intel/common/rcba_pirq.c:
https://review.coreboot.org/c/coreboot/+/50857/comment/1d35bb5e_cc480cce
PS7, Line 57: int_pin - PCI_INT_A
This appears four times in the loop body. Maybe:
const u8 pci_dev = PCI_SLOT(dev->path.pci.devfn);
const u8 int_pin = pci_read_config8(dev, PCI_INTERRUPT_PIN);
if (int_pin < PCI_INT_A || int_pin > PCI_INT_D)
continue;
const u8 int_idx = int_pin - PCI_INT_A;
if (matrix[pci_dev][int_idx] != PIRQ_NONE)
continue;
matrix[pci_dev][int_idx] = map_pirq(dev, int_pin);
printk(BIOS_SPEW, "ACPI_PIRQ_GEN: %s: pin=%d pirq=%d\n",
dev_path(dev), int_idx, matrix[pci_dev][int_idx] - PIRQ_A);
num_devs++;
Thoughts?
--
To view, visit https://review.coreboot.org/c/coreboot/+/50857
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ica420a3d12fd1d64c8fe6e4b326fd779b3f10868
Gerrit-Change-Number: 50857
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 02 Mar 2021 10:26:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Arthur Heymans, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51158 )
Change subject: sb/intel/common: Use new acpigen_write_PRT_*_entry functions
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/51158
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9f573b9bd40260ab963c5a4a965a6ac483af91ec
Gerrit-Change-Number: 51158
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 02 Mar 2021 10:16:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Ivy Jian, Patrick Rudolph, EricR Lai.
Amanda Hwang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51167 )
Change subject: util: Add new memory part for brya boards
......................................................................
Patch Set 6:
(4 comments)
Patchset:
PS3:
> Please note: This change will have to be rebased once CB:51057 lands.
Done.
File util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt:
https://review.coreboot.org/c/coreboot/+/51167/comment/b7bc8bf6_e9d72af8
PS1, Line 238: 4
> (2 channels x 16) correct.
Done.
https://review.coreboot.org/c/coreboot/+/51167/comment/e010e4c5_4fdbb23d
PS1, Line 239: 1
> Yes, that's correct.
Done
File util/spd_tools/lp4x/global_lp4x_mem_parts.json.txt:
https://review.coreboot.org/c/coreboot/+/51167/comment/8911dc40_25bc350a
PS3, Line 233:
> nit: Use spaces here just like other entries.
Done.
--
To view, visit https://review.coreboot.org/c/coreboot/+/51167
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic06e9d672a2d3db2b4ea12d15b462843c90db8f6
Gerrit-Change-Number: 51167
Gerrit-PatchSet: 6
Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 09:36:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Martin Roth, Tim Wawrzynczak, Amanda Hwang, Ivy Jian.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51170 )
Change subject: mb/google/brya: Add DRAM support for Micron MT53E1G32D2NP-046 and MT53E2G32D4NQ-046
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/51170
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I413e35cdb7c34388c3e159f8f9584fae2d21a355
Gerrit-Change-Number: 51170
Gerrit-PatchSet: 1
Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>
Gerrit-Attention: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 09:31:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Xi Chen, Nico Huber, Martin Roth, Paul Menzel, Julius Werner.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50294 )
Change subject: vendor: mediatek: Add mediatek mt8192 dram initialization codes
......................................................................
Patch Set 16:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50294/comment/4f4fbad5_f0a2b921
PS16, Line 7: codes
code
https://review.coreboot.org/c/coreboot/+/50294/comment/8e2191a1_7cb811b6
PS16, Line 12: an
a
https://review.coreboot.org/c/coreboot/+/50294/comment/6ff159d8_4befe1c9
PS16, Line 12: took
"taken" or "considered"
https://review.coreboot.org/c/coreboot/+/50294/comment/b79bbc2a_6e732eec
PS16, Line 14: (
One space before "("
--
To view, visit https://review.coreboot.org/c/coreboot/+/50294
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3853204578069c6abf52689ea6f5d88841414bd4
Gerrit-Change-Number: 50294
Gerrit-PatchSet: 16
Gerrit-Owner: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Cindy Ching <cindy.ching(a)mediatek.corp-partner.google.com>
Gerrit-CC: Joel Kitching <kitching(a)google.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Stefan Reinauer <reinauer(a)chromium.org>
Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-CC: Yidi Lin <yidi.lin(a)mediatek.com>
Gerrit-CC: Yidi Lin <yidi.lin(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Tue, 02 Mar 2021 09:14:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Xi Chen, Nico Huber, Martin Roth, Paul Menzel, Julius Werner, Yu-Ping Wu.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50294 )
Change subject: vendor: mediatek: Add mediatek mt8192 dram initialization codes
......................................................................
Patch Set 16:
(9 comments)
File MAINTAINERS:
https://review.coreboot.org/c/coreboot/+/50294/comment/5a9056c4_5da72340
PS15, Line 617: MT8192
Will you work on DRAM for more platforms in future?
If yes, I wonder if you'd like to add a section to the "Platforms" above, say
MEDKATEK DRAM CALIBRATION
M:..
S:...
F: src/vendor/mediatek
https://review.coreboot.org/c/coreboot/+/50294/comment/a159eeb8_19bf77d3
PS15, Line 618: Xi Chen <xixi.chen(a)mediatek.com>
If you'll work on non-dram topics as well, feel free to move your self to "MEDIATEK SOCS"
https://review.coreboot.org/c/coreboot/+/50294/comment/4dcafa51_4c92076a
PS15, Line 620: /
if this is for 8192 then the path should be mediatek/mt8192/
https://review.coreboot.org/c/coreboot/+/50294/comment/1f48d569_64e81555
PS15, Line 621: 2
mt8192/
File src/vendorcode/mediatek/Kconfig:
https://review.coreboot.org/c/coreboot/+/50294/comment/862de68f_d6e027a8
PS16, Line 1: SOC_MEDIATEK_MT8192
can we move this to mt8192/Kconfig?
https://review.coreboot.org/c/coreboot/+/50294/comment/7ca7e6c8_5bf6c2b0
PS16, Line 3: config DEBUG_DRAM
: bool "Output verbose DRAM related debug messages"
: default y
: help
: This option enables additional DRAM related debug messages.
this is the only config in dramc.
I wonder if we can revise it to something like
extern int mtk_dramc_debug;
#define dramc_dbg(_x_...) { if (mtk_dramc_debug) printk(BIOS_INFO, _x_); }
And in soc memory.c add
int mtk_drac_debug = CONFIG(DEBUG_DRAM);
Then we don't need KCONFIG in vendor code.
https://review.coreboot.org/c/coreboot/+/50294/comment/733df007_e22101ba
PS16, Line 9: config MEDIATEK_DRAM_DVFS
: bool
: default n
: help
: This option enables DRAM calibration with multiple frequencies (low,
: medium and high frequency groups, with total 7 frequencies) for DVFS
: feature. All supported data rates are: 800, 1200, 1600, 1866, 2400,
: 3200, 4266.
:
: config MEDIATEK_DRAM_DVFS_LIMIT_FREQ_CNT
: bool
: default y
: select MEDIATEK_DRAM_DVFS
: help
: This options limit DRAM frequency calibration count from total 7 to 3,
: other frequency will directly use the low frequency shu result.
:
: config MEMORY_TEST
: bool
: default y
: help
: This option enables memory basic compare test to verify the DRAM read
: or write is as expected.
these options are not really used in the dramc implementation here. I think you can remove them? especially MEMORY_TEST is in src/soc/mediatek/mt8192/memory.c. You can consider moving that to src/soc/mediakte/common or mt8192.
File src/vendorcode/mediatek/mt8192/dramc/ANA_init_config.c:
PS13:
> Even vendorcode files always need to have an SPDX header
Hi Julius, we found that current submit hooks didn't check SPDX for files in vendor folder, was that on purpose?
File src/vendorcode/mediatek/mt8192/include/print.h:
https://review.coreboot.org/c/coreboot/+/50294/comment/ce59c436_c3afe4ab
PS16, Line 7: #define printf print
What about just add
#define print(fmt...) printk(BIOS_INFO, fmt)
Then we don't need print.c ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/50294
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3853204578069c6abf52689ea6f5d88841414bd4
Gerrit-Change-Number: 50294
Gerrit-PatchSet: 16
Gerrit-Owner: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Cindy Ching <cindy.ching(a)mediatek.corp-partner.google.com>
Gerrit-CC: Joel Kitching <kitching(a)google.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Stefan Reinauer <reinauer(a)chromium.org>
Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-CC: Yidi Lin <yidi.lin(a)mediatek.com>
Gerrit-CC: Yidi Lin <yidi.lin(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 09:13:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Xi Chen, Hung-Te Lin, Nico Huber, Martin Roth, Paul Menzel, Julius Werner, Yu-Ping Wu.
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50294 )
Change subject: vendor: mediatek: Add mediatek mt8192 dram initialization codes
......................................................................
Patch Set 16:
(17 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50294/comment/282e15a9_bf695bf7
PS5, Line 11:
> > However, in case Mediatek maintains a public Git repository, we could also integrated that as a su […]
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/50294/comment/378c9437_4672d5c6
PS9, Line 9: Add the DRAM initialization code based on Mediatek reference implementation.
> This is the DRAM calibration code from the reference […]
Done
https://review.coreboot.org/c/coreboot/+/50294/comment/ce8919bf_9f2ae4c8
PS9, Line 11: Mediatek internally maintains the DRAM initialization code, following
: different coding style.
:
: To prevent maintaining a different branch for coreboot
: (which may lead to typo or errors which switching between different coding
: style), we want to directly use the reference implementation as vendor code.
> The ETT is a standalone library, used by different boot loaders […]
Done
Patchset:
PS13:
> Please also update the file `MAINTAINERS`.
Add mt8192 maintainers on MEDIATEK MT8192 SOC.
File src/vendorcode/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/50294/comment/88b4148e_1b6c5927
PS8, Line 7: #
> If that's the case, I'd prefer adding a new Kconfig, say MEDIATEK_VENDOR_DRAM_CALIBRATION or MEDIATE […]
Done
File src/vendorcode/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/50294/comment/d5859fbf_e3246a50
PS13, Line 7: # subdirs-y += mediatek
> Why is this still commented out?
move this to CB:51125.
File src/vendorcode/mediatek/Kconfig:
https://review.coreboot.org/c/coreboot/+/50294/comment/bb032b7b_ba8ad279
PS9, Line 9: config MT8192_DRAM_EMCP
: bool
: default y
: help
: The eMCP platform should select this option to run at different DRAM
: frequencies.
> no used, will remove.
Done
File src/vendorcode/mediatek/Kconfig:
https://review.coreboot.org/c/coreboot/+/50294/comment/6a981c66_aed9b061
PS13, Line 9: config MT8192_DRAM_DVFS
> Please try to design this from the start so it will make sense when more SoCs are added (i.e. […]
It's a common option for mediatek, change to MEDIATEK_DRAM_DVFS.
File src/vendorcode/mediatek/mt8192/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/50294/comment/1f82741f_f5e4fd1c
PS9, Line 7: ramstage-y += dpm.c
> will move to soc folder.
Done
https://review.coreboot.org/c/coreboot/+/50294/comment/4fb1126d_7f01b51d
PS9, Line 9: BL31_MAKEARGS += PLAT=mt8192
> yes, no need, will remove.
Done
File src/vendorcode/mediatek/mt8192/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/50294/comment/ae5a6d01_30d3a8be
PS13, Line 1: ifeq ($(CONFIG_SOC_MEDIATEK_MT8192),y)
> This could just be `subdirs-$(CONFIG_SOC_MEDIATEK_MT8192) +=` in the Makefile one level higher.
Ack
File src/vendorcode/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/50294/comment/ab4e9177_5c69b1cd
PS9, Line 1: License
> will move to soc folder.
Done
File src/vendorcode/mediatek/mt8192/dramc/ANA_init_config.c:
PS13:
> Even vendorcode files always need to have an SPDX header (it would be great if MediaTek could just p […]
Add SPDX license.
File src/vendorcode/mediatek/mt8192/dramc/emi.c:
https://review.coreboot.org/c/coreboot/+/50294/comment/956545f9_c949d07d
PS13, Line 5: Without
: * the prior written permission of MediaTek inc. and/or its licensors, any
: * reproduction, modification, use or disclosure of MediaTek Software, and
: * information contained herein, in whole or in part, shall be strictly
: * prohibited.
> This doesn't work. It still needs to be open-source code.
change to SPDX license.
File src/vendorcode/mediatek/mt8192/driver/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/50294/comment/20c36ba2_a4d095a3
PS13, Line 5: romstage-y += uart.c
> Doesn't coreboot have all these already? Please avoid duplicating drivers between vendorcode and cor […]
already remove them.
File src/vendorcode/mediatek/mt8192/include/stdint.h:
PS13:
> Why is this file duplicated here? (Not to mention the fact that this is clearly an original coreboot […]
Already remove it.
File src/vendorcode/mediatek/mt8192/lib/print.c:
https://review.coreboot.org/c/coreboot/+/50294/comment/f98d132a_444ce9e2
PS13, Line 101: static int vprint(const char *fmt, va_list ap)
> This too is something we really don't want to have a duplicate of in the codebase.
already remove this function.
--
To view, visit https://review.coreboot.org/c/coreboot/+/50294
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3853204578069c6abf52689ea6f5d88841414bd4
Gerrit-Change-Number: 50294
Gerrit-PatchSet: 16
Gerrit-Owner: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Cindy Ching <cindy.ching(a)mediatek.corp-partner.google.com>
Gerrit-CC: Joel Kitching <kitching(a)google.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Stefan Reinauer <reinauer(a)chromium.org>
Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-CC: Yidi Lin <yidi.lin(a)mediatek.com>
Gerrit-CC: Yidi Lin <yidi.lin(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 09:02:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Comment-In-Reply-To: Xi Chen <xixi.chen(a)mediatek.com>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment