[RFC PATCH] Revert "arm: Show cache warnings in U-Boot proper only"

[Marek Vasut]

On 12/19/19 2:23 AM, André Przywara wrote:
> On 19/12/2019 00:55, Marek Vasut wrote:
> 
> Hi Marek,

Hi,

>> On 12/19/19 1:52 AM, Andre Przywara wrote:
>>> According to commit 11aa6a32eb5f ("arm: cache: Implement cache range
>>> check for v7"), which introduced check_cache_range(), this was meant
>>> as a pure debugging feature, only to be compiled in when a developer
>>> explicitly #defined DEBUG in cache.c. Presumably the intention was to
>>> help with finding *certain* alignment issues with DMA buffers.
>>>
>>> Commit bcc53bf09589 ("arm: Show cache warnings in U-Boot proper only")
>>> compiled this in *unconditionally* into U-Boot proper.
>>>
>>> This has the annoying side effect of producing tons of somewhat
>>> pointless warnings about non-aligned clean&invalidate operations, which
>>> tend to be appeased by even more pointless rounding operations in many
>>> drivers (mostly those used on ARM boards).
>>>
>>> Bring back the old behaviour, of only compiling this in for DEBUG
>>> situations, but staying silent otherwise.
>>>
>>> This reverts commit bcc53bf095893fbdae531a9a7b5d4ef4a125a7fc.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
>>> ---
>>> Hi,
>>>
>>> if the intention was indeed to always force cache maintenance range
>>> alignments, I would like to open a discussion on this, because I believe
>>> it is not useful, especially in the clean&invalidate case.
>>
>> Why don't you rather fix the cache op alignment bugs ?
> 
> Which bugs do you mean?

Well, the ones reported by this warning.

> Those that would currently trigger those warnings?
> I don't think they are actual bugs, besides I don't think they are any
> cases left for 32-bit ARM boards (leave alone the new RPi4 Ethernet
> driver in the rpi-4-32b_defconfig).

If your cache isn't flushed or invalidated properly, it's a bug.
On ARM32, this is (was, before this warning) a real problem.

> Or those that are currently hidden because we *force* an alignment on
> the *arguments* passed to invalidate_dcache_range, for instance?
> These are quite numerous, so I would rather get some input first before
> spending a lot of time on this.

Make sure your buffers are properly aligned and you have no cache
issues. It's really that easy. If you see numerous, then your platform
has a real problem which must be fixed ; I didn't see any for quite a
while on platforms I have access to.

> Starting a discussion on this topic and getting some feedback was the
> actual reason for this patch - even though it is still valid, IMHO.

[...]