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

Add possibility to setup Dma and max_transfer_size #76

Merged
merged 3 commits into from
May 26, 2022

Conversation

pyaillet
Copy link
Contributor

Following the previous PRs, here is a proposition to enable configuration of max_transfer_size and Dma activation.

However, I am not satisfied with the content, because as said before if we use transfer_size > 64 without Dma it panics :\

Do you think max_transfer_size should be silently lowered when Dma is not used ?
Any idea of another to notify the user he's doing something wrong ?
Would some documentation on the method be enough ?

@pyaillet pyaillet changed the title add possibility to setup Dma and max_transfer_size Add possibility to setup Dma and max_transfer_size May 19, 2022
@ivmarkov
Copy link
Collaborator

Following the previous PRs, here is a proposition to enable configuration of max_transfer_size and Dma activation.

However, I am not satisfied with the content, because as said before if we use transfer_size > 64 without Dma it panics :\

Do you think max_transfer_size should be silently lowered when Dma is not used ? Any idea of another to notify the user he's doing something wrong ? Would some documentation on the method be enough ?

I would move max_transfer_size as a parameter of the Dma enum. I.e. all values except Disabled should have it. I would also implement a max_transfer_size() const fn method on the enum that returns 64 (or however the current value for non-dma transfer is calculated) for Dma::Disabled and min(max_transfer_size, 1440(?)) for all dma-enabled transfers in the enum. I would also do some checks in this method for when dma is enabled, like whether the number is divisible by 4, > 0 etc. and either round it down, or panic with a good message.

@pyaillet
Copy link
Contributor Author

Thank you for the suggestions!

I applied most of them, though:

  • I set the upper limit to 4096 (according to this) and confirmed it worked for me
  • It seems min can't be called from a const fn

@ivmarkov ivmarkov merged commit 37d09e7 into esp-rs:master May 26, 2022
@pyaillet pyaillet mentioned this pull request Jun 13, 2022
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.

None yet

2 participants