Skip to content

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

funkeleinhorn
Copy link

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

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.
Copy link
Contributor

@neuschaefer neuschaefer left a 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);
}
Copy link
Contributor

@neuschaefer neuschaefer Apr 14, 2024

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);
}

Copy link
Author

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.
@funkeleinhorn
Copy link
Author

@neuschaefer thanks for your review! I addressed the mentioned points. Please have a look again :)

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