[PATCH v3 09/12] riscv: p8700: Add Coherence Manager (CM) and IOCU support

[Uros Stajic]

On 1. 8. 25. 10:47, Yao Zi wrote:
> [Some people who received this message don't often get email from ziyao at disroot.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> On Tue, Jul 29, 2025 at 04:24:36PM +0000, Uros Stajic wrote:
>> From: Chao-ying Fu <cfu at mips.com>
>>
>> Add support for Coherence Manager (CM) and IOCU discovery and
>> configuration on the P8700 platform.
>>
>> Signed-off-by: Chao-ying Fu <cfu at mips.com>
>> Signed-off-by: Uros Stajic <uros.stajic at htecgroup.com>
>> ---
>>   arch/riscv/cpu/p8700/Makefile             |   2 +
>>   arch/riscv/cpu/p8700/cache.c              |  10 +++
>>   arch/riscv/cpu/p8700/cm-iocu.c            |  75 ++++++++++++++++
>>   arch/riscv/cpu/p8700/cm.c                 |  92 +++++++++++++++++++
>>   arch/riscv/include/asm/arch-p8700/cm.h    |  61 +++++++++++++
>>   arch/riscv/include/asm/arch-p8700/p8700.h |  31 +++++++
>>   arch/riscv/include/asm/global_data.h      |   2 +
>>   arch/riscv/include/asm/io.h               |  86 ++++++++++++++++++
>>   board/mips/boston-riscv/Makefile          |   1 +
>>   board/mips/boston-riscv/iocu.c            | 104 ++++++++++++++++++++++
>>   10 files changed, 464 insertions(+)
>>   create mode 100644 arch/riscv/cpu/p8700/cm-iocu.c
>>   create mode 100644 arch/riscv/cpu/p8700/cm.c
>>   create mode 100644 arch/riscv/include/asm/arch-p8700/cm.h
>>   create mode 100644 board/mips/boston-riscv/iocu.c
>>
> 
> ...
> 
>> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
>> index da165858034..fb79d0a8d32 100644
>> --- a/arch/riscv/include/asm/io.h
>> +++ b/arch/riscv/include/asm/io.h
> 
> These changes don't seem to be related to the commit message. Please
> split them into a separate patch (if these are really necessary, see
> my comments below).
> 
>> @@ -15,6 +15,91 @@ static inline void sync(void)
>>   {
>>   }
>>
>> +/*
>> + * Generic virtual read/write.  Note that we don't support half-word
>> + * read/writes.  We define __arch_*[bl] here, and leave __arch_*w
>> + * to the architecture specific code.
>> + */
> 
> But you do define half-word operations. This comment looks like the one
> removed in d5af15bf515 ("riscv: Clean up asm/io.h"), and seems already
> out-of-date.
> 
>> +#if CONFIG_P8700_RISCV
>> +#ifdef CONFIG_ARCH_MAP_SYSMEM
>> +static inline void *map_sysmem(phys_addr_t paddr, unsigned long len)
>> +{
>> +     if (paddr < PHYS_SDRAM_0_SIZE + PHYS_SDRAM_1_SIZE)
>> +             paddr = paddr | 0x40000000;
>> +     return (void *)(uintptr_t)paddr;
>> +}
>> +
>> +static inline void *unmap_sysmem(const void *vaddr)
>> +{
>> +     phys_addr_t paddr = (phys_addr_t)vaddr;
>> +
>> +     paddr = paddr & ~0x40000000;
>> +     return (void *)(uintptr_t)paddr;
>> +}
>> +
>> +static inline phys_addr_t map_to_sysmem(const void *ptr)
>> +{
>> +     return (phys_addr_t)(uintptr_t)ptr;
>> +}
>> +#endif
>> +
>> +static inline unsigned char __arch_getb(const volatile void __iomem *mem)
>> +{
>> +     unsigned char value;
>> +
>> +     asm volatile("lbu %0,0(%1)" : "=r"(value) : "r"(mem));
>> +
>> +     return value;
>> +}
>> +
>> +static inline unsigned short __arch_getw(const volatile void __iomem *mem)
>> +{
>> +     unsigned short value;
>> +
>> +     asm volatile("lhu %0,0(%1)" : "=r"(value) : "r"(mem));
>> +
>> +     return value;
>> +}
>> +
>> +static inline unsigned int __arch_getl(const volatile void __iomem *mem)
>> +{
>> +     unsigned int value;
>> +
>> +     asm volatile("lw %0,0(%1)" : "=r"(value) : "r"(mem));
>> +
>> +     return value;
>> +}
>> +
>> +static inline unsigned long long __arch_getq(const volatile void __iomem *mem)
>> +{
>> +     unsigned long long value;
>> +
>> +     asm volatile("ld %0,0(%1)" : "=r"(value) : "r"(mem));
>> +
>> +     return value;
>> +}
>> +
>> +static inline void __arch_putb(unsigned char value, volatile void __iomem *mem)
>> +{
>> +     asm volatile("sb %0,0(%1)"::"r"(value), "r"(mem));
>> +}
>> +
>> +static inline void __arch_putw(unsigned short value, volatile void __iomem *mem)
>> +{
>> +     asm volatile("sh %0,0(%1)"::"r"(value), "r"(mem));
>> +}
>> +
>> +static inline void __arch_putl(unsigned int value, volatile void __iomem *mem)
>> +{
>> +     asm volatile("sw %0,0(%1)"::"r"(value), "r"(mem));
>> +}
>> +
>> +static inline void __arch_putq(unsigned int value, volatile void __iomem *mem)
>> +{
>> +     asm volatile("sd %0,0(%1)"::"r"(value), "r"(mem));
>> +}
> 
> I don't see any reason not to use the generic macros for P8700, these
> inline assembly should basically behave the same as the C-version below.
> Could you please explain it futher?
> 
> Thanks,
> Yao Zi
> 
>> +#else
>>   #define __arch_getb(a)                       (*(volatile unsigned char *)(a))
>>   #define __arch_getw(a)                       (*(volatile unsigned short *)(a))
>>   #define __arch_getl(a)                       (*(volatile unsigned int *)(a))
>> @@ -24,6 +109,7 @@ static inline void sync(void)
>>   #define __arch_putw(v, a)            (*(volatile unsigned short *)(a) = (v))
>>   #define __arch_putl(v, a)            (*(volatile unsigned int *)(a) = (v))
>>   #define __arch_putq(v, a)            (*(volatile unsigned long long *)(a) = (v))
>> +#endif
>>
>>   #define __raw_writeb(v, a)           __arch_putb(v, a)
>>   #define __raw_writew(v, a)           __arch_putw(v, a)
>> +}

Thank you for the feedback! After review, the use of generic macros
already provides equivalent behavior, which makes the inline assembly
versions redundant.

The patch will be updated to adopt the generic implementation
in the next revision.

Regards,
Uros