[U-Boot] [PATCH] cmd_sf: Fix problem with "sf update" and unaligned length

[Gerlando Falauto]

Hi Stefan,

thanks for the patch.
Please compare it with my v1 revision of the patch:
http://patchwork.ozlabs.org/patch/150468/
Back then, Simon suggested to use two separate SPI flash operations to 
avoid the memcpy() operation. I guess noone would have guessed about 
QSPI devices at the time.

Anyway, your patch looks even better than my v1 then, so:

Acked-by: Gerlando Falauto <gerlando.falauto at keymile.com>

Thank you,
Gerlando

On 01/09/2015 02:39 PM, Stefan Roese wrote:
> On SoCFPGA, using "sf update" with an non-4byte aligned length leads
> to a hangup (and reboot via watchdog). This is because of the unaligned
> access in the cadence QSPI driver which is hard to prevent since the
> data is written into a 4-byte wide FIFO. This patch fixes this problem
> by changing the behavior of the last sector write (not sector aligned).
>
> The new code is even simpler and copies the source data into the temp
> buffer and now uses the temp buffer to write the complete sector. So
> only one SPI sector write is used now instead of 2 in the old version.
>
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Gerlando Falauto <gerlando.falauto at keymile.com>
> Cc: Valentin Longchamp <valentin.longchamp at keymile.com>
> Cc: Holger Brunck <holger.brunck at keymile.com>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki at gmail.com>
> ---
>   common/cmd_sf.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index 5c788e9..dd82290 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -163,6 +163,8 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>   static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset,
>   		size_t len, const char *buf, char *cmp_buf, size_t *skipped)
>   {
> +	char *ptr = (char *)buf;
> +
>   	debug("offset=%#x, sector_size=%#x, len=%#zx\n",
>   	      offset, flash->sector_size, len);
>   	/* Read the entire sector so to allow for rewriting */
> @@ -178,16 +180,14 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset,
>   	/* Erase the entire sector */
>   	if (spi_flash_erase(flash, offset, flash->sector_size))
>   		return "erase";
> -	/* Write the initial part of the block from the source */
> -	if (spi_flash_write(flash, offset, len, buf))
> -		return "write";
> -	/* If it's a partial sector, rewrite the existing part */
> +	/* If it's a partial sector, copy the data into the temp-buffer */
>   	if (len != flash->sector_size) {
> -		/* Rewrite the original data to the end of the sector */
> -		if (spi_flash_write(flash, offset + len,
> -				    flash->sector_size - len, &cmp_buf[len]))
> -			return "write";
> +		memcpy(cmp_buf, buf, len);
> +		ptr = cmp_buf;
>   	}
> +	/* Write one complete sector */
> +	if (spi_flash_write(flash, offset, flash->sector_size, ptr))
> +		return "write";
>
>   	return NULL;
>   }
>