Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39195 )
Change subject: mb/intel/jasperlake_rvp: Add memory config for JSLRVP ......................................................................
Patch Set 10:
(13 comments)
https://review.coreboot.org/c/coreboot/+/39195/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39195/3//COMMIT_MSG@7 PS3, Line 7: mb/intel/jasperlake_rvp: memory config for JSLRVP
Please make it a statement by adding a verb (in imperative mood).
Done
https://review.coreboot.org/c/coreboot/+/39195/3//COMMIT_MSG@8 PS3, Line 8:
Please write a commit message.
Done
https://review.coreboot.org/c/coreboot/+/39195/3/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39195/3/src/mainboard/intel/jasperl... PS3, Line 30: {.spd_smbus_address[1] = 0xA0, : .spd_smbus_address[2] = 0xA2, : .spd_smbus_address[3] = 0xA4, : .spd_smbus_address[1] = 0xA6}
Please add appropriate spaces.
Ack
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... PS4, Line 41: };
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... PS4, Line 41: };
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... PS4, Line 45: if ( board_id == jsl_ddr4 )
space prohibited before that close parenthesis ')'
Done
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... PS4, Line 45: if ( board_id == jsl_ddr4 )
space prohibited after that open parenthesis '('
Done
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... PS4, Line 47: else if ( board_id == jsl_lpddr4 )
space prohibited before that close parenthesis ')'
Done
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... PS4, Line 47: else if ( board_id == jsl_lpddr4 )
space prohibited after that open parenthesis '('
Done
https://review.coreboot.org/c/coreboot/+/39195/4/src/mainboard/intel/jasperl... PS4, Line 50: die(" Unsupported Board id : 0x%x \n", board_id);
unnecessary whitespace before a quoted newline
Done
https://review.coreboot.org/c/coreboot/+/39195/5/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39195/5/src/mainboard/intel/jasperl... PS5, Line 50: die("Unsupported Board id : 0x%x \n", board_id);
unnecessary whitespace before a quoted newline
Done
https://review.coreboot.org/c/coreboot/+/39195/9/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39195/9/src/mainboard/intel/jasperl... PS9, Line 40: 1
4?
Ack
https://review.coreboot.org/c/coreboot/+/39195/3/src/mainboard/intel/jasperl... File src/mainboard/intel/jasperlake_rvp/variants/jslrvp/memory_ddr4.c:
https://review.coreboot.org/c/coreboot/+/39195/3/src/mainboard/intel/jasperl... PS3, Line 58: */
One line, and remove the dot/period at the end.
Done