Meera Ravindranath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39136 )
Change subject: mb/google/dedede: Add memory initialization support for dedede ......................................................................
Patch Set 18:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39136/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/romstage.c:
https://review.coreboot.org/c/coreboot/+/39136/5/src/mainboard/google/dedede... PS5, Line 4: * Copyright (C) 2020 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.
Please refer to 4.11 release notes - https://doc.coreboot.org/releases/coreboot-4.11-relnotes. […]
Ack
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 23: ;
Should we have a default config here instead of NULL. […]
Ack. A default config would mean the memory initialization code to die again. How about a simple die function?
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledoo/memory.c:
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 28: 0xff, 0x0
As per the available documentation, 0xff corresponds to CS signal used for all the 8 bytes in a pack […]
There is no 1:1 mapping between the board value we are passing and the schematics of Waddledoo. The values we are passing are to the FSP UPD structure. Here is a snippet of the value to be filled according to FSP.
///< DQByteMap[2] - CtlDQByteMap : Always program to [0xFF, 0] since we have 1 CTL / rank ///< Variable only exists to make the code easier to use