Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
mainboard/google: Move variant_memory_sku() to baseboard/
The variant_memory_sku() symbol within romstage.c unreasonably depends on pre-processor #define's in variants that may not define them. Provide a default symbol and configuration within the basebase in the form of a weak symbol and allow the two variants that differ to override using a strong symbol.
In the case of variants that do not need this symbol they can override with a strong symbol that is a nop.
Change-Id: I41532e3067a989db5776887fac0459a000d07ff0 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M src/mainboard/google/hatch/romstage.c M src/mainboard/google/hatch/variants/akemi/include/variant/gpio.h M src/mainboard/google/hatch/variants/baseboard/memory.c M src/mainboard/google/hatch/variants/dratini/include/variant/gpio.h M src/mainboard/google/hatch/variants/hatch/Makefile.inc M src/mainboard/google/hatch/variants/hatch/include/variant/gpio.h A src/mainboard/google/hatch/variants/hatch/memory.c M src/mainboard/google/hatch/variants/helios/include/variant/gpio.h M src/mainboard/google/hatch/variants/kindred/include/variant/gpio.h M src/mainboard/google/hatch/variants/kohaku/include/variant/gpio.h M src/mainboard/google/hatch/variants/kohaku/memory.c 11 files changed, 76 insertions(+), 48 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/36492/1
diff --git a/src/mainboard/google/hatch/romstage.c b/src/mainboard/google/hatch/romstage.c index a94fab5..50d9066 100644 --- a/src/mainboard/google/hatch/romstage.c +++ b/src/mainboard/google/hatch/romstage.c @@ -29,18 +29,6 @@ */ #define GPIO_MEM_CH_SEL GPP_F2
-int __weak variant_memory_sku(void) -{ - const gpio_t spd_gpios[] = { - GPIO_MEM_CONFIG_0, - GPIO_MEM_CONFIG_1, - GPIO_MEM_CONFIG_2, - GPIO_MEM_CONFIG_3, - }; - - return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); -} - void mainboard_memory_init_params(FSPM_UPD *memupd) { struct cnl_mb_cfg memcfg; diff --git a/src/mainboard/google/hatch/variants/akemi/include/variant/gpio.h b/src/mainboard/google/hatch/variants/akemi/include/variant/gpio.h index b257589..5d69eed 100644 --- a/src/mainboard/google/hatch/variants/akemi/include/variant/gpio.h +++ b/src/mainboard/google/hatch/variants/akemi/include/variant/gpio.h @@ -18,10 +18,4 @@
#include <baseboard/gpio.h>
-/* Memory configuration board straps */ -#define GPIO_MEM_CONFIG_0 GPP_H19 -#define GPIO_MEM_CONFIG_1 GPP_H22 -#define GPIO_MEM_CONFIG_2 GPP_F10 -#define GPIO_MEM_CONFIG_3 GPP_F3 - #endif diff --git a/src/mainboard/google/hatch/variants/baseboard/memory.c b/src/mainboard/google/hatch/variants/baseboard/memory.c index bcfc49f..ada20a8 100644 --- a/src/mainboard/google/hatch/variants/baseboard/memory.c +++ b/src/mainboard/google/hatch/variants/baseboard/memory.c @@ -15,9 +15,28 @@
#include <baseboard/variants.h> #include <baseboard/gpio.h> +#include <gpio.h> #include <soc/cnl_memcfg_init.h> #include <string.h>
+/* Default memory configuration board straps */ +#define GPIO_MEM_CONFIG_0 GPP_H19 +#define GPIO_MEM_CONFIG_1 GPP_H22 +#define GPIO_MEM_CONFIG_2 GPP_F10 +#define GPIO_MEM_CONFIG_3 GPP_F3 + +int __weak variant_memory_sku(void) +{ + const gpio_t spd_gpios[] = { + GPIO_MEM_CONFIG_0, + GPIO_MEM_CONFIG_1, + GPIO_MEM_CONFIG_2, + GPIO_MEM_CONFIG_3, + }; + + return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); +} + static const struct cnl_mb_cfg baseboard_memcfg = { /* Baseboard uses 121, 81 and 100 rcomp resistors */ .rcomp_resistor = {121, 81, 100}, diff --git a/src/mainboard/google/hatch/variants/dratini/include/variant/gpio.h b/src/mainboard/google/hatch/variants/dratini/include/variant/gpio.h index 92f9d41..d99e2bb 100644 --- a/src/mainboard/google/hatch/variants/dratini/include/variant/gpio.h +++ b/src/mainboard/google/hatch/variants/dratini/include/variant/gpio.h @@ -18,10 +18,4 @@
#include <baseboard/gpio.h>
-/* Memory configuration board straps */ -#define GPIO_MEM_CONFIG_0 GPP_H19 -#define GPIO_MEM_CONFIG_1 GPP_H22 -#define GPIO_MEM_CONFIG_2 GPP_F10 -#define GPIO_MEM_CONFIG_3 GPP_F3 - #endif diff --git a/src/mainboard/google/hatch/variants/hatch/Makefile.inc b/src/mainboard/google/hatch/variants/hatch/Makefile.inc index a990b5a..47ca158 100644 --- a/src/mainboard/google/hatch/variants/hatch/Makefile.inc +++ b/src/mainboard/google/hatch/variants/hatch/Makefile.inc @@ -19,5 +19,7 @@ SPD_SOURCES += 16G_2400 # 0b100 SPD_SOURCES += 16G_2666 # 0b101
+romstage-y += memory.c + ramstage-y += gpio.c bootblock-y += gpio.c diff --git a/src/mainboard/google/hatch/variants/hatch/include/variant/gpio.h b/src/mainboard/google/hatch/variants/hatch/include/variant/gpio.h index e7d8a75..5d69eed 100644 --- a/src/mainboard/google/hatch/variants/hatch/include/variant/gpio.h +++ b/src/mainboard/google/hatch/variants/hatch/include/variant/gpio.h @@ -18,10 +18,4 @@
#include <baseboard/gpio.h>
-/* Memory configuration board straps */ -#define GPIO_MEM_CONFIG_0 GPP_F20 -#define GPIO_MEM_CONFIG_1 GPP_F21 -#define GPIO_MEM_CONFIG_2 GPP_F11 -#define GPIO_MEM_CONFIG_3 GPP_F22 - #endif diff --git a/src/mainboard/google/hatch/variants/hatch/memory.c b/src/mainboard/google/hatch/variants/hatch/memory.c new file mode 100644 index 0000000..2712b24 --- /dev/null +++ b/src/mainboard/google/hatch/variants/hatch/memory.c @@ -0,0 +1,36 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2018 Google LLC + * + * 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 <gpio.h> + +/* Memory configuration board straps */ +#define GPIO_MEM_CONFIG_0 GPP_F20 +#define GPIO_MEM_CONFIG_1 GPP_F21 +#define GPIO_MEM_CONFIG_2 GPP_F11 +#define GPIO_MEM_CONFIG_3 GPP_F22 + +int variant_memory_sku(void) +{ + const gpio_t spd_gpios[] = { + GPIO_MEM_CONFIG_0, + GPIO_MEM_CONFIG_1, + GPIO_MEM_CONFIG_2, + GPIO_MEM_CONFIG_3, + }; + + return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); +} diff --git a/src/mainboard/google/hatch/variants/helios/include/variant/gpio.h b/src/mainboard/google/hatch/variants/helios/include/variant/gpio.h index 92f9d41..d99e2bb 100644 --- a/src/mainboard/google/hatch/variants/helios/include/variant/gpio.h +++ b/src/mainboard/google/hatch/variants/helios/include/variant/gpio.h @@ -18,10 +18,4 @@
#include <baseboard/gpio.h>
-/* Memory configuration board straps */ -#define GPIO_MEM_CONFIG_0 GPP_H19 -#define GPIO_MEM_CONFIG_1 GPP_H22 -#define GPIO_MEM_CONFIG_2 GPP_F10 -#define GPIO_MEM_CONFIG_3 GPP_F3 - #endif diff --git a/src/mainboard/google/hatch/variants/kindred/include/variant/gpio.h b/src/mainboard/google/hatch/variants/kindred/include/variant/gpio.h index 92f9d41..d99e2bb 100644 --- a/src/mainboard/google/hatch/variants/kindred/include/variant/gpio.h +++ b/src/mainboard/google/hatch/variants/kindred/include/variant/gpio.h @@ -18,10 +18,4 @@
#include <baseboard/gpio.h>
-/* Memory configuration board straps */ -#define GPIO_MEM_CONFIG_0 GPP_H19 -#define GPIO_MEM_CONFIG_1 GPP_H22 -#define GPIO_MEM_CONFIG_2 GPP_F10 -#define GPIO_MEM_CONFIG_3 GPP_F3 - #endif diff --git a/src/mainboard/google/hatch/variants/kohaku/include/variant/gpio.h b/src/mainboard/google/hatch/variants/kohaku/include/variant/gpio.h index 29e5904..d99e2bb 100644 --- a/src/mainboard/google/hatch/variants/kohaku/include/variant/gpio.h +++ b/src/mainboard/google/hatch/variants/kohaku/include/variant/gpio.h @@ -18,10 +18,4 @@
#include <baseboard/gpio.h>
-/* Memory configuration board straps */ -#define GPIO_MEM_CONFIG_0 GPP_F20 -#define GPIO_MEM_CONFIG_1 GPP_F21 -#define GPIO_MEM_CONFIG_2 GPP_F11 -#define GPIO_MEM_CONFIG_3 GPP_F22 - #endif diff --git a/src/mainboard/google/hatch/variants/kohaku/memory.c b/src/mainboard/google/hatch/variants/kohaku/memory.c index 4901247..de33ead 100644 --- a/src/mainboard/google/hatch/variants/kohaku/memory.c +++ b/src/mainboard/google/hatch/variants/kohaku/memory.c @@ -15,9 +15,28 @@
#include <baseboard/variants.h> #include <baseboard/gpio.h> +#include <gpio.h> #include <soc/cnl_memcfg_init.h> #include <string.h>
+/* Memory configuration board straps */ +#define GPIO_MEM_CONFIG_0 GPP_F20 +#define GPIO_MEM_CONFIG_1 GPP_F21 +#define GPIO_MEM_CONFIG_2 GPP_F11 +#define GPIO_MEM_CONFIG_3 GPP_F22 + +int variant_memory_sku(void) +{ + const gpio_t spd_gpios[] = { + GPIO_MEM_CONFIG_0, + GPIO_MEM_CONFIG_1, + GPIO_MEM_CONFIG_2, + GPIO_MEM_CONFIG_3, + }; + + return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); +} + static const struct cnl_mb_cfg baseboard_memcfg = { /* * The dqs_map arrays map the SoC pins to the lpddr3 pins
Edward O'Callaghan has removed Martin Roth from this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Removed reviewer Martin Roth.
Edward O'Callaghan has removed Patrick Georgi from this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Removed reviewer Patrick Georgi.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG@9 PS1, Line 9: unreasonably I would say the assumptions are breaking down from original implementation: memory down and memory strapping pins vs dimms. Does variant_memory_sku() even make sense in the dimm platforms? I would argue not. Are you following up in making it not used in that case? You were already splitting up the spd-in-cbfs case vs spd-on-dimms.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG@9 PS1, Line 9: unreasonably
I would say the assumptions are breaking down from original implementation: memory down and memory s […]
Yep that's fine as a better reword to clarify that 'unreasonable' means the assumption changed here.
This patch is to exercise out the ideas set out by Furquan in https://review.coreboot.org/c/coreboot/+/36250 where djkurtz and I discussed locally to flesh it out for comparison. Talking with djkurtz, he asked me to take the journey and see which solution looks best in the end.
It is my feeling that variant_memory_sku() doesn't make sense at all for seated dimms nor mainboard_get_dram_part_num() in the same file leaving only mainboard_memory_init_params(). However in the case of mainboard_memory_init_params() the actual implementation is different enough that they may as well be separate functions in separate object files depending on use-case rather than splitting control-flow with the pre-processor involvement. The benefit of doing the split is that it is less invasive as well.
Hope this add some extra context why this got forked off as a off-shot to the side.
Daniel Kurtz has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG@9 PS1, Line 9: unreasonably
Yep that's fine as a better reword to clarify that 'unreasonable' means the assumption changed here. […]
The request from me was to follow furquan's advice on CL:36250 on how to implement a single romstage.c that handles both strapping pins and dimms.
This CL is a first attempt at handling the "GPIO straps" problem as suggested by furquan - to move variant_memory_sku() into variants/baseboard/ which provides default GPIO straps".
Can you please follow up with CLs that add SMBUS support on top to see what the result looks like?
https://review.coreboot.org/c/coreboot/+/36492/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/36492/1/src/mainboard/google/hatch/... PS1, Line 22: #define GPIO_MEM_CONFIG_0 GPP_H19 This breaks helios b/c it defines its own variant_memory_sku which overrides the weak in baseboard and relies on these #defines.
Hello Daniel Kurtz, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36492
to look at the new patch set (#2).
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
mainboard/google: Move variant_memory_sku() to baseboard/
The variant_memory_sku() symbol within romstage.c depends on pre-processor #define's in variants that may not define them. Weaken this assumption by providing a default symbol and configuration within the basebase in the form of a weak symbol and allow the two variants that differ to override using a strong symbol.
In the case of variants that do not need this symbol they can override with a strong symbol that is a nop.
BRANCH=none BUG=b:143134702 TEST=./util/abuild/abuild -p none -t google/hatch -x -a
Change-Id: I41532e3067a989db5776887fac0459a000d07ff0 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M src/mainboard/google/hatch/romstage.c M src/mainboard/google/hatch/variants/akemi/include/variant/gpio.h M src/mainboard/google/hatch/variants/baseboard/memory.c M src/mainboard/google/hatch/variants/dratini/include/variant/gpio.h M src/mainboard/google/hatch/variants/hatch/Makefile.inc M src/mainboard/google/hatch/variants/hatch/include/variant/gpio.h A src/mainboard/google/hatch/variants/hatch/memory.c M src/mainboard/google/hatch/variants/kindred/include/variant/gpio.h M src/mainboard/google/hatch/variants/kohaku/include/variant/gpio.h M src/mainboard/google/hatch/variants/kohaku/memory.c 10 files changed, 76 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/36492/2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG@9 PS1, Line 9: unreasonably
The request from me was to follow furquan's advice on CL:36250 on how to implement a single romstage […]
Edward, makes sense. I was trying to point how I was reading the various efforts and wording. Technically speaking, I think we're on the same page.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... PS2, Line 39: mem_sku = variant_memory_sku(); When it is a nop, what value would this return so that things continue to work in both memory down and dimms? In your other patch where we were reading the spd contents ourselves, I think that's ok and likely better. Along w/ my first question I'm wondering about this is_single_ch_mem discovery and if that makes sense. I think I may like the other approach, fwiw, as there's lots of specifics that become odd in comparing hatch to puff -- largely this memory topology.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... PS2, Line 39: mem_sku = variant_memory_sku();
When it is a nop, what value would this return so that things continue to work in both memory down and dimms?
If we decide to share the same code for SPDs and DIMMs, then it would be better to reorganize the function such that the paths that do not matter for one type of memory are not taken at all by the other path.
Thus, this function should look like:
void mainboard_memory_init_params(FSPM_UPD *memupd) { is_single_ch_mem = gpio_get(GPIO_MEM_CH_SEL); variant_memory_params(&memcfg);
if (CONFIG(...SMBUS)) memory_init_params_smbus(&memcfg, is_single_ch_mem); else memory_init_params_spd(&memcfg, is_single_ch_mem); }
Along w/ my first question I'm wondering about this is_single_ch_mem discovery and if that makes sense.
I see the single channel GPIO on Puff, but I am really not sure if that is a valid case.
I think I may like the other approach, fwiw, as there's lots of specifics that become odd in comparing hatch to puff -- largely this memory topology.
That sounds okay to me. If the overall changes diverge too much, it would be better to just separate them out as Edward did in his other CL.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36492/1//COMMIT_MSG@9 PS1, Line 9: unreasonably
Edward, makes sense. I was trying to point how I was reading the various efforts and wording. […]
Np just my usual poor grammar from dyslexia don't read too deeply into my wording however thanks for the feedback Aaron.
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... PS2, Line 39: mem_sku = variant_memory_sku();
When it is a nop, what value would this return so that things continue to work in both memory down […]
I think we are all reaching the same conclusion here.
In regards to the case of `variant_memory_sku()` being a nop - in the follow up patch I moved it to be close to the call site. I did that in a separate patch to deal with cornering it out. I am not sure 100% about the is_single_ch_mem discovery logic is actually needed for Puff, although the schematic does mention the strap so I will look further. I have to fill in the gap in my understanding of whats right to do there however it seems simple enough.
Ultimately I think it best to drop this route of exploration and focus on the simple slice down the middle of having a romstage.c that is appropriate for down and seated dimms respectively. My reasoning is that:
- it is super easy to do, - makes progress with puff, - is easy to reason about without pre-processor and linker tricks and, - I think most importantly the least invasive way forwards.
We pick a path and compile in the appropriate object file that provides the correct worker function, specifically - `void mainboard_memory_init_params(FSPM_UPD *memupd)`, fixes to one path then do not run the risk of breaking the other so easily.
At the very core of this I think it comes down to where to bifurcate, be it at the source/runtime level or at the build time level. In essence it would be my view to slice and dice with the kconfig knob at build time. Apologies in advance if I didn't explain myself well, hopefully it all makes sense?
However, thank you for taking the time to look over both solutions!
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/c/coreboot/+/36492/2/src/mainboard/google/hatch/... PS2, Line 39: mem_sku = variant_memory_sku();
I think we are all reaching the same conclusion here.
In regards to the case of `variant_memory_sku()` being a nop - in the follow up patch I moved it to be close to the call site. I did that in a separate patch to deal with cornering it out. I am not sure 100% about the is_single_ch_mem discovery logic is actually needed for Puff, although the schematic does mention the strap so I will look further. I have to fill in the gap in my understanding of whats right to do there however it seems simple enough.
I don't think single channel needs/requires a gpio if the memory subsystem is using dimms. That can be inferred from the presence of the dimms.
Ultimately I think it best to drop this route of exploration and focus on the simple slice down the middle of having a romstage.c that is appropriate for down and seated dimms respectively. My reasoning is that:
Are you abandoning this patchset then?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 2: Code-Review+1
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
Are you abandoning this patchset then?
Well I prefer to go the https://review.coreboot.org/c/coreboot/+/36250 route for the reasons stated above, I think that is what we decided here or have I miss understood? Regardless, I just need a decision so we can keep things moving forwards.
Either; i) we do this refactor, ii) just move romstage's, iii) just create a new board.
I have patches for all these paths up so I just need an actual decision (as in +2's less obviously any technical adjustments) so we can proceed.
Edward O'Callaghan has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36492 )
Change subject: mainboard/google: Move variant_memory_sku() to baseboard/ ......................................................................
Abandoned
resolved now.