-
Notifications
You must be signed in to change notification settings - Fork 146
Replace old BLOCK_SIZES with definitions of the blocks themselves
#971
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
I think for the v5 release we should carefully reconsider the efuse API - e.g. I agree the After #969 it's probably good enough for espflash's internal use but not for consumption by users |
bd3a56d to
9754275
Compare
9754275 to
24425e0
Compare
xtask/Cargo.toml
Outdated
| clap = { version = "4.5", features = ["derive"] } | ||
| env_logger = "0.11" | ||
| log = "0.4" | ||
| pyo3 = { version = "0.27.1", features = ["auto-initialize"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have this behind a feature? This way we would only need python3 when regenerating the efuses, but the rest of the xtask commands can be run without it. We could add an alias, just like in https://github.com/esp-rs/esp-hal/blob/cd8addbedde320c9352020b2dde142e7e83c2aa7/.cargo/config.toml#L4 so its easier too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like this? 03f86c8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! Thanks!
|
Hmm, that's kind of problematic. Ubuntu has only had Is 20.04 a requirement or can we switch to 24.04? Or should we install the dependencies using pip instead? Oh wait, I'm silly, with the conditional dep change we don't need it in CI for now. |
585dcc7 to
e51c60a
Compare
The actual block definitions only exist within Python classes so this commit uses PyO3 to import the esptool.py eFuse block definitions and generates arrays of the length and read address of each eFuse block. We need the actual read address here rather than using the old offset method because not all eFuse blocks are sequential. (On ESP32 the block 0 read registers are followed by the block 0 write registers.)
This prevents Python being necessary to build the xtask binary for other purposes.
e51c60a to
f367a32
Compare
The actual block definitions only exist within Python classes so this commit uses PyO3 to import the esptool.py eFuse block definitions and generates arrays of the length and read address of each eFuse block.
We need the actual read address here rather than using the old offset method because not all eFuse blocks are sequential. (On ESP32 the block 0 read registers are followed by the block 0 write registers.)
TODO:
Should we deprecate the
block_sizemethod?It's unfortunately a public method, but it doesn't seem to me like there's any safe way for API consumers to use it for anything.
On the other hand, that kind of also applies to
efuse_regandblock0_offset. To use them safely you have to have pretty good knowledge of the register layout per-chip, and we don't expose that right now.Should we also replace
efuse_regwith ones generated from theDR_REG_EFUSE_BASEdefinitions inesptool.py?I decided to leave
blockandEfuseBlockprivate for now. It might make sense to eventually make them public, but it's much harder to make any necessary changes later if we make them public.I'll wait for #969 to be merged and rebase on top of that before undrafting this.