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

Ownership based SD-RAM #95

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Ownership based SD-RAM #95

wants to merge 3 commits into from

Conversation

oli-obk
Copy link
Member

@oli-obk oli-obk commented Apr 9, 2019

@phil-opp what do you think? is this sort of handling ok?

@phil-opp
Copy link
Contributor

phil-opp commented Apr 9, 2019

From a quick look, this seems like a good improvement to me!

The use of [volatile::Volatile<u8>] might be a bit problematic for use cases where multiple bytes should be accessed at once. But at least it allows the user to use the remaining SDRAM capacity, so it's definitely an improvement.

Instead of taking and returning ownership of the sdram, we could introduce a SdramMemory wrapper type and borrow it. But that's just a small cosmetic suggestion.

@oli-obk
Copy link
Member Author

oli-obk commented Apr 9, 2019

Instead of taking and returning ownership of the sdram, we could introduce a SdramMemory wrapper type and borrow it. But that's just a small cosmetic suggestion.

I was considering that, but that would give all the users an additional lifetime. Alternatively we can give the memory to an allocator and see how well liballoc supports custom allocators

@phil-opp
Copy link
Contributor

phil-opp commented Apr 9, 2019

Ah, didn't think of that. How about something like this?

see how well liballoc supports custom allocators

I think this is not ready yet. AFAIK only raw_vec supports custom allocators right now.

@oli-obk
Copy link
Member Author

oli-obk commented Aug 28, 2019

Updated to be less hacky.

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