[flashrom] [PATCH] Partial writes for SPI

[Michael Karcher]

Am Donnerstag, den 25.03.2010, 03:10 +0100 schrieb Carl-Daniel
Hailfinger:
> +	/* Warning: This loop has a very unusual condition and body.
> +	 * The loop needs to go through each page with at least one affected
> +	 * byte. The lowest page number is (start / page_size) since that
> +	 * division rounds down. The highest page number we want is the page
> +	 * where the last byte of the range lives. That last byte has the
> +	 * address (start + len - 1), thus the highest page number is
> +	 * (start + len - 1) / page_size. Since we want to include that last
> +	 * page as well, the loop condition uses <=.
> +	 */
> +	for (i = start / page_size; i <= (start + len - 1) / page_size; i++) {
> +		/* Byte position of the first byte in the range in this page. */
> +		/* starthere is an offset to the base address of the chip. */
> +		starthere = max(start, i * page_size);
> +		/* Length of bytes in the range in this page. */
> +		lenhere = min(start + len, (i + 1) * page_size) - starthere;
> +		for (j = 0; j < lenhere; j += chunksize) {
> +			towrite = min(chunksize, lenhere - j);
> +			rc = spi_nbyte_program(starthere + j, buf + starthere - start + j, towrite);
> +			if (rc)
> +				break;
> +			while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> +				programmer_delay(10);
> +		}
> +		if (rc)
> +			break;
> +	}

i would write this as (carful, untested):

    startofs = start % pagesize;  /* offset relative to page begin */
    /* iterate over all pages to write */
    for(pageaddr = start - startofs; pageaddr < start + len; 
        pageaddr += page_size) {
        endofs = min(pagesize, start + len - pageaddr);
        /* iterate over chunks in this page */
        for(j = startofs; j < endofs; j += chunksize) {
            towrite = min(chunksize, endofs - j);
            rc = spi_nbyte_program(pageaddr + j, buf, towrite);
            buf += towrite;
            if (rc)
                goto leave_page_loop;
            /* Copy/paste wait here */
        }
        startofs = 0;  /* all pages but the first need to be programmed
                          from the beginning */
    }
leave_page_loop:

Your logic seem right, too, so this is a matter of taste. I think my
version needs less explanation, that's why I would prefer it that way.

We should aim to remove all the foo_spi_write_n functions and put the
max write size into the spi controller structure. This probably means to
write one entry for Intel (64) and one for VIA (16). Currently, some of
the write functions do erase, others don't. Some of the write functions
do unprotect, others don't. But that's for a later patch.

Regards,
  Michael Karcher