On 03/07/2017 03:41 PM, Gary Bisson wrote: > Hi Marek, All, > > On Tue, Mar 07, 2017 at 03:52:23AM +0100, Marek Vasut wrote: >> 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....@raedomain.com> Reported-by: Gary >>> Bisson <gary.bis...@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. > > This will align the first chunk but there's no guarantee the next one > will be aligned.
OK, so the protocol is basically crappy ? >>> 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. > > Actually fast isn't the problem, it is more than we can't afford to copy > each chunk to an aligned buffer because of size constraints. Although we > could break down "big" chunks to smaller one I guess. Great >>> 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. > > Yes I agree that hiding the warning isn't solving anything. However I > expected that disabling dcache would remove the warning but it doesn't. Removing the warning if dcache is OFF is fine. > Shouldn't the cache maintenance functions like flush_dcache_range() > check the dcache_status() before doing anything? They should. > Regards, > Gary > -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot