On Wed, 27 Jan 2016 03:12:11 +0200 Urja Rannikko urjaman@gmail.com wrote:
Hi,
Review-ish... with some only-commentary included too.
On Mon, Jan 18, 2016 at 1:28 AM, Stefan Tauner stefan.tauner@alumni.tuwien.ac.at wrote:
Signed-off-by: Urja Rannikko urjaman@gmail.com Signed-off-by: Stefan Tauner stefan.tauner@alumni.tuwien.ac.at
Makefile | 23 ++- ch341a_spi.c | 531 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ flashrom.8.tmpl | 12 +- flashrom.c | 12 ++ programmer.h | 13 ++ 5 files changed, 588 insertions(+), 3 deletions(-) create mode 100644 ch341a_spi.c
free_idx = (free_idx + 1) % USB_IN_TRANSFERS; /* Increment (and wrap around). */Yeah this obvious construct is something I didnt use because I'm such an embedded guy that I'd never ever use % in performance related code... (i think i had some if idx>=USB_IN_TRANSFERS) idx = 0 thing)... thanks for making it saner.
:)
I had a fancier delay implementation done (all the way upto ms scale using the I2C commands), but since i didnt get that fully tested, this one will do, i'll post it as an improvement later.
Please do. I don't deem it that important but since you have investigated the whole issue in such detail it would be a waste to not exploit this knowledge.
unsigned int i;for (i = 0; i < readcnt; i++) {/* FIXME: does working on words instead of bytes improve speed? */... drop the comment? i dont really care, but i think our consensus on IRC was that a words-wide implementation would have more issues (alignment and such) than performance. Or make a comment with that info.
It's gone.
*readarr++ = swap_byte(rbuf[writecnt + i]);}return 0;+}
+static const struct spi_master spi_master_ch341a_spi = {
.type = SPI_CONTROLLER_CH341A_SPI,/* TODO: flashrom's current maximum is 256 B. Device was tested on Linux to accept atleast 16 kB. */.max_data_read = 16 * 1024,.max_data_write = 16 * 1024,On linux. Not on windows usb "stack" i think... set these back to 1k? Or test for the limit (maybe something like 4k?) and change the 1000ms timeout, since i think the big packet would timeout on windows if nothing else failed...
As discussed, I have tested up to 128 kB on Windows (in VM) and on Linux with a bigger timeout and it works fine. We have agreed upon 4 kB chunks now and using VLAs (I had an intermediate version that used bigger chunks and relied on mallocs for them).
With atleast the limits dropped (or tested), this is: Acked-by: Urja Rannikko urjaman@gmail.com
Thanks, r1921.