Attention is currently required from: Bora Guvendik, Cliff Huang, Jamie Ryu, Jérémy Compostella, Saurabh Mishra, Wonkyu Kim.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83981?usp=email )
Change subject: soc/intel/common/gpio: support 16-bit CPU Port ID and vw mapping fix ......................................................................
Patch Set 4:
(3 comments)
File src/soc/intel/common/block/gpio/gpio.c:
https://review.coreboot.org/c/coreboot/+/83981/comment/44a99ec4_82da113a?usp... : PS4, Line 1063: if (comm->vw_map) { ```
/* Get the community (potentially updating 'comm' if vw_map exists) */ comm = gpio_get_community(pad);
/* Find the VW entry containing 'pad' */ for (i = 0; i < comm->num_vw_entries; i++) { if (pad >= comm->vw_entries[i].first_pad && pad <= comm->vw_entries[i].last_pad) break;
offset += 1 + comm->vw_entries[i].last_pad - comm->vw_entries[i].first_pad; }
/* Check if we found a valid entry */ if (i == comm->num_vw_entries) return false;
/* Adjust offset and calculate vw_index based on the mapping type */ if (comm->vw_map) { offset = pad - comm->vw_entries[i].first_pad; offset += comm->vw_map[i].start_pos; *vw_index = comm->vw_map[i].base + offset / 8; } else { offset += pad - comm->vw_entries[i].first_pad; offset += comm->vw_base; *vw_index = offset / 8; }
/* Calculate vw_bit */ *vw_bit = offset % 8; ```
https://review.coreboot.org/c/coreboot/+/83981/comment/631ed785_1e0d8c82?usp... : PS4, Line 1064: comm = gpio_get_community(pad); this is redundant? (line #1060 added the same)
https://review.coreboot.org/c/coreboot/+/83981/comment/b5cded38_d73a2399?usp... : PS4, Line 1071: } what will happen when the `if` clause is not meeting?
``` if (pad >= comm->vw_entries[i].first_pad && pad <= comm->vw_entries[i].last_pad) ``` This if statement checks if a given value `pad` falls within the range defined by the `first_pad` and `last_pad` within the GPIO community, current VW entry.
If a pad is within this range, the break statement immediately exits the loop (after adjusting the offset)
If not found, we are not doing anything with offset. I assume this `start_pos` is now added to mitigate the problem where VM offsets are not incremental ?