Re: [flashrom] [PATCH 1/3] serprog: add SPI support

[Carl-Daniel Hailfinger]

Am 08.09.2011 00:56 schrieb Stefan Tauner:
> +/* FIXME: This function is optimized so that it does not split each
> transaction into chip page_size long blocks unnecessarily like
> spi_read_chunked. When spi_read_chunked is fixed this method can be
> removed. */

That "fix" would break all chips with a non-contiguous address space,
i.e. all chips with a page size which is not a power of two. This
shortcut here means that serprog will never work with those chips.
Please add a FIXME comment which mentions this.
> +int serprog_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len)
> +{
> int i;
> const int max_read = spi_programmer_serprog.max_data_read;
> for(i = 0; i < len; ) {

Space after for.
You could indeed move the i+=cur_len into the for AFAICS.
> int ret;
> int cur_len = min(max_read, (len - i));
> ret = spi_nbyte_read(start + i, buf + i, cur_len);
> if (ret)
> 	return ret;
> i += cur_len;
> }
> return 0;
>
> +}

Regards,
Carl-Daniel