[U-Boot] [PATCH v2 1/2] armv7m: add instruction & data cache support
Marek Vasut
marex at denx.de
Sun Mar 12 06:02:51 UTC 2017
On 03/12/2017 01:13 AM, Vikas Manocha wrote:
> This patch adds armv7m instruction & data cache support.
>
> Signed-off-by: Vikas Manocha <vikas.manocha at st.com>
> ---
>
> Changed in v2:
> - changed strucures for memory mapped cache registers to MACROs
Macro is written in lowercase, FYI ...
> - added lines better readability.
> - replaced magic numbers with MACROs.
>
> arch/arm/cpu/armv7m/Makefile | 2 +-
> arch/arm/cpu/armv7m/cache.c | 294 ++++++++++++++++++++++++++++++++++++++++++
> arch/arm/include/asm/armv7m.h | 26 +++-
> arch/arm/lib/Makefile | 2 +
> 4 files changed, 321 insertions(+), 3 deletions(-)
> create mode 100644 arch/arm/cpu/armv7m/cache.c
>
> diff --git a/arch/arm/cpu/armv7m/Makefile b/arch/arm/cpu/armv7m/Makefile
> index aff60e8..41efe11 100644
> --- a/arch/arm/cpu/armv7m/Makefile
> +++ b/arch/arm/cpu/armv7m/Makefile
> @@ -6,4 +6,4 @@
> #
>
> extra-y := start.o
> -obj-y += cpu.o
> +obj-y += cpu.o cache.o
> diff --git a/arch/arm/cpu/armv7m/cache.c b/arch/arm/cpu/armv7m/cache.c
> new file mode 100644
> index 0000000..cc17366
> --- /dev/null
> +++ b/arch/arm/cpu/armv7m/cache.c
> @@ -0,0 +1,294 @@
> +/*
> + * (C) Copyright 2017
> + * Vikas Manocha, ST Micoelectronics, vikas.manocha at st.com.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <asm/armv7m.h>
> +#include <asm/io.h>
> +#include <errno.h>
> +
> +/* Cache maintenance operation registers */
> +
> +#define IC_IALLU 0x00
> +#define INVAL_ICACHE_POU 0
> +
> +#define IC_IMVALU 0x08
> +#define DC_IMVAC 0x0C
> +#define DC_ISW 0x10
> +#define DC_CMVAU 0x14
> +#define DC_CMVAC 0x18
> +#define DC_CSW 0x1C
> +#define DC_CIMVAC 0x20
> +#define DC_CISW 0x24
Would be nice to have some more distinguishing name here, so one can
easily git grep for those reg names and make sense of their name without
reading the datasheet .
> +#define WAYS_SHIFT 30
> +#define SETS_SHIFT 5
Is this always 30 and 5 , on all CPUs ?
> +/* armv7m processor feature registers */
> +
> +#define CLIDR 0x00
> +#define CTR 0x04
> +
> +#define CCSIDR 0x08
> +#define MASK_NUM_WAYS GENMASK(12, 3)
> +#define MASK_NUM_SETS GENMASK(27, 13)
> +#define NUM_WAYS_SHIFT 3
> +#define NUM_SETS_SHIFT 13
> +
> +#define CSSELR 0x0C
> +#define SEL_I_OR_D BIT(0)
> +
> +void *const v7m_cache_maint = (void *)V7M_CACHE_MAINT_BASE;
> +void *const v7m_processor = (void *)V7M_PROC_FTR_BASE;
Needed ? Why don't you just use the macro directly ?
> +
> +
> +enum cache_type {
> + DCACHE = 0,
> + ICACHE,
> +};
> +
> +/* PoU : Point of Unification, Poc: Point of Coherency */
> +enum cache_action {
> + INVALIDATE_POU, /* for i-cache invalidate by address */
> + INVALIDATE_POC, /* for d-cache invalidate by address */
> + INVALIDATE_SET_WAY, /* for d-cache invalidate by sets/ways */
> + FLUSH_POU,
> + FLUSH_POC,
> + FLUSH_SET_WAY,
> + FLUSH_INVAL_POC,
> + FLUSH_INVAL_SET_WAY,
> +};
> +
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +struct dcache_config {
> + uint32_t ways;
> + uint32_t sets;
u32 , this is not userspace ...
> +};
> +
> +static void get_cache_ways_sets(struct dcache_config *cache)
> +{
> + cache->ways = (readl(v7m_processor + CCSIDR) & MASK_NUM_WAYS)
> + >> NUM_WAYS_SHIFT;
> + cache->sets = (readl(v7m_processor + CCSIDR) & MASK_NUM_SETS)
> + >> NUM_SETS_SHIFT;
Do you need to read the reg twice ?
> +}
> +
> +static uint32_t *get_action_reg_set_ways(enum cache_action action)
> +{
> + switch (action) {
> + case INVALIDATE_SET_WAY:
> + return v7m_cache_maint + DC_ISW;
> + case FLUSH_SET_WAY:
> + return v7m_cache_maint + DC_CSW;
> + case FLUSH_INVAL_SET_WAY:
> + return v7m_cache_maint + DC_CISW;
> + default:
> + break;
> + };
> +
> + return NULL;
> +}
> +
> +static uint32_t *get_action_reg_range(enum cache_action action)
> +{
> + switch (action) {
> + case INVALIDATE_POU:
> + return v7m_cache_maint + IC_IMVALU;
> + case INVALIDATE_POC:
> + return v7m_cache_maint + DC_IMVAC;
> + case FLUSH_POU:
> + return v7m_cache_maint + DC_CMVAU;
> + case FLUSH_POC:
> + return v7m_cache_maint + DC_CMVAC;
> + case FLUSH_INVAL_POC:
> + return v7m_cache_maint + DC_CIMVAC;
> + default:
> + break;
> + }
> +
> + return NULL;
> +}
> +
> +static uint32_t get_cline_size(enum cache_type type)
> +{
> + uint32_t size;
> +
> + if (type == DCACHE)
> + clrbits_le32(v7m_processor + CSSELR, BIT(SEL_I_OR_D));
> + else if (type == ICACHE)
> + setbits_le32(v7m_processor + CSSELR, BIT(SEL_I_OR_D));
> + dsb();
> +
> + size = readl(v7m_processor + CCSIDR) & GENMASK(2, 0);
define the mask as some macro ....
> + size = size * 4 + 4; /* 0 means 4, 1 means 8 & so on */
2 means 12 or 16 ? The comment is useless ...
Is the size basically 1 << (size + 2) ?
> + debug("cache line size is %d\n", size);
> +
> + return size;
> +}
> +
> +int action_cache_range(enum cache_action action, uint32_t start_addr,
> + int64_t size)
static ?
You're never checking if start_addr and size are cache-line aligned ,
see arm926ejs and armv7a
> +{
> + uint32_t cline_size;
> + uint32_t *action_reg;
u32 , fix globally
> + enum cache_type type;
> +
> + action_reg = get_action_reg_range(action);
> + if (!action_reg)
> + return -EINVAL;
> + if (action == INVALIDATE_POU)
> + type = ICACHE;
> + else
> + type = DCACHE;
> +
> + /* cache line size is minium size for the cache action */
> + cline_size = get_cline_size(type);
> + do {
> + writel(start_addr, action_reg);
> + size -= cline_size;
> + start_addr += cline_size;
> + } while (size > cline_size);
> + debug("cache action on range done\n");
> + dsb();
> + isb();
> +
> + return 0;
> +}
> +
> +static int action_dcache_all(enum cache_action action)
> +{
> + struct dcache_config cache;
> + uint32_t *action_reg;
> + int i, j;
> +
> + action_reg = get_action_reg_set_ways(action);
> + if (!action_reg)
> + return -EINVAL;
> +
> + clrbits_le32(v7m_processor + CSSELR, BIT(SEL_I_OR_D));
> + dsb();
Needed ?
> + get_cache_ways_sets(&cache); /* Get number of ways & sets */
> + debug("cache: ways= %d, sets= %d\n", cache.ways + 1, cache.sets + 1);
> + for (i = cache.sets; i >= 0; i--) {
> + for (j = cache.ways; j >= 0; j--) {
> + writel((j << WAYS_SHIFT) | (i << SETS_SHIFT),
> + action_reg);
> + }
> + }
> + dsb();
> + isb();
Are all those barriers needed ?
> + return 0;
> +}
> +
> +void dcache_enable(void)
> +{
> + if (dcache_status()) /* return if cache already enabled */
> + return;
> +
> + if (action_dcache_all(INVALIDATE_SET_WAY)) {
> + printf("ERR: D-cache not enabled\n");
> + return;
> + }
> +
> + setbits_le32(&V7M_SCB->ccr, BIT(V7M_CCR_DCACHE));
> + dsb();
> + isb();
> +}
> +
> +void dcache_disable(void)
> +{
> + /* if dcache is enabled-> dcache disable & then flush */
> + if (dcache_status()) {
Invert the condition here ...
> + if (action_dcache_all(FLUSH_SET_WAY)) {
> + printf("ERR: D-cahe not flushed\n");
> + return;
> + }
> +
> + clrbits_le32(&V7M_SCB->ccr, BIT(V7M_CCR_DCACHE));
> + dsb();
> + isb();
> + }
> +}
> +
> +int dcache_status(void)
> +{
> + return (readl(&V7M_SCB->ccr) & BIT(V7M_CCR_DCACHE)) != 0;
> +}
> +
> +#else
> +void dcache_enable(void)
> +{
> + return;
> +}
> +
> +void dcache_disable(void)
> +{
> + return;
> +}
> +
> +int dcache_status(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +
> +void invalidate_icache_all(void)
> +{
> + writel(INVAL_ICACHE_POU, v7m_cache_maint + IC_IALLU);
> + dsb();
> + isb();
> +}
> +
> +void icache_enable(void)
> +{
> + if (icache_status())
> + return;
> +
> + invalidate_icache_all();
> + setbits_le32(&V7M_SCB->ccr, BIT(V7M_CCR_ICACHE));
> + dsb();
> + isb();
> +}
> +
> +int icache_status(void)
> +{
> + return (readl(&V7M_SCB->ccr) & BIT(V7M_CCR_ICACHE)) != 0;
> +}
> +
> +void icache_disable(void)
> +{
> + isb();
> + clrbits_le32(&V7M_SCB->ccr, BIT(V7M_CCR_ICACHE));
> + isb();
> +}
> +#else
> +void icache_enable(void)
> +{
> + return;
> +}
> +
> +void icache_disable(void)
> +{
> + return;
> +}
> +
> +int icache_status(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +void enable_caches(void)
> +{
> +#ifndef CONFIG_SYS_ICACHE_OFF
> + icache_enable();
> +#endif
> +#ifndef CONFIG_SYS_DCACHE_OFF
> + dcache_enable();
> +#endif
> +}
> diff --git a/arch/arm/include/asm/armv7m.h b/arch/arm/include/asm/armv7m.h
> index 54d8a2b..67cb0e4 100644
> --- a/arch/arm/include/asm/armv7m.h
> +++ b/arch/arm/include/asm/armv7m.h
> @@ -16,8 +16,15 @@
> .thumb
> #endif
>
> -#define V7M_SCB_BASE 0xE000ED00
> -#define V7M_MPU_BASE 0xE000ED90
> +/* armv7m fixed base addresses */
> +#define V7M_SCS_BASE 0xE000E000
> +#define V7M_NVIC_BASE (V7M_SCS_BASE + 0x0100)
> +#define V7M_SCB_BASE (V7M_SCS_BASE + 0x0D00)
> +#define V7M_PROC_FTR_BASE (V7M_SCS_BASE + 0x0D78)
> +#define V7M_MPU_BASE (V7M_SCS_BASE + 0x0D90)
> +#define V7M_FPU_BASE (V7M_SCS_BASE + 0x0F30)
> +#define V7M_CACHE_MAINT_BASE (V7M_SCS_BASE + 0x0F50)
> +#define V7M_ACCESS_CNTL_BASE (V7M_SCS_BASE + 0x0F90)
Does all this stuff need to be in global namespace ?
> #define V7M_SCB_VTOR 0x08
>
> @@ -27,6 +34,18 @@ struct v7m_scb {
> uint32_t icsr; /* Interrupt Control and State Register */
> uint32_t vtor; /* Vector Table Offset Register */
> uint32_t aircr; /* App Interrupt and Reset Control Register */
> + uint32_t scr; /* offset 0x10 */
> + uint32_t ccr; /* offset 0x14 */
> + uint32_t shpr1; /* offset 0x18 */
> + uint32_t shpr2; /* offset 0x1c */
> + uint32_t shpr3; /* offset 0x20 */
> + uint32_t shcrs; /* offset 0x24 */
> + uint32_t cfsr; /* offset 0x28 */
> + uint32_t hfsr; /* offset 0x2C */
> + uint32_t res; /* offset 0x30 */
> + uint32_t mmar; /* offset 0x34 */
> + uint32_t bfar; /* offset 0x38 */
> + uint32_t afsr; /* offset 0x3C */
The comments are real useless compared to the previous comments in this
block ...
> };
> #define V7M_SCB ((struct v7m_scb *)V7M_SCB_BASE)
>
> @@ -39,6 +58,9 @@ struct v7m_scb {
>
> #define V7M_ICSR_VECTACT_MSK 0xFF
>
> +#define V7M_CCR_DCACHE 16
> +#define V7M_CCR_ICACHE 17
> +
> struct v7m_mpu {
> uint32_t type; /* Type Register */
> uint32_t ctrl; /* Control Register */
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index 166fa9e..52b36b3 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -55,8 +55,10 @@ endif
>
> obj-y += cache.o
> ifndef CONFIG_ARM64
> +ifndef CONFIG_CPU_V7M
> obj-y += cache-cp15.o
> endif
> +endif
>
> obj-y += psci-dt.o
>
>
--
Best regards,
Marek Vasut
More information about the U-Boot
mailing list