Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add OpenTitan Earl Grey ROM_EXT board to KNOWN_BOARDS #116

Merged
merged 2 commits into from
Jul 23, 2024
Merged

feat: Add OpenTitan Earl Grey ROM_EXT board to KNOWN_BOARDS #116

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b04068
feat: Add opentitan_earlgrey_rom_ext board to KNOWN_BOARDS
pqcfox Jul 19, 2024
8fb60c9
fix: Set max_size to fill remainder of 512kB eFlash
pqcfox Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions tockloader/board_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,17 @@ class BoardInterface:
"max_size": 0x00100000,
},
},
"opentitan_earlgrey_rom_ext": {
"description": "OpenTitan EarlGrey chip with ROM_EXT targeting the Earlgrey-M2.5.2-RC0 hardware",
"arch": "rv32imc",
"page_size": 512,
"no_attribute_table": True,
"address_translator": lambda addr: addr - 0x20010000,
"flash_file": {
# Set to the maximum flash size.
"max_size": 0x00100000,
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to adjust the flash-file max size to the size that's actually available for Tockloader to use now? Assuming I added this correctly for opentitan_earlgrey, this should probably be 0x000F0000 now?

This makes sure that Tockloader errors out when it would overrun the actual device flash for the image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, thank you so much. Let me double check the flash map to confirm, then will amend the commit!

Copy link
Contributor Author

@pqcfox pqcfox Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Just finished my check, it looks like from this issue the eFlash has a maximum storage of 512kB (-> 0x00080000) at 256 pages and 2 banks. I just looked at top_earlgrey.hjson in the OpenTitan repo which confirms that the page and bank numbers are still current.

From that, it sounds like 0x00070000 would be the right size--I'll tentatively push a commit based on that, but if there's any details I'm missing let me know and I'll amend right away :)

Edit, the PR I was referencing was out of date, my dearest apologies! Added max_size as 0x000F0000 in the commit below.

Copy link
Member

Choose a reason for hiding this comment

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

I think 0x00070000 makes sense in this case!

The impact of setting this value too small is that you might get a spurious error, and setting it too large might cause Tockloader to produce a huge image file (which happened to me in development once, hence introducing this flag 😅) and producing an image that overflow's the actual device flash. Neither's a subtle bug, so this should be a fairly low-risk (hah!) change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point! I'll go ahead and set it low for now, thanks so much!

},
},
}

def __init__(self, args):
Expand Down
Loading