Attention is currently required from: Thomas Heijligen, Angel Pons, qianfan, Anastasia Klimchuk.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 )
Change subject: ch347_spi.c: Add initial support for the WCH CH347 ......................................................................
Patch Set 3:
(2 comments)
File ch347_spi.c:
https://review.coreboot.org/c/flashrom/+/70573/comment/fc1405ae_3ad1f03b PS3, Line 69: free(data);
Do you have to free this? `ch341a_spi` does not, but `ft2232_spi` does.
Both my code and `ft2232_spi` allocate memory for the data passed around in flashctx, so I would think that memory should be freed later. `ch341a_spi` doesn't free anything because it doesn't pass around data in flashctx and thus there isn't anything to free in the first place.
https://review.coreboot.org/c/flashrom/+/70573/comment/ec846349_ebf3cf86 PS3, Line 261: 64 * 1024
Where does this come from?
With the CH347, you can basically read an unlimited amount of bytes with one command. The read command accepts a 32 bit unsigned value representing the number of bytes that requested, and then you just continually issue USB_BULK_IN transfers until it has returned that number of bytes. I just chose some decently large number (it's the same as what the ft2232 chooses), and chose the same value for writes (for writes, you just continually issue write commands with a little over 500 bytes of payload data each until you've sent out all the bits you need) 64k is probably a bit overkill though for writes, as for SPI flash I think the most number of bytes would be a page program, which generally seem to be 256 bytes/page)
-
Nicholas Chin (Code Review)