platform: generic: andes: Refine Andes PMA related code

This patch refines the Andes PMA related code. The main change is
refactor andes_pma_[read|write]_cfg() and andes_pma_[read|write]_addr()
into new functions andes_pma_[read|write]_num().

Also, fix some coding style problems.

Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
Reviewed-by: Anup Patel <anup@brainfault.org>

diff --git a/platform/generic/andes/andes_pma.c b/platform/generic/andes/andes_pma.c
index d5ea594a6e2844f6d01943628878a15762fd6602..321074ace366e96867dad35fed071451b06dbed1 100644 (file)
--- a/platform/generic/andes/andes_pma.c
+++ b/platform/generic/andes/andes_pma.c
@@ -1,12 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) 2023 Renesas Electronics Corp.
- *
- * Copyright (c) 2020 Andes Technology Corporation
+ * Copyright (c) 2024 Andes Technology Corporation
  *
  * Authors:
- *      Nick Hu <nickhu@andestech.com>
- *      Nylon Chen <nylon7@andestech.com>
+ *      Ben Zong-You Xie <ben717@andestech.com>
  *      Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
  */
 
@@ -19,124 +17,81 @@
 #include <sbi/sbi_error.h>
 #include <sbi_utils/fdt/fdt_helper.h>
 
-static inline unsigned long andes_pma_read_cfg(unsigned int pma_cfg_off)
+static unsigned long andes_pma_read_num(unsigned int csr_num)
 {
-#define switchcase_pma_cfg_read(__pma_cfg_off, __val)          \
-       case __pma_cfg_off:                                     \
-               __val = csr_read(__pma_cfg_off);                \
+#define switchcase_csr_read(__csr_num, __val)          \
+       case __csr_num:                                 \
+               __val = csr_read(__csr_num);            \
                break;
-#define switchcase_pma_cfg_read_2(__pma_cfg_off, __val)                \
-       switchcase_pma_cfg_read(__pma_cfg_off + 0, __val)       \
-       switchcase_pma_cfg_read(__pma_cfg_off + 2, __val)
+#define switchcase_csr_read_2(__csr_num, __val)                \
+       switchcase_csr_read(__csr_num + 0, __val)       \
+       switchcase_csr_read(__csr_num + 1, __val)
+#define switchcase_csr_read_4(__csr_num, __val)                \
+       switchcase_csr_read_2(__csr_num + 0, __val)     \
+       switchcase_csr_read_2(__csr_num + 2, __val)
+#define switchcase_csr_read_8(__csr_num, __val)                \
+       switchcase_csr_read_4(__csr_num + 0, __val)     \
+       switchcase_csr_read_4(__csr_num + 4, __val)
+#define switchcase_csr_read_16(__csr_num, __val)       \
+       switchcase_csr_read_8(__csr_num + 0, __val)     \
+       switchcase_csr_read_8(__csr_num + 8, __val)
 
        unsigned long ret = 0;
 
-       switch (pma_cfg_off) {
-       switchcase_pma_cfg_read_2(CSR_PMACFG0, ret)
-
+       switch (csr_num) {
+       switchcase_csr_read_4(CSR_PMACFG0, ret)
+       switchcase_csr_read_16(CSR_PMAADDR0, ret)
        default:
-               sbi_panic("%s: Unknown PMA CFG offset %#x", __func__, pma_cfg_off);
+               sbi_panic("%s: Unknown Andes PMA CSR %#x", __func__, csr_num);
                break;
        }
 
        return ret;
 
-#undef switchcase_pma_cfg_read_2
-#undef switchcase_pma_cfg_read
+#undef switchcase_csr_read_16
+#undef switchcase_csr_read_8
+#undef switchcase_csr_read_4
+#undef switchcase_csr_read_2
+#undef switchcase_csr_read
 }
 
-static inline void andes_pma_write_cfg(unsigned int pma_cfg_off, unsigned long val)
+static void andes_pma_write_num(unsigned int csr_num, unsigned long val)
 {
-#define switchcase_pma_cfg_write(__pma_cfg_off, __val)         \
-       case __pma_cfg_off:                                     \
-               csr_write(__pma_cfg_off, __val);                \
+#define switchcase_csr_write(__csr_num, __val)         \
+       case __csr_num:                                 \
+               csr_write(__csr_num, __val);            \
                break;
-#define switchcase_pma_cfg_write_2(__pma_cfg_off, __val)       \
-       switchcase_pma_cfg_write(__pma_cfg_off + 0, __val)      \
-       switchcase_pma_cfg_write(__pma_cfg_off + 2, __val)
-
-       switch (pma_cfg_off) {
-       switchcase_pma_cfg_write_2(CSR_PMACFG0, val)
-
+#define switchcase_csr_write_2(__csr_num, __val)       \
+       switchcase_csr_write(__csr_num + 0, __val)      \
+       switchcase_csr_write(__csr_num + 1, __val)
+#define switchcase_csr_write_4(__csr_num, __val)       \
+       switchcase_csr_write_2(__csr_num + 0, __val)    \
+       switchcase_csr_write_2(__csr_num + 2, __val)
+#define switchcase_csr_write_8(__csr_num, __val)       \
+       switchcase_csr_write_4(__csr_num + 0, __val)    \
+       switchcase_csr_write_4(__csr_num + 4, __val)
+#define switchcase_csr_write_16(__csr_num, __val)      \
+       switchcase_csr_write_8(__csr_num + 0, __val)    \
+       switchcase_csr_write_8(__csr_num + 8, __val)
+
+       switch (csr_num) {
+       switchcase_csr_write_4(CSR_PMACFG0, val)
+       switchcase_csr_write_16(CSR_PMAADDR0, val)
        default:
-               sbi_panic("%s: Unknown PMA CFG offset %#x", __func__, pma_cfg_off);
+               sbi_panic("%s: Unknown Andes PMA CSR %#x", __func__, csr_num);
                break;
        }
 
-#undef switchcase_pma_cfg_write_2
-#undef switchcase_pma_cfg_write
+#undef switchcase_csr_write_16
+#undef switchcase_csr_write_8
+#undef switchcase_csr_write_4
+#undef switchcase_csr_write_2
+#undef switchcase_csr_write
 }
 
-static inline void andes_pma_write_addr(unsigned int pma_addr_off, unsigned long val)
+static inline bool not_napot(unsigned long addr, unsigned long size)
 {
-#define switchcase_pma_write(__pma_addr_off, __val)            \
-       case __pma_addr_off:                                    \
-               csr_write(__pma_addr_off, __val);               \
-               break;
-#define switchcase_pma_write_2(__pma_addr_off, __val)          \
-       switchcase_pma_write(__pma_addr_off + 0, __val)         \
-       switchcase_pma_write(__pma_addr_off + 1, __val)
-#define switchcase_pma_write_4(__pma_addr_off, __val)          \
-       switchcase_pma_write_2(__pma_addr_off + 0, __val)       \
-       switchcase_pma_write_2(__pma_addr_off + 2, __val)
-#define switchcase_pma_write_8(__pma_addr_off, __val)          \
-       switchcase_pma_write_4(__pma_addr_off + 0, __val)       \
-       switchcase_pma_write_4(__pma_addr_off + 4, __val)
-#define switchcase_pma_write_16(__pma_addr_off, __val)         \
-       switchcase_pma_write_8(__pma_addr_off + 0, __val)       \
-       switchcase_pma_write_8(__pma_addr_off + 8, __val)
-
-       switch (pma_addr_off) {
-       switchcase_pma_write_16(CSR_PMAADDR0, val)
-
-       default:
-               sbi_panic("%s: Unknown PMA ADDR offset %#x", __func__, pma_addr_off);
-               break;
-       }
-
-#undef switchcase_pma_write_16
-#undef switchcase_pma_write_8
-#undef switchcase_pma_write_4
-#undef switchcase_pma_write_2
-#undef switchcase_pma_write
-}
-
-static inline unsigned long andes_pma_read_addr(unsigned int pma_addr_off)
-{
-#define switchcase_pma_read(__pma_addr_off, __val)             \
-       case __pma_addr_off:                                    \
-               __val = csr_read(__pma_addr_off);               \
-               break;
-#define switchcase_pma_read_2(__pma_addr_off, __val)           \
-       switchcase_pma_read(__pma_addr_off + 0, __val)          \
-       switchcase_pma_read(__pma_addr_off + 1, __val)
-#define switchcase_pma_read_4(__pma_addr_off, __val)           \
-       switchcase_pma_read_2(__pma_addr_off + 0, __val)        \
-       switchcase_pma_read_2(__pma_addr_off + 2, __val)
-#define switchcase_pma_read_8(__pma_addr_off, __val)           \
-       switchcase_pma_read_4(__pma_addr_off + 0, __val)        \
-       switchcase_pma_read_4(__pma_addr_off + 4, __val)
-#define switchcase_pma_read_16(__pma_addr_off, __val)          \
-       switchcase_pma_read_8(__pma_addr_off + 0, __val)        \
-       switchcase_pma_read_8(__pma_addr_off + 8, __val)
-
-       unsigned long ret = 0;
-
-       switch (pma_addr_off) {
-       switchcase_pma_read_16(CSR_PMAADDR0, ret)
-
-       default:
-               sbi_panic("%s: Unknown PMA ADDR offset %#x", __func__, pma_addr_off);
-               break;
-       }
-
-       return ret;
-
-#undef switchcase_pma_read_16
-#undef switchcase_pma_read_8
-#undef switchcase_pma_read_4
-#undef switchcase_pma_read_2
-#undef switchcase_pma_read
+       return ((size & (size - 1)) || (addr & (size - 1)));
 }
 
 static unsigned long andes_pma_setup(const struct andes_pma_region *pma_region,
@@ -149,36 +104,24 @@ static unsigned long andes_pma_setup(const struct andes_pma_region *pma_region,
        unsigned long pmaaddr;
        char *pmaxcfg;
 
-       /* Check for 4KiB granularity */
-       if (size < (1 << 12))
-               return SBI_EINVAL;
-
-       /* Check size is power of 2 */
-       if (size & (size - 1))
-               return SBI_EINVAL;
-
-       if (entry_id > 15)
-               return SBI_EINVAL;
-
-       if (!(pma_region->flags & ANDES_PMACFG_ETYP_NAPOT))
+       /* Check for a 4KiB granularity NAPOT region*/
+       if (size < ANDES_PMA_GRANULARITY || not_napot(addr, size) ||
+           !(pma_region->flags & ANDES_PMACFG_ETYP_NAPOT))
                return SBI_EINVAL;
 
-       if ((addr & (size - 1)) != 0)
-               return SBI_EINVAL;
-
-       pma_cfg_addr = entry_id / 8 ? CSR_PMACFG0 + 2 : CSR_PMACFG0;
-       pmacfg_val = andes_pma_read_cfg(pma_cfg_addr);
+       pma_cfg_addr = CSR_PMACFG0 + ((entry_id / 8) ? 2 : 0);
+       pmacfg_val = andes_pma_read_num(pma_cfg_addr);
        pmaxcfg = (char *)&pmacfg_val + (entry_id % 8);
        *pmaxcfg = 0;
        *pmaxcfg = pma_region->flags;
 
-       andes_pma_write_cfg(pma_cfg_addr, pmacfg_val);
+       andes_pma_write_num(pma_cfg_addr, pmacfg_val);
 
        pmaaddr = (addr >> 2) + (size >> 3) - 1;
 
-       andes_pma_write_addr(CSR_PMAADDR0 + entry_id, pmaaddr);
+       andes_pma_write_num(CSR_PMAADDR0 + entry_id, pmaaddr);
 
-       return andes_pma_read_addr(CSR_PMAADDR0 + entry_id) == pmaaddr ?
+       return andes_pma_read_num(CSR_PMAADDR0 + entry_id) == pmaaddr ?
               pmaaddr : SBI_EINVAL;
 }
 
@@ -202,19 +145,20 @@ static int andes_fdt_pma_resv(void *fdt, const struct andes_pma_region *pma,
 
        if (na > 1 && addr_high) {
                sbi_snprintf(name, sizeof(name),
-                            "pma_resv%d@%x,%x", index,
-                            addr_high, addr_low);
+                            "pma_resv%d@%x,%x",
+                            index, addr_high, addr_low);
        } else {
                sbi_snprintf(name, sizeof(name),
-                            "pma_resv%d@%x", index,
-                            addr_low);
+                            "pma_resv%d@%x",
+                            index, addr_low);
        }
        subnode = fdt_add_subnode(fdt, parent, name);
        if (subnode < 0)
                return subnode;
 
        if (pma->shared_dma) {
-               err = fdt_setprop_string(fdt, subnode, "compatible", "shared-dma-pool");
+               err = fdt_setprop_string(fdt, subnode, "compatible",
+                                        "shared-dma-pool");
                if (err < 0)
                        return err;
        }
@@ -259,14 +203,14 @@ static int andes_fdt_reserved_memory_fixup(void *fdt,
 {
        int parent;
 
-       /* try to locate the reserved memory node */
+       /* Try to locate the reserved memory node */
        parent = fdt_path_offset(fdt, "/reserved-memory");
        if (parent < 0) {
                int na = fdt_address_cells(fdt, 0);
                int ns = fdt_size_cells(fdt, 0);
                int err;
 
-               /* if such node does not exist, create one */
+               /* If such node does not exist, create one */
                parent = fdt_add_subnode(fdt, 0, "reserved-memory");
                if (parent < 0)
                        return parent;
@@ -307,17 +251,13 @@ int andes_pma_setup_regions(const struct andes_pma_region *pma_regions,
                return SBI_ENOTSUPP;
 
        /* Configure the PMA regions */
+       dt_populate_cnt = 0;
        for (i = 0; i < pma_regions_count; i++) {
                pa = andes_pma_setup(&pma_regions[i], i);
                if (pa == SBI_EINVAL)
                        return SBI_EINVAL;
-       }
-
-       dt_populate_cnt = 0;
-       for (i = 0; i < pma_regions_count; i++) {
-               if (!pma_regions[i].dt_populate)
-                       continue;
-               dt_populate_cnt++;
+               else if (pma_regions[i].dt_populate)
+                       dt_populate_cnt++;
        }
 
        if (!dt_populate_cnt)
@@ -325,7 +265,8 @@ int andes_pma_setup_regions(const struct andes_pma_region *pma_regions,
 
        fdt = fdt_get_address();
 
-       ret = fdt_open_into(fdt, fdt, fdt_totalsize(fdt) + (64 * dt_populate_cnt));
+       ret = fdt_open_into(fdt, fdt,
+                           fdt_totalsize(fdt) + (64 * dt_populate_cnt));
        if (ret < 0)
                return ret;
 
@@ -333,7 +274,9 @@ int andes_pma_setup_regions(const struct andes_pma_region *pma_regions,
                if (!pma_regions[i].dt_populate)
                        continue;
 
-               ret = andes_fdt_reserved_memory_fixup(fdt, &pma_regions[i], j++);
+               ret = andes_fdt_reserved_memory_fixup(fdt,
+                                                     &pma_regions[i],
+                                                     j++);
                if (ret)
                        return ret;
        }

diff --git a/platform/generic/include/andes/andes_pma.h b/platform/generic/include/andes/andes_pma.h
index bbc09cdd7741751a9c89acbf0c0ca48add497517..5ea12475ee95dce8e08c862d018d41ec42bc4bbd 100644 (file)
--- a/platform/generic/include/andes/andes_pma.h
+++ b/platform/generic/include/andes/andes_pma.h
@@ -10,6 +10,8 @@
 
 #define ANDES_MAX_PMA_REGIONS                  16
 
+#define ANDES_PMA_GRANULARITY                  (1 << 12)
+
 /* Naturally aligned power of 2 region */
 #define ANDES_PMACFG_ETYP_NAPOT                        3