diff mbox series

[v4,08/14] lib: utils/timer: Add Andes fdt timer support

Message ID 20221014003252.17765-9-peterlin@andestech.com
State Accepted
Headers show
Series Add Andes AE350 fdt driver support | expand

Commit Message

Yu Chien Peter Lin Oct. 14, 2022, 12:32 a.m. UTC
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@e6000000 {
          compatible = "andestech,plmt0";
          reg = <0x00 0xe6000000 0x00 0x100000>;
          interrupts-extended = <&cpu0_intc 0x07
                                 &cpu1_intc 0x07
                                 &cpu2_intc 0x07
                                 &cpu3_intc 0x07>;
      };
      ...
  }

Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 include/sbi_utils/fdt/fdt_helper.h   |   3 +
 include/sbi_utils/timer/andes_plmt.h |  29 ++++++++
 lib/utils/fdt/fdt_helper.c           |  54 ++++++++++++++
 lib/utils/timer/Kconfig              |   9 +++
 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         |   2 +
 platform/andes/ae350/objects.mk      |   2 +-
 platform/andes/ae350/platform.c      |  19 +----
 platform/andes/ae350/platform.h      |   2 -
 platform/andes/ae350/plmt.c          | 107 ---------------------------
 platform/andes/ae350/plmt.h          |  17 -----
 13 files changed, 259 insertions(+), 144 deletions(-)
 create mode 100644 include/sbi_utils/timer/andes_plmt.h
 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/plmt.c
 delete mode 100644 platform/andes/ae350/plmt.h

Comments

Samuel Holland Oct. 16, 2022, 7:15 p.m. UTC | #1
On 10/13/22 19:32, Yu Chien Peter Lin wrote:
> 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@e6000000 {
>           compatible = "andestech,plmt0";
>           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
Yu Chien Peter Lin Oct. 19, 2022, 9:23 p.m. UTC | #2
On Sun, Oct 16, 2022 at 02:15:42PM -0500, Samuel Holland wrote:
> On 10/13/22 19:32, Yu Chien Peter Lin wrote:
> > 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@e6000000 {
> >           compatible = "andestech,plmt0";
> >           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
> 

Hi Samuel,

Thanks for pointing this out!
As PLMT and PLICSW are not used by any driver in kernel, I think it's
better to maintain within our own repository. Here is the branch [1]
we've updated the schemas for these fdt drivers.
If this works for you, we'll send PATCH v5 to fix dt binding check
violation.

[1] https://github.com/andestech/linux/commits/v6.0.y_ae350-ax45mp/Documentation/devicetree/bindings
Anup Patel Oct. 21, 2022, 5:11 a.m. UTC | #3
On Mon, Oct 17, 2022 at 12:45 AM Samuel Holland <samuel@sholland.org> wrote:
>
> On 10/13/22 19:32, Yu Chien Peter Lin wrote:
> > 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@e6000000 {
> >           compatible = "andestech,plmt0";
> >           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.
>

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
Atish Patra Oct. 21, 2022, 6:01 a.m. UTC | #4
On Thu, Oct 20, 2022 at 10:12 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Oct 17, 2022 at 12:45 AM Samuel Holland <samuel@sholland.org> wrote:
> >
> > On 10/13/22 19:32, Yu Chien Peter Lin wrote:
> > > 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@e6000000 {
> > >           compatible = "andestech,plmt0";
> > >           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.
> >
>
> 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.
>

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.

> Regards,
> Anup
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
Samuel Holland Oct. 29, 2022, 3:51 p.m. UTC | #5
On 10/21/22 01:01, Atish Patra wrote:
> On Thu, Oct 20, 2022 at 10:12 PM Anup Patel <anup@brainfault.org> wrote:
>>
>> On Mon, Oct 17, 2022 at 12:45 AM Samuel Holland <samuel@sholland.org> wrote:
>>>
>>> On 10/13/22 19:32, Yu Chien Peter Lin wrote:
>>>> 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@e6000000 {
>>>>           compatible = "andestech,plmt0";
>>>>           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.
>>>
>>
>> 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.
>>
> 
> 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.

Bindings still need to be in the Linux kernel (or dt-schema) repository
for several reasons. Most importantly, the DT maintainers there provide
a thorough review and enforce backward compatibility. If a binding is
only documented in some downstream repository, there is nothing ensuring
its validity or stability. Bindings hosted elsewhere cannot be trusted.

Linux is the de facto "upstream" for both devicetrees and DT bindings,
even when no Linux driver exists. U-Boot's policy is to use DTS files
from Linux verbatim. When using the standard U-Boot boot flow, that
devicetree is passed at runtime from U-Boot SPL to OpenSBI. That means
any (static) node used by either U-Boot or OpenSBI needs to be in the
Linux DTS. And any node in the Linux DTS needs to have its binding
documented in the Linux repository (or the dt-schema repository).

See for example the U-Boot patch driven by this very thread:
https://lore.kernel.org/u-boot/20221025150350.19300-1-peterlin@andestech.com/

The drivers in OpenSBI consume DT nodes which are provided by U-Boot. So
unless the bindings are upstreamed and can be considered stable, we have
now introduced a version dependency between OpenSBI and U-Boot.

Finally, Linux supports running in M-mode. And so does U-Boot, which
again takes its DTSs from Linux. So a device being limited to M-mode is
not a good reason to exclude it from the Linux DTS or DT bindings.

Regards,
Samuel
diff mbox series

Patch

diff --git a/include/sbi_utils/fdt/fdt_helper.h b/include/sbi_utils/fdt/fdt_helper.h
index bcd4996..7ef63c9 100644
--- a/include/sbi_utils/fdt/fdt_helper.h
+++ b/include/sbi_utils/fdt/fdt_helper.h
@@ -95,6 +95,9 @@  int fdt_parse_aclint_node(void *fdt, int nodeoffset, bool for_timer,
 			  unsigned long *out_addr2, unsigned long *out_size2,
 			  u32 *out_first_hartid, u32 *out_hart_count);
 
+int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
+			  unsigned long *plmt_size, u32 *hart_count);
+
 int fdt_parse_compat_addr(void *fdt, uint64_t *addr,
 			  const char *compatible);
 
diff --git a/include/sbi_utils/timer/andes_plmt.h b/include/sbi_utils/timer/andes_plmt.h
new file mode 100644
index 0000000..08bce33
--- /dev/null
+++ b/include/sbi_utils/timer/andes_plmt.h
@@ -0,0 +1,29 @@ 
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2022 Andes Technology Corporation
+ *
+ * Authors:
+ *   Zong Li <zong@andestech.com>
+ *   Nylon Chen <nylon7@andestech.com>
+ *   Yu Chien Peter Lin <peterlin@andestech.com>
+ */
+
+#ifndef __TIMER_ANDES_PLMT_H__
+#define __TIMER_ANDES_PLMT_H__
+
+#define DEFAULT_AE350_PLMT_FREQ 60000000
+#define PLMT_REGION_ALIGN 0x1000
+
+struct plmt_data {
+	u32 hart_count;
+	unsigned long size;
+	unsigned long timer_freq;
+	volatile u64 *time_val;
+	volatile u64 *time_cmp;
+};
+
+int plmt_cold_timer_init(struct plmt_data *plmt);
+int plmt_warm_timer_init(void);
+
+#endif /* __TIMER_ANDES_PLMT_H__ */
diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
index 6a75d6f..ce52fca 100644
--- a/lib/utils/fdt/fdt_helper.c
+++ b/lib/utils/fdt/fdt_helper.c
@@ -835,6 +835,60 @@  int fdt_parse_aclint_node(void *fdt, int nodeoffset, bool for_timer,
 	return 0;
 }
 
+int fdt_parse_plmt_node(void *fdt, int nodeoffset, unsigned long *plmt_base,
+			  unsigned long *plmt_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 || !plmt_base ||
+	    !hart_count || !plmt_size)
+		return SBI_EINVAL;
+
+	rc = fdt_get_node_addr_size(fdt, nodeoffset, 0,
+				    &reg_addr, &reg_size);
+	if (rc < 0 || !plmt_base || !plmt_size)
+		return SBI_ENODEV;
+	*plmt_base = reg_addr;
+	*plmt_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_TIMER)
+			hcount++;
+	}
+
+	*hart_count = hcount;
+
+	return 0;
+}
+
 int fdt_parse_compat_addr(void *fdt, uint64_t *addr,
 			  const char *compatible)
 {
diff --git a/lib/utils/timer/Kconfig b/lib/utils/timer/Kconfig
index 23c48c5..ba211b6 100644
--- a/lib/utils/timer/Kconfig
+++ b/lib/utils/timer/Kconfig
@@ -14,10 +14,19 @@  config FDT_TIMER_MTIMER
 	select TIMER_MTIMER
 	default n
 
+config FDT_TIMER_PLMT
+	bool "Andes PLMT FDT driver"
+	select TIMER_PLMT
+	default n
+
 endif
 
 config TIMER_MTIMER
 	bool "ACLINT MTIMER support"
 	default n
 
+config TIMER_PLMT
+	bool "Andes PLMT support"
+	default n
+
 endmenu
diff --git a/lib/utils/timer/andes_plmt.c b/lib/utils/timer/andes_plmt.c
new file mode 100644
index 0000000..94a86cc
--- /dev/null
+++ b/lib/utils/timer/andes_plmt.c
@@ -0,0 +1,104 @@ 
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2022 Andes Technology Corporation
+ *
+ * Authors:
+ *   Yu Chien Peter Lin <peterlin@andestech.com>
+ */
+
+#include <sbi/riscv_asm.h>
+#include <sbi/riscv_io.h>
+#include <sbi/sbi_domain.h>
+#include <sbi/sbi_error.h>
+#include <sbi/sbi_timer.h>
+#include <sbi_utils/timer/andes_plmt.h>
+
+struct plmt_data plmt;
+
+static u64 plmt_timer_value(void)
+{
+#if __riscv_xlen == 64
+	return readq_relaxed(plmt.time_val);
+#else
+	u32 lo, hi;
+
+	do {
+		hi = readl_relaxed((void *)plmt.time_val + 0x04);
+		lo = readl_relaxed(plmt.time_val);
+	} while (hi != readl_relaxed((void *)plmt.time_val + 0x04));
+
+	return ((u64)hi << 32) | (u64)lo;
+#endif
+}
+
+static void plmt_timer_event_stop(void)
+{
+	u32 target_hart = current_hartid();
+
+	if (plmt.hart_count <= target_hart)
+		ebreak();
+
+	/* Clear PLMT Time Compare */
+#if __riscv_xlen == 64
+	writeq_relaxed(-1ULL, &plmt.time_cmp[target_hart]);
+#else
+	writel_relaxed(-1UL, &plmt.time_cmp[target_hart]);
+	writel_relaxed(-1UL, (void *)(&plmt.time_cmp[target_hart]) + 0x04);
+#endif
+}
+
+static void plmt_timer_event_start(u64 next_event)
+{
+	u32 target_hart = current_hartid();
+
+	if (plmt.hart_count <= target_hart)
+		ebreak();
+
+	/* Program PLMT Time Compare */
+#if __riscv_xlen == 64
+	writeq_relaxed(next_event, &plmt.time_cmp[target_hart]);
+#else
+	u32 mask = -1UL;
+
+	writel_relaxed(next_event & mask, &plmt.time_cmp[target_hart]);
+	writel_relaxed(next_event >> 32,
+		       (void *)(&plmt.time_cmp[target_hart]) + 0x04);
+#endif
+}
+
+static struct sbi_timer_device plmt_timer = {
+	.name		   = "andes_plmt",
+	.timer_freq	   = DEFAULT_AE350_PLMT_FREQ,
+	.timer_value	   = plmt_timer_value,
+	.timer_event_start = plmt_timer_event_start,
+	.timer_event_stop  = plmt_timer_event_stop
+};
+
+int plmt_cold_timer_init(struct plmt_data *plmt)
+{
+	int rc;
+
+	/* Add PLMT region to the root domain */
+	rc = sbi_domain_root_add_memrange(
+		(unsigned long)plmt->time_val, plmt->size, PLMT_REGION_ALIGN,
+		SBI_DOMAIN_MEMREGION_MMIO | SBI_DOMAIN_MEMREGION_READABLE);
+	if (rc)
+		return rc;
+
+	plmt_timer.timer_freq = plmt->timer_freq;
+
+	sbi_timer_set_device(&plmt_timer);
+
+	return 0;
+}
+
+int plmt_warm_timer_init(void)
+{
+	if (!plmt.time_val)
+		return SBI_ENODEV;
+
+	plmt_timer_event_stop();
+
+	return 0;
+}
diff --git a/lib/utils/timer/fdt_timer_plmt.c b/lib/utils/timer/fdt_timer_plmt.c
new file mode 100644
index 0000000..e8be91b
--- /dev/null
+++ b/lib/utils/timer/fdt_timer_plmt.c
@@ -0,0 +1,51 @@ 
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2022 Andes Technology Corporation
+ *
+ * Authors:
+ *   Yu Chien Peter Lin <peterlin@andestech.com>
+ */
+
+#include <sbi_utils/fdt/fdt_helper.h>
+#include <sbi_utils/timer/fdt_timer.h>
+#include <sbi_utils/timer/andes_plmt.h>
+
+extern struct plmt_data plmt;
+
+static int fdt_plmt_cold_timer_init(void *fdt, int nodeoff,
+				    const struct fdt_match *match)
+{
+	int rc;
+	unsigned long plmt_base;
+
+	rc = fdt_parse_plmt_node(fdt, nodeoff, &plmt_base, &plmt.size,
+				 &plmt.hart_count);
+	if (rc)
+		return rc;
+
+	plmt.time_val = (u64 *)plmt_base;
+	plmt.time_cmp = (u64 *)(plmt_base + 0x8);
+
+	rc = fdt_parse_timebase_frequency(fdt, &plmt.timer_freq);
+	if (rc)
+		return rc;
+
+	rc = plmt_cold_timer_init(&plmt);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static const struct fdt_match timer_plmt_match[] = {
+	{ .compatible = "andestech,plmt0" },
+	{},
+};
+
+struct fdt_timer fdt_timer_plmt = {
+	.match_table = timer_plmt_match,
+	.cold_init   = fdt_plmt_cold_timer_init,
+	.warm_init   = plmt_warm_timer_init,
+	.exit	     = NULL,
+};
diff --git a/lib/utils/timer/objects.mk b/lib/utils/timer/objects.mk
index 7f5f3ce..9360a76 100644
--- a/lib/utils/timer/objects.mk
+++ b/lib/utils/timer/objects.mk
@@ -8,9 +8,13 @@ 
 #
 
 libsbiutils-objs-$(CONFIG_TIMER_MTIMER) += timer/aclint_mtimer.o
+libsbiutils-objs-$(CONFIG_TIMER_PLMT) += timer/andes_plmt.o
 
 libsbiutils-objs-$(CONFIG_FDT_TIMER) += timer/fdt_timer.o
 libsbiutils-objs-$(CONFIG_FDT_TIMER) += timer/fdt_timer_drivers.o
 
 carray-fdt_timer_drivers-$(CONFIG_FDT_TIMER_MTIMER) += fdt_timer_mtimer
 libsbiutils-objs-$(CONFIG_FDT_TIMER_MTIMER) += timer/fdt_timer_mtimer.o
+
+carray-fdt_timer_drivers-$(CONFIG_FDT_TIMER_PLMT) += fdt_timer_plmt
+libsbiutils-objs-$(CONFIG_FDT_TIMER_PLMT) += timer/fdt_timer_plmt.o
diff --git a/platform/andes/ae350/Kconfig b/platform/andes/ae350/Kconfig
index 8dd8ebe..f6f50eb 100644
--- a/platform/andes/ae350/Kconfig
+++ b/platform/andes/ae350/Kconfig
@@ -6,6 +6,8 @@  config PLATFORM_ANDES_AE350
 	select IRQCHIP_PLIC
 	select FDT_SERIAL
 	select FDT_SERIAL_UART8250
+	select FDT_TIMER
+	select FDT_TIMER_PLMT
 	default y
 
 if PLATFORM_ANDES_AE350
diff --git a/platform/andes/ae350/objects.mk b/platform/andes/ae350/objects.mk
index 80f0737..1ccb894 100644
--- a/platform/andes/ae350/objects.mk
+++ b/platform/andes/ae350/objects.mk
@@ -15,7 +15,7 @@  platform-asflags-y =
 platform-ldflags-y =
 
 # Objects to build
-platform-objs-y += cache.o platform.o plicsw.o plmt.o
+platform-objs-y += cache.o platform.o plicsw.o
 
 # Blobs to build
 FW_TEXT_START=0x00000000
diff --git a/platform/andes/ae350/platform.c b/platform/andes/ae350/platform.c
index 04428d1..79736c0 100644
--- a/platform/andes/ae350/platform.c
+++ b/platform/andes/ae350/platform.c
@@ -19,9 +19,9 @@ 
 #include <sbi_utils/fdt/fdt_fixup.h>
 #include <sbi_utils/irqchip/plic.h>
 #include <sbi_utils/serial/fdt_serial.h>
+#include <sbi_utils/timer/fdt_timer.h>
 #include "platform.h"
 #include "plicsw.h"
-#include "plmt.h"
 #include "cache.h"
 
 static struct plic_data plic = {
@@ -81,21 +81,6 @@  static int ae350_ipi_init(bool cold_boot)
 	return plicsw_warm_ipi_init();
 }
 
-/* Initialize platform timer for current HART. */
-static int ae350_timer_init(bool cold_boot)
-{
-	int ret;
-
-	if (cold_boot) {
-		ret = plmt_cold_timer_init(AE350_PLMT_ADDR,
-					   AE350_HART_COUNT);
-		if (ret)
-			return ret;
-	}
-
-	return plmt_warm_timer_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,
@@ -150,7 +135,7 @@  const struct sbi_platform_operations platform_ops = {
 
 	.ipi_init     = ae350_ipi_init,
 
-	.timer_init	   = ae350_timer_init,
+	.timer_init	   = fdt_timer_init,
 
 	.vendor_ext_provider = ae350_vendor_ext_provider
 };
diff --git a/platform/andes/ae350/platform.h b/platform/andes/ae350/platform.h
index c699b7f..6a29fe5 100644
--- a/platform/andes/ae350/platform.h
+++ b/platform/andes/ae350/platform.h
@@ -18,8 +18,6 @@ 
 
 #define AE350_PLICSW_ADDR		0xe6400000
 
-#define AE350_PLMT_ADDR			0xe6000000
-
 #define AE350_L2C_ADDR			0xe0500000
 
 /*Memory and Miscellaneous Registers*/
diff --git a/platform/andes/ae350/plmt.c b/platform/andes/ae350/plmt.c
deleted file mode 100644
index 54dcb94..0000000
--- a/platform/andes/ae350/plmt.c
+++ /dev/null
@@ -1,107 +0,0 @@ 
-/*
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Copyright (c) 2019 Andes Technology Corporation
- *
- * Authors:
- *   Zong Li <zong@andestech.com>
- *   Nylon Chen <nylon7@andestech.com>
- */
-
-#include <sbi/riscv_asm.h>
-#include <sbi/riscv_io.h>
-#include <sbi/sbi_timer.h>
-
-static u32 plmt_time_hart_count;
-static volatile void *plmt_time_base;
-static volatile u64 *plmt_time_val;
-static volatile u64 *plmt_time_cmp;
-
-static u64 plmt_timer_value(void)
-{
-#if __riscv_xlen == 64
-	return readq_relaxed(plmt_time_val);
-#else
-	u32 lo, hi;
-
-	do {
-		hi = readl_relaxed((void *)plmt_time_val + 0x04);
-		lo = readl_relaxed(plmt_time_val);
-	} while (hi != readl_relaxed((void *)plmt_time_val + 0x04));
-
-	return ((u64)hi << 32) | (u64)lo;
-#endif
-}
-
-static void plmt_timer_event_stop(void)
-{
-	u32 target_hart = current_hartid();
-
-	if (plmt_time_hart_count <= target_hart)
-		return;
-
-	/* Clear PLMT Time Compare */
-#if __riscv_xlen == 64
-	writeq_relaxed(-1ULL, &plmt_time_cmp[target_hart]);
-#else
-	writel_relaxed(-1UL, &plmt_time_cmp[target_hart]);
-	writel_relaxed(-1UL, (void *)(&plmt_time_cmp[target_hart]) + 0x04);
-#endif
-}
-
-static void plmt_timer_event_start(u64 next_event)
-{
-	u32 target_hart = current_hartid();
-
-	if (plmt_time_hart_count <= target_hart)
-		return;
-
-	/* Program PLMT Time Compare */
-#if __riscv_xlen == 64
-	writeq_relaxed(next_event, &plmt_time_cmp[target_hart]);
-#else
-	u32 mask = -1UL;
-
-	writel_relaxed(next_event & mask, &plmt_time_cmp[target_hart]);
-	writel_relaxed(next_event >> 32,
-		       (void *)(&plmt_time_cmp[target_hart]) + 0x04);
-#endif
-
-}
-
-static struct sbi_timer_device plmt_timer = {
-	.name = "ae350_plmt",
-	.timer_value = plmt_timer_value,
-	.timer_event_start = plmt_timer_event_start,
-	.timer_event_stop = plmt_timer_event_stop
-};
-
-int plmt_warm_timer_init(void)
-{
-	u32 target_hart = current_hartid();
-
-	if (plmt_time_hart_count <= target_hart || !plmt_time_base)
-		return -1;
-
-	/* Clear PLMT Time Compare */
-#if __riscv_xlen == 64
-	writeq_relaxed(-1ULL, &plmt_time_cmp[target_hart]);
-#else
-	writel_relaxed(-1UL, &plmt_time_cmp[target_hart]);
-	writel_relaxed(-1UL, (void *)(&plmt_time_cmp[target_hart]) + 0x04);
-#endif
-
-	return 0;
-}
-
-int plmt_cold_timer_init(unsigned long base, u32 hart_count)
-{
-	plmt_time_hart_count = hart_count;
-	plmt_time_base	     = (void *)base;
-	plmt_time_val        = (u64 *)(plmt_time_base);
-	plmt_time_cmp        = (u64 *)(plmt_time_base + 0x8);
-
-	sbi_timer_set_device(&plmt_timer);
-
-	return 0;
-}
diff --git a/platform/andes/ae350/plmt.h b/platform/andes/ae350/plmt.h
deleted file mode 100644
index db093e0..0000000
--- a/platform/andes/ae350/plmt.h
+++ /dev/null
@@ -1,17 +0,0 @@ 
-/*
- * SPDX-License-Identifier: BSD-2-Clause
- *
- * Copyright (c) 2019 Andes Technology Corporation
- *
- * Authors:
- *   Zong Li <zong@andestech.com>
- */
-
-#ifndef _AE350_PLMT_H_
-#define _AE350_PLMT_H_
-
-int plmt_warm_timer_init(void);
-
-int plmt_cold_timer_init(unsigned long base, u32 hart_count);
-
-#endif /* _AE350_PLMT_H_ */