-
Notifications
You must be signed in to change notification settings - Fork 46
Add support for SPI and CS mode commands #15
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
base: main
Are you sure you want to change the base?
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@neuschaefer thanks for your review! I addressed the mentioned points. Please have a look again :) |
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