Attention is currently required from: Subrata Banik.
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
15 comments:
Patchset:
Looks good overall, just some mostly minor comments.
File util/ifdtool/ifdtool.c:
Patch Set #3, Line 47: CSE_REGION_DATA_PARTITION_OFFSET
This applies to BPDT v1.7 (TGL onwards) only, referring to util/cbfstool/bpdt_formats/{bpdt_1_6.c,bpdt_1_7.c}. Do we want to support v1.6 too? Or just say it's unsupported on platforms older than TGL for now?
Patch Set #3, Line 1556: uint32_t gprd
nit: make `union gprd_info` public and use it here instead of manual bit shifting?
Patch Set #3, Line 1610: fpsba->pchstrp[gpr0_offset] & 0x80000000
nit: use `union gprd_info` here too
FITC?
FPT?
for (size_t index = 1; index <= fpt->count; index++) {
struct cse_fpt_sub_part *fpt_sub_part = (struct cse_fpt_sub_part *)(fpt + index);
This is kind of a hack which only works because `struct cse_fpt` and `struct cse_fpt_sub_part` happen to both be 32 bytes.
Can you increment fpt by sizeof(struct cse_fpt) first, then start the index at 0?
Patch Set #3, Line 1641: &fpt_sub_part->signature[0]
nit: just `fpt_sub_part->signature` should work?
Fail if FITC is not found? Right now it will silently continue and fill in an incorrect range.
Patch Set #3, Line 1663: region.size <= 0xfff
Why 0xfff (not 0)?
Patch Set #3, Line 1712: fpsba->pchstrp[gpr0_offset] & 0x80000000
nit: use `union gprd_info` here too
Patch Set #3, Line 1724: union gprd_info
nit: just `union gprd`? This is more consistent with other places where we just use the register name.
Patch Set #3, Line 1726: start:16
I think bit 16 is actually read protect enable, referring to `union spi_bios_gpr0` in src/soc/intel/common/block/spi/spi.c.
/*
* Start Address: bit 0-15 of the GPRD represents the protected region start address,
* where bit 0-11 of the start address are assumed to be zero.
*/
nit: maybe move these comments to the struct definition, so we don't have to duplicate them here and in print_gpr0_range().
Patch Set #3, Line 1737: 0xffff
0x7fff, based on my comment about read protect enable.
To view, visit change 80235. To unsubscribe, or for help writing mail filters, visit settings.