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.

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

RFC: Add statically checked macro for safe struct Peripherals creation #105

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mbuesch
Copy link
Contributor

@mbuesch mbuesch commented Aug 7, 2022

The Peripherals::take() function has two disadvantages:

  • It uses one byte of precious RAM for runtime memory safety checking.
  • It uses a couple of bytes of precious program memory for establishing a critical section.

This change uses the linker to statically ensure that only one Peripherals instance is constructed globally.
It introduces a new macro to avr_device that is used just like:

let dp = avr_device::peripherals!(atmega328p);

In case of multiple macro invocations, the linker will abort with

multiple definition of `__ERROR__avr_device__peripherals__macro_must_only_be_used_once__'

TODO:

  • For actually not wasting the one byte of memory, the DEVICE_PERIPHERALS access has to be removed from the generated Peripherals::steal() function.
  • In order to be sound, all Peripherals::take() function have to be removed from the generated code.

@mbuesch
Copy link
Contributor Author

mbuesch commented Aug 8, 2022

As we need support from the svd2rust tool to properly implement this, the macro should probably also be moved into the generated code.
The svd2rust could get a new option to select between static and dynamic peripherals struct checking. avr-device could then always request generation of the static macro.

@mbuesch mbuesch force-pushed the peripherals_static_check branch from 61fe2ac to 6a73d56 Compare August 9, 2022 13:29
@Rahix
Copy link
Owner

Rahix commented Jan 5, 2023

Hi, sorry, I completely lost track of this. I agree with you, I think this should be discussed with the svd2rust project and integrated there.

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.

2 participants