Skip to content

Fixing a bug in reverse lookup of ID to enum - #125

Merged
glguida merged 1 commit into
aifoundry-org:masterfrom
rvs:firmware-flash-tool
May 7, 2026
Merged

glguida merged 1 commit into
aifoundry-org:masterfrom
rvs:firmware-flash-tool

Conversation

@rvs

@rvs rvs commented Apr 5, 2026

Copy link
Copy Markdown
Member

When I was trying to replace SP_BL2 in the image I've noticed that SP_BL2 resolves to numeric ID 0x9 in the enum at firmware-flash-tool/include/esperanto_flash_image.h:18. The replace path then matches regions by numeric region_id, not by the printed string, at firmware-flash-tool/src/esperanto_flash_image_replace.c:90.

The bad output comes from region_id_to_name(), which assumes the lookup table is in enum order, but the table has PMIC_FW_S0 and PMIC_FW_S1 inserted before SP_BL1/SP_BL2 at firmware-flash-tool/src/parse_template_file.c:31. So ID 0x9 gets mislabeled as PMIC_FW_S1 when printed at firmware-flash-tool/src/parse_template_file.c:71.

Arguably this can be maintained at the cost of precise ordering, but it seems that a quick scan of the table is a better option

@rvs
rvs requested review from AFOliveira and glguida April 5, 2026 17:56

@vidas vidas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AFOliveira

Copy link
Copy Markdown
Member

Code-wise looks good to me, but I still didn't get to get hands-on with FW on the ET, neither do I have a board to test the change on, so I'll refrain from the approval and let Vidas and Gianluca properly approve it.

@glguida glguida left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is something that I noticed back when I was playing with it.

Thanks for the patch!

@glguida
glguida merged commit 85a5585 into aifoundry-org:master May 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants