Furquan Shaikh 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:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... File src/mainboard/google/dedede/romstage.c:
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 25: extra tab not required.
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 26: same here.
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 117: RAM_STRAP_0 These straps should be configured in gpio_table above as well.
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledoo/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 16: empty So, ID 0 is empty on waddledoo? Why are we leaving empty slots. We should start indexing from 0. This comment is more from our board hardware design standpoint.
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 23: /* Access memory info through CBFS */ Comment seems misplaced.