ch341a: split oversized SPI streams to stay inside the USB timeout - #2
Merged
Merged
Conversation
A bulk transfer only drains as fast as the CH341A clocks out the SPI stream, measured at ~60 kB/s of payload on a 16 MB XT25F128B. snor_read() reads a whole sector per call, so on any chip with 64 kB sectors the driver submitted a single 67683-byte OUT transfer that needed ~1.05 s to complete and tripped the 1000 ms USB_TIMEOUT: cb_out: error: LIBUSB_TRANSFER_TIMED_OUT ch341a_spi_send_command: Failed to write 67683 bytes Being only ~7% over the deadline, this failed on the very first chunk and made those chips unreadable. Measured read times before the fix: 4 kB 0.08 s, 16 kB 0.28 s, 32 kB 0.54 s, 48 kB 0.80 s, 64 kB timeout at 1.02 s. Cap the payload per USB transaction at 16 kB and loop. CS is asserted by the caller and held across transactions, so the flash still sees one uninterrupted read and the sub-transactions cost nothing measurable: a full 16 MB read takes 270 s, matching flashrom on the same part, and the dump is byte-identical to two flashrom reference reads. Also scale the transfer deadline with the payload rather than using a flat 1000 ms, assuming a pessimistic 8 kB/s floor so slow hubs and clone programmers get headroom instead of a hard cliff. Finally, snor_read() signalled read errors by assigning -1 to its unsigned long len, which wrapped and printed a nonsense progress line: Read 100% [18446744073692774399] of [18446744073709551615] bytes Use a separate flag and report the address the read actually failed at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
|
Merged — thanks for the excellent PR! The diagnosis and fix are really solid. I especially appreciate the thorough testing with byte-identical verification against two independent flashrom reads. Clean, surgical change. |
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.
Problem
Reading a 16 MB XT25F128B fails immediately and every time:
snor_read()reads one whole sector perSPI_CONTROLLER_Read_NByte()call. For any chip with 64 kB sectors that becomes a single 65536-byte SPI transaction, whichch341a_spi_send_command()submits as one 67683-byte bulk OUT transfer under a flat 1000 msUSB_TIMEOUT.A bulk transfer only drains as fast as the CH341A clocks out the SPI stream. Measured on this part, that is ~63 kB/s of payload and it is dead linear:
So a 64 kB transaction needs ~1.05 s against a 1000 ms deadline — about 7% over the cliff, which is why it fails deterministically on the very first chunk rather than intermittently. flashrom reads the same chip on the same programmer without trouble because it never hands the driver a chunk this large.
Fix
enable_pins()) and held across transactions, so the flash still sees one uninterrupted sequential read — the split is invisible below the USB layer. Write bytes are drained first so reads always trail the command they belong to.snor_read()signalled failure by assigning-1to itsunsigned long len, which wrapped toULONG_MAXand printedRead 100% [18446744073692774399] of [18446744073709551615] bytes. It now uses a separate flag and reports the address the read actually failed at.Testing
Hardware: CH341A rev 3.0.4, XT25F128B (16 MB), libusb 1.0.29.
md5 ad9b48d89e0f217b3ec96160245fe018) to two independentflashrom -p ch341a_spi -c XT25F128B -rreference reads, which are themselves identical to each other.-ichip ID detection and other small transfers unaffected (they stay under the cap and take the unchanged single-transaction path).Only the CH341A transport and the NOR read error path are touched; EEPROM, microwire and NAND paths are unchanged and all issue transfers far below the cap.
🤖 Generated with Claude Code