STM32 I2C size improvements, STM32G030 support #1540
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FixedMap contains a runtime capacity check that has, as far as I can tell, never fired. This means it doesn't need an explanatory error message -- file/line should be sufficient. Removing the message saves a good chunk of text.
The hl::recv facility is _almost_ dead, but the single user I can find (the I2C server) is quite popular, so we have to maintain it. Sigh. It's entirely possible for rm.message_len to be larger than the buffer: if the client sends an overly long message, the kernel will report that. So, this code has been a client-controlled server panic for ages. This switches it to check the message length when slicing, and transfer the panic to the client, where it belongs. This happens to remove a hundred bytes or so of text, by removing the setup and call to the slice index bounds fail routine.
Previously, sleep_for was using normal arithmetic to compute a deadline from the given number of ticks. We don't expect ticks to overflow in practice (it's 64-bit) but normal arithmetic checks for overflow. This means sleeping with a very very large interval would produce a panic. A panic from arithmetic overflow is somewhat more expensive than a no-message explicit panic, or an unwrap_lite. This commit changes the arithmetic to do checks explicitly and unwrap_lite on failure, saving a chunk of text.
task_slot checks its own build system behavior at runtime at a nontrivial cost, particularly in tasks with few panics. I'd argue this is unnecessary because 1. This has, as far as I can tell, never failed. 2. Should it fail, the kernel will fault us with a "task index out of range" fault, so it's not like it'll be silently ignored. Fixes #1538.
I'm pretty sure the "laps" counter variable couldn't overflow, but this was not obvious to the compiler, which was generating checks and panics. Restructuring the loop to use explicit iteration bounds fixes this. This knocks about 100 bytes off the I2C server.
I'm pretty sure the wiggles variable (a u8) could not overflow here, but the compiler is less convinced and was generating checks. Switched it to explicitly wrapping arithmetic for some text savings.
cbiffle
force-pushed
the
oxcon2023g0-i2c
branch
2 times, most recently
from
0e1437e to
ff8f4aa
Compare
mkeeter
reviewed
Sep 18, 2023
app/oxcon2023g0/app.toml
Outdated
| scl.pin = 11 | ||
| sda.pin = 10 | ||
| af = 6 | ||
|
|
There was a problem hiding this comment.
Nit: bonus whitespace here
mkeeter
approved these changes
Sep 18, 2023
This is the shotgun change required to teach all the various bits that g030 exists, plus adding it to the oxcon2023g0 demo app. The main functional change here: panic messages can now be disabled in the i2c server for specific targets. It'd be nice to have explicit control over this, instead of having it depend on the target, but the way things are currently structured that's not practical. With panic messages off we can fit the server in less than 4 kiB, making it practical on STM32G030.
cbiffle
force-pushed
the
oxcon2023g0-i2c
branch
from
ff8f4aa to
eabedcd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On the 0xCon demo board, I included a "standard" Sparkfun/Adafruit I2C connector. I later realized that we don't actually have I2C support working on the G030 for space reasons. (Last night, while I was attempting to sleep.) So this morning I've thoroughly beaten the i2c server with the "get smaller" stick, and have it fitting in under 4 kiB. (For comparison, on donglet, the next larger target, it takes 16 kiB.)
This has required turning panic messages off (code contains a lot of panics) and disabling ringbuf (code contains a lot of ringbuf). Among other things.
There appear to be more opportunities for flash reclamation in these crates -- as far as I can tell nobody's been keeping an eye on overflow panics or bounds check panics, which are generating a lot of goo. So, I may have follow-ups. TBD.