-
Notifications
You must be signed in to change notification settings - Fork 139
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
Adds PPI support and example #162
Conversation
nrf-hal-common/src/ppi.rs
Outdated
regs.ch[P::CH].tep.write(|w| unsafe { w.bits(addr) }); | ||
} | ||
|
||
fn set_event_endpoint(&mut self, addr: u32) { |
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.
The manual says that these registers "accept only addresses to registers from the Event/Task group". Does that mean that writing an incorrect address has no effect, or will that cause trouble?
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.
Hmm, I just tried here on hardware, I used another timer's CC0 register (TIMER1) to see if it would write something to it or hardfault, but nothing happened, no write nor fault, so I guess it just doesn't do anything ?
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 also tried with a RAM address now, same thing.
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.
It might be really tedious, but we could define an extension trait for all of the events and tasks, it would also let us get rid of the awkward &p.RADIO.tasks_disable as *const _ as u32
dance.
Awesome! I am keen to try this out! Unfortunately I have little to no experience with the PPI, so I can't really judge about the API ... |
Co-authored-by: Jonas Schievink <[email protected]>
Regarding the "extension trait" idea, here's a quick look for the nrf52832-pac: Events:
|
Let me know what you think of https://github.com/thalesfragoso/nrf-hal/pull/1/files, if you like it, I can do all the target gating to get the correct tasks/events for each chip. Right now it (probably) only works on the nrf52832. |
Looks good! |
* Add extension trait for tasks and events * Easier to maintain structure * Implement other crates, fix regex a bit * That's gross, but it works * Apply suggestions from code review Co-authored-by: Thales <[email protected]> * Remove nrf9160 impls * More inline always Co-authored-by: Thales <[email protected]>
This abstraction allows for the usage of PPI channels independently.
I would like an opinion on the
enable
method, I think that it might have the potential to break an hypothetical abstraction, but probably won't cause any memory safety problems, should we mark it asunsafe
?