31 comments:
Patch Set #40, Line 11: chrome
Chromium OS?
Done
Patch Set #40, Line 18: the coreboot
Remove the article.
Done
Patch Set #40, Line 19: partition(either BP1(RO) or BP2)
Please add a space after *partition* before (.
Done
File src/soc/intel/common/block/cse/cse_bp.c:
Patch Set #38, Line 103: force_cse_init
What is the use of this variable? […]
cse_init_bp_info() call is must in every API to intialize boot partition info. It helps initializing the boot_partition_info when any boot partition info API is called.
Patch Set #38, Line 105: cse_bp_init
Can we just not use the "total_number_of_bp" field in cse_boot_partition_info to check if the partit […]
Agreed. Implemented the same
if (!cse_init_bp_info(true)) {
printk(BIOS_ERR, "Failed to update boot partition info\n");
return false;
}
just call the function to update the partition info, check comment above.
Please see my first comment
Patch Set #38, Line 192: cse_init_bp_info
check comment on cse_init_bp_info()
Please see my first comment.
Patch Set #38, Line 201: cse_init_bp_info
check comment on cse_init_bp_info()
Please see my first comment.
Patch Set #38, Line 210: cse_init_bp_info
check comment on cse_init_bp_info()
Please see my first comment.
Patch Set #38, Line 221: cse_init_bp_info
check comment on cse_init_bp_info()
Please see my first comment.
Patch Set #38, Line 232: cse_init_bp_info
check comment on cse_init_bp_info()
Please see my first comment.
Patch Set #38, Line 304: cse_init_bp_info
check comment on cse_init_bp_info()
Please see my first comment.
File src/soc/intel/common/block/cse/cse_bp.c:
Please add a space before.
Done
Patch Set #40, Line 47: uint8_t
Why not `unsigned int`?
The structure definition is align with the response buffer of get_boot_partition_info. Hence, data types of the struct members have been defined accordingly
Use CB_SUCCESS and friends?
bool type seems to be ok here.
Patch Set #40, Line 89: printk(BIOS_ERR, "Get partition info resp failed:%d\n", info_resp.hdr.result);
Please add a space after the colon.
Done
Remove the blank line.
Done
failed
Done
Patch Set #40, Line 135: Command(BP%d)
Please add a space before (.
Done
Patch Set #40, Line 146: printk(BIOS_ERR, "Set Boot Partion Info Response Failed:%d\n",
Ditto.
Done
Number
Done
Patch Set #40, Line 308: =0x%x
Please add a space.
Done
File src/soc/intel/common/block/include/intelblocks/cse.h:
Patch Set #40, Line 172: Enabled
lowercase
Done
Patch Set #40, Line 174: * So, CSE Image contains 3 partitions when redundancy is enabled
Period/dot at the end?
Done
Remove blank line.
Done
Patch Set #40, Line 183: * and it's a valid bootable partition.
Fits on one line?
Done
Patch Set #40, Line 195: * not a bootable partition on it's own.
One line?
Done
Remove blank line.
Done
Might this be available in other parts of coreboot already?
I think No!
Patch Set #40, Line 274: partition(BP1
Space.
Done
File src/soc/intel/common/block/include/intelblocks/cse.h:
Patch Set #25, Line 155: * Otherwise, it contains only 2 partitions(BP1+BP3)
Thanks. Alas, I still don't get it. Below it says BP3 is loaded by BP2. […]
I agree with your inputs, and in line with comments mentioned for enum definitions of Boot partition ids, I have modified comment for CSE_MAX_BOOT_PARTITIONS macro.
To view, visit change 35402. To unsubscribe, or for help writing mail filters, visit settings.