Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29749 )
Change subject: mb/google/dragonegg: Add initial mainboard code support ......................................................................
Patch Set 9:
(3 comments)
https://review.coreboot.org/#/c/29749/9/src/mainboard/google/dragonegg/dsdt.... File src/mainboard/google/dragonegg/dsdt.asl:
https://review.coreboot.org/#/c/29749/9/src/mainboard/google/dragonegg/dsdt.... PS9, Line 22: 0x05
the revision is 0x02
Although there is no harm to declare this value as 0x5 :) but still i will make this as 0x02 as you have asked.
here is the recommendation from spec
"A revision field value greater than or equal to 2 signifies that integers declared within the Definition Block are to be evaluated as 64 bit values"
https://review.coreboot.org/#/c/29749/9/src/mainboard/google/dragonegg/romst... File src/mainboard/google/dragonegg/romstage_fsp_params.c:
https://review.coreboot.org/#/c/29749/9/src/mainboard/google/dragonegg/romst... PS9, Line 23: void mainboard_memory_init_params(FSPM_UPD *mupd)
Can this file be removed, since it contains 1 empty function only?
We can't remove this file because we might have some overrides as epatch which will apply over this file during chromium build.
https://review.coreboot.org/#/c/29749/9/src/mainboard/google/dragonegg/spd/e... File src/mainboard/google/dragonegg/spd/empty.spd.hex:
https://review.coreboot.org/#/c/29749/9/src/mainboard/google/dragonegg/spd/e... PS9, Line 1: 00
Why this empty spd file? Remove it if not used
make sense