Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33456
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
mb/google/hatch/variants/helios: Use LPDDR3 memory
Change the SPD makefile to use the LPDDR3 SPDs. Set up the arrays for mapping SoC DQS pins to LPDDR3 pins.
BRANCH=none BUG=b:133455595 TEST=`FEATURES="noclean" FW_NAME="helios" emerge-hatch chromeos-ec depthcharge vboot_reference libpayload coreboot-private-files intel-cmlfsp coreboot-private-files-hatch coreboot chromeos-bootimage` Ensure the firmware builds without error.
Change-Id: Iebaba2ec65dfcf36674b4733b421ada107b22b09 Signed-off-by: Paul Fagerburg pfagerburg@chromium.org --- M src/mainboard/google/hatch/variants/helios/Makefile.inc A src/mainboard/google/hatch/variants/helios/memory.c 2 files changed, 73 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/33456/1
diff --git a/src/mainboard/google/hatch/variants/helios/Makefile.inc b/src/mainboard/google/hatch/variants/helios/Makefile.inc index cf6ee5a..b2a4095 100644 --- a/src/mainboard/google/hatch/variants/helios/Makefile.inc +++ b/src/mainboard/google/hatch/variants/helios/Makefile.inc @@ -12,9 +12,8 @@ ## GNU General Public License for more details. ##
-SPD_SOURCES = 4G_2400 # 0b000 -SPD_SOURCES += empty_ddr4 # 0b001 -SPD_SOURCES += 8G_2400 # 0b010 -SPD_SOURCES += 8G_2666 # 0b011 -SPD_SOURCES += 16G_2400 # 0b100 -SPD_SOURCES += 16G_2666 # 0b101 +SPD_SOURCES = LP_8G_2133 # 0b0000 +SPD_SOURCES += empty_ddr4 # 0b0001 +SPD_SOURCES += LP_16G_2133 # 0b0010 + +romstage-y += memory.c diff --git a/src/mainboard/google/hatch/variants/helios/memory.c b/src/mainboard/google/hatch/variants/helios/memory.c new file mode 100644 index 0000000..64b4cac --- /dev/null +++ b/src/mainboard/google/hatch/variants/helios/memory.c @@ -0,0 +1,68 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <baseboard/variants.h> +#include <baseboard/gpio.h> +#include <soc/cnl_memcfg_init.h> +#include <string.h> + +static const struct cnl_mb_cfg baseboard_memcfg = { + /* + * The dqs_map arrays map the SoC pins to the lpddr3 pins + * for both channels. + * + * "The index of the array is CPU byte number, the values are DRAM byte + * numbers." - doc #573387 + * + * the index = pin number on SoC + * the value = pin number on lpddr3 part + */ + .dqs_map[DDR_CH0] = {4, 7, 5, 6, 0, 3, 2, 1}, + .dqs_map[DDR_CH1] = {0, 3, 2, 1, 4, 7, 6, 5}, + + .dq_map[DDR_CH0] = { + {0xf0, 0xf}, + {0x0, 0xf}, + {0xf0, 0xf}, + {0xf0, 0x0}, + {0xff, 0x0}, + {0xff, 0x0} + }, + .dq_map[DDR_CH1] = { + {0xf, 0xf0}, + {0x0, 0xf0}, + {0xf, 0xf0}, + {0xf, 0x0}, + {0xff, 0x0}, + {0xff, 0x0} + }, + + /* Helios uses 200, 80.6 and 162 rcomp resistors */ + .rcomp_resistor = {200, 81, 162}, + + /* Helios Rcomp target values */ + .rcomp_targets = {100, 40, 40, 23, 40}, + + /* Set CaVref config to 0 for LPDDR3 */ + .vref_ca_config = 0, + + /* Disable Early Command Training */ + .ect = 0, +}; + +void variant_memory_params(struct cnl_mb_cfg *bcfg) +{ + memcpy(bcfg, &baseboard_memcfg, sizeof(baseboard_memcfg)); +}
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33456/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/helios/Makefile.inc:
https://review.coreboot.org/#/c/33456/1/src/mainboard/google/hatch/variants/... PS1, Line 17: LP_16G_2133 Shouldn't this be at index 1?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
Patch Set 1: Code-Review+1
Hello Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33456
to look at the new patch set (#2).
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
mb/google/hatch/variants/helios: Use LPDDR3 memory
Change the SPD makefile to use the LPDDR3 SPDs. Set up the arrays for mapping SoC DQS pins to LPDDR3 pins.
BRANCH=none BUG=b:133455595 TEST=`FEATURES="noclean" FW_NAME="helios" emerge-hatch chromeos-ec depthcharge vboot_reference libpayload coreboot-private-files intel-cmlfsp coreboot-private-files-hatch coreboot chromeos-bootimage` Ensure the firmware builds without error.
Change-Id: Iebaba2ec65dfcf36674b4733b421ada107b22b09 Signed-off-by: Paul Fagerburg pfagerburg@chromium.org --- M src/mainboard/google/hatch/variants/helios/Makefile.inc A src/mainboard/google/hatch/variants/helios/memory.c 2 files changed, 72 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/33456/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
Patch Set 2: Code-Review+2
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33456/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/helios/Makefile.inc:
https://review.coreboot.org/#/c/33456/1/src/mainboard/google/hatch/variants/... PS1, Line 17: LP_16G_2133
Shouldn't this be at index 1?
According to the chart in comment #2 on the bug, 0000 is for the 512Mbit parts, and 0010 is for the 1024Mbit parts. Do we want to have them change the strapping, and then the other vendor's parts (if not covered by these SPDs) can have slots 0010 and 0011?
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33456/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/helios/Makefile.inc:
https://review.coreboot.org/#/c/33456/1/src/mainboard/google/hatch/variants/... PS1, Line 17: LP_16G_2133
According to the chart in comment #2 on the bug, 0000 is for the 512Mbit parts, and 0010 is for the […]
Done
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
Patch Set 2: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33456 )
Change subject: mb/google/hatch/variants/helios: Use LPDDR3 memory ......................................................................
mb/google/hatch/variants/helios: Use LPDDR3 memory
Change the SPD makefile to use the LPDDR3 SPDs. Set up the arrays for mapping SoC DQS pins to LPDDR3 pins.
BRANCH=none BUG=b:133455595 TEST=`FEATURES="noclean" FW_NAME="helios" emerge-hatch chromeos-ec depthcharge vboot_reference libpayload coreboot-private-files intel-cmlfsp coreboot-private-files-hatch coreboot chromeos-bootimage` Ensure the firmware builds without error.
Change-Id: Iebaba2ec65dfcf36674b4733b421ada107b22b09 Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/33456 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Shelley Chen shchen@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/hatch/variants/helios/Makefile.inc A src/mainboard/google/hatch/variants/helios/memory.c 2 files changed, 72 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Shelley Chen: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/helios/Makefile.inc b/src/mainboard/google/hatch/variants/helios/Makefile.inc index cf6ee5a..ddafa32 100644 --- a/src/mainboard/google/hatch/variants/helios/Makefile.inc +++ b/src/mainboard/google/hatch/variants/helios/Makefile.inc @@ -12,9 +12,7 @@ ## GNU General Public License for more details. ##
-SPD_SOURCES = 4G_2400 # 0b000 -SPD_SOURCES += empty_ddr4 # 0b001 -SPD_SOURCES += 8G_2400 # 0b010 -SPD_SOURCES += 8G_2666 # 0b011 -SPD_SOURCES += 16G_2400 # 0b100 -SPD_SOURCES += 16G_2666 # 0b101 +SPD_SOURCES = LP_8G_2133 # 0b0000 +SPD_SOURCES += LP_16G_2133 # 0b0001 + +romstage-y += memory.c diff --git a/src/mainboard/google/hatch/variants/helios/memory.c b/src/mainboard/google/hatch/variants/helios/memory.c new file mode 100644 index 0000000..64b4cac --- /dev/null +++ b/src/mainboard/google/hatch/variants/helios/memory.c @@ -0,0 +1,68 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <baseboard/variants.h> +#include <baseboard/gpio.h> +#include <soc/cnl_memcfg_init.h> +#include <string.h> + +static const struct cnl_mb_cfg baseboard_memcfg = { + /* + * The dqs_map arrays map the SoC pins to the lpddr3 pins + * for both channels. + * + * "The index of the array is CPU byte number, the values are DRAM byte + * numbers." - doc #573387 + * + * the index = pin number on SoC + * the value = pin number on lpddr3 part + */ + .dqs_map[DDR_CH0] = {4, 7, 5, 6, 0, 3, 2, 1}, + .dqs_map[DDR_CH1] = {0, 3, 2, 1, 4, 7, 6, 5}, + + .dq_map[DDR_CH0] = { + {0xf0, 0xf}, + {0x0, 0xf}, + {0xf0, 0xf}, + {0xf0, 0x0}, + {0xff, 0x0}, + {0xff, 0x0} + }, + .dq_map[DDR_CH1] = { + {0xf, 0xf0}, + {0x0, 0xf0}, + {0xf, 0xf0}, + {0xf, 0x0}, + {0xff, 0x0}, + {0xff, 0x0} + }, + + /* Helios uses 200, 80.6 and 162 rcomp resistors */ + .rcomp_resistor = {200, 81, 162}, + + /* Helios Rcomp target values */ + .rcomp_targets = {100, 40, 40, 23, 40}, + + /* Set CaVref config to 0 for LPDDR3 */ + .vref_ca_config = 0, + + /* Disable Early Command Training */ + .ect = 0, +}; + +void variant_memory_params(struct cnl_mb_cfg *bcfg) +{ + memcpy(bcfg, &baseboard_memcfg, sizeof(baseboard_memcfg)); +}
-
Furquan Shaikh (Code Review) -
Paul Fagerburg (Code Review)
-
Shelley Chen (Code Review)
-
Tim Wawrzynczak (Code Review)