[U-Boot] [ PATCH] ARM: cache: runtime flag to suppress the "cache misaligned" message

[Marek Vasut]

On 03/06/2017 11:21 PM, Steve Rae wrote:
> The "chunks" in the "fastboot sparse image" are not aligned,
> resulting in many "cached misaligned" messages from
> check_cache_range(). Implement a runtime flag to suppress this
> message, and use this flag when processing the "fastboot sparse
> image".

Let me understand this, what we add here is a patch to silence a warning
which correctly indicates something totally wrong is happening instead
of fixing that problem ?

 From me, that is a NAK and if my understanding of this is correct, this
patch will go in only over my dead body.

> Signed-off-by: Steve Rae <steve.rae at raedomain.com> Reported-by: Gary
> Bisson <gary.bisson at boundarydevices.com>
>
> --- A little background: In the code that I am testing, the "sparse"
> image is 350,723,024 bytes and is downloaded into an aligned buffer
> on the device. This image/buffer contains a header ('a') and 13903
> "chunks" (b1/c1 through bx/cx):

OT: Can you please fix your mailer to break lines at 80 chars ?
I reflowed the mail and the ascii art turned crap.

> +---+----+----+----+----+     +----+----+ | a | b1 | c1 | b2 | c2 |
> ... | bx | cx | +---+----+----+----+----+     +----+----+ where: a  =
> the "sparse image" header (28 bytes) b* = the "chunk" header
> (12 bytes) c* = the "chunk" data          (which is always an exact
> multiple of CONFIG_SYS_CACHELINE_SIZE) Whenever the "chunk" type is
> CHUNK_TYPE_RAW, then the data in the current 'c' (the "chunk" data)
> needs to be written into the flash as is, using the pointer to the
> first byte of this 'c' as the starting address for the blk_dwrite().
> Because of the 28 byte image header, and the 12 byte "chunk"
> header(s), it is extremely unlikely that this starting address will
> be aligned - resulting in many "CACHE: Misaligned operation at range
> [XXXX, YYYY]" messages while these 13093 "chunks" are processed. In
> the code that I am testing, this message is being generated by the
> call to flush_cache(start_addr, trans_bytes) in the
> sdhci_send_command() function.

You can set this up such that the [a] part starts at offset +24 bytes, 
then the [c] will be aligned.

> Copying each "chuck data" to an aligned buffer before calling
> blk_dwrite() in order to avoid this message would be highly
> inefficient -- the desire is to download the code and flash it as
> fast as possible!

Fast is great, but not if this makes the transfer unreliable, which will 
happen if you cannot reliably flush/invalidate cache and you decide to 
ignore this clear warning you're getting.

> Therefore, I propose this patch which provides a runtime flag to
> suppress this warning message, which is used when processing these
> fastboot "sparse" images.

This is wrong.

> arch/arm/include/asm/cache.h |  2 ++ arch/arm/lib/cache.c         |
> 15 ++++++++++++++- common/fb_mmc.c              |  8 +++++++- 3 files
> changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/cache.h
> b/arch/arm/include/asm/cache.h index 5400cbe..1895b8a 100644 ---
> a/arch/arm/include/asm/cache.h +++ b/arch/arm/include/asm/cache.h @@
> -29,6 +29,8 @@ static inline void invalidate_l2_cache(void) } #endif
>
> +void set_suppress_warning_flag(void); +void
> clr_suppress_warning_flag(void); int check_cache_range(unsigned long
> start, unsigned long stop);
>
> void l2_cache_enable(void); diff --git a/arch/arm/lib/cache.c
> b/arch/arm/lib/cache.c index 4f72f89..174a167 100644 ---
> a/arch/arm/lib/cache.c +++ b/arch/arm/lib/cache.c @@ -46,6 +46,19 @@
> __weak void flush_dcache_range(unsigned long start, unsigned long
> stop) /* An empty stub, real implementation should be in platform
> code */ }
>
> +/* suppress "cache misaligned" warning message */ +static int
> suppress_warning; + +void set_suppress_warning_flag(void) +{ +
> suppress_warning = 1; +} + +void clr_suppress_warning_flag(void) +{ +
> suppress_warning = 0; +} + int check_cache_range(unsigned long start,
> unsigned long stop) { int ok = 1; @@ -56,7 +69,7 @@ int
> check_cache_range(unsigned long start, unsigned long stop) if (stop &
> (CONFIG_SYS_CACHELINE_SIZE - 1)) ok = 0;
>
> -	if (!ok) { +	if (!ok && !suppress_warning) { warn_non_spl("CACHE:
> Misaligned operation at range [%08lx, %08lx]\n", start, stop); } diff
> --git a/common/fb_mmc.c b/common/fb_mmc.c index 6cc113d..c8b1e17
> 100644 --- a/common/fb_mmc.c +++ b/common/fb_mmc.c @@ -58,8 +58,14 @@
> static lbaint_t fb_mmc_sparse_write(struct sparse_storage *info, {
> struct fb_mmc_sparse *sparse = info->priv; struct blk_desc *dev_desc
> = sparse->dev_desc; +	lbaint_t blks; + +	/* suppress "cache
> misaligned" warning message during this write */ +
> set_suppress_warning_flag(); +	blks = blk_dwrite(dev_desc, blk,
> blkcnt, buffer); +	clr_suppress_warning_flag();
>
> -	return blk_dwrite(dev_desc, blk, blkcnt, buffer); +	return blks; }
>
> static lbaint_t fb_mmc_sparse_reserve(struct sparse_storage *info,
>