Thejaswani Putta 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 5:
(8 comments)
https://review.coreboot.org/c/coreboot/+/35141/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35141/3//COMMIT_MSG@15 PS3, Line 15: 3. Include spd configuration(still need spd for 16G_3200.spd.hex to complete)
Please add a space before (.
Done
https://review.coreboot.org/c/coreboot/+/35141/3//COMMIT_MSG@15 PS3, Line 15: need
needs
Done
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... PS3, Line 21: /* Access memory info through SMBUS. */ : .spd[0] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa0}, : }, : .spd[1] = {.read_type = NOT_EXISTING}, : .spd[2] = { : .read_type = READ_SMBUS, : .spd_spec = {.spd_smbus_address = 0xa4}, : }, : .spd[3] = {.read_type = NOT_EXISTING},
We should change to read by CBFS, right?
Done
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... PS3, Line 20: ##
ToDo
Done
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... PS3, Line 21:
consistent spacing as above.
Done
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... PS3, Line 180: /* EMMC_DATA0 */ PAD_NC(GPP_F12, NONE), : /* EMMC_DATA1 */ PAD_NC(GPP_F13, NONE), : /* EMMC_DATA2 */ PAD_NC(GPP_F14, NONE), : /* EMMC_DATA3 */ PAD_NC(GPP_F15, NONE), : /* EMMC_DATA4 */ PAD_NC(GPP_F16, NONE),
change this rather.
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
sorry,memory. […]
So, There is no duplicate right. This function is defined only in memory.c file.
https://review.coreboot.org/c/coreboot/+/35141/3/src/mainboard/google/dralli... PS3, Line 70: case 17: : val = 0; : break; : case 18: : val = 2; : break; : case 19: : case 20: : val = 0; : break; : case 25: : val = 2; : break; : case 26: : val = 3; : break; : } case 27: : val = 2; : break; : case 28: : val = 1; : break; : }
club similar cases.
Done