Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/romstage.c A src/mainboard/up/squared/romstage.h 2 files changed, 109 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/1
diff --git a/src/mainboard/up/squared/romstage.c b/src/mainboard/up/squared/romstage.c index 2cfaa00..c1c52ad 100644 --- a/src/mainboard/up/squared/romstage.c +++ b/src/mainboard/up/squared/romstage.c @@ -15,10 +15,20 @@
#include <string.h> #include <soc/romstage.h> +#include <soc/gpio_apl.h> #include <fsp/api.h> #include <FspmUpd.h> #include <console/console.h> +#include <gpio.h> #include "gpio.h" +#include "romstage.h" + +/* + * Offsets: + * - GPIO_27: 0xd8 + * - GPIO_28: 0xe0 + */ +const uint8_t memory_skuid_pads[] = { GPIO_27, GPIO_28 };
static const uint8_t ch0_bit_swizzling[] = { 0x0D, 0x0A, 0x08, 0x0B, 0x0C, 0x0F, 0x0E, 0x09, @@ -48,49 +58,101 @@ 0x19, 0x1F, 0x1D, 0x1B, 0x1E, 0x18, 0x1C, 0x1A };
- void mainboard_memory_init_params(FSPM_UPD *memupd) { printk(BIOS_DEBUG, "MAINBOARD: %s/%s called\n", __FILE__, __func__);
+ FSP_M_CONFIG *config = &memupd->FspmConfig; + + uint8_t memory_skuid = get_memory_skuid(); + + switch (memory_skuid) { + case 0: /* 2GB */ + config->DualRankSupportEnable = 0; + config->Ch0_RankEnable = 1; + config->Ch0_DramDensity = 2; + config->Ch1_RankEnable = 1; + config->Ch1_DramDensity = 2; + config->Ch2_RankEnable = 0; + config->Ch3_RankEnable = 0; + break; + case 1: /* 4GB */ + config->DualRankSupportEnable = 1; + config->Ch0_RankEnable = 1; + config->Ch0_DramDensity = 2; + config->Ch1_RankEnable = 1; + config->Ch1_DramDensity = 2; + config->Ch2_RankEnable = 1; + config->Ch2_DramDensity = 2; + config->Ch3_RankEnable = 1; + config->Ch3_DramDensity = 2; + break; + case 2: /* 8GB */ + config->DualRankSupportEnable = 1; + config->Ch0_RankEnable = 3; + config->Ch0_DramDensity = 2; + config->Ch1_RankEnable = 3; + config->Ch1_DramDensity = 2; + config->Ch2_RankEnable = 3; + config->Ch2_DramDensity = 2; + config->Ch3_RankEnable = 3; + config->Ch3_DramDensity = 2; + break; + default: + break; + } + gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table));
- memupd->FspmConfig.Package = 0x1; // 0x0 - memupd->FspmConfig.Profile = 0xB; // 0x19 - memupd->FspmConfig.MemoryDown = 0x1; // 0x0 - memupd->FspmConfig.DDR3LPageSize = 0x0; // 0x1 - memupd->FspmConfig.DIMM0SPDAddress = 0x0; // 0xa0 - memupd->FspmConfig.DIMM1SPDAddress = 0x0; // 0xa4 - memupd->FspmConfig.RmtCheckRun = 0x3; // 0x0 - memupd->FspmConfig.RmtMarginCheckScaleHighThreshold = 0xC8; // 0x0 - memupd->FspmConfig.EnhancePort8xhDecoding = 0x0; // 0x1 - memupd->FspmConfig.NpkEn = 0x0; // 0x3 - memupd->FspmConfig.PrimaryVideoAdaptor = 0x2; // 0x0 + config->Package = 0x1; // 0x0 + config->Profile = 0xB; // 0x19 + config->MemoryDown = 0x1; // 0x0 + config->DDR3LPageSize = 0x0; // 0x1 + config->DIMM0SPDAddress = 0x0; // 0xa0 + config->DIMM1SPDAddress = 0x0; // 0xa4 + config->RmtCheckRun = 0x3; // 0x0 + config->RmtMarginCheckScaleHighThreshold = 0xC8; // 0x0 + config->EnhancePort8xhDecoding = 0x0; // 0x1 + config->NpkEn = 0x0; // 0x3 + config->PrimaryVideoAdaptor = 0x2; // 0x0
- memupd->FspmConfig.Ch0_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch0_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch0_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch0_Option = 0x3; // 0x0 - memupd->FspmConfig.Ch1_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch1_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch1_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch1_Option = 0x3; // 0x0 - memupd->FspmConfig.Ch2_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch2_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch2_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch2_Option = 0x3; // 0x0 - memupd->FspmConfig.Ch3_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch3_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch3_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch3_Option = 0x3; // 0x0 - memupd->FspmConfig.StartTimerTickerOfPfetAssert = 0x4E20; // 0x0 + config->Ch0_RankEnable = 0x1; // 0x0 + config->Ch0_DeviceWidth = 0x1; // 0x0 + config->Ch0_DramDensity = 0x2; // 0x0 + config->Ch0_Option = 0x3; // 0x0 + config->Ch1_RankEnable = 0x1; // 0x0 + config->Ch1_DeviceWidth = 0x1; // 0x0 + config->Ch1_DramDensity = 0x2; // 0x0 + config->Ch1_Option = 0x3; // 0x0 + config->Ch2_RankEnable = 0x1; // 0x0 + config->Ch2_DeviceWidth = 0x1; // 0x0 + config->Ch2_DramDensity = 0x2; // 0x0 + config->Ch2_Option = 0x3; // 0x0 + config->Ch3_RankEnable = 0x1; // 0x0 + config->Ch3_DeviceWidth = 0x1; // 0x0 + config->Ch3_DramDensity = 0x2; // 0x0 + config->Ch3_Option = 0x3; // 0x0 + config->StartTimerTickerOfPfetAssert = 0x4E20; // 0x0
- memcpy(memupd->FspmConfig.Ch0_Bit_swizzling, &ch0_bit_swizzling, + memcpy(config->Ch0_Bit_swizzling, &ch0_bit_swizzling, sizeof(ch0_bit_swizzling)); - memcpy(memupd->FspmConfig.Ch1_Bit_swizzling, &ch1_bit_swizzling, + memcpy(config->Ch1_Bit_swizzling, &ch1_bit_swizzling, sizeof(ch1_bit_swizzling)); - memcpy(memupd->FspmConfig.Ch2_Bit_swizzling, &ch2_bit_swizzling, + memcpy(config->Ch2_Bit_swizzling, &ch2_bit_swizzling, sizeof(ch2_bit_swizzling)); - memcpy(memupd->FspmConfig.Ch3_Bit_swizzling, &ch3_bit_swizzling, + memcpy(config->Ch3_Bit_swizzling, &ch3_bit_swizzling, sizeof(ch3_bit_swizzling)); } + +uint8_t get_memory_skuid(void) +{ + uint8_t memory_skuid = 0; + + for (uint8_t i = 0; i < ARRAY_SIZE(memory_skuid_pads); i++) { + uint32_t gpio_value = gpio_get(memory_skuid_pads[i]); + uint8_t rx_state = gpio_value & 0x2; + memory_skuid |= rx_state << i; + } + memory_skuid = memory_skuid & 0x3; + return memory_skuid; +} diff --git a/src/mainboard/up/squared/romstage.h b/src/mainboard/up/squared/romstage.h new file mode 100644 index 0000000..5c0f4ba --- /dev/null +++ b/src/mainboard/up/squared/romstage.h @@ -0,0 +1,14 @@ +/* + * This file is part of the coreboot project. + * + * 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. + */ + +uint8_t get_memory_skuid(void);
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 4:
This change is ready for review.
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#5).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/romstage.c A src/mainboard/up/squared/romstage.h 2 files changed, 115 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/5
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34838/2/src/mainboard/up/squared/ro... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/2/src/mainboard/up/squared/ro... PS2, Line 31: cons
Do you know what each bit combination corresponds to? Something like this table I made up: […]
Done
https://review.coreboot.org/c/coreboot/+/34838/2/src/mainboard/up/squared/ro... PS2, Line 149: return memory_skuid;
I think it is better to do this in mainboard_memory_init_params(), since the evaluation is there.
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 5: Code-Review-1
(3 comments)
Please test this code on hardware.
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... PS5, Line 31: const uint8_t memory_skuid_pads[] = { GPIO_27, GPIO_28 }; static
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... PS5, Line 159: uint8_t rx_state = gpio_value & 0x2; rx_state is always 0
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... PS5, Line 162: memory_skuid = memory_skuid & 0x3; pointless operation
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#6).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/romstage.c A src/mainboard/up/squared/romstage.h 2 files changed, 112 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/6
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... PS5, Line 159: uint8_t rx_state = gpio_value & 0x2;
rx_state is always 0
Done. Somehow I messed up two versions.. Will test it tomorrow.
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#7).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/romstage.c A src/mainboard/up/squared/romstage.h 2 files changed, 111 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/7
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#8).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/romstage.c A src/mainboard/up/squared/romstage.h 2 files changed, 111 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/8
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... PS5, Line 31: const uint8_t memory_skuid_pads[] = { GPIO_27, GPIO_28 };
static
Done
https://review.coreboot.org/c/coreboot/+/34838/5/src/mainboard/up/squared/ro... PS5, Line 162: memory_skuid = memory_skuid & 0x3;
pointless operation
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34838/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34838/9//COMMIT_MSG@8 PS9, Line 8: tested on?
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... PS9, Line 28: * - GPIO_27: 0xd8 doesn't match memory_skuid_pads
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... PS9, Line 31: static const uint8_t memory_skuid_pads[] = { GPIO_214, GPIO_215 }; make sure to activate 20K pull ups on those as we don't know if there are pull ups on every hardware revision.
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... PS9, Line 145: * GPIO27 GPIO28 Memory size same
Hello Patrick Rudolph, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#10).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/romstage.c A src/mainboard/up/squared/romstage.h 2 files changed, 111 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/10
Christoph Pomaska has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 10: Code-Review+1
Hello Patrick Rudolph, Christoph Pomaska, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#11).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
TESTED=UP Squared
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/gpio.h M src/mainboard/up/squared/romstage.c A src/mainboard/up/squared/romstage.h 3 files changed, 113 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/11
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 11:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... PS9, Line 28: * - GPIO_27: 0xd8
doesn't match memory_skuid_pads
Done
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... PS9, Line 31: static const uint8_t memory_skuid_pads[] = { GPIO_214, GPIO_215 };
make sure to activate 20K pull ups on those as we don't know if there are pull ups on every hardware […]
Done
https://review.coreboot.org/c/coreboot/+/34838/9/src/mainboard/up/squared/ro... PS9, Line 145: * GPIO27 GPIO28 Memory size
same
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 11: Code-Review+2
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34838/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34838/9//COMMIT_MSG@8 PS9, Line 8:
tested on?
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34838/11/src/mainboard/up/squared/r... File src/mainboard/up/squared/romstage.h:
PS11: is this really needed? I don't see this function being called outside of romstage.c, so I'd rather put the function prototype there or move the function before the function calling it and mark the function as static
https://review.coreboot.org/c/coreboot/+/34838/11/src/mainboard/up/squared/r... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/11/src/mainboard/up/squared/r... PS11, Line 69: memory_skuid maybe use an enum for this?
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34838/11/src/mainboard/up/squared/r... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/11/src/mainboard/up/squared/r... PS11, Line 69: memory_skuid
maybe use an enum for this?
ok, with an enum type here, the follow-up patch would be a bit uglier
Hello Patrick Rudolph, Christoph Pomaska, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#12).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
TESTED=UP Squared
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/gpio.h M src/mainboard/up/squared/romstage.c 2 files changed, 97 insertions(+), 34 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/12
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34838/11/src/mainboard/up/squared/r... File src/mainboard/up/squared/romstage.h:
PS11:
is this really needed? I don't see this function being called outside of romstage. […]
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34838/12/src/mainboard/up/squared/r... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/12/src/mainboard/up/squared/r... PS12, Line 30: static const uint8_t memory_skuid_pads[] = { GPIO_214, GPIO_215 }; are the gpio pins in the right order here? if i read the code below correctly, the array element 0 becomes bit 0 in memory_skuid; the table beginning on line 61 suggests though that the state of gpio125 should become the lsb of memory_skuid.
Hello Patrick Rudolph, Christoph Pomaska, Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34838
to look at the new patch set (#13).
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
TESTED=UP Squared
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com --- M src/mainboard/up/squared/gpio.h M src/mainboard/up/squared/romstage.c 2 files changed, 97 insertions(+), 34 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/34838/13
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34838/12/src/mainboard/up/squared/r... File src/mainboard/up/squared/romstage.c:
https://review.coreboot.org/c/coreboot/+/34838/12/src/mainboard/up/squared/r... PS12, Line 30: static const uint8_t memory_skuid_pads[] = { GPIO_214, GPIO_215 };
are the gpio pins in the right order here? if i read the code below correctly, the array element 0 b […]
These pins are in the correct order, just the column names were mixed up.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 13: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
Patch Set 13: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/34838 )
Change subject: mb/up/squared: Do RAM config based on SKU ID ......................................................................
mb/up/squared: Do RAM config based on SKU ID
TESTED=UP Squared
Change-Id: Ic121652213d5b1f65cff2f3096e919a3cf88db72 Signed-off-by: Felix Singer felix.singer@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34838 Reviewed-by: Patrick Rudolph siro@das-labor.org Reviewed-by: Felix Held felix-coreboot@felixheld.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/up/squared/gpio.h M src/mainboard/up/squared/romstage.c 2 files changed, 97 insertions(+), 34 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Patrick Rudolph: Looks good to me, approved
diff --git a/src/mainboard/up/squared/gpio.h b/src/mainboard/up/squared/gpio.h index b4ac4e9..eb3081b 100644 --- a/src/mainboard/up/squared/gpio.h +++ b/src/mainboard/up/squared/gpio.h @@ -342,10 +342,10 @@ _PAD_CFG_STRUCT(GPIO_213, 0x44000201, 0x00003000),
// GPIO - _PAD_CFG_STRUCT(GPIO_214, 0x44000102, 0x00000000), + _PAD_CFG_STRUCT(GPIO_214, 0x44000102, 0x00003300),
// GPIO - _PAD_CFG_STRUCT(GPIO_215, 0x44000100, 0x00000000), + _PAD_CFG_STRUCT(GPIO_215, 0x44000100, 0x00003300),
// *THERMTRIP_N _PAD_CFG_STRUCT(PMIC_THERMTRIP_B, 0x44000400, 0x00003000), diff --git a/src/mainboard/up/squared/romstage.c b/src/mainboard/up/squared/romstage.c index 2cfaa00..5bef64d 100644 --- a/src/mainboard/up/squared/romstage.c +++ b/src/mainboard/up/squared/romstage.c @@ -15,11 +15,20 @@
#include <string.h> #include <soc/romstage.h> +#include <soc/gpio_apl.h> #include <fsp/api.h> #include <FspmUpd.h> #include <console/console.h> +#include <gpio.h> #include "gpio.h"
+/* + * Offsets: + * - GPIO_214: 0xd8 + * - GPIO_215: 0xe0 + */ +static const uint8_t memory_skuid_pads[] = { GPIO_214, GPIO_215 }; + static const uint8_t ch0_bit_swizzling[] = { 0x0D, 0x0A, 0x08, 0x0B, 0x0C, 0x0F, 0x0E, 0x09, 0x06, 0x00, 0x03, 0x04, 0x07, 0x01, 0x05, 0x02, @@ -48,49 +57,103 @@ 0x19, 0x1F, 0x1D, 0x1B, 0x1E, 0x18, 0x1C, 0x1A };
+/* + * GPIO215 GPIO214 Memory size + * 0 0 2 GiB + * 0 1 4 GiB + * 1 0 8 GiB + * 1 1 Reserved + */ +static uint8_t get_memory_skuid(void) +{ + uint8_t memory_skuid = 0; + + for (uint8_t i = 0; i < ARRAY_SIZE(memory_skuid_pads); i++) { + uint8_t rx_state = gpio_get(memory_skuid_pads[i]); + memory_skuid |= rx_state << i; + } + return memory_skuid; +}
void mainboard_memory_init_params(FSPM_UPD *memupd) { printk(BIOS_DEBUG, "MAINBOARD: %s/%s called\n", __FILE__, __func__);
+ FSP_M_CONFIG *config = &memupd->FspmConfig; + gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table));
- memupd->FspmConfig.Package = 0x1; // 0x0 - memupd->FspmConfig.Profile = 0xB; // 0x19 - memupd->FspmConfig.MemoryDown = 0x1; // 0x0 - memupd->FspmConfig.DDR3LPageSize = 0x0; // 0x1 - memupd->FspmConfig.DIMM0SPDAddress = 0x0; // 0xa0 - memupd->FspmConfig.DIMM1SPDAddress = 0x0; // 0xa4 - memupd->FspmConfig.RmtCheckRun = 0x3; // 0x0 - memupd->FspmConfig.RmtMarginCheckScaleHighThreshold = 0xC8; // 0x0 - memupd->FspmConfig.EnhancePort8xhDecoding = 0x0; // 0x1 - memupd->FspmConfig.NpkEn = 0x0; // 0x3 - memupd->FspmConfig.PrimaryVideoAdaptor = 0x2; // 0x0 + uint8_t memory_skuid = get_memory_skuid(); + printk(BIOS_DEBUG, "MAINBOARD: Found memory SKU ID: 0x%02x\n", memory_skuid);
- memupd->FspmConfig.Ch0_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch0_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch0_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch0_Option = 0x3; // 0x0 - memupd->FspmConfig.Ch1_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch1_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch1_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch1_Option = 0x3; // 0x0 - memupd->FspmConfig.Ch2_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch2_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch2_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch2_Option = 0x3; // 0x0 - memupd->FspmConfig.Ch3_RankEnable = 0x1; // 0x0 - memupd->FspmConfig.Ch3_DeviceWidth = 0x1; // 0x0 - memupd->FspmConfig.Ch3_DramDensity = 0x2; // 0x0 - memupd->FspmConfig.Ch3_Option = 0x3; // 0x0 - memupd->FspmConfig.StartTimerTickerOfPfetAssert = 0x4E20; // 0x0 + switch (memory_skuid) { + case 0: /* 2GB */ + config->DualRankSupportEnable = 0; + config->Ch0_RankEnable = 1; + config->Ch0_DramDensity = 2; + config->Ch1_RankEnable = 1; + config->Ch1_DramDensity = 2; + config->Ch2_RankEnable = 0; + config->Ch3_RankEnable = 0; + printk(BIOS_INFO, "MAINBOARD: Found supported memory: 2GB\n"); + break; + case 1: /* 4GB */ + config->DualRankSupportEnable = 1; + config->Ch0_RankEnable = 1; + config->Ch0_DramDensity = 2; + config->Ch1_RankEnable = 1; + config->Ch1_DramDensity = 2; + config->Ch2_RankEnable = 1; + config->Ch2_DramDensity = 2; + config->Ch3_RankEnable = 1; + config->Ch3_DramDensity = 2; + printk(BIOS_INFO, "MAINBOARD: Found supported memory: 4GB\n"); + break; + case 2: /* 8GB */ + config->DualRankSupportEnable = 1; + config->Ch0_RankEnable = 3; + config->Ch0_DramDensity = 2; + config->Ch1_RankEnable = 3; + config->Ch1_DramDensity = 2; + config->Ch2_RankEnable = 3; + config->Ch2_DramDensity = 2; + config->Ch3_RankEnable = 3; + config->Ch3_DramDensity = 2; + printk(BIOS_INFO, "MAINBOARD: Found supported memory: 8GB\n"); + break; + default: + printk(BIOS_INFO, "MAINBOARD: No supported memory found!\n"); + break; + }
- memcpy(memupd->FspmConfig.Ch0_Bit_swizzling, &ch0_bit_swizzling, + config->Package = 0x1; // 0x0 + config->Profile = 0xB; // 0x19 + config->MemoryDown = 0x1; // 0x0 + config->DDR3LPageSize = 0x0; // 0x1 + config->DIMM0SPDAddress = 0x0; // 0xa0 + config->DIMM1SPDAddress = 0x0; // 0xa4 + config->RmtCheckRun = 0x3; // 0x0 + config->RmtMarginCheckScaleHighThreshold = 0xC8; // 0x0 + config->EnhancePort8xhDecoding = 0x0; // 0x1 + config->NpkEn = 0x0; // 0x3 + config->PrimaryVideoAdaptor = 0x2; // 0x0 + + config->Ch0_DeviceWidth = 0x1; // 0x0 + config->Ch0_Option = 0x3; // 0x0 + config->Ch1_DeviceWidth = 0x1; // 0x0 + config->Ch1_Option = 0x3; // 0x0 + config->Ch2_DeviceWidth = 0x1; // 0x0 + config->Ch2_Option = 0x3; // 0x0 + config->Ch3_DeviceWidth = 0x1; // 0x0 + config->Ch3_Option = 0x3; // 0x0 + config->StartTimerTickerOfPfetAssert = 0x4E20; // 0x0 + + memcpy(config->Ch0_Bit_swizzling, &ch0_bit_swizzling, sizeof(ch0_bit_swizzling)); - memcpy(memupd->FspmConfig.Ch1_Bit_swizzling, &ch1_bit_swizzling, + memcpy(config->Ch1_Bit_swizzling, &ch1_bit_swizzling, sizeof(ch1_bit_swizzling)); - memcpy(memupd->FspmConfig.Ch2_Bit_swizzling, &ch2_bit_swizzling, + memcpy(config->Ch2_Bit_swizzling, &ch2_bit_swizzling, sizeof(ch2_bit_swizzling)); - memcpy(memupd->FspmConfig.Ch3_Bit_swizzling, &ch3_bit_swizzling, + memcpy(config->Ch3_Bit_swizzling, &ch3_bit_swizzling, sizeof(ch3_bit_swizzling)); }
-
Christoph Pomaska (Code Review) -
Felix Held (Code Review)
-
Felix Singer (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)