Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35141 )
Change subject: mb/google/drallion: Add memory init setup for drallion ......................................................................
Patch Set 43:
(12 comments)
Please remember to mark comments as "resolved" once they're handled, otherwise the change isn't ready for submit.
https://review.coreboot.org/c/coreboot/+/35141/17/src/mainboard/google/drall... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35141/17/src/mainboard/google/drall... PS17, Line 48: struct cnl_mb_cfg memcfg;
We should guard CONFIG_DRALLION here.
Done
https://review.coreboot.org/c/coreboot/+/35141/19/src/mainboard/google/drall... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35141/19/src/mainboard/google/drall... PS19, Line 62: int mem_sku;
Done!
Done
https://review.coreboot.org/c/coreboot/+/35141/25/src/mainboard/google/drall... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35141/25/src/mainboard/google/drall... PS25, Line 29: static const struct cnl_mb_cfg memcfg = {
Remove static const here, because we want to override it.
Now done through the weak get_variant_memory_cfg() function.
https://review.coreboot.org/c/coreboot/+/35141/5/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35141/5/src/mainboard/google/dralli... PS5, Line 18: 3200
Let me confirm with Tim about exact speed, before I make further changes.
The entire scheme changed, so closing
https://review.coreboot.org/c/coreboot/+/35141/10/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35141/10/src/mainboard/google/drall... PS10, Line 17: SPD_SOURCES = 8G_2666 # 0b00000
We will update the SPD. We update this in another CL.
Done
https://review.coreboot.org/c/coreboot/+/35141/28/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/include/variant/variant.h:
https://review.coreboot.org/c/coreboot/+/35141/28/src/mainboard/google/drall... PS28, Line 28: #endif /* VARIANT_H */
you can drop this
Done
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/memory.c:
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... PS3, Line 49: int variant_memory_sku
No. This is fine.
Done
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... PS3, Line 69: switch(val) {
We will have SPD file for each memory. […]
Done
https://review.coreboot.org/c/coreboot/+/35141/10/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/memory.c:
https://review.coreboot.org/c/coreboot/+/35141/10/src/mainboard/google/drall... PS10, Line 59: int val = gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios));
I think this change is needn't. We will upload SPD for each within few days.
Done
https://review.coreboot.org/c/coreboot/+/35141/35/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/memory.c:
https://review.coreboot.org/c/coreboot/+/35141/35/src/mainboard/google/drall... PS35, Line 36: mem_cfg->rcomp_resistor = { 120, 81, 100 };
rcomp_resistor[]
Ack
https://review.coreboot.org/c/coreboot/+/35141/35/src/mainboard/google/drall... PS35, Line 37: mem_cfg->rcomp_targets = { 100, 40, 20, 20, 26 };
rcomp_targets[]
Ack
https://review.coreboot.org/c/coreboot/+/35141/35/src/mainboard/google/drall... PS35, Line 39: mem_cfg->.ect = 1;
mem_cfg->ect
Ack