EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
mb/google/drallion: dynamic disable memory channel
Disable memory channel by HW strap pin. Using for factory debug. Keep this until the real board come out.
BUG=b:139773082 BRANCH=N/A TEST=N/A
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/romstage.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/1
diff --git a/src/mainboard/google/drallion/romstage.c b/src/mainboard/google/drallion/romstage.c index 20eee7f..6e62a1e 100644 --- a/src/mainboard/google/drallion/romstage.c +++ b/src/mainboard/google/drallion/romstage.c @@ -16,6 +16,8 @@ #include <ec/google/wilco/romstage.h> #include <soc/cnl_memcfg_init.h> #include <soc/romstage.h> +#include <gpio.h> +#include <variant/gpio.h>
static const struct cnl_mb_cfg memcfg = { /* Access memory info through SMBUS. */ @@ -59,5 +61,9 @@ { wilco_ec_romstage_init();
+ /* Disable memory channel by HW strap pin */ + memupd->FspmConfig.DisableDimmChannel0 = gpio_get(GPP_F1)? 0 : 3; + memupd->FspmConfig.DisableDimmChannel1 = gpio_get(GPP_F2)? 0 : 3; + cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35241/1/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/1/src/mainboard/google/dralli... PS1, Line 65: memupd->FspmConfig.DisableDimmChannel0 = gpio_get(GPP_F1)? 0 : 3; spaces required around that '?' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/35241/1/src/mainboard/google/dralli... PS1, Line 66: memupd->FspmConfig.DisableDimmChannel1 = gpio_get(GPP_F2)? 0 : 3; spaces required around that '?' (ctx:VxW)
EricR Lai has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
mb/google/drallion: dynamic disable memory channel
Disable memory channel by HW strap pin. Using for factory debug. Keep this until the real board come out.
BUG=b:139773082 BRANCH=N/A TEST=N/A
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/romstage.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
@Intel, please leave comment in here or the issue if any thoughts.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: GPP_F1 This might be in a different commit but GPP_F1/GPP_F2 currently seem to be set to PAD_NC() and not PAD_CFG_GPI()
These two GPIOs probably need to also be listed in the early_gpio_table[] so they are set up before memory init.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: GPP_F1
This might be in a different commit but GPP_F1/GPP_F2 currently seem to be set to PAD_NC() and not P […]
Yes, I will keep this CL until I verified in factory. We are going to modify GPIO table. https://review.coreboot.org/c/coreboot/+/35175
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: DisableDimmChannel0 Shouldn't this be set in https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/cannonlake/... based on spd[].read_type.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: DisableDimmChannel0
Shouldn't this be set in https://review.coreboot.org/cgit/coreboot. […]
I didn't get this, we will use soldered down memory in this project. We will fix the type cbfs if I am right. https://review.coreboot.org/c/coreboot/+/35141. Intel still work on this.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: DisableDimmChannel0
I didn't get this, we will use soldered down memory in this project. […]
I don't understand the comment.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: DisableDimmChannel0
I don't understand the comment.
I means this is Drallion feature only, and I didn't know why this need based on read_type? This project will remove the other variant later. So we only have one read type.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: DisableDimmChannel0
I means this is Drallion feature only, and I didn't know why this need based on read_type? This proj […]
I meant something like this:
diff --git a/src/soc/intel/cannonlake/cnl_memcfg_init.c b/src/soc/intel/cannonlake/cnl_memcfg_init.c index 6c551ad563..894bcfe2e5 100644 --- a/src/soc/intel/cannonlake/cnl_memcfg_init.c +++ b/src/soc/intel/cannonlake/cnl_memcfg_init.c @@ -134,6 +134,10 @@ void cannonlake_memcfg_init(FSP_M_CONFIG *mem_cfg, spdi = &(cnl_cfg->spd[i]); switch (spdi->read_type) { case NOT_EXISTING: + if ((i == 0) || (i == 1)) + mem_cfg->DisableDimmChannel0 = 1 << i; + else + mem_cfg->DisableDimmChannel1 = 1 << (i % 2); break; case READ_SMBUS: mem_cfg->SpdAddressTable[i] =
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
Oh, the SPD always exist. This disable channel temporary for debug usage. We always enable the channel. The strap pin can be rework by HW.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 24: spd[0] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa0}, : }, : .spd[1] = {.read_type = NOT_EXISTING}, : .spd[2] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa4}, : }, : .spd[3] = {.read_type = NOT_EXISTING}, Also, this will have to be updated based on GPP_F1 and GPP_F2?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 24: spd[0] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa0}, : }, : .spd[1] = {.read_type = NOT_EXISTING}, : .spd[2] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa4}, : }, : .spd[3] = {.read_type = NOT_EXISTING},
Also, this will have to be updated based on GPP_F1 and GPP_F2?
No, as I said, this currently copy from Sarien and we will remove this in Drallion, we only use on-board ram and use the SPD files by CBFS.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
I elaborate more in the issue. Please have a look.
Hello Thejaswani Putta, Bernardo Perez Priego, Mathew King, Selma Bensaid, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35241
to look at the new patch set (#3).
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
mb/google/drallion: dynamic disable memory channel
Disable memory channel by HW strap pin. Using for factory debug. Keep this until the real board come out.
BUG=b:139773082 BRANCH=N/A TEST=N/A
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/3
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 3:
Please have a review to see it's clear to you or not.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35241/1/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/1/src/mainboard/google/dralli... PS1, Line 65: memupd->FspmConfig.DisableDimmChannel0 = gpio_get(GPP_F1)? 0 : 3;
spaces required around that '?' (ctx:VxW)
Ack
https://review.coreboot.org/c/coreboot/+/35241/1/src/mainboard/google/dralli... PS1, Line 66: memupd->FspmConfig.DisableDimmChannel1 = gpio_get(GPP_F2)? 0 : 3;
spaces required around that '?' (ctx:VxW)
Ack
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 24: spd[0] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa0}, : }, : .spd[1] = {.read_type = NOT_EXISTING}, : .spd[2] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa4}, : }, : .spd[3] = {.read_type = NOT_EXISTING},
No, as I said, this currently copy from Sarien and we will remove this in Drallion, we only use on-b […]
Ack
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: GPP_F1
Yes, I will keep this CL until I verified in factory. We are going to modify GPIO table. […]
Ack
https://review.coreboot.org/c/coreboot/+/35241/2/src/mainboard/google/dralli... PS2, Line 65: DisableDimmChannel0
I meant something like this: […]
Ack
Hello Thejaswani Putta, Bernardo Perez Priego, Mathew King, Selma Bensaid, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35241
to look at the new patch set (#4).
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
mb/google/drallion: dynamic disable memory channel
Disable memory channel by HW strap pin. Using for factory debug. Keep this until the real board come out.
BUG=b:139773082 BRANCH=N/A TEST=N/A
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/4
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 4: Code-Review+2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 4: Code-Review-1
Let me hold this after I verified at factory.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/35241/4/src/mainboard/google/dralli... PS4, Line 31: Sensor detection pin Comment needs update?
Hello Thejaswani Putta, Bernardo Perez Priego, Mathew King, Selma Bensaid, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35241
to look at the new patch set (#5).
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
mb/google/drallion: dynamic disable memory channel
Disable memory channel by HW strap pin. Using for factory debug. Keep this until the real board come out.
BUG=b:139773082 BRANCH=N/A TEST=N/A
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/5
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/35241/4/src/mainboard/google/dralli... PS4, Line 31: Sensor detection pin
Comment needs update?
Thanks for the heads up. done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: dynamic disable memory channel ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35241/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35241/5//COMMIT_MSG@7 PS5, Line 7: dynamic Dynamicly
https://review.coreboot.org/c/coreboot/+/35241/5//COMMIT_MSG@10 PS5, Line 10: board boards?
Hello Thejaswani Putta, Bernardo Perez Priego, Mathew King, Selma Bensaid, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35241
to look at the new patch set (#6).
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
mb/google/drallion: Dynamicly disable memory channel
Disable memory channel by HW strap pin. Using for factory debug. Keep this until the real boards come out.
BUG=b:139773082 BRANCH=N/A TEST=N/A
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/6
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 6:
(2 comments)
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35241/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35241/5//COMMIT_MSG@7 PS5, Line 7: dynamic
Dynamicly
Done
https://review.coreboot.org/c/coreboot/+/35241/5//COMMIT_MSG@10 PS5, Line 10: board
boards?
Done. I will remove this line after verified this.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/6/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35241/6/src/mainboard/google/dralli... PS6, Line 291: * 0:Enable both DIMMs, 3:Disable both DIMMs Please look at the allowed comment styles in the coding style document. Add space after the colons.
/* Disable memory … 0: Enable both DIMMs, 3: Disable both DIMMs */
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/6/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35241/6/src/mainboard/google/dralli... PS6, Line 291: * 0:Enable both DIMMs, 3:Disable both DIMMs
Please look at the allowed comment styles in the coding style document. Add space after the colons. […]
I checked this, I think it ask for the code not comment but fine.
https://github.com/torvalds/linux/blob/master/Documentation/process/coding-s... Use one space around (on each side of) most binary and ternary operators.
Hello Thejaswani Putta, Bernardo Perez Priego, Mathew King, Selma Bensaid, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35241
to look at the new patch set (#7).
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
mb/google/drallion: Dynamicly disable memory channel
Disable memory channel by HW strap pin. Using for factory debug. Keep this until the real boards come out.
BUG=b:139773082 BRANCH=N/A TEST=N/A
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/7
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/6/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35241/6/src/mainboard/google/dralli... PS6, Line 291: * 0:Enable both DIMMs, 3:Disable both DIMMs
I checked this, I think it ask for the code not comment but fine. […]
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/7/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35241/7/src/mainboard/google/dralli... PS7, Line 292: */ I meant the [coreboot coding style][1] which slightly differs. Please see the asterisks in my previous comment.
[1]: https://doc.coreboot.org/coding_style.html#commenting
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35241/7/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35241/7/src/mainboard/google/dralli... PS7, Line 292: */
I meant the [coreboot coding style][1] which slightly differs. […]
Ack
Hello Thejaswani Putta, Bernardo Perez Priego, Mathew King, Selma Bensaid, Duncan Laurie, Bora Guvendik, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35241
to look at the new patch set (#8).
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
mb/google/drallion: Dynamicly disable memory channel
Disable memory channel by HW strap pin. Using for factory debug.
BUG=b:139773082 BRANCH=N/A TEST=Rework HW strap pin and check /proc/mem_info
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/8
Frank Wu has uploaded a new patch set (#9) to the change originally created by EricR Lai. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
mb/google/drallion: Dynamicly disable memory channel
Disable memory channel by HW strap pin. Using for factory debug.
BUG=b:139773082 BRANCH=N/A TEST=Rework HW strap pin and check /proc/mem_info
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/35241/9
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 9: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
Patch Set 9: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35241 )
Change subject: mb/google/drallion: Dynamicly disable memory channel ......................................................................
mb/google/drallion: Dynamicly disable memory channel
Disable memory channel by HW strap pin. Using for factory debug.
BUG=b:139773082 BRANCH=N/A TEST=Rework HW strap pin and check /proc/mem_info
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ic5f53f0ba3bd432fbcb7513d2a8aa49d42f7a23e Reviewed-on: https://review.coreboot.org/c/coreboot/+/35241 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/drallion/variants/drallion/gpio.c M src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h 2 files changed, 11 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved EricR Lai: Looks good to me, approved
diff --git a/src/mainboard/google/drallion/variants/drallion/gpio.c b/src/mainboard/google/drallion/variants/drallion/gpio.c index 154fc5a..f0fc55e 100644 --- a/src/mainboard/google/drallion/variants/drallion/gpio.c +++ b/src/mainboard/google/drallion/variants/drallion/gpio.c @@ -286,4 +286,11 @@ FSP_M_CONFIG *fsp_m_cfg = &mupd->FspmConfig; if (fsp_m_cfg->PchIshEnable) fsp_m_cfg->PchIshEnable = is_ish_device_enabled(); + + /* + * Disable memory channel by HW strap pin, HW default is enable + * 0: Enable both DIMMs, 3: Disable both DIMMs + */ + mupd->FspmConfig.DisableDimmChannel0 = gpio_get(DDR_CH0_EN) ? 0 : 3; + mupd->FspmConfig.DisableDimmChannel1 = gpio_get(DDR_CH1_EN) ? 0 : 3; } diff --git a/src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h b/src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h index 251b40e..219e0c4 100644 --- a/src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h +++ b/src/mainboard/google/drallion/variants/drallion/include/variant/gpio.h @@ -28,6 +28,10 @@ /* Sensor detection pin */ #define SENSOR_DET_360 GPP_H5
+/* DDR channel enable pin */ +#define DDR_CH0_EN GPP_F1 +#define DDR_CH1_EN GPP_F2 + /* Memory configuration board straps */ #define GPIO_MEM_CONFIG_0 GPP_F12 #define GPIO_MEM_CONFIG_1 GPP_F13
-
build bot (Jenkins) (Code Review) -
Duncan Laurie (Code Review)
-
EricR Lai (Code Review)
-
Frank Wu (Code Review)
-
Furquan Shaikh (Code Review)
-
Mathew King (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)