Attention is currently required from: Robert Zieba, Raul Rangel, Reka Norman, Nick Vaccaro. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62905 )
Change subject: util/spd_tools: Add support for exclusive IDs ......................................................................
Patch Set 3:
(4 comments)
File util/spd_tools/src/part_id_gen/part_id_gen.go:
https://review.coreboot.org/c/coreboot/+/62905/comment/0a79a238_5f5986e1 PS3, Line 96: mappingType Do we need a new type
https://review.coreboot.org/c/coreboot/+/62905/comment/5e131021_f4230eb5 PS3, Line 102: This can be:
const ( Auto = iota Fixed Exclusive )
https://review.coreboot.org/c/coreboot/+/62905/comment/9c3835fe_76f80c87 PS3, Line 284: map[int]mappingType{} Feels weird to use a map for the concerned use-case. Feels like an array is sufficient.
If planning to switch to array - extend the array along with partIdList as and when needed and initialize it as Auto.
https://review.coreboot.org/c/coreboot/+/62905/comment/df8e046d_bd5a1fca PS3, Line 314: if mapping, present := assignedMapping[p.index]; present { : // Don't allow explicit and non-explicit parts to use the same index : if mapping != p.mapping && (mapping == Exclusive || p.mapping == Exclusive) { : return nil, fmt.Errorf("Exclusive/non-exclusive conflict in assigning %s to ID %d", p.partName, p.index) : } : } else { : assignedMapping[p.index] = p.mapping : } if planning to switch assignedMapping as array, this can be changed as:
``` if (assignedMapping[p.index] == Exclusive && p.mapping == Fixed) || (assignedMapping[p.Index] == Fixed && p.mapping == Exclusive) { return nil, fmt.Errorf("...") } else { assignedMapping[p.Index] = p.mapping } ```