-
Notifications
You must be signed in to change notification settings - Fork 29
Implement support for 32-bit memory access scheme #39
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
Conversation
@usbalbin If you could test this on STM32H5 and confirm if this makes things work on H5 as well then that would be great. (Full disclosure: I have not looked at the STM32H5 refrence manual and have no idea if H5 USB implementation is in any way compatible with this driver. I just know from your issue that it uses a 32-bit memory access scheme.) |
I will try! |
Works great from what I can tell. Tested with this on a nucleo-h533 |
src/endpoint_memory.rs
Outdated
} else { | ||
self.mem[index * 2].set(value); | ||
} | ||
fn get_mem_slice<T>(&self) -> &mut [VolatileCell<T>] { |
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.
I believe this function is very unsafe because calling it twice within the same block is unsound.
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.
That's fair. I realized that I can return &[VolatileCell<T>]
, which relaxes the safety requirements quite a bit. I did also mark it as unsafe for posterity.
Looks good to me overall. I'm happy to approve the PR after changes to |
Thank you! |
Ok, CI is broken (fixed in #40), so merging the PR after running corresponding checks locally. |
Closes #37 and unblocks USB support for C0 and possibly some other STM32 families. Tested on STM32C071 devboard.
Note: I changed the UsbPeripheral trait a little, which unfortunately makes this a breaking change. In practice this should not be a big issue, since existing HAL-s can keep using the previous version pretty much indefinitely and the migration is very simple.