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
Index: dediprog.c =================================================================== --- dediprog.c (Revision 1545) +++ dediprog.c (Arbeitskopie) @@ -31,6 +31,9 @@ static int dediprog_firmwareversion; static int dediprog_endpoint;
+#define DEDI_SPI_CMD_PAGEWRITE 0x1 +#define DEDI_SPI_CMD_AAIWRITE 0x4 + #if 0 /* Might be useful for other pieces of code as well. */ static void print_hex(void *buf, size_t len) @@ -307,13 +310,15 @@ return 0; }
-/* Bulk write interface, will write multiple page_size byte chunks aligned to page_size bytes. - * @start start address - * @len length - * @return 0 on success, 1 on failure +/* Bulk write interface, will write multiple chunksize byte chunks aligned to chunksize bytes. + * @chunksize length of data chunks, only 256 supported by now + * @start start address + * @len length + * @dedi_spi_cmd dediprog specific write command for spi bus + * @return 0 on success, 1 on failure */ -static int dediprog_spi_bulk_write(struct flashctx *flash, uint8_t *buf, - unsigned int start, unsigned int len) +static int dediprog_spi_bulk_write(struct flashctx *flash, uint8_t *buf, unsigned int chunksize, + unsigned int start, unsigned int len, uint8_t dedi_spi_cmd) { int ret; unsigned int i; @@ -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); + } + if ((start % chunksize) || (len % chunksize)) { msg_perr("%s: Unaligned start=%i, len=%i! Please report a bug " "at flashrom@flashrom.org\n", __func__, start, len); @@ -341,10 +352,9 @@ /* Command Write SPI Bulk. No idea which write command is used on the * SPI side. */ - ret = usb_control_msg(dediprog_handle, 0x42, 0x30, start % 0x10000, - start / 0x10000, (char *)count_and_chunk, - sizeof(count_and_chunk), DEFAULT_TIMEOUT); - if (ret != sizeof(count_and_chunk)) { + ret = usb_control_msg(dediprog_handle, 0x42, 0x30, start % 0x10000, start / 0x10000, + (char *)count_and_cmd, sizeof(count_and_cmd), DEFAULT_TIMEOUT); + if (ret != sizeof(count_and_cmd)) { msg_perr("Command Write SPI Bulk failed, %i %s!\n", ret, usb_strerror()); return 1; @@ -366,8 +376,8 @@ return 0; }
-static int dediprog_spi_write_256(struct flashctx *flash, uint8_t *buf, - unsigned int start, unsigned int len) +static int dediprog_spi_write(struct flashctx *flash, uint8_t *buf, + unsigned int start, unsigned int len, uint8_t dedi_spi_cmd) { int ret; const unsigned int chunksize = flash->page_size; @@ -376,6 +386,13 @@
dediprog_set_leds(PASS_OFF|BUSY_ON|ERROR_OFF);
+ if (chunksize != 256) { + msg_pdbg("Page sizes other than 256 bytes are unsupported as " + "we don't know how dediprog\nhandles them.\n"); + /* Write everything like it was residue. */ + residue = len; + } + if (residue) { msg_pdbg("Slow write for partial block from 0x%x, length 0x%x\n", start, residue); @@ -389,8 +406,7 @@
/* Round down. */ bulklen = (len - residue) / chunksize * chunksize; - ret = dediprog_spi_bulk_write(flash, buf + residue, start + residue, - bulklen); + ret = dediprog_spi_bulk_write(flash, buf + residue, chunksize, start + residue, bulklen, dedi_spi_cmd); if (ret) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); return ret; @@ -412,6 +428,16 @@ return 0; }
+static int dediprog_spi_write_256(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) +{ + return dediprog_spi_write(flash, buf, start, len, DEDI_SPI_CMD_PAGEWRITE); +} + +static int dediprog_spi_write_aai(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) +{ + return dediprog_spi_write(flash, buf, start, len, DEDI_SPI_CMD_AAIWRITE); +} + static int dediprog_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, @@ -717,7 +743,7 @@ .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, .write_256 = dediprog_spi_write_256, - .write_aai = default_spi_write_aai, + .write_aai = dediprog_spi_write_aai, };
static int dediprog_shutdown(void *data)
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
Hello Carl-Daniel,
Am 19.06.2012 08:44, schrieb Carl-Daniel Hailfinger:
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
Yes, that's what I had in mind.
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.
Sounds good to me.
Next revision:
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
Index: dediprog.c =================================================================== --- dediprog.c (Revision 1545) +++ dediprog.c (Arbeitskopie) @@ -31,6 +31,9 @@ static int dediprog_firmwareversion; static int dediprog_endpoint;
+#define DEDI_SPI_CMD_PAGEWRITE 0x1 +#define DEDI_SPI_CMD_AAIWRITE 0x4 + #if 0 /* Might be useful for other pieces of code as well. */ static void print_hex(void *buf, size_t len) @@ -307,13 +310,15 @@ return 0; }
-/* Bulk write interface, will write multiple page_size byte chunks aligned to page_size bytes. - * @start start address - * @len length - * @return 0 on success, 1 on failure +/* Bulk write interface, will write multiple chunksize byte chunks aligned to chunksize bytes. + * @chunksize length of data chunks, only 256 supported by now + * @start start address + * @len length + * @dedi_spi_cmd dediprog specific write command for spi bus + * @return 0 on success, 1 on failure */ -static int dediprog_spi_bulk_write(struct flashctx *flash, uint8_t *buf, - unsigned int start, unsigned int len) +static int dediprog_spi_bulk_write(struct flashctx *flash, uint8_t *buf, unsigned int chunksize, + unsigned int start, unsigned int len, uint8_t dedi_spi_cmd) { int ret; unsigned int i; @@ -321,14 +326,21 @@ * 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];
+ /* + * We should change this check to + * chunksize > 512 + * once we know how to handle different chunk sizes. + */ + 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 ((start % chunksize) || (len % chunksize)) { msg_perr("%s: Unaligned start=%i, len=%i! Please report a bug " "at flashrom@flashrom.org\n", __func__, start, len); @@ -341,10 +353,9 @@ /* Command Write SPI Bulk. No idea which write command is used on the * SPI side. */ - ret = usb_control_msg(dediprog_handle, 0x42, 0x30, start % 0x10000, - start / 0x10000, (char *)count_and_chunk, - sizeof(count_and_chunk), DEFAULT_TIMEOUT); - if (ret != sizeof(count_and_chunk)) { + ret = usb_control_msg(dediprog_handle, 0x42, 0x30, start % 0x10000, start / 0x10000, + (char *)count_and_cmd, sizeof(count_and_cmd), DEFAULT_TIMEOUT); + if (ret != sizeof(count_and_cmd)) { msg_perr("Command Write SPI Bulk failed, %i %s!\n", ret, usb_strerror()); return 1; @@ -366,8 +377,8 @@ return 0; }
-static int dediprog_spi_write_256(struct flashctx *flash, uint8_t *buf, - unsigned int start, unsigned int len) +static int dediprog_spi_write(struct flashctx *flash, uint8_t *buf, + unsigned int start, unsigned int len, uint8_t dedi_spi_cmd) { int ret; const unsigned int chunksize = flash->page_size; @@ -376,6 +387,13 @@
dediprog_set_leds(PASS_OFF|BUSY_ON|ERROR_OFF);
+ if (chunksize != 256) { + msg_pdbg("Page sizes other than 256 bytes are unsupported as " + "we don't know how dediprog\nhandles them.\n"); + /* Write everything like it was residue. */ + residue = len; + } + if (residue) { msg_pdbg("Slow write for partial block from 0x%x, length 0x%x\n", start, residue); @@ -389,8 +407,7 @@
/* Round down. */ bulklen = (len - residue) / chunksize * chunksize; - ret = dediprog_spi_bulk_write(flash, buf + residue, start + residue, - bulklen); + ret = dediprog_spi_bulk_write(flash, buf + residue, chunksize, start + residue, bulklen, dedi_spi_cmd); if (ret) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); return ret; @@ -412,6 +429,16 @@ return 0; }
+static int dediprog_spi_write_256(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) +{ + return dediprog_spi_write(flash, buf, start, len, DEDI_SPI_CMD_PAGEWRITE); +} + +static int dediprog_spi_write_aai(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) +{ + return dediprog_spi_write(flash, buf, start, len, DEDI_SPI_CMD_AAIWRITE); +} + static int dediprog_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, @@ -717,7 +744,7 @@ .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, .write_256 = dediprog_spi_write_256, - .write_aai = default_spi_write_aai, + .write_aai = dediprog_spi_write_aai, };
static int dediprog_shutdown(void *data)
Am 19.06.2012 09:22 schrieb Nico Huber:
Hello Carl-Daniel,
Am 19.06.2012 08:44, schrieb Carl-Daniel Hailfinger:
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
Yes, that's what I had in mind.
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.
Sounds good to me.
Next revision:
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
Thank you for the patch. Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net and committed in r1546.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Nico Huber