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

[Carl-Daniel Hailfinger]

Hi Nico,

Am 18.06.2012 11:30 schrieb Nico Huber:
> Second try for this patch.
>
> 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 fourth byte sent with the usb command is used. The second word was
> mistaken for the size of the chunks sent over usb earlier. The third
> byte (first of the second word) is now set to zero. This also adds some
> checks for the size of data chunks sent over usb.
>
> Signed-off-by: Nico Huber <nico.huber at secunet.com>

Looks very good, just a small question:


> Index: dediprog.c
> ===================================================================
> --- dediprog.c	(Revision 1545)
> +++ dediprog.c	(Arbeitskopie)
> @@ -321,14 +326,20 @@
>  	 * 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 count = len / chunksize;
> -	const char count_and_chunk[] = {count & 0xff,
> -					(count >> 8) & 0xff,
> -					chunksize & 0xff,
> -					(chunksize >> 8) & 0xff};
> +	const char count_and_cmd[] = {count & 0xff, (count >> 8) & 0xff, 0x00, dedi_spi_cmd};
>  	char usbbuf[512];
>  
> +	if (chunksize != 256) {
> +		msg_perr("%s: Chunk sizes other than 256 bytes are unsupported, chunksize=%u!\n"
> +			 "Please report a bug at flashrom at flashrom.org\n", __func__, chunksize);
> +		return 1;
> +	}
> +	if (chunksize > 512) {
> +		msg_perr("%s: Maximum chunk size is 512 bytes, chunksize=%u!\n"
> +			 "Please report a bug at flashrom at flashrom.org\n", __func__, chunksize);
> +	}
> +

Some compilers might complain about unreachable code in the second if
clause. I think I see what you tried to do here (allow removal of the
first if clause once we know how to handle different chunk sizes), but
I'd rather have the second if clause replaced by a comment above the
first if clause. AFAICS code behaviour won't change, and we get
documentation for the 512 byte limit.

What do you think?

Regards,
Carl-Daniel

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