[PATCH v2 2/2] powerpc: introduce CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD

[Rasmus Villemoes]

On 15/04/2021 07.34, Stefan Roese wrote:
> On 13.04.21 21:38, Rasmus Villemoes wrote:
>>
>> I have checked with objdump that the generated code doesn't change
>> when this option is left at its default value of 0: gcc is smart
>> enough to see that wd_limit is constant 0, the ">=" comparisons are
>> tautologically true, hence all assignments to "flushed" are eliminated
> 
> Nitpicking:
> s/tautologically/autologically. 

No.

https://www.merriam-webster.com/dictionary/tautological
https://en.wikipedia.org/wiki/Tautology_(logic)
"In Mathematical logic, a tautology (from Greek: ταυτολογία) is a
formula or assertion that is true in every possible interpretation."

It even exists in gcc docs in the form of -Wtautological-compare. [But I
do admit that "tautologically true" is a pleonasm; "tautologies" or
"automatically true" could have been used instead. And going full meta,
now that I look up pleonasm in wikipedia to check that I'm using it
right, that article has a reference to "tautology (language)" in its
second sentence.]

>>       /* wait for all dcbst to complete on bus */
>>       asm volatile("sync" : : : "memory");
>> @@ -27,7 +33,11 @@ void flush_cache(ulong start_addr, ulong size)
>>       for (addr = start; (addr <= end) && (addr >= start);
>>               addr += CONFIG_SYS_CACHELINE_SIZE) {
>>           asm volatile("icbi 0,%0" : : "r" (addr) : "memory");
>> -        WATCHDOG_RESET();
>> +        flushed += CONFIG_SYS_CACHELINE_SIZE;
>> +        if (flushed >= wd_limit) {
>> +            WATCHDOG_RESET();
>> +            flushed = 0;
>> +        }
> 
> It might make sense to pull these 2 identical code snippets into a
> function to not duplicate this code.

Something like

static ulong maybe_reset(ulong flushed)
{
  const ulong wd_limit = ...;
  flushed += CONFIG_SYS_CACHELINE_SIZE;
  if (flushed >= wd_limit) {
    WATCHDOG_RESET();
    flushed = 0;
  }
  return flushed;
}

and "flushed = maybe_reset(flushed);"? I don't think that improves
readability.

Thanks,
Rasmus