Add support for SPI and CS mode commands#15
Open
funkeleinhorn wants to merge 5 commits intostacksmashing:mainfrom
Open
Add support for SPI and CS mode commands#15funkeleinhorn wants to merge 5 commits intostacksmashing:mainfrom
funkeleinhorn wants to merge 5 commits intostacksmashing:mainfrom
Conversation
This has the advantage that the pico-serprog can stay attached to the board even while a CPU etc. runs on the board and uses the flash.
neuschaefer
suggested changes
Apr 14, 2024
Contributor
neuschaefer
left a comment
There was a problem hiding this comment.
Hi, thanks for your PR!
It looks good overall, but I left some more specific comments.
I reviewed the top three commits (which are not in the multiple-cs branch).
| } | ||
| } | ||
| fflush(stdout); | ||
| } |
Contributor
There was a problem hiding this comment.
This is suboptimal performance-wise, because of the lack of chunking.
I gave optimization a try and came up with the follow (untested!), but it's not very concise:
void spi_full_duplex(const pio_spi_inst_t *spi, uint32_t wlen, uint32_t rlen) {
size_t len = MAX(wlen, rlen);
uint8_t buffer[128];
putchar(S_ACK);
while (wlen || rlen) {
size_t len = MIN(rlen, wlen);
size_t chunk_size = MIN(sizeof(buffer), len);
memset(buffer, 0, chunk_size);
if (wlen) {
size_t chunk = MIN(wlen, chunk_size);
fread(buffer, 1, chunk, stdin);
wlen -= chunk;
}
pio_spi_write8_read8_blocking(spi, buffer, buffer, chunk_size);
if (rlen) {
size_t chunk = MIN(rlen, chunk_size);
fwrite(buffer, 1, chunk, stdout);
rlen -= chunk;
}
}
fflush(stdout);
}
Author
There was a problem hiding this comment.
I added this suggested change but was not able to test it yet with hardware.
Flashprog recently added support for multiple chip selects to serprog: https://review.sourcearcade.org/plugins/gitiles/flashprog/+/ddb6d926783d4f9cbee04c7392718ed8f89daa0e This commit adds support for this feature to pico-serprog. This is useful if you want to drive multiple SPI devices on one bus with pico-serprog as it eliminates the need for reflashing the pico or changing cables.
This serprog spec addition added support for selecting the SPI mode for SPI_OP commands: https://review.coreboot.org/c/flashrom/+/81428 This commit adds support for the 0x17 S_SPI_MODE command.
This serprog spec addition added support for selecting the CS mode for SPI_OP commands: https://review.coreboot.org/c/flashrom/+/81428 This commit adds support for the 0x18 S_CS_MODE command.
Author
|
@neuschaefer thanks for your review! I addressed the mentioned points. Please have a look again :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Flashrom and Flashprog recently added support for a command to select the SPI mode between half and full duplex SPI and a command to manually control the CS. This PR adds support for these commands. They allow more applications than just interacting with flashes over serprog as for example programming AVR microcontrollers which can be tried with my fork of avrdude. This PR is based on #13