Skip to content

Allow unaligned input/output to SPI::transferBytes #5709

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

Merged
merged 5 commits into from
Feb 3, 2019

Conversation

earlephilhower
Copy link
Collaborator

Fixes #4967

Support any alignment of input and output pointers and transfer lengths
in SPI::transferBytes. Use 32-bit transfers and FIFO as much as
possible.

Fixes esp8266#4967

Support any alignment of input and output pointers and transfer lengths
in SPI::transferBytes.  Use 32-bit transfers and FIFO as much as
possible.
@earlephilhower earlephilhower changed the title Allow unaligned input/output to SPI::transferBytes WIP - Allow unaligned input/output to SPI::transferBytes Feb 1, 2019
@earlephilhower
Copy link
Collaborator Author

Code is done, but need to do some testing (thinking of a wrapper that makes aligned/misaligned buffers at random). Feel free to test on your own on non-critical data, of course.

@earlephilhower earlephilhower added this to the 2.5.0 milestone Feb 1, 2019
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

Can some of the code pieces be factored out? e.g.:
transmitMisaligned()
transmitAligned()
receiveMisaligned()
receiveAligned()

then implement something like:

void trasmit(out)
{
    if(out & 3)
        transmitMisaligned();
    else
        transmitAligned();
}

same for receive(in).

@earlephilhower
Copy link
Collaborator Author

I realized after testing that you can't do bytewise updates to the FIFO space because it's RmW. When you read, you're not reading what was writen by the core but the SPI HW read FIFO. So the original refactoring could not work.

Rewrote it with a small shim with handles alignment and then calls the existing transfer operation with aligned input and output buffers, and then handles unaligning output as needed.

The SPI FIFO can't properly do RMW (i.e. bytewise updates) because when
you read the FIFO you are actually reading the SPI read data, not what
was written into the write FIFO.

Refactor the transferBytes to take account of this.  For aligned input
and outputs, perform as before (but handle non-x4 sizes properly).  For
misaligned inputs, if it's unidirectional then do bytewise until the
direction data pointer is aligned and then do 32b accesses.  Fod
bidirectional and misaligned inputs, copy the output data to an aligned
buffer, do the transfer, then copy the read back data from temp aligned
buffer to the real input buffer.
@earlephilhower
Copy link
Collaborator Author

Testing with SDFS and forcing both misaligned read and write buffers as well as no-by-4 lengths, looks good in all testing I can throw at it.

@earlephilhower earlephilhower changed the title WIP - Allow unaligned input/output to SPI::transferBytes Allow unaligned input/output to SPI::transferBytes Feb 2, 2019
Add more comments and adjust naming to be more informative in
transferBytes_ and *aligned_.  Save 64bytes of stack in double
misaligned case.
On any misaligned input or output, always use a temp buffer.  No need
for special casing and bytewise ::transfer().  This should be faster as
bytewise ::transfer involves a significant number of IO register
accesses and setup.  Thanks to @devyte for the suggestion.
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.

2 participants