Add Andes AE350 fdt driver support

Fix minor grammar issue in function description.

Signed-off-by Yu Chien Peter Lin <[email protected]>

---
 include/sbi/sbi_domain.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/sbi/sbi_domain.h +1 -2

index b90f59c..00662a1 100644
@@ -175,7 +175,7 @@ int sbi_domain_register(struct sbi_domain *dom,
  * @param reg pointer to the memory region to be added
  *
  * @return 0 on success
- * @return SBI_EALREADY if memory region conflicts with existing
+ * @return SBI_EALREADY if memory region conflicts with the existing one
  * @return SBI_EINVAL otherwise
  */
 int sbi_domain_root_add_memregion(const struct sbi_domain_memregion *reg);
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Since we can get the PLMT base address and timer frequency from device tree, move plmt timer device to fdt timer framework.

dts example (Quad-core AX45MP):

  cpus {
      ...
      timebase-frequency = <0x3938700>;
      ...
  }
  soc {
      ...
      plmt0 at e6000000 {
          compatible = "andestech,plmt0";

          reg = <0x00 0xe6000000 0x00 0x100000>;
          interrupts-extended = <&cpu0_intc 0x07
                                 &cpu1_intc 0x07
                                 &cpu2_intc 0x07
                                 &cpu3_intc 0x07>;
      };
      ...
  }

Quoted 14 lines · Yu Chien Peter Lin

Since we can get the PLMT base address and timer frequency from device tree, move plmt timer device to fdt timer framework.

dts example (Quad-core AX45MP):

  cpus {
      ...
      timebase-frequency = <0x3938700>;
      ...
  }
  soc {
      ...
      plmt0 at e6000000 {
          compatible = "andestech,plmt0";

Quoted 8 lines

          reg = <0x00 0xe6000000 0x00 0x100000>;
          interrupts-extended = <&cpu0_intc 0x07
                                 &cpu1_intc 0x07
                                 &cpu2_intc 0x07
                                 &cpu3_intc 0x07>;
      };
      ...
  }

Where is the binding for this compatible string? I do not see anything either upstream or downstream:

https://lore.kernel.org/linux-devicetree/?q=andestech%2Cplmt0
https://github.com/andestech/linux/commits/RISCV-Linux-5.4-ast-v5_1_0-branch/Documentation/devicetree/bindings

We really shouldn't be accepting any devicetree compatible unless a YAML binding for it exists, ideally one acked by the DT schema maintainers.

Regards,
Samuel

Quoted 14 lines · Yu Chien Peter Lin

Since we can get the PLMT base address and timer frequency from device tree, move plmt timer device to fdt timer framework.

dts example (Quad-core AX45MP):

  cpus {
      ...
      timebase-frequency = <0x3938700>;
      ...
  }
  soc {
      ...
      plmt0 at e6000000 {
          compatible = "andestech,plmt0";

Quoted 8 lines

          reg = <0x00 0xe6000000 0x00 0x100000>;
          interrupts-extended = <&cpu0_intc 0x07
                                 &cpu1_intc 0x07
                                 &cpu2_intc 0x07
                                 &cpu3_intc 0x07>;
      };
      ...
  }

Where is the binding for this compatible string? I do not see anything either upstream or downstream:

https://lore.kernel.org/linux-devicetree/?q=andestech%2Cplmt0
https://github.com/andestech/linux/commits/RISCV-Linux-5.4-ast-v5_1_0-branch/Documentation/devicetree/bindings

We really shouldn't be accepting any devicetree compatible unless a YAML binding for it exists, ideally one acked by the DT schema maintainers.

Quoted 25 lines · Samuel Holland

Quoted 14 lines · Yu Chien Peter Lin

Since we can get the PLMT base address and timer frequency from device tree, move plmt timer device to fdt timer framework.

dts example (Quad-core AX45MP):

  cpus {
      ...
      timebase-frequency = <0x3938700>;
      ...
  }
  soc {
      ...
      plmt0 at e6000000 {
          compatible = "andestech,plmt0";

Quoted 8 lines

          reg = <0x00 0xe6000000 0x00 0x100000>;
          interrupts-extended = <&cpu0_intc 0x07
                                 &cpu1_intc 0x07
                                 &cpu2_intc 0x07
                                 &cpu3_intc 0x07>;
      };
      ...
  }

Quoted 9 lines

Where is the binding for this compatible string? I do not see anything either upstream or downstream:

https://lore.kernel.org/linux-devicetree/?q=andestech%2Cplmt0
https://github.com/andestech/linux/commits/RISCV-Linux-5.4-ast-v5_1_0-branch/Documentation/devicetree/bindings

We really shouldn't be accepting any devicetree compatible unless a YAML binding for it exists, ideally one acked by the DT schema maintainers.

Ideally, we should only accept patches for which DT bindings have been accepted in kernel but at the moment we are not strict about this as long as:
1) It does not affect other platforms
2) The platform/SoC vendor are willing to keep things up-to-date whenever DT bindings are merged in kernel

I am still not sure about from which OpenSBI release we should change our patch acceptance policy and mandate DT bindings to be present in the kernel.

Regards,
Anup

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Quoted 44 lines · Anup Patel

Quoted 25 lines · Samuel Holland

Quoted 14 lines · Yu Chien Peter Lin

Since we can get the PLMT base address and timer frequency from device tree, move plmt timer device to fdt timer framework.

dts example (Quad-core AX45MP):

  cpus {
      ...
      timebase-frequency = <0x3938700>;
      ...
  }
  soc {
      ...
      plmt0 at e6000000 {
          compatible = "andestech,plmt0";

Quoted 8 lines

          reg = <0x00 0xe6000000 0x00 0x100000>;
          interrupts-extended = <&cpu0_intc 0x07
                                 &cpu1_intc 0x07
                                 &cpu2_intc 0x07
                                 &cpu3_intc 0x07>;
      };
      ...
  }

Quoted 9 lines

Where is the binding for this compatible string? I do not see anything either upstream or downstream:

https://lore.kernel.org/linux-devicetree/?q=andestech%2Cplmt0
https://github.com/andestech/linux/commits/RISCV-Linux-5.4-ast-v5_1_0-branch/Documentation/devicetree/bindings

We really shouldn't be accepting any devicetree compatible unless a YAML binding for it exists, ideally one acked by the DT schema maintainers.

Ideally, we should only accept patches for which DT bindings have been accepted in kernel but at the moment we are not strict about this as long as:
1) It does not affect other platforms
2) The platform/SoC vendor are willing to keep things up-to-date whenever DT bindings are merged in kernel

Quoted 4 lines

I am still not sure about from which OpenSBI release we should change our patch acceptance policy and mandate DT bindings to be present in the kernel.

That should be the policy for drivers present in the kernel. However, there may be some other driver
that only operates in M-mode and may not need to be present in the kernel.

The SBI PMU extension also falls into this category. The DT bindings have to be maintained in the OpenSBI
as the kernel driver doesn't need to be aware of PMU DT bindings.

Add ATCWDT200 as reset device of AE350 platform, this driver requires SMU to program the reset vector registers before triggering WDT software restart signal.

dts example:

  smu at f0100000 {
    compatible = "andestech,atcsmu";
    reg = <0x00000000 0xf0100000 0x00000000 0x00001000>;
  };

  wdt: wdt at f0500000 {
    compatible = "andestech,atcwdt200";
    reg = <0x00000000 0xf0500000 0x00000000 0x00001000>;
    interrupts = <3 4>;
    interrupt-parent = <&plic0>;
    clock-frequency = <15000000>;
  };

Move Andes PLICSW ipi device to fdt ipi framework, this patch is based on Leo's modified IPI scheme on PLICSW.

Current IPI scheme uses bit 0 of pending reigster on PLICSW to send IPI from hart 0 to hart 7, but bit 0 needs to be hardwired to 0 according to spec. After some investigation, self-IPI seems to be seldom or never used, so we re-order the IPI scheme to support 8 core platforms.

dts example (Quad-core AX45MP):

  plicsw: interrupt-controller at e6400000 {
          compatible = "andestech,plicsw";
          reg = <0x00000000 0xe6400000 0x00000000 0x00400000>;
          interrupts-extended = <&CPU0_intc 3
                                 &CPU1_intc 3
                                 &CPU2_intc 3
                                 &CPU3_intc 3>;
          interrupt-controller;
          #address-cells = <2>;
          #interrupt-cells = <2>;
  };

Signed-off-by Yu Chien Peter Lin <[email protected]>

---
 include/sbi_utils/fdt/fdt_helper.h   |   3 +
 include/sbi_utils/ipi/andes_plicsw.h |  46 +++++++++
 lib/utils/fdt/fdt_helper.c           |  54 +++++++++++
 lib/utils/ipi/Kconfig                |   9 ++
 lib/utils/ipi/andes_plicsw.c         | 137 ++++++++++++++++++++++++++
 lib/utils/ipi/fdt_ipi_plicsw.c       |  47 +++++++++
 lib/utils/ipi/objects.mk             |   4 +
 platform/andes/ae350/Kconfig         |   2 +
 platform/andes/ae350/objects.mk      |   2 +-
 platform/andes/ae350/platform.c      |  27 +-----
 platform/andes/ae350/platform.h      |   2 -
 platform/andes/ae350/plicsw.c        | 139 ---------------------------
 platform/andes/ae350/plicsw.h        |  44 ---------
 13 files changed, 305 insertions(+), 211 deletions(-)
 create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
 create mode 100644 lib/utils/ipi/andes_plicsw.c
 create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
 delete mode 100644 platform/andes/ae350/plicsw.c
 delete mode 100644 platform/andes/ae350/plicsw.h

include/sbi_utils/fdt/fdt_helper.h +3 -0

index 7ef63c9..c39f77a 100644
@@ -98,6 +98,9 @@ int fdt_parse_aclint_node(void *fdt, int nodeoffset, bool for_timer,
 int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
                          unsigned long *plmt_size, u32 *hart_count);
 
+int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
+                         unsigned long *size, u32 *hart_count);
+
 int fdt_parse_compat_addr(void *fdt, uint64_t *addr,
                          const char *compatible);
 

include/sbi_utils/ipi/andes_plicsw.h new file +46 -0

new file mode 100644
index 0000000..e93cda0
@@ -0,0 +1,46 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2022 Andes Technology Corporation
+ *
+ * Authors:
+ *   Zong Li <[email protected]>
+ *   Nylon Chen <[email protected]>
+ *   Leo Yu-Chi Liang <[email protected]>
+ *   Yu Chien Peter Lin <[email protected]>
+ */
+
+#ifndef _IPI_ANDES_PLICSW_H_
+#define _IPI_ANDES_PLICSW_H_
+
+#define PLICSW_PRIORITY_BASE 0x4
+
+#define PLICSW_PENDING_BASE 0x1000
+#define PLICSW_PENDING_STRIDE 0x8
+
+#define PLICSW_ENABLE_BASE 0x2000
+#define PLICSW_ENABLE_STRIDE 0x80
+
+#define PLICSW_CONTEXT_BASE 0x200000
+#define PLICSW_CONTEXT_STRIDE 0x1000
+#define PLICSW_CONTEXT_CLAIM 0x4
+
+#define PLICSW_HART_MASK 0x01010101
+
+#define PLICSW_HART_MAX_NR 8
+
+#define PLICSW_REGION_ALIGN 0x1000
+
+struct plicsw_data {
+       unsigned long addr;
+       unsigned long size;
+       uint32_t hart_count;
+       /* hart id to source id table */
+       uint32_t source_id[PLICSW_HART_MAX_NR];
+};
+
+int plicsw_warm_ipi_init(void);
+
+int plicsw_cold_ipi_init(struct plicsw_data *plicsw);
+
+#endif /* _IPI_ANDES_PLICSW_H_ */

lib/utils/fdt/fdt_helper.c +54 -0

index ce52fca..d390436 100644
@@ -889,6 +889,60 @@ int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
        return 0;
 }
 
+int fdt_parse_plicsw_node(void *fdt, int nodeoffset, unsigned long *plicsw_base,
+                         unsigned long *size, u32 *hart_count)
+{
+       const fdt32_t *val;
+       int rc, i, count;
+       uint64_t reg_addr, reg_size, cpu_offset, cpu_intc_offset;
+       u32 phandle, hwirq, hartid, hcount;
+
+       if (nodeoffset < 0 || !fdt || !plicsw_base ||
+           !hart_count || !size)
+               return SBI_EINVAL;
+
+       rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
+                                   &reg_addr, &reg_size);
+       if (rc < 0 || !plicsw_base || !size)
+               return SBI_ENODEV;
+       *plicsw_base = reg_addr;
+       *size = reg_size;
+
+       val = fdt_getprop(fdt, nodeoffset, "interrupts-extended", &count);
+       if (!val || count < sizeof(fdt32_t))
+               return 0;
+       count = count / sizeof(fdt32_t);
+
+       hcount = 0;
+       for (i = 0; i < (count / 2); i++) {
+               phandle = fdt32_to_cpu(val[2 * i]);
+               hwirq = fdt32_to_cpu(val[2 * i + 1]);
+
+               cpu_intc_offset = fdt_node_offset_by_phandle(fdt, phandle);
+               if (cpu_intc_offset < 0)
+                       continue;
+
+               cpu_offset = fdt_parent_offset(fdt, cpu_intc_offset);
+               if (cpu_intc_offset < 0)
+                       continue;
+
+               rc = fdt_parse_hart_id(fdt, cpu_offset, &hartid);
+
+               if (rc)
+                       continue;
+
+               if (SBI_HARTMASK_MAX_BITS <= hartid)
+                       continue;
+
+               if (hwirq == IRQ_M_SOFT)
+                       hcount++;
+       }
+
+       *hart_count = hcount;
+
+       return 0;
+}
+
 int fdt_parse_compat_addr(void *fdt, uint64_t *addr,
                          const char *compatible)
 {

lib/utils/ipi/Kconfig +9 -0

index efc8960..3718852 100644
@@ -14,10 +14,19 @@ config FDT_IPI_MSWI
        select IPI_MSWI
        default n
 
+config FDT_IPI_PLICSW
+       bool "Andes PLICSW FDT driver"
+       select IPI_PLICSW
+       default n
+
 endif
 
 config IPI_MSWI
        bool "ACLINT MSWI support"
        default n
 
+config IPI_PLICSW
+       bool "Andes PLICSW support"
+       default n
+
 endmenu

lib/utils/ipi/andes_plicsw.c new file +137 -0

new file mode 100644
index 0000000..db25ae2
@@ -0,0 +1,137 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2022 Andes Technology Corporation
+ *
+ * Authors:
+ *   Zong Li <[email protected]>
+ *   Nylon Chen <[email protected]>
+ *   Leo Yu-Chi Liang <[email protected]>
+ *   Yu Chien Peter Lin <[email protected]>
+ */
+
+#include <sbi/riscv_asm.h>
+#include <sbi/riscv_io.h>
+#include <sbi/sbi_domain.h>
+#include <sbi/sbi_ipi.h>
+#include <sbi_utils/ipi/andes_plicsw.h>
+
+struct plicsw_data plicsw;
+
+static inline void plicsw_claim(void)
+{
+       u32 hartid = current_hartid();
+
+       if (plicsw.hart_count <= hartid)
+               ebreak();
+
+       plicsw.source_id[hartid] =
+               readl((void *)plicsw.addr + PLICSW_CONTEXT_BASE +
+                     PLICSW_CONTEXT_CLAIM + PLICSW_CONTEXT_STRIDE * hartid);
+}
+
+static inline void plicsw_complete(void)
+{
+       u32 hartid = current_hartid();
+       u32 source = plicsw.source_id[hartid];
+
+       writel(source, (void *)plicsw.addr + PLICSW_CONTEXT_BASE +
+                              PLICSW_CONTEXT_CLAIM +
+                              PLICSW_CONTEXT_STRIDE * hartid);
+}
+
+static inline void plic_sw_pending(u32 target_hart)
+{
+       /*
+        * The pending array registers are w1s type.
+        * IPI pending array mapping as following:
+        *
+        * Pending array start address: base + 0x1000
+        * ---------------------------------
+        * | hart3 | hart2 | hart1 | hart0 |
+        * ---------------------------------
+        * Each hartX can send IPI to another hart by setting the
+        * bitY to its own region (see the below).
+        *
+        * In each hartX region:
+        * <---------- PICSW_PENDING_STRIDE -------->
+        * | bit7 | ... | bit3 | bit2 | bit1 | bit0 |
+        * ------------------------------------------
+        * The bitY of hartX region indicates that hartX sends an
+        * IPI to hartY.
+        */
+       u32 hartid          = current_hartid();
+       u32 word_index      = hartid / 4;
+       u32 per_hart_offset = PLICSW_PENDING_STRIDE * hartid;
+       u32 val             = 1 << target_hart << per_hart_offset;
+
+       writel(val, (void *)plicsw.addr + PLICSW_PENDING_BASE + word_index * 4);
+}
+
+static void plicsw_ipi_send(u32 target_hart)
+{
+       if (plicsw.hart_count <= target_hart)
+               ebreak();
+
+       /* Set PLICSW IPI */
+       plic_sw_pending(target_hart);
+}
+
+static void plicsw_ipi_clear(u32 target_hart)
+{
+       if (plicsw.hart_count <= target_hart)
+               ebreak();
+
+       /* Clear PLICSW IPI */
+       plicsw_claim();
+       plicsw_complete();
+}
+
+static struct sbi_ipi_device plicsw_ipi = {
+       .name      = "andes_plicsw",
+       .ipi_send  = plicsw_ipi_send,
+       .ipi_clear = plicsw_ipi_clear
+};
+
+int plicsw_warm_ipi_init(void)
+{
+       u32 hartid = current_hartid();
+
+       /* Clear PLICSW IPI */
+       plicsw_ipi_clear(hartid);
+
+       return 0;
+}
+
+int plicsw_cold_ipi_init(struct plicsw_data *plicsw)
+{
+       int rc;
+
+       /* Setup source priority */
+       uint32_t *priority = (void *)plicsw->addr + PLICSW_PRIORITY_BASE;
+
+       for (int i = 0; i < plicsw->hart_count; i++)
+               writel(1, &priority[i]);
+
+       /* Setup target enable */
+       uint32_t enable_mask = PLICSW_HART_MASK;
+
+       for (int i = 0; i < plicsw->hart_count; i++) {
+               uint32_t *enable = (void *)plicsw->addr + PLICSW_ENABLE_BASE +
+                                  PLICSW_ENABLE_STRIDE * i;
+               writel(enable_mask, enable);
+               writel(enable_mask, enable + 1);
+               enable_mask <<= 1;
+       }
+
+       /* Add PLICSW region to the root domain */
+       rc = sbi_domain_root_add_memrange(plicsw->addr, plicsw->size,
+                                         PLICSW_REGION_ALIGN,
+                                         SBI_DOMAIN_MEMREGION_MMIO);
+       if (rc)
+               return rc;
+
+       sbi_ipi_set_device(&plicsw_ipi);
+
+       return 0;
+}

lib/utils/ipi/fdt_ipi_plicsw.c new file +47 -0

new file mode 100644
index 0000000..bf29bfc
@@ -0,0 +1,47 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2022 Andes Technology Corporation
+ *
+ * Authors:
+ *   Zong Li <[email protected]>
+ *   Nylon Chen <[email protected]>
+ *   Leo Yu-Chi Liang <[email protected]>
+ *   Yu Chien Peter Lin <[email protected]>
+ */
+
+#include <sbi/riscv_io.h>
+#include <sbi_utils/fdt/fdt_helper.h>
+#include <sbi_utils/ipi/fdt_ipi.h>
+#include <sbi_utils/ipi/andes_plicsw.h>
+
+extern struct plicsw_data plicsw;
+
+int fdt_plicsw_cold_ipi_init(void *fdt, int nodeoff,
+                               const struct fdt_match *match)
+{
+       int rc;
+
+       rc = fdt_parse_plicsw_node(fdt, nodeoff, &plicsw.addr, &plicsw.size,
+                                  &plicsw.hart_count);
+       if (rc)
+               return rc;
+
+       rc = plicsw_cold_ipi_init(&plicsw);
+       if (rc)
+               return rc;
+
+       return 0;
+}
+
+static const struct fdt_match ipi_plicsw_match[] = {
+       { .compatible = "andestech,plicsw" },
+       {},
+};
+
+struct fdt_ipi fdt_ipi_plicsw = {
+       .match_table = ipi_plicsw_match,
+       .cold_init   = fdt_plicsw_cold_ipi_init,
+       .warm_init   = plicsw_warm_ipi_init,
+       .exit        = NULL,
+};

lib/utils/ipi/objects.mk +4 -0

index 0600cac..999e7bd 100644
@@ -8,9 +8,13 @@
 #
 
 libsbiutils-objs-$(CONFIG_IPI_MSWI) += ipi/aclint_mswi.o
+libsbiutils-objs-$(CONFIG_IPI_PLICSW) += ipi/andes_plicsw.o
 
 libsbiutils-objs-$(CONFIG_FDT_IPI) += ipi/fdt_ipi.o
 libsbiutils-objs-$(CONFIG_FDT_IPI) += ipi/fdt_ipi_drivers.o
 
 carray-fdt_ipi_drivers-$(CONFIG_FDT_IPI_MSWI) += fdt_ipi_mswi
 libsbiutils-objs-$(CONFIG_FDT_IPI_MSWI) += ipi/fdt_ipi_mswi.o
+
+carray-fdt_ipi_drivers-$(CONFIG_FDT_IPI_PLICSW) += fdt_ipi_plicsw
+libsbiutils-objs-$(CONFIG_FDT_IPI_PLICSW) += ipi/fdt_ipi_plicsw.o

platform/andes/ae350/Kconfig +2 -0

index 084b27f..38e4001 100644
@@ -11,6 +11,8 @@ config PLATFORM_ANDES_AE350
        select FDT_RESET_ATCWDT200
        select FDT_IRQCHIP
        select FDT_IRQCHIP_PLIC
+       select FDT_IPI
+       select FDT_IPI_PLICSW
        default y
 
 if PLATFORM_ANDES_AE350

platform/andes/ae350/objects.mk +1 -1

index 1ccb894..a4a2332 100644
@@ -15,7 +15,7 @@ platform-asflags-y =
 platform-ldflags-y =
 
 # Objects to build
-platform-objs-y += cache.o platform.o plicsw.o
+platform-objs-y += cache.o platform.o
 
 # Blobs to build
 FW_TEXT_START=0x00000000

platform/andes/ae350/platform.c +2 -25

index cf5417c..30e4606 100644
@@ -25,8 +25,8 @@
 #include <sbi_utils/reset/fdt_reset.h>
 #include <sbi_utils/serial/fdt_serial.h>
 #include <sbi_utils/timer/fdt_timer.h>
+#include <sbi_utils/ipi/fdt_ipi.h>
 #include "platform.h"
-#include "plicsw.h"
 #include "cache.h"
 
 struct sbi_platform platform;

@@ -88,29 +88,6 @@ static int ae350_final_init(bool cold_boot)
        return 0;
 }
 
-static struct sbi_ipi_device plicsw_ipi = {
-       .name = "ae350_plicsw",
-       .ipi_send = plicsw_ipi_send,
-       .ipi_clear = plicsw_ipi_clear
-};
-
-/* Initialize IPI for current HART. */
-static int ae350_ipi_init(bool cold_boot)
-{
-       int ret;
-
-       if (cold_boot) {
-               ret = plicsw_cold_ipi_init(AE350_PLICSW_ADDR,
-                                          AE350_HART_COUNT);
-               if (ret)
-                       return ret;
-
-               sbi_ipi_set_device(&plicsw_ipi);
-       }
-
-       return plicsw_warm_ipi_init();
-}
-
 /* Vendor-Specific SBI handler */
 static int ae350_vendor_ext_provider(long extid, long funcid,
        const struct sbi_trap_regs *regs, unsigned long *out_value,

@@ -163,7 +140,7 @@ const struct sbi_platform_operations platform_ops = {
 
        .irqchip_init = fdt_irqchip_init,
 
-       .ipi_init     = ae350_ipi_init,
+       .ipi_init     = fdt_ipi_init,
 
        .timer_init        = fdt_timer_init,
 

platform/andes/ae350/platform.h +0 -2

index 3003bb4..903bef0 100644
@@ -11,8 +11,6 @@
 #ifndef _AE350_PLATFORM_H_
 #define _AE350_PLATFORM_H_
 
-#define AE350_PLICSW_ADDR              0xe6400000
-
 #define AE350_L2C_ADDR                 0xe0500000
 
 /*Memory and Miscellaneous Registers*/

platform/andes/ae350/plicsw.c deleted +0 -139

deleted file mode 100644
index edbbb11..0000000
@@ -1,139 +0,0 @@
-/*
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Copyright (c) 2019 Andes Technology Corporation
- *
- * Authors:
- *   Zong Li <[email protected]>
- *   Nylon Chen <[email protected]>
- */
-
-#include <sbi/riscv_asm.h>
-#include <sbi/riscv_io.h>
-#include <sbi/sbi_types.h>
-#include "plicsw.h"
-#include "platform.h"
-
-static u32 plicsw_ipi_hart_count;
-static struct plicsw plicsw_dev[AE350_HART_COUNT];
-
-static inline void plicsw_claim(void)
-{
-       u32 source_hart = current_hartid();
-
-       plicsw_dev[source_hart].source_id =
-               readl(plicsw_dev[source_hart].plicsw_claim);
-}
-
-static inline void plicsw_complete(void)
-{
-       u32 source_hart = current_hartid();
-       u32 source = plicsw_dev[source_hart].source_id;
-
-       writel(source, plicsw_dev[source_hart].plicsw_claim);
-}
-
-static inline void plic_sw_pending(u32 target_hart)
-{
-       /*
-        * The pending array registers are w1s type.
-        * IPI pending array mapping as following:
-        *
-        * Pending array start address: base + 0x1000
-        * -------------------------------------
-        * | hart 3 | hart 2 | hart 1 | hart 0 |
-        * -------------------------------------
-        * Each hart X can send IPI to another hart by setting the
-        * corresponding bit in hart X own region(see the below).
-        *
-        * In each hart region:
-        * -----------------------------------------------
-        * | bit 7 | bit 6 | bit 5 | bit 4 | ... | bit 0 |
-        * -----------------------------------------------
-        * The bit 7 is used to send IPI to hart 0
-        * The bit 6 is used to send IPI to hart 1
-        * The bit 5 is used to send IPI to hart 2
-        * The bit 4 is used to send IPI to hart 3
-        */
-       u32 source_hart = current_hartid();
-       u32 target_offset = (PLICSW_PENDING_PER_HART - 1) - target_hart;
-       u32 per_hart_offset = PLICSW_PENDING_PER_HART * source_hart;
-       u32 val = 1 << target_offset << per_hart_offset;
-
-       writel(val, plicsw_dev[source_hart].plicsw_pending);
-}
-
-void plicsw_ipi_send(u32 target_hart)
-{
-       if (plicsw_ipi_hart_count <= target_hart)
-               return;
-
-       /* Set PLICSW IPI */
-       plic_sw_pending(target_hart);
-}
-
-void plicsw_ipi_clear(u32 target_hart)
-{
-       if (plicsw_ipi_hart_count <= target_hart)
-               return;
-
-       /* Clear PLICSW IPI */
-       plicsw_claim();
-       plicsw_complete();
-}
-
-int plicsw_warm_ipi_init(void)
-{
-       u32 hartid = current_hartid();
-
-       if (!plicsw_dev[hartid].plicsw_pending
-           && !plicsw_dev[hartid].plicsw_enable
-           && !plicsw_dev[hartid].plicsw_claim)
-               return -1;
-
-       /* Clear PLICSW IPI */
-       plicsw_ipi_clear(hartid);
-
-       return 0;
-}
-
-int plicsw_cold_ipi_init(unsigned long base, u32 hart_count)
-{
-       /* Setup source priority */
-       uint32_t *priority = (void *)base + PLICSW_PRIORITY_BASE;
-
-       for (int i = 0; i < AE350_HART_COUNT * PLICSW_PENDING_PER_HART; i++)
-               writel(1, &priority[i]);
-
-       /* Setup target enable */
-       uint32_t enable_mask = PLICSW_HART_MASK;
-
-       for (int i = 0; i < AE350_HART_COUNT; i++) {
-               uint32_t *enable = (void *)base + PLICSW_ENABLE_BASE
-                       + PLICSW_ENABLE_PER_HART * i;
-               writel(enable_mask, &enable[0]);
-               enable_mask >>= 1;
-       }
-
-       /* Figure-out PLICSW IPI register address */
-       plicsw_ipi_hart_count = hart_count;
-
-       for (u32 hartid = 0; hartid < AE350_HART_COUNT; hartid++) {
-               plicsw_dev[hartid].source_id = 0;
-               plicsw_dev[hartid].plicsw_pending =
-                       (void *)base
-                       + PLICSW_PENDING_BASE
-                       + ((hartid / 4) * 4);
-               plicsw_dev[hartid].plicsw_enable  =
-                       (void *)base
-                       + PLICSW_ENABLE_BASE
-                       + PLICSW_ENABLE_PER_HART * hartid;
-               plicsw_dev[hartid].plicsw_claim   =
-                       (void *)base
-                       + PLICSW_CONTEXT_BASE
-                       + PLICSW_CONTEXT_CLAIM
-                       + PLICSW_CONTEXT_PER_HART * hartid;
-       }
-
-       return 0;
-}

platform/andes/ae350/plicsw.h deleted +0 -45

deleted file mode 100644
index 58728f6..0000000
@@ -1,44 +0,0 @@
-/*
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Copyright (c) 2019 Andes Technology Corporation
- *
- * Authors:
- *   Zong Li <[email protected]>
- *   Nylon Chen <[email protected]>
- */
-
-#ifndef _AE350_PLICSW_H_
-#define _AE350_PLICSW_H_
-
-#define PLICSW_PRIORITY_BASE           0x4
-
-#define PLICSW_PENDING_BASE            0x1000
-#define PLICSW_PENDING_PER_HART                0x8
-
-#define PLICSW_ENABLE_BASE             0x2000
-#define PLICSW_ENABLE_PER_HART         0x80
-
-#define PLICSW_CONTEXT_BASE            0x200000
-#define PLICSW_CONTEXT_PER_HART                0x1000
-#define PLICSW_CONTEXT_CLAIM           0x4
-
-#define PLICSW_HART_MASK               0x80808080
-
-struct plicsw {
-       u32 source_id;
-
-       volatile uint32_t *plicsw_pending;
-       volatile uint32_t *plicsw_enable;
-       volatile uint32_t *plicsw_claim;
-};
-
-void plicsw_ipi_send(u32 target_hart);
-
-void plicsw_ipi_clear(u32 target_hart);
-
-int plicsw_warm_ipi_init(void);
-
-int plicsw_cold_ipi_init(unsigned long base, u32 hart_count);
-
-#endif /* _AE350_PLICSW_H_ */
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

We move the devices used by AE350 to fdt driver framework, reuse existing driver and add timer, ipi and reset drivers. The OpenSBI domain support is also enabled. As of now, we can use Kconfig to manage these configurations that can support variants of Andes platform and SoC like Kendryte K510.

Changes v1 -> v2
- Rename Andes-specific devices (PLMT, PLICSW) compatible string
- Update ISA string in CPU nodes
Changes v2 -> v3
- Add PATCH1 and PATCH2 to fix typos
- Add PATCH3 which factors out a function for adding mtimer and ipi device regions to root domain
- Remove unused variable "num_src" and "riscv,ndev" property of ipi device node
Changes v3 -> v4
- Add PATCH3 to fix grammar
- Move plmt_{cold,warm}_timer_init() to andes_plmt.c and define fdt_plmt_cold_timer_init() which adds PLMT region and sets timer device
- Move plicsw_{cold,warm}_ipi_init() to andes_plicsw.c and define fdt_plicsw_cold_ipi_init() which adds PLICSW region and sets ipi device

Yu Chien Peter Lin (14):
include: sbi: Fix typo in comment
lib: sbi: Fix typo in comment
include: sbi: Fix grammar in comment
lib: sbi: Add sbi_domain_root_add_memrange() API
platform: andes/ae350: Remove enabling cache from an350_final_init
platform: andes/ae350: Use kconfig to set platform version and default
name
platform: andes/ae350: Use fdt serial driver
lib: utils/timer: Add Andes fdt timer support

lib: utils/reset: Add Andes fdt reset driver support
platform: andes/ae350: Use fdt irqchip driver
platform: andes/ae350: Add fw_platform_init for platform
initialization
lib: utils/ipi: Add Andes fdt ipi driver support
platform: andes/ae350: Add AE350 domain support
docs: andes-ae350.md: Update ae350 documentation for fdt driver
support

 docs/platform/andes-ae350.md            | 184 +++++++++++++++++++++++-
 include/sbi/sbi_domain.h                |  16 ++-
 include/sbi/sbi_scratch.h               |   2 +-
 include/sbi_utils/fdt/fdt_helper.h      |   6 +
 include/sbi_utils/ipi/andes_plicsw.h    |  46 ++++++
 include/sbi_utils/timer/aclint_mtimer.h |   2 +
 include/sbi_utils/timer/andes_plmt.h    |  29 ++++
 lib/sbi/sbi_domain.c                    |  27 ++++
 lib/sbi/sbi_expected_trap.S             |   2 +-
 lib/utils/fdt/fdt_helper.c              | 108 ++++++++++++++
 lib/utils/ipi/Kconfig                   |   9 ++
 lib/utils/ipi/andes_plicsw.c            | 137 ++++++++++++++++++
 lib/utils/ipi/fdt_ipi_plicsw.c          |  47 ++++++
 lib/utils/ipi/objects.mk                |   4 +
 lib/utils/reset/Kconfig                 |   4 +
 lib/utils/reset/fdt_reset_atcwdt200.c   | 122 ++++++++++++++++
 lib/utils/reset/objects.mk              |   3 +
 lib/utils/timer/Kconfig                 |   9 ++
 lib/utils/timer/aclint_mtimer.c         |  50 ++-----
 lib/utils/timer/andes_plmt.c            | 104 ++++++++++++++
 lib/utils/timer/fdt_timer_plmt.c        |  51 +++++++
 lib/utils/timer/objects.mk              |   4 +
 platform/andes/ae350/Kconfig            |  30 +++-
 platform/andes/ae350/objects.mk         |   2 +-
 platform/andes/ae350/platform.c         | 165 +++++++++------------
 platform/andes/ae350/platform.h         |  17 ---
 platform/andes/ae350/plicsw.c           | 139 ------------------
 platform/andes/ae350/plicsw.h           |  44 ------
 platform/andes/ae350/plmt.c             | 107 --------------
 platform/andes/ae350/plmt.h             |  17 ---
 30 files changed, 1021 insertions(+), 466 deletions(-)
 create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
 create mode 100644 include/sbi_utils/timer/andes_plmt.h
 create mode 100644 lib/utils/ipi/andes_plicsw.c
 create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
 create mode 100644 lib/utils/reset/fdt_reset_atcwdt200.c
 create mode 100644 lib/utils/timer/andes_plmt.c
 create mode 100644 lib/utils/timer/fdt_timer_plmt.c
 delete mode 100644 platform/andes/ae350/plicsw.c
 delete mode 100644 platform/andes/ae350/plicsw.h
 delete mode 100644 platform/andes/ae350/plmt.c
 delete mode 100644 platform/andes/ae350/plmt.h

--
2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Quoted 36 lines · Yu Chien Peter Lin

We move the devices used by AE350 to fdt driver framework, reuse existing driver and add timer, ipi and reset drivers. The OpenSBI domain support is also enabled. As of now, we can use Kconfig to manage these configurations that can support variants of Andes platform and SoC like Kendryte K510.

Changes v1 -> v2
- Rename Andes-specific devices (PLMT, PLICSW) compatible string
- Update ISA string in CPU nodes
Changes v2 -> v3
- Add PATCH1 and PATCH2 to fix typos
- Add PATCH3 which factors out a function for adding mtimer and ipi device regions to root domain
- Remove unused variable "num_src" and "riscv,ndev" property of ipi device node
Changes v3 -> v4
- Add PATCH3 to fix grammar
- Move plmt_{cold,warm}_timer_init() to andes_plmt.c and define fdt_plmt_cold_timer_init() which adds PLMT region and sets timer device
- Move plicsw_{cold,warm}_ipi_init() to andes_plicsw.c and define fdt_plicsw_cold_ipi_init() which adds PLICSW region and sets ipi device

Yu Chien Peter Lin (14):
include: sbi: Fix typo in comment
lib: sbi: Fix typo in comment
include: sbi: Fix grammar in comment
lib: sbi: Add sbi_domain_root_add_memrange() API
platform: andes/ae350: Remove enabling cache from an350_final_init
platform: andes/ae350: Use kconfig to set platform version and default
name
platform: andes/ae350: Use fdt serial driver
lib: utils/timer: Add Andes fdt timer support

Quoted 8 lines

lib: utils/reset: Add Andes fdt reset driver support
platform: andes/ae350: Use fdt irqchip driver
platform: andes/ae350: Add fw_platform_init for platform
initialization
lib: utils/ipi: Add Andes fdt ipi driver support
platform: andes/ae350: Add AE350 domain support
docs: andes-ae350.md: Update ae350 documentation for fdt driver
support

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Thanks,
Anup

Quoted 53 lines

 docs/platform/andes-ae350.md            | 184 +++++++++++++++++++++++-
 include/sbi/sbi_domain.h                |  16 ++-
 include/sbi/sbi_scratch.h               |   2 +-
 include/sbi_utils/fdt/fdt_helper.h      |   6 +
 include/sbi_utils/ipi/andes_plicsw.h    |  46 ++++++
 include/sbi_utils/timer/aclint_mtimer.h |   2 +
 include/sbi_utils/timer/andes_plmt.h    |  29 ++++
 lib/sbi/sbi_domain.c                    |  27 ++++
 lib/sbi/sbi_expected_trap.S             |   2 +-
 lib/utils/fdt/fdt_helper.c              | 108 ++++++++++++++
 lib/utils/ipi/Kconfig                   |   9 ++
 lib/utils/ipi/andes_plicsw.c            | 137 ++++++++++++++++++
 lib/utils/ipi/fdt_ipi_plicsw.c          |  47 ++++++
 lib/utils/ipi/objects.mk                |   4 +
 lib/utils/reset/Kconfig                 |   4 +
 lib/utils/reset/fdt_reset_atcwdt200.c   | 122 ++++++++++++++++
 lib/utils/reset/objects.mk              |   3 +
 lib/utils/timer/Kconfig                 |   9 ++
 lib/utils/timer/aclint_mtimer.c         |  50 ++-----
 lib/utils/timer/andes_plmt.c            | 104 ++++++++++++++
 lib/utils/timer/fdt_timer_plmt.c        |  51 +++++++
 lib/utils/timer/objects.mk              |   4 +
 platform/andes/ae350/Kconfig            |  30 +++-
 platform/andes/ae350/objects.mk         |   2 +-
 platform/andes/ae350/platform.c         | 165 +++++++++------------
 platform/andes/ae350/platform.h         |  17 ---
 platform/andes/ae350/plicsw.c           | 139 ------------------
 platform/andes/ae350/plicsw.h           |  44 ------
 platform/andes/ae350/plmt.c             | 107 --------------
 platform/andes/ae350/plmt.h             |  17 ---
 30 files changed, 1021 insertions(+), 466 deletions(-)
 create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
 create mode 100644 include/sbi_utils/timer/andes_plmt.h
 create mode 100644 lib/utils/ipi/andes_plicsw.c
 create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
 create mode 100644 lib/utils/reset/fdt_reset_atcwdt200.c
 create mode 100644 lib/utils/timer/andes_plmt.c
 create mode 100644 lib/utils/timer/fdt_timer_plmt.c
 delete mode 100644 platform/andes/ae350/plicsw.c
 delete mode 100644 platform/andes/ae350/plicsw.h
 delete mode 100644 platform/andes/ae350/plmt.c
 delete mode 100644 platform/andes/ae350/plmt.h

--
2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Quoted 51 lines · Anup Patel

Quoted 36 lines · Yu Chien Peter Lin

We move the devices used by AE350 to fdt driver framework, reuse existing driver and add timer, ipi and reset drivers. The OpenSBI domain support is also enabled. As of now, we can use Kconfig to manage these configurations that can support variants of Andes platform and SoC like Kendryte K510.

Changes v1 -> v2
- Rename Andes-specific devices (PLMT, PLICSW) compatible string
- Update ISA string in CPU nodes
Changes v2 -> v3
- Add PATCH1 and PATCH2 to fix typos
- Add PATCH3 which factors out a function for adding mtimer and ipi device regions to root domain
- Remove unused variable "num_src" and "riscv,ndev" property of ipi device node
Changes v3 -> v4
- Add PATCH3 to fix grammar
- Move plmt_{cold,warm}_timer_init() to andes_plmt.c and define fdt_plmt_cold_timer_init() which adds PLMT region and sets timer device
- Move plicsw_{cold,warm}_ipi_init() to andes_plicsw.c and define fdt_plicsw_cold_ipi_init() which adds PLICSW region and sets ipi device

Yu Chien Peter Lin (14):
include: sbi: Fix typo in comment
lib: sbi: Fix typo in comment
include: sbi: Fix grammar in comment
lib: sbi: Add sbi_domain_root_add_memrange() API
platform: andes/ae350: Remove enabling cache from an350_final_init
platform: andes/ae350: Use kconfig to set platform version and default
name
platform: andes/ae350: Use fdt serial driver
lib: utils/timer: Add Andes fdt timer support

Quoted 8 lines

lib: utils/reset: Add Andes fdt reset driver support
platform: andes/ae350: Use fdt irqchip driver
platform: andes/ae350: Add fw_platform_init for platform
initialization
lib: utils/ipi: Add Andes fdt ipi driver support
platform: andes/ae350: Add AE350 domain support
docs: andes-ae350.md: Update ae350 documentation for fdt driver
support

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Quoted 2 lines

Thanks,
Anup

Hi Anup,

The FW_TEXT_START on AE350 is different from generic platform. Will it become a configuration?

I think we would keep the platform specific files in andes/ae350 until we have to refactor the duplicate code.

Thanks,
Peter Lin

Quoted 53 lines

Quoted 53 lines

 docs/platform/andes-ae350.md            | 184 +++++++++++++++++++++++-
 include/sbi/sbi_domain.h                |  16 ++-
 include/sbi/sbi_scratch.h               |   2 +-
 include/sbi_utils/fdt/fdt_helper.h      |   6 +
 include/sbi_utils/ipi/andes_plicsw.h    |  46 ++++++
 include/sbi_utils/timer/aclint_mtimer.h |   2 +
 include/sbi_utils/timer/andes_plmt.h    |  29 ++++
 lib/sbi/sbi_domain.c                    |  27 ++++
 lib/sbi/sbi_expected_trap.S             |   2 +-
 lib/utils/fdt/fdt_helper.c              | 108 ++++++++++++++
 lib/utils/ipi/Kconfig                   |   9 ++
 lib/utils/ipi/andes_plicsw.c            | 137 ++++++++++++++++++
 lib/utils/ipi/fdt_ipi_plicsw.c          |  47 ++++++
 lib/utils/ipi/objects.mk                |   4 +
 lib/utils/reset/Kconfig                 |   4 +
 lib/utils/reset/fdt_reset_atcwdt200.c   | 122 ++++++++++++++++
 lib/utils/reset/objects.mk              |   3 +
 lib/utils/timer/Kconfig                 |   9 ++
 lib/utils/timer/aclint_mtimer.c         |  50 ++-----
 lib/utils/timer/andes_plmt.c            | 104 ++++++++++++++
 lib/utils/timer/fdt_timer_plmt.c        |  51 +++++++
 lib/utils/timer/objects.mk              |   4 +
 platform/andes/ae350/Kconfig            |  30 +++-
 platform/andes/ae350/objects.mk         |   2 +-
 platform/andes/ae350/platform.c         | 165 +++++++++------------
 platform/andes/ae350/platform.h         |  17 ---
 platform/andes/ae350/plicsw.c           | 139 ------------------
 platform/andes/ae350/plicsw.h           |  44 ------
 platform/andes/ae350/plmt.c             | 107 --------------
 platform/andes/ae350/plmt.h             |  17 ---
 30 files changed, 1021 insertions(+), 466 deletions(-)
 create mode 100644 include/sbi_utils/ipi/andes_plicsw.h
 create mode 100644 include/sbi_utils/timer/andes_plmt.h
 create mode 100644 lib/utils/ipi/andes_plicsw.c
 create mode 100644 lib/utils/ipi/fdt_ipi_plicsw.c
 create mode 100644 lib/utils/reset/fdt_reset_atcwdt200.c
 create mode 100644 lib/utils/timer/andes_plmt.c
 create mode 100644 lib/utils/timer/fdt_timer_plmt.c
 delete mode 100644 platform/andes/ae350/plicsw.c
 delete mode 100644 platform/andes/ae350/plicsw.h
 delete mode 100644 platform/andes/ae350/plmt.c
 delete mode 100644 platform/andes/ae350/plmt.h

--
2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Quoted 11 lines

Quoted 6 lines

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Thanks,
Anup

Hi Anup,

The FW_TEXT_START on AE350 is different from generic platform. Will it become a configuration?

The generic platform has FW_PIC enabled so you can load and run generic firmwares from any address.

Quoted 2 lines

I think we would keep the platform specific files in andes/ae350 until we have to refactor the duplicate code.

You can add platform specific overrides to generic platforms which are selected based on compatible string in the root DT node.

Regards,
Anup

Quoted 22 lines

Quoted 5 lines

Quoted 5 lines

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Quoted 7 lines

Quoted 2 lines

Thanks,
Anup

Hi Anup,

The FW_TEXT_START on AE350 is different from generic platform. Will it become a configuration?

The generic platform has FW_PIC enabled so you can load and run generic firmwares from any address.

Quoted 2 lines

I think we would keep the platform specific files in andes/ae350 until we have to refactor the duplicate code.

You can add platform specific overrides to generic platforms which are selected based on compatible string in the root DT node.

Quoted 2 lines

Regards,
Anup

Hi Anup,

We're considering using generic platform, but I don't find any vendor_ext_provider is implemented as a platform_override hook, does it mean we need to drop it if we move to generic platform?

Best regards,
Peter Lin

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Quoted 22 lines · Yu-Chien Peter Lin

Quoted 22 lines

Quoted 5 lines

Quoted 5 lines

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Quoted 7 lines

Quoted 2 lines

Thanks,
Anup

Hi Anup,

The FW_TEXT_START on AE350 is different from generic platform. Will it become a configuration?

The generic platform has FW_PIC enabled so you can load and run generic firmwares from any address.

Quoted 2 lines

I think we would keep the platform specific files in andes/ae350 until we have to refactor the duplicate code.

You can add platform specific overrides to generic platforms which are selected based on compatible string in the root DT node.

Quoted 8 lines

Quoted 2 lines

Regards,
Anup

Hi Anup,

We're considering using generic platform, but I don't find any vendor_ext_provider is implemented as a platform_override hook, does it mean we need to drop it if we move to generic platform?

vendor_ext_provider() is available in platform_override of generic platform.

Please check the latest OpenSBI sources.

Regards,
Anup

Quoted 33 lines · Anup Patel

Quoted 22 lines · Yu-Chien Peter Lin

Quoted 22 lines

Quoted 5 lines

Quoted 5 lines

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Quoted 7 lines

Quoted 2 lines

Thanks,
Anup

Hi Anup,

The FW_TEXT_START on AE350 is different from generic platform. Will it become a configuration?

The generic platform has FW_PIC enabled so you can load and run generic firmwares from any address.

Quoted 2 lines

I think we would keep the platform specific files in andes/ae350 until we have to refactor the duplicate code.

You can add platform specific overrides to generic platforms which are selected based on compatible string in the root DT node.

Quoted 8 lines

Quoted 2 lines

Regards,
Anup

Hi Anup,

We're considering using generic platform, but I don't find any vendor_ext_provider is implemented as a platform_override hook, does it mean we need to drop it if we move to generic platform?

Quoted 8 lines

vendor_ext_provider() is available in platform_override of generic platform.

Please check the latest OpenSBI sources.

Regards,
Anup

Hi Anup,

Sorry, I know there is vendor_ext_provider in platform_override, except AE350, no other platforms actually have such hook in their platform specific files, so I was wondering if the current acceptance policy won't let us keep that function even in platform/andes/ae350.

Quoted 44 lines · Yu-Chien Peter Lin

Quoted 33 lines · Anup Patel

Quoted 22 lines · Yu-Chien Peter Lin

Quoted 22 lines

Quoted 5 lines

Quoted 5 lines

Applied this series to the riscv/opensbi repo.

Any thoughts on using the generic platform ?

Quoted 7 lines

Quoted 2 lines

Thanks,
Anup

Hi Anup,

The FW_TEXT_START on AE350 is different from generic platform. Will it become a configuration?

The generic platform has FW_PIC enabled so you can load and run generic firmwares from any address.

Quoted 2 lines

I think we would keep the platform specific files in andes/ae350 until we have to refactor the duplicate code.

You can add platform specific overrides to generic platforms which are selected based on compatible string in the root DT node.

Quoted 8 lines

Quoted 2 lines

Regards,
Anup

Hi Anup,

We're considering using generic platform, but I don't find any vendor_ext_provider is implemented as a platform_override hook, does it mean we need to drop it if we move to generic platform?

Quoted 8 lines

vendor_ext_provider() is available in platform_override of generic platform.

Please check the latest OpenSBI sources.

Regards,
Anup

Quoted 6 lines

Hi Anup,

Sorry, I know there is vendor_ext_provider in platform_override, except AE350, no other platforms actually have such hook in their platform specific files, so I was wondering if the current acceptance policy won't let us keep that function even in platform/andes/ae350.

We generally encourage platform vendors to prefer standard SBI extensions over custom SBI extensions but it might not be always possible to have a standard extension for everything out there.

In other words, we do expect a few platform_override to have vendor_ext_provider() for platform specific quirks and currently we are OKAY with it but platform vendor will have to maintain their vendor_ext_provider() implementation.

Regards,
Anup