Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/23585 )
Change subject: libpayload/drivers/nvram: Add function to write RTC
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/23585/7/payloads/libpayload/driver…
File payloads/libpayload/drivers/nvram.c:
https://review.coreboot.org/c/coreboot/+/23585/7/payloads/libpayload/driver…
PS7, Line 185: !(statusB & NVRAM_RTC_FORMAT_BINARY)
Maybe place this check inside a wrapper for nvram_write()? That way, the writes don't have to be duplicated
--
To view, visit https://review.coreboot.org/c/coreboot/+/23585
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I17b4c1ee0dcc649738ac6a7400b087d07213eaf0
Gerrit-Change-Number: 23585
Gerrit-PatchSet: 7
Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 16 Mar 2020 09:10:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 )
Change subject: nb/intel/i945: Program CxODT value for each channel
......................................................................
Patch Set 35:
(1 comment)
https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94…
File src/northbridge/intel/i945/raminit.c:
https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94…
PS34, Line 2450: one
> no […]
We are talking about `(populated_dimms & 3) == 1 != (populated_dimms & 3) == 2)`, right?
If `(populated_dimms & 3) == 0`, the statement above evaluates to false as `0 != 0` is not true. The value wouldn’t be written.
--
To view, visit https://review.coreboot.org/c/coreboot/+/18548
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c
Gerrit-Change-Number: 18548
Gerrit-PatchSet: 35
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Mon, 16 Mar 2020 08:54:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-MessageType: comment
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 )
Change subject: nb/intel/i945: Program CxODT value for each channel
......................................................................
Patch Set 35:
(1 comment)
https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94…
File src/northbridge/intel/i945/raminit.c:
https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94…
PS34, Line 2450: one
> Looking at patch set 35, you agree with my statement?
no
we write on C0ODT here if ( (dimm0 populated and dimm1 not populated) or (dimm0 not populated and dimm1 is populated )
old mask (let say only for channel 0) is :
case #1: 00b & 11b = 0 <-- here we don't have to write on C0ODT
case #2: 10b & 11b = 2
case #3: 01b & 11b = 1
case #4: 11b & 11b = 3 <-- here we don't have to write on C0ODT
in your statement, (popp & 3 == 0) is missing and also you are using only case #3
--
To view, visit https://review.coreboot.org/c/coreboot/+/18548
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c
Gerrit-Change-Number: 18548
Gerrit-PatchSet: 35
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Mon, 16 Mar 2020 08:33:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-MessageType: comment
Hello Duan huayang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/39097
to review the following change.
Change subject: soc/mediatek/mt8183: Add get dram type API at ATF
......................................................................
soc/mediatek/mt8183: Add get dram type API at ATF
DVFS module need know the dram type at ATF for loading the correct spm binary.
BRANCH=kukui
BUG=none
TEST=bootup pass
Change-Id: I20afc00c4c671abcbb6f36eb8e3e364529e9389c
Signed-off-by: Huayang Duan <huayang.duan(a)mediatek.com>
---
M src/mainboard/google/kukui/mainboard.c
1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/39097/1
diff --git a/src/mainboard/google/kukui/mainboard.c b/src/mainboard/google/kukui/mainboard.c
index 844496d..da39c1c 100644
--- a/src/mainboard/google/kukui/mainboard.c
+++ b/src/mainboard/google/kukui/mainboard.c
@@ -184,8 +184,19 @@
.gpio = { .polarity = ARM_TF_GPIO_LEVEL_HIGH },
};
+ /* use .polarity to save dram type value */
+ static struct bl_aux_param_gpio param_dram_type = {
+ .h = { .type = BL_AUX_PARAM_MTK_DRAM_TYPE },
+#if CONFIG(MT8183_DRAM_EMCP)
+ .gpio = { .polarity = DRAMC_CONFIG_EMCP },
+#else
+ .gpio = { .polarity = 0 },
+#endif
+ };
+
param_reset.gpio.index = GPIO_RESET.id;
register_bl31_aux_param(¶m_reset.h);
+ register_bl31_aux_param(¶m_dram_type.h);
}
static void mainboard_init(struct device *dev)
--
To view, visit https://review.coreboot.org/c/coreboot/+/39097
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I20afc00c4c671abcbb6f36eb8e3e364529e9389c
Gerrit-Change-Number: 39097
Gerrit-PatchSet: 1
Gerrit-Owner: huayang duan <huayangduan(a)gmail.com>
Gerrit-Reviewer: Duan huayang <huayang.duan(a)mediatek.com>
Gerrit-MessageType: newchange
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39401 )
Change subject: soc/intel/tigerlake:add support to read spd data from SMBUS
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39401/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/39401/1//COMMIT_MSG@6
PS1, Line 6:
: soc/intel/tigerlake:add
Please add one space after the colon.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39401
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I94f8707c731c8afa1106e387a246c000bd53a654
Gerrit-Change-Number: 39401
Gerrit-PatchSet: 1
Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 16 Mar 2020 07:58:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39196 )
Change subject: jasperlake_rvp: UART config for JSLRVP
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39196/5//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/39196/5//COMMIT_MSG@7
PS5, Line 7: jasperlake_rvp: UART config for JSLRVP
Please make it a statement by adding a verb (in imperative mood).
--
To view, visit https://review.coreboot.org/c/coreboot/+/39196
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic042cdc449b77c2d9bdd7a63777328e47f910654
Gerrit-Change-Number: 39196
Gerrit-PatchSet: 5
Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 16 Mar 2020 07:55:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 )
Change subject: nb/intel/i945: Program CxODT value for each channel
......................................................................
Patch Set 35:
(1 comment)
https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94…
File src/northbridge/intel/i945/raminit.c:
https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94…
PS34, Line 2450: one
> here we are not looking for *at least* but *only one* […]
Looking at patch set 35, you agree with my statement?
--
To view, visit https://review.coreboot.org/c/coreboot/+/18548
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c
Gerrit-Change-Number: 18548
Gerrit-PatchSet: 35
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Mon, 16 Mar 2020 07:48:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-MessageType: comment