Re: [flashrom] [PATCH] Enable Dediprog SF100 programmers with FW 5.5.1

[Pablo Cases]

Playing around with non "Chip Erasers" (e.g. Block and Sector Erasers) uncovered a bug and a few problems in the previous patch submission:

Fairly trivial:
* Block and Sector Erase USB commands were not modified appropriately (writecnt != 1 for these commands). This is easily patched.

Major problem(?):
* Partial writes to the chip does not work with dediprog SF100 FW 5.5.1. The reason is that the start address supplied to dediprog through dediprog_spi_bulk_write() is not honored by dediprog. Writes will always start at address 0x0.  
As side note; the Dediprog Software application, which actually has functionality for partial writes, will actually not perform a partial write in its true meaning. The Dediprog Software will prepend relevant parts of the existing chip data to every partial write and then start writing from address 0x0.

* Partial reads are also broken as the start address supplied to dediprog through dediprog_spi_bulk_read() is not honored either. This error is more subtle though; when validating the erase status dediprog will always supply the chip data with start at address 0x0, not at the specified start address. The result is that only the first block is checked for successful erasing.

I'm not really sure on how to overcome these problems with the dediprog programmer...

Regards,
Pablo
> -----Original Message-----
> From: Pablo Cases
> Sent: Thursday, July 31, 2014 1:51 PM
> To: '[e-mail]'
> Subject: [PATCH] Enable Dediprog SF100 programmers with FW 5.5.1
>
> This patch enables Dediprog SF100 programmers with FW 5.5.1 to work
> properly with flashrom.
>
> The following functionality has been fixed and verified on a W25Q64.V flash
> chip. All fixes relate to changes in the USB  commands issued to the SF100
> programmer:
> chip discovery (flashrom could not read out the ManufacturerID and
>
> DeviceID fields properly, got parts of the devicestring instead)
> chip data readout (SF100 issued SPI Fast Read from address 0xF30000
>
> instead of 0x0)
> chip data write (SF100 issued SPI Page Program to address 0xF30000 instead
>
> of 0x0)
> chip erase (verification failed as a consequence of that chip data readout
>
> was problematic)
> SF100 LED management (LEDs could be turned on but not turned off)
>
> Signed-off-by: Pablo Cases <pablo.cases at flatfrog.com>
> Index: dediprog.c
>
> =========
> --- dediprog.c	(revision 1832)
> +++ dediprog.c	(working copy)
> @@ -101,8 +101,13 @@
>    	target_leds = leds;
>    }
> ret = usb_control_msg(dediprog_handle, 0x42, 0x07, 0x09,
>
> target_leds,
> if (dediprog_firmwareversion >= FIRMWARE_VERSION(5,5,1))
> ret = usb_control_msg(dediprog_handle, 0x42, 0x07,
>
> (target_leds << 8)
> +| 0x09, 0x0,
>    		      NULL, 0x0, DEFAULT_TIMEOUT);
> else
> ret = usb_control_msg(dediprog_handle, 0x42, 0x07, 0x09,
>
> target_leds,
> 	      NULL, 0x0, DEFAULT_TIMEOUT);
> if (ret != 0x0) {
>   msg_perr("Command Set LED 0x%x failed (%s)!\n",
>   	 leds, usb_strerror());
>
> @@ -216,8 +221,22 @@
>    const char count_and_chunk[] = {count & 0xff,
>    				(count >> 8) & 0xff,
>    				chunksize & 0xff,
> 			(chunksize >> 8) & 0xff};
> 			(chunksize >> 8) & 0xff,
> 			0xFF, 0,0,0,0,0};
>    unsigned int count_and_chunk_size;
>
>    /* Reverse engineering note:
>     * ========================
> * Omitting the {0xFF, 0,0,0,0,0} appendage of SF100 FW 5.5.1
>     * results in SF100 to incorrectly issue 0x0B (Fast Read) command with
>     * start at SPI address 0xF30000 instead of the expected address 0x0.
>     * (On a 8MiB W25Q64.V chip this wraps around to physical address
>
> 0x730000)
>     */
>    if (dediprog_firmwareversion >= FIRMWARE_VERSION(5,5,1))
>            count_and_chunk_size = 10;
>    else
>            count_and_chunk_size = 4;
> if ((start % chunksize) || (len % chunksize)) {
>   msg_perr("%s: Unaligned start=%i, len=%i! Please report a
>
> bug "
>    		 "at [e-mail]\n", __func__, start, len);
> @@ -232,8 +251,8 @@
>     */
>    ret = usb_control_msg(dediprog_handle, 0x42, 0x20, start %
> 0x10000,
>    		      start / 0x10000, (char *)count_and_chunk,
> 	      sizeof(count_and_chunk), DEFAULT_TIMEOUT);
> if (ret != sizeof(count_and_chunk)) {
> 	      count_and_chunk_size, DEFAULT_TIMEOUT);
> if (ret != count_and_chunk_size) {
> msg_perr("Command Read SPI Bulk failed, %i %s!\n", ret,
> 	 usb_strerror());
> return 1;
>
> @@ -316,9 +335,27 @@
>     * space in a USB bulk transfer must be filled with 0xff padding.
>     */
>    const unsigned int count = len / chunksize;
> const char count_and_cmd[] = {count & 0xff, (count >> 8) & 0xff,
>
> 0x00, dedi_spi_cmd};
> const char count_and_cmd[] = {count & 0xff, (count >> 8) & 0xff,
>
> 0x00, dedi_spi_cmd, 0xFF, 0,0,0,0,0};
> unsigned int count_and_cmd_size;
> char usbbuf[512];
> /* Reverse engineering note:
> * ========================
> * Omitting the {0xFF, 0,0,0,0,0} appendage of SF100 FW 5.5.1
> * results in SF100 to incorrectly issue 0x02 (Page Program) command
>
> with
> * start at SPI address 0xF30000 instead of the expected address 0x0.
> * (On a 8MiB W25Q64.V chip this wraps around to physical address
>
> 0x730000)
> * Issuing a single spi_bulk_write() command with this appendage will
> * correct the addressing so that all further spi_bulk_write()
>
> commands
> * (with or without the appendage) will start at SPI address 0x0.
> * Only a  power cycle of the SF100 will reset the behavior
> * (i.e. reintroduce the error).
> */
> if (dediprog_firmwareversion >= FIRMWARE_VERSION(5,5,1))
> count_and_cmd_size = 10;
> else
> count_and_cmd_size = 4;
> /*
> We should change this check to
> chunksize > 512
>
> @@ -343,8 +380,8 @@
>     * SPI side.
>     */
>    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)) {
> 	      (char *)count_and_cmd, count_and_cmd_size,
>
> DEFAULT_TIMEOUT);
> if (ret != count_and_cmd_size) {
> msg_perr("Command Write SPI Bulk failed, %i %s!\n", ret,
> 	 usb_strerror());
> return 1;
>
> @@ -447,10 +484,29 @@
>    	msg_perr("Untested readcnt=%i, aborting.\n", readcnt);
>    	return 1;
>    }
> ret = usb_control_msg(dediprog_handle, 0x42, 0x1, 0xff,
> 	      readcnt ? 0x1 : 0x0, (char *)writearr, writecnt,
> 	      DEFAULT_TIMEOUT);
> if (dediprog_firmwareversion >= FIRMWARE_VERSION(5,5,1))
> if (writecnt == 1 &&
> 	(*writearr == 0x6 ||			 // Write
>
> Enable
> 	*writearr == 0x60 || *writearr == 0xc7 ||// Chip
>
> Erase
> 	*writearr == 0x52 || *writearr == 0xd8 ||// Block
>
> Erase
> 	*writearr == 0x20))			 // Sector
>
> Erase
> 	/* These commands are treated differently
>
> otherwise
> 	 * Read Status Register-1 (05h) will return incorrect
> 	 * results for WEL and WIP bits
> 	 * (at least on Winbond W25Q64BV Flash)
> 	 */
> 	ret = usb_control_msg(dediprog_handle, 0x42, 0x1,
>
> 0x0,
>                             0x0, (char *)writearr, writecnt,
>                             DEFAULT_TIMEOUT);
> else
> 	ret = usb_control_msg(dediprog_handle, 0x42, 0x1,
>
> 0x1,
>                             0x0, (char *)writearr, writecnt,
> 	         DEFAULT_TIMEOUT);
> else
> ret = usb_control_msg(dediprog_handle, 0x42, 0x1, 0xff,
> 	         readcnt ? 0x1 : 0x0, (char *)writearr, writecnt,
> 	         DEFAULT_TIMEOUT);
> if (ret != writecnt) {
> msg_perr("Send SPI failed, expected %i, got %i %s!\n",
> 	 writecnt, ret, usb_strerror());
>
> @@ -459,8 +515,13 @@
>    if (!readcnt)
>    	return 0;
>    memset(readarr, 0, readcnt);
> ret = usb_control_msg(dediprog_handle, 0xc2, 0x01, 0xbb8, 0x0000,
> 	     (char *)readarr, readcnt, DEFAULT_TIMEOUT);
> if (dediprog_firmwareversion >= FIRMWARE_VERSION(5,5,1))
> ret = usb_control_msg(dediprog_handle, 0xc2, 0x1, 0x1, 0x0,
> 	         (char *)readarr, readcnt, DEFAULT_TIMEOUT);
> else
> ret = usb_control_msg(dediprog_handle, 0xc2, 0x1, 0xbb8,
>
> 0x0000,
> 	         (char *)readarr, readcnt, DEFAULT_TIMEOUT);
> if (ret != readcnt) {
> msg_perr("Receive SPI failed, expected %i, got %i %s!\n",
> 	 readcnt, ret, usb_strerror());
>
> @@ -485,6 +546,12 @@
>    		 " String!\n");
>    	return 1;
>    }
> /* Reverse engineering note:
> * ========================
> * Dediprog Software SF6.0.4.33 application sends the devicestring
> * command with wValue and wIndex values of 0x0 instead of 0xFF.
> * Both commands yield the same result, so nothing is changed for
>
> now.
>     */
> /* Command Receive Device String. */
>  memset(buf, 0, sizeof(buf));
>  ret = usb_control_msg(dediprog_handle, 0xc2, 0x8, 0xff, 0xff, buf,
>
> Pablo Cases, M.Sc.
> Senior Software Engineer
> FlatFrog Laboratories AB