Hi Nico,
Am 18.06.2012 11:30 schrieb Nico Huber:
Second try for this patch.
This enables native AAI transfer support in the dediprog driver. The function to write chunks of data, dediprog_spi_bulk_write(), is reused. To tell the programmer how to handle the data on the spi bus, a flag in the fourth byte sent with the usb command is used. The second word was mistaken for the size of the chunks sent over usb earlier. The third byte (first of the second word) is now set to zero. This also adds some checks for the size of data chunks sent over usb.
Signed-off-by: Nico Huber nico.huber@secunet.com
Looks very good, just a small question:
Index: dediprog.c
--- dediprog.c (Revision 1545) +++ dediprog.c (Arbeitskopie) @@ -321,14 +326,20 @@ * chunksize is the real data size per USB bulk transfer. The remaining * space in a USB bulk transfer must be filled with 0xff padding. */
- const unsigned int chunksize = flash->page_size; const unsigned int count = len / chunksize;
- const char count_and_chunk[] = {count & 0xff,
(count >> 8) & 0xff,chunksize & 0xff,(chunksize >> 8) & 0xff};
const char count_and_cmd[] = {count & 0xff, (count >> 8) & 0xff, 0x00, dedi_spi_cmd}; char usbbuf[512];
if (chunksize != 256) {
msg_perr("%s: Chunk sizes other than 256 bytes are unsupported, chunksize=%u!\n""Please report a bug at flashrom@flashrom.org\n", __func__, chunksize);return 1;}
if (chunksize > 512) {
msg_perr("%s: Maximum chunk size is 512 bytes, chunksize=%u!\n""Please report a bug at flashrom@flashrom.org\n", __func__, chunksize);}
Some compilers might complain about unreachable code in the second if clause. I think I see what you tried to do here (allow removal of the first if clause once we know how to handle different chunk sizes), but I'd rather have the second if clause replaced by a comment above the first if clause. AFAICS code behaviour won't change, and we get documentation for the 512 byte limit.
What do you think?
Regards, Carl-Daniel