Sign conversion warnings

[HTRamsey]

When using gcc 11.3 & 12.3, there are sign conversion warnings in armv7m_cachel1.h with most usages of __SCB_DCACHE_LINE_SIZE and __SCB_ICACHE_LINE_SIZE in that file.

[JonatanAntoni]

Hi @holden-zenith,

Thanks for pointing this out. The cache maintenance functions convert forth and back between signed and unsigned which is probably causing your warnings. Would you like to contribute reworked versions of the functions that work for you?

Thanks, Jonatan

[ReinhardKeil]

I think the issue is the definition

__STATIC_FORCEINLINE void SCB_InvalidateICache_by_Addr (volatile void *addr, int32_t isize)
{
  #if defined (__ICACHE_PRESENT) && (__ICACHE_PRESENT == 1U)
    if ( isize > 0 ) {
       int32_t op_size = isize + (((uint32_t)addr) & (__SCB_ICACHE_LINE_SIZE - 1U));
      uint32_t op_addr = (uint32_t)addr /* & ~(__SCB_ICACHE_LINE_SIZE - 1U) */;

      __DSB();

      do {
        SCB->ICIMVAU = op_addr;             /* register accepts only 32byte aligned values, only bits 31..5 are valid */
        op_addr += __SCB_ICACHE_LINE_SIZE;
        op_size -= __SCB_ICACHE_LINE_SIZE;
      } while ( op_size > 0 );

      __DSB();
      __ISB();
    }
  #endif
}

See: https://arm-software.github.io/CMSIS_6/latest/Core/html/group__Icache__functions__m7.html#gaeb1a2bf181afcfb837ce0502e6bfa4fb

Changing this as below should solve the problem. But perhaps there is a timing reason for the previous code:

__STATIC_FORCEINLINE void SCB_InvalidateICache_by_Addr (volatile void *addr, uint32_t isize)
{
  #if defined (__ICACHE_PRESENT) && (__ICACHE_PRESENT == 1U)
    if ( isize > 0 ) {
      uint32_t op_size = isize + (((uint32_t)addr) & (__SCB_ICACHE_LINE_SIZE - 1U));
      uint32_t op_addr = (uint32_t)addr /* & ~(__SCB_ICACHE_LINE_SIZE - 1U) */;

      __DSB();

      while ( op_size >=  __SCB_ICACHE_LINE_SIZE ) {
        SCB->ICIMVAU = op_addr;             /* register accepts only 32byte aligned values, only bits 31..5 are valid */
        op_addr += __SCB_ICACHE_LINE_SIZE;
        op_size -= __SCB_ICACHE_LINE_SIZE;
      };

      __DSB();
      __ISB();
    }
  #endif
}

[JonatanAntoni]

@holden-zenith, can you confirm the reworked code provided by Reinhard solves the issue your you while it is still performing correctly?

[HTRamsey]

@JonatanAntoni well it definitely resolves the warning in that function which would also need to be applied to the others, but changing the do while to a while will obviously change the functionality depending on isize. Are you supposed to manually verify isize is >= __SCB_ICACHE_LINE_SIZE before calling this?

[JonatanAntoni]

Good spot. Basically, the idea of the loop is that at least isize bytes gets invalidated. Effectively, cache maintenance can only happen in junks of __SCB_ICACHE_LINE_SIZE. So, an alternative implementation might be calculating the number of cache lines to be invalidated and use a for-loop based on the address only. E.g.

__STATIC_FORCEINLINE void SCB_InvalidateICache_by_Addr (volatile void *addr, uint32_t isize)
{
  #if defined (__ICACHE_PRESENT) && (__ICACHE_PRESENT == 1U)
    const uint32_t op_start_addr = (uint32_t)addr /* & ~(__SCB_ICACHE_LINE_SIZE - 1U) */;
    const uint32_t op_end_addr = (uint32_t)addr + isize; 

    __DSB();

    for (uint32_t op_addr = op_start_addr; op_addr < op_end_addr; op_addr += __SCB_ICACHE_LINE_SIZE) {
      SCB->ICIMVAU = op_addr;             /* register accepts only 32byte aligned values, only bits 31..5 are valid */
    };

    __DSB();
    __ISB();
  #endif
}

A problem could result from an integer overflow in op_end_addr calculation.