Re: [flashrom] [PATCH] Add native AAI transfer support to the dediprog driver

[Carl-Daniel Hailfinger]

Hi Nico,

I have a few questions about AAI with the Dediprog SF100:
- Do you know how the completion of one AAI write command is detected?
The public SST25VF032B datasheet mentions three methods: "There are
three methods to determine completion of a program cycle during AAI Word
programming: hardware detection by reading the Serial Output, software
detection by polling the BUSY bit in the Software Status Register or
wait TBP (Byte Programming Time)."
- Is AAI programming done in 256 byte blocks (i.e. WRDI at the end of
each page) or in bigger blocks (i.e. WRDI only at the end of the
to-be-programmed region)?

Am 13.06.2012 11:46 schrieb Nico Huber:
> 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 second word sent with the usb command is used. This word was
> mistaken for the size of the chunks sent over usb earlier.
>
> This patch requires "[PATCH] Let the programmer driver decide how to do
> AAI transfers".
>
> Signed-off-by: Nico Huber
>
> diff -urp -x.svn flashrom-spi_write_aai/dediprog.c flashrom-dediprog-own-spi_write_aai/dediprog.c
> --- flashrom-spi_write_aai/dediprog.c	2012-06-12 13:33:43.301025936 +0200
> +++ flashrom-dediprog-own-spi_write_aai/dediprog.c	2012-06-12 13:40:52.223633529 +0200
> @@ -31,6 +31,9 @@ static usb_dev_handle *dediprog_handle;
>  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)
> @@ -305,7 +308,8 @@ static int dediprog_spi_read(struct flas
> @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)
> 		   unsigned int start, unsigned int len,
> 		   uint16_t dedi_spi_cmd)

112 column limit.
> {
>    int ret;
>    unsigned int i;
> @@ -313,12 +317,12 @@ static int dediprog_spi_bulk_write(struc
>     * 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 chunksize = 256; /* always? flash->page_size; */
> const unsigned int count = len / chunksize;
> const char count_and_chunk[] = {count & 0xff,
> const char count_and_cmd[] = {count & 0xff,
> 			(count >> 8) & 0xff,
> 			chunksize & 0xff,
> 			(chunksize >> 8) & 0xff};
> 			(dedi_spi_cmd >> 8) & 0xff,
> 			dedi_spi_cmd & 0xff};

Have you ever seen the field containing ((dedi_spi_cmd >> 8) & 0xff) at a nonzero value? Should we replace that field with 0 if we have no idea about it (and make dedi_spi_cmd an uint8_t)?
> char usbbuf[512];
>
> if ((start % chunksize) || (len % chunksize)) {
> @@ -334,9 +338,9 @@ static int dediprog_spi_bulk_write(struc
>     * 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)) {
> 	      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;
>
> @@ -358,8 +362,9 @@ static int dediprog_spi_bulk_write(struc
>    return 0;
>  }
>
> -static int dediprog_spi_write_256(struct flashctx *flash, uint8_t *buf,
> 		  unsigned int start, unsigned int len)
>
> +static int dediprog_write_256(struct flashctx *flash, uint8_t *buf,

Not sure if dediprog_wite_256() still is an appropriate name for the
function.
> 	      unsigned int start, unsigned int len,
> 	      uint16_t dedi_spi_cmd)
>
> {
>    int ret;
>    const unsigned int chunksize = flash->page_size;
> @@ -382,7 +387,7 @@ static int dediprog_spi_write_256(struct
>    /* Round down. */
>    bulklen = (len - residue) / chunksize * chunksize;
>    ret = dediprog_spi_bulk_write(flash, buf + residue, start + residue,
> 		     bulklen);
> 		     bulklen, dedi_spi_cmd);
> if (ret) {
> dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON);
> return ret;
>
> @@ -404,6 +409,20 @@ static int dediprog_spi_write_256(struct
>    return 0;
>  }
>
> +static int dediprog_spi_write_256(struct flashctx *flash, uint8_t *buf,
> 		  unsigned int start, unsigned int len)
>
> +{
> return dediprog_write_256(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_write_256(flash, buf, start, len,
> 		  DEDI_SPI_CMD_AAIWRITE);
>
> +}
>
> static int dediprog_spi_send_command(struct flashctx *flash,
>    			     unsigned int writecnt,
>    			     unsigned int readcnt,
> @@ -709,7 +728,7 @@ static const struct spi_programmer spi_p
>    .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)

Regards,
Carl-Daniel