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 Jasper Lake RVP ......................................................................
Patch Set 23:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39195/21/src/mainboard/intel/jasper... File src/mainboard/intel/jasperlake_rvp/board_id.c:
https://review.coreboot.org/c/coreboot/+/39195/21/src/mainboard/intel/jasper... PS21, Line 33: /*
trailing whitespace
Done
https://review.coreboot.org/c/coreboot/+/39195/21/src/mainboard/intel/jasper... PS21, Line 34: * Get Board ID via EC I/O port write/read
trailing whitespace
Done
https://review.coreboot.org/c/coreboot/+/39195/22/src/mainboard/intel/jasper... File src/mainboard/intel/jasperlake_rvp/board_id.c:
https://review.coreboot.org/c/coreboot/+/39195/22/src/mainboard/intel/jasper... PS22, Line 50: id = (buffer[0] << 8) | buffer[1];
id = (buffer[0] << 8) | buffer[1] & 0x1F? if 5 bits only apply for EC_ACPI
5 bit will be applied for chrome-EC too. We had this check for other platforms too.
https://review.coreboot.org/c/coreboot/+/39195/23/src/mainboard/intel/jasper... File src/mainboard/intel/jasperlake_rvp/board_id.c:
https://review.coreboot.org/c/coreboot/+/39195/23/src/mainboard/intel/jasper... PS23, Line 55: 1F
0x1f
Done
https://review.coreboot.org/c/coreboot/+/39195/23/src/mainboard/intel/jasper... File src/mainboard/intel/jasperlake_rvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/39195/23/src/mainboard/intel/jasper... PS23, Line 43:
remove
Ack
https://review.coreboot.org/c/coreboot/+/39195/23/src/mainboard/intel/jasper... PS23, Line 44: } else { : die("Unsupported Board id : 0x%x\n", board_id); : }
isn't this check is duplicate between line #25 above and https://review.coreboot. […]
Ack