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

UART and TWIM: Implement large buffer splitting and check for readonly buffers in flash #50

Closed
wants to merge 14 commits into from

Conversation

simonsso
Copy link
Contributor

@simonsso simonsso commented Feb 5, 2019

Same type of changes as done for SPI.

I have not split up the dma transfer for: pub fn write_then_read()
it is not clear to me what assumptions can be made about read and write - for instance if a write and read always completes in sync or if all writes shall be done before data starts to be revived.

There have been no attempts to verify this change -- other than it is almost the same as I did for SPI.

@simonsso simonsso changed the title UART and TWIM: Implement large buffer splinting and check for readonly buffers in flash UART and TWIM: Implement large buffer splitting and check for readonly buffers in flash Feb 5, 2019
@simonsso
Copy link
Contributor Author

simonsso commented Mar 4, 2019

@jamesmunns / @hannobraun / @Yatekii Should we try to do something about this change?

This patch includes the fail-if-DMA is not in RAM check and replaces the 255 limit on easy DMA by splitting it up in several if there is a limit.

@hannobraun
Copy link
Contributor

Sorry, I don't have time to take a closer look right now. I've added it to my list.

nrf52-hal-common/src/twim.rs Outdated Show resolved Hide resolved
@Yatekii
Copy link
Contributor

Yatekii commented Mar 5, 2019

@simonsso I tried to speed up discussions about the implicit copy and batching.
We did also think about possible compiletime solutions.

Maybe it should be merged for now and adapted later on (even tho I think we'll never adapt it then :P).
If @jamesmunns and @hannobraun agree, I am ok with merging this for now if you fix the minor issue with bondary checks pointed out.

What do you think about this @jamesmunns @hannobraun? Appart from the ongoing issue about implicity, this seems to be sound otherwise (except for the minor issue pointed out by @somemetricprefix which can be fixed easily.).

nrf52-hal-common/src/twim.rs Outdated Show resolved Hide resolved
nrf52-hal-common/src/twim.rs Outdated Show resolved Hide resolved
@Yatekii Yatekii mentioned this pull request Mar 5, 2019
@simonsso
Copy link
Contributor Author

Close in favor of #74 and #75

@simonsso simonsso closed this Mar 14, 2019
hargoniX pushed a commit to hargoniX/nrf-hal that referenced this pull request Jul 28, 2021
50: Copy xtask version bumper from nrf-hal r=therealprof a=robyoung

Minor changes to accomodate the slightly different CHANGELOG format.

Co-authored-by: Rob Young <rob@robyoung.digital>
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.

3 participants