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

Return the number of blocks completely read when querying read completion #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MichaelBell
Copy link
Contributor

Especially when doing a longer async transfer, it can be useful to start reading it before it has finished.

This change allows the user to read the number of blocks already read by checking how far the chain DMA has got.

It's a breaking change, but I don't think there are many users yet, and you can just use NULL if you don't care.

@kilograham
Copy link
Contributor

nice, yes i had planned such a feature! will take a look

@@ -828,6 +828,10 @@ bool sd_scatter_read_complete(int *status) {
}
check_crc_count = 0;
}
else if (blocks_complete)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there should be a code-comment that blocks_complete will only be set when sd_scatter_read_complete returns false? 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, I would've added a comment to the API if the API had any comments at all 🤣

@@ -828,6 +828,10 @@ bool sd_scatter_read_complete(int *status) {
}
check_crc_count = 0;
}
else if (blocks_complete)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brace-error 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do normally try to fit in with surrounding code's style, but I'm very much used to braces being on their own line! Will try to remember.

@MichaelBell
Copy link
Contributor Author

I've just realised that sd_readblocks_scatter_async is an externally exposed function, and if you use it then this won't work.

It should record the control words pointer passed into scatter_async and compare with that. I'll fix that and update the PR, hopefully today.

@kilograham
Copy link
Contributor

I've just realised that sd_readblocks_scatter_async is an externally exposed function, and if you use it then this won't work.

It should record the control words pointer passed into scatter_async and compare with that. I'll fix that and update the PR, hopefully today.

yeah, the API is obviously not currenlty ideal :-)

@MichaelBell
Copy link
Contributor Author

I'm going to switch to use the scatter read function directly in my video code.

So, really I just want a call to get the number of control words that have been consumed. But that's not a particularly user friendly interface if you are using the normal methods.

Options:

  1. Keep this as is, document it as not working if you call scatter directly.
  2. Record the chain control pointer used for the last read, if it matches then do as at present. If it doesn't, return the number of chain entries consumed, as the caller might be doing something different that 2 chain entries per block.
  3. Always return the number of chain entries consumed, document that when using the friendly read functions the number of sectors completed is equal to half the value returned.

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