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

-- 
http://www.hailfinger.org/