arm: aspeed: Basic support for Aspeed AST2500 part and eval board

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/Kconfig              |  7 +++++++
 arch/arm/Makefile             |  1 +
 arch/arm/mach-aspeed/Kconfig  | 15 +++++++++++++++
 arch/arm/mach-aspeed/Makefile |  8 ++++++++
 4 files changed, 31 insertions(+)
 create mode 100644 arch/arm/mach-aspeed/Kconfig
 create mode 100644 arch/arm/mach-aspeed/Makefile

arch/arm/Kconfig +7 -0

index 714dd8b514..135c544335 100644
@@ -882,8 +882,15 @@ config TARGET_THUNDERX_88XX
        select OF_CONTROL
        select SYS_CACHE_SHIFT_7
 
+config ARCH_ASPEED
+       bool "Support Aspeed SoCs"
+       select OF_CONTROL
+       select DM
+
 endchoice
 
+source "arch/arm/mach-aspeed/Kconfig"
+
 source "arch/arm/mach-at91/Kconfig"
 
 source "arch/arm/mach-bcm283x/Kconfig"

arch/arm/Makefile +1 -0

index 236debb452..cc73e1038e 100644
@@ -50,6 +50,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)
 
 # Machine directory name.  This list is sorted alphanumerically
 # by CONFIG_* macro name.
+machine-$(CONFIG_ARCH_ASPEED)          += aspeed
 machine-$(CONFIG_ARCH_AT91)            += at91
 machine-$(CONFIG_ARCH_BCM283X)         += bcm283x
 machine-$(CONFIG_ARCH_DAVINCI)         += davinci

arch/arm/mach-aspeed/Kconfig new file +15 -0

new file mode 100644
index 0000000000..43cdbeda84
@@ -0,0 +1,15 @@
+if ARCH_ASPEED
+
+config SYS_ARCH
+       default "arm"
+
+config SYS_SOC
+       default "aspeed"
+
+config ASPEED_AST2500
+       bool "Support Aspeed AST2500 SoC"
+       select CPU_ARM1176
+       help
+         The Aspeed AST2500 is a ARM-based SoC with arm1176 CPU
+
+endif

arch/arm/mach-aspeed/Makefile new file +0 -0

new file mode 100644
index 0000000000..8e276b4a9f

--
2.11.0.390.gc69c2f50cf-goog

Quoted 73 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/Kconfig              |  7 +++++++
 arch/arm/Makefile             |  1 +
 arch/arm/mach-aspeed/Kconfig  | 15 +++++++++++++++
 arch/arm/mach-aspeed/Makefile |  8 ++++++++
 4 files changed, 31 insertions(+)
 create mode 100644 arch/arm/mach-aspeed/Kconfig
 create mode 100644 arch/arm/mach-aspeed/Makefile

arch/arm/Kconfig +7 -0

index 714dd8b514..135c544335 100644
@@ -882,8 +882,15 @@ config TARGET_THUNDERX_88XX
        select OF_CONTROL
        select SYS_CACHE_SHIFT_7
 
+config ARCH_ASPEED
+       bool "Support Aspeed SoCs"
+       select OF_CONTROL
+       select DM
+
 endchoice
 
+source "arch/arm/mach-aspeed/Kconfig"
+
 source "arch/arm/mach-at91/Kconfig"
 
 source "arch/arm/mach-bcm283x/Kconfig"

arch/arm/Makefile +1 -0

index 236debb452..cc73e1038e 100644
@@ -50,6 +50,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)
 
 # Machine directory name.  This list is sorted alphanumerically
 # by CONFIG_* macro name.
+machine-$(CONFIG_ARCH_ASPEED)          += aspeed
 machine-$(CONFIG_ARCH_AT91)            += at91
 machine-$(CONFIG_ARCH_BCM283X)         += bcm283x
 machine-$(CONFIG_ARCH_DAVINCI)         += davinci

arch/arm/mach-aspeed/Kconfig new file +15 -0

new file mode 100644
index 0000000000..43cdbeda84
@@ -0,0 +1,15 @@
+if ARCH_ASPEED
+
+config SYS_ARCH
+       default "arm"
+
+config SYS_SOC
+       default "aspeed"
+
+config ASPEED_AST2500
+       bool "Support Aspeed AST2500 SoC"
+       select CPU_ARM1176
+       help
+         The Aspeed AST2500 is a ARM-based SoC with arm1176 CPU
+
+endif

arch/arm/mach-aspeed/Makefile new file +3 -0

new file mode 100644
index 0000000000..8e276b4a9f
@@ -0,0 +1,8 @@
+#
+# Copyright (c) 2014 Google, Inc
+#

arch/arm/mach-aspeed/Makefile line 8

Quoted 6 lines

arch/arm/mach-aspeed/Makefile new file +5 -0

+# SPDX-License-Identifier:     GPL-2.0+
+#
+
+obj-$(CONFIG_ARCH_ASPEED) += ast_wdt.o
+obj-$(CONFIG_ASPEED_AST2500) += ast2500/ ast2500-board.o

These files don't exist yet.

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/Kconfig              |  7 +++++++
 arch/arm/Makefile             |  1 +
 arch/arm/mach-aspeed/Kconfig  | 15 +++++++++++++++
 arch/arm/mach-aspeed/Makefile |  8 ++++++++
 4 files changed, 31 insertions(+)
 create mode 100644 arch/arm/mach-aspeed/Kconfig
 create mode 100644 arch/arm/mach-aspeed/Makefile

arch/arm/Kconfig +7 -0

index 714dd8b514..135c544335 100644
@@ -882,8 +882,15 @@ config TARGET_THUNDERX_88XX
        select OF_CONTROL
        select SYS_CACHE_SHIFT_7
 
+config ARCH_ASPEED
+       bool "Support Aspeed SoCs"
+       select OF_CONTROL
+       select DM
+
 endchoice
 
+source "arch/arm/mach-aspeed/Kconfig"
+
 source "arch/arm/mach-at91/Kconfig"
 
 source "arch/arm/mach-bcm283x/Kconfig"

arch/arm/Makefile +1 -0

index 236debb452..cc73e1038e 100644
@@ -50,6 +50,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)
 
 # Machine directory name.  This list is sorted alphanumerically
 # by CONFIG_* macro name.
+machine-$(CONFIG_ARCH_ASPEED)          += aspeed
 machine-$(CONFIG_ARCH_AT91)            += at91
 machine-$(CONFIG_ARCH_BCM283X)         += bcm283x
 machine-$(CONFIG_ARCH_DAVINCI)         += davinci

arch/arm/mach-aspeed/Kconfig new file +3 -0

new file mode 100644
index 0000000000..43cdbeda84
@@ -0,0 +1,15 @@
+if ARCH_ASPEED
+
+config SYS_ARCH

arch/arm/mach-aspeed/Kconfig new file +2 -0

+
+endif

arch/arm/mach-aspeed/Makefile new file +8 -1

new file mode 100644
index 0000000000..8e276b4a9f
@@ -0,0 +1,8 @@
+#
+# Copyright (c) 2014 Google, Inc
+#
+# SPDX-License-Identifier:     GPL-2.0+
+#
+
+obj-$(CONFIG_ARCH_ASPEED) += ast_wdt.o
+obj-$(CONFIG_ASPEED_AST2500) += ast2500/ ast2500-board.o
--

2.11.0.390.gc69c2f50cf-goog

Hi Maxim,

Quoted 57 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/Kconfig              |  7 +++++++
 arch/arm/Makefile             |  1 +
 arch/arm/mach-aspeed/Kconfig  | 15 +++++++++++++++
 arch/arm/mach-aspeed/Makefile |  8 ++++++++
 4 files changed, 31 insertions(+)
 create mode 100644 arch/arm/mach-aspeed/Kconfig
 create mode 100644 arch/arm/mach-aspeed/Makefile

arch/arm/Kconfig +7 -0

index 714dd8b514..135c544335 100644
@@ -882,8 +882,15 @@ config TARGET_THUNDERX_88XX
        select OF_CONTROL
        select SYS_CACHE_SHIFT_7
 
+config ARCH_ASPEED
+       bool "Support Aspeed SoCs"
+       select OF_CONTROL
+       select DM
+
 endchoice
 
+source "arch/arm/mach-aspeed/Kconfig"
+
 source "arch/arm/mach-at91/Kconfig"
 
 source "arch/arm/mach-bcm283x/Kconfig"

arch/arm/Makefile +1 -0

index 236debb452..cc73e1038e 100644
@@ -50,6 +50,7 @@ PLATFORM_CPPFLAGS += $(arch-y) $(tune-y)
 
 # Machine directory name.  This list is sorted alphanumerically
 # by CONFIG_* macro name.
+machine-$(CONFIG_ARCH_ASPEED)          += aspeed
 machine-$(CONFIG_ARCH_AT91)            += at91
 machine-$(CONFIG_ARCH_BCM283X)         += bcm283x
 machine-$(CONFIG_ARCH_DAVINCI)         += davinci

arch/arm/mach-aspeed/Kconfig new file +8 -0

new file mode 100644
index 0000000000..43cdbeda84
@@ -0,0 +1,15 @@
+if ARCH_ASPEED
+
+config SYS_ARCH
+       default "arm"
+
+config SYS_SOC
+       default "aspeed"
+

arch/arm/mach-aspeed/Kconfig line 13

Quoted 5 lines

arch/arm/mach-aspeed/Kconfig new file +5 -0

+config ASPEED_AST2500
+       bool "Support Aspeed AST2500 SoC"
+       select CPU_ARM1176
+       help
+         The Aspeed AST2500 is a ARM-based SoC with arm1176 CPU

Can you please expand this a bit? E.g. a summary of peripherals it has, what the chip is used for...

arch/arm/mach-aspeed/

Quoted 3 lines

arch/arm/mach-aspeed/Kconfig new file +2 -0

+
+endif

arch/arm/mach-aspeed/ renamed from arch/arm/mach-aspeed/Makefile +0 -0

Makefile

Quoted 14 lines

new file mode 100644
index 0000000000..8e276b4a9f

arch/arm/mach-aspeed/Makefile new file +8 -1

@@ -0,0 +1,8 @@
+#
+# Copyright (c) 2014 Google, Inc
+#
+# SPDX-License-Identifier:     GPL-2.0+
+#
+
+obj-$(CONFIG_ARCH_ASPEED) += ast_wdt.o
+obj-$(CONFIG_ASPEED_AST2500) += ast2500/ ast2500-board.o
--

Quoted 2 lines

2.11.0.390.gc69c2f50cf-goog

Regards,
Simon

The driver is compatible with AST2400 and AST2500 watchdogs. There is no uclass for Watchdog yet, so the driver does not follow the driver model. It also uses fixed clock, so no clock driver is needed.

The driver is compatible with AST2400 and AST2500 watchdogs. There is no uclass for Watchdog yet, so the driver does not follow the driver model. It also uses fixed clock, so no clock driver is needed.

# Conflicts:
# arch/arm/mach-aspeed/Makefile

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/include/asm/arch-aspeed/wdt.h | 89 ++++++++++++++++++++++++++++++++++
 arch/arm/mach-aspeed/Makefile          |  3 +-
 arch/arm/mach-aspeed/ast_wdt.c         | 44 +++++++++++++++++
 3 files changed, 134 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-aspeed/wdt.h
 create mode 100644 arch/arm/mach-aspeed/ast_wdt.c

arch/arm/include/asm/arch-aspeed/wdt.h new file +0 -0

new file mode 100644
index 0000000000..32774b1a70

Add support for timer for Aspeed ast2400/ast2500 devices. The driver actually controls several devices, but because all devices share the same Control Register, it is somewhat difficult to completely decouple them. Since only one timer is needed at the moment, this should be OK.

The timer uses fixed clock, so does not rely on a clock driver.

+U_BOOT_DRIVER(sandbox_timer) = {

Add support for timer for Aspeed ast2400/ast2500 devices. The driver actually controls several devices, but because all devices share the same Control Register, it is somewhat difficult to completely decouple them. Since only one timer is needed at the moment, this should be OK.

The timer uses fixed clock, so does not rely on a clock driver.

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/include/asm/arch-aspeed/timer.h | 54 ++++++++++++++++++
 drivers/timer/Kconfig                    |  7 +++
 drivers/timer/Makefile                   |  1 +
 drivers/timer/ast_timer.c                | 96 ++++++++++++++++++++++++++++++++
 4 files changed, 158 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-aspeed/timer.h
 create mode 100644 drivers/timer/ast_timer.c

arch/arm/include/asm/arch-aspeed/timer.h new file +54 -0

new file mode 100644
index 0000000000..87c5b354ec
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#ifndef _ASM_ARCH_TIMER_H
+#define _ASM_ARCH_TIMER_H
+
+/* Each timer has 4 control bits in ctrl1 register.
+ * Timer1 uses bits 0:3, Timer2 uses bits 4:7 and so on,
+ * such that timer X uses bits (4 * X - 4):(4 * X - 1)
+ * If the timer does not support PWM, bit 4 is reserved.
+ */
+#define AST_TMC_EN                     (1 << 0)
+#define AST_TMC_1MHZ                   (1 << 1)
+#define AST_TMC_OVFINTR                        (1 << 2)
+#define AST_TMC_PWM                    (1 << 3)
+
+/* Timers are counted from 1 in the datasheet. */
+#define AST_TMC_CTRL1_SHIFT(n)                 (4 * ((n) - 1))
+
+#define AST_TMC_RATE  (1000*1000)
+
+#ifndef __ASSEMBLY__
+
+/*
+ * All timers share control registers, which makes it harder to make them
+ * separate devices. Since only one timer is needed at the moment, making
+ * it this just one device.
+ */
+
+struct ast_timer_counter {
+       u32 status;
+       u32 reload_val;
+       u32 match1;
+       u32 match2;
+};
+
+struct ast_timer {
+       struct ast_timer_counter timers1[3];
+       u32 ctrl1;
+       u32 ctrl2;
+#ifdef CONFIG_ASPEED_AST2500
+       u32 ctrl3;
+       u32 ctrl1_clr;
+#else
+       u32 reserved[2];
+#endif
+       struct ast_timer_counter timers2[5];
+};
+
+#endif  /* __ASSEMBLY__ */
+
+#endif  /* _ASM_ARCH_TIMER_H */

drivers/timer/Kconfig +0 -0

index cb18f12fc9..9c5f98bb88 100644

drivers/timer/Kconfig +1 -0

+
 endmenu

drivers/timer/Makefile +1 -0

index f351fbb4e0..a4b1a486b0 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_ALTERA_TIMER)      += altera_timer.o
 obj-$(CONFIG_SANDBOX_TIMER)    += sandbox_timer.o
 obj-$(CONFIG_X86_TSC_TIMER)    += tsc_timer.o
 obj-$(CONFIG_OMAP_TIMER)       += omap-timer.o
+obj-$(CONFIG_AST_TIMER)        += ast_timer.o

drivers/timer/ast_timer.c new file +26 -0

new file mode 100644
index 0000000000..f644882f40
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <timer.h>
+#include <asm/io.h>
+#include <asm/arch/timer.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define AST_TICK_TIMER  1
+#define AST_TMC_RELOAD_VAL  0xffffffff
+
+struct ast_timer_priv {
+       struct ast_timer *regs;
+};
+
+static struct ast_timer_counter *ast_get_timer_counter(struct ast_timer *timer,
+                                                      int n)
+{
+       if (n > 3)

drivers/timer/ast_timer.c new file +26 -0

+       struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+       writel(AST_TMC_RELOAD_VAL, &tmc->reload_val);
+
+       /*
+        * Stop the timer. This will also load reload_val into
+        * the status register.
+        */
+       clrbits_le32(&priv->regs->ctrl1,
+                    AST_TMC_EN << AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));
+       /* Start the timer from the fixed 1MHz clock. */
+       setbits_le32(&priv->regs->ctrl1,
+                    (AST_TMC_EN | AST_TMC_1MHZ) <<
+                    AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));
+
+       uc_priv->clock_rate = AST_TMC_RATE;
+
+       return 0;
+}
+
+static int ast_timer_get_count(struct udevice *dev, u64 *count)
+{
+       struct ast_timer_priv *priv = dev_get_priv(dev);
+       struct ast_timer_counter *tmc = ast_get_timer_counter(priv->regs,
+                                                             AST_TICK_TIMER);
+

drivers/timer/ast_timer.c new file +5 -0

+
+       return 0;
+}
+
+static const struct timer_ops ast_timer_ops = {

drivers/timer/ast_timer.c new file +9 -1

+       .name = "ast_timer",
+       .id = UCLASS_TIMER,
+       .of_match = ast_timer_ids,
+       .probe = ast_timer_probe,
+       .priv_auto_alloc_size = sizeof(struct ast_timer_priv),
+       .ofdata_to_platdata = ast_timer_ofdata_to_platdata,
+       .ops = &ast_timer_ops,
+       .flags = DM_FLAG_PRE_RELOC,
+};
--

2.11.0.390.gc69c2f50cf-goog

Hi Maxim,

Quoted 8 lines · Maxim Sloyko

Add support for timer for Aspeed ast2400/ast2500 devices. The driver actually controls several devices, but because all devices share the same Control Register, it is somewhat difficult to completely decouple them. Since only one timer is needed at the moment, this should be OK.

The timer uses fixed clock, so does not rely on a clock driver.

Quoted 7 lines

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/include/asm/arch-aspeed/timer.h | 54 ++++++++++++++++++
 drivers/timer/Kconfig                    |  7 +++
 drivers/timer/Makefile                   |  1 +
 drivers/timer/ast_timer.c                | 96

++++++++++++++++++++++++++++++++

Quoted 3 lines

 4 files changed, 158 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-aspeed/timer.h
 create mode 100644 drivers/timer/ast_timer.c

Reviewed-by Simon Glass <[email protected]>

nits below.

Review

Quoted 2 lines

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-aspeed/timer.h

Quoted 22 lines

new file mode 100644
index 0000000000..87c5b354ec

arch/arm/include/asm/arch-aspeed/timer.h new file +17 -0

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#ifndef _ASM_ARCH_TIMER_H
+#define _ASM_ARCH_TIMER_H
+
+/* Each timer has 4 control bits in ctrl1 register.
+ * Timer1 uses bits 0:3, Timer2 uses bits 4:7 and so on,
+ * such that timer X uses bits (4 * X - 4):(4 * X - 1)
+ * If the timer does not support PWM, bit 4 is reserved.
+ */
+#define AST_TMC_EN                     (1 << 0)
+#define AST_TMC_1MHZ                   (1 << 1)
+#define AST_TMC_OVFINTR                        (1 << 2)
+#define AST_TMC_PWM                    (1 << 3)

arch/arm/include/asm/arch-aspeed/timer.h line 27

Quoted 10 lines

arch/arm/include/asm/arch-aspeed/timer.h new file +10 -0

+
+/* Timers are counted from 1 in the datasheet. */
+#define AST_TMC_CTRL1_SHIFT(n)                 (4 * ((n) - 1))
+
+#define AST_TMC_RATE  (1000*1000)
+
+#ifndef __ASSEMBLY__
+
+/*
+ * All timers share control registers, which makes it harder to make

them

arch/arm/include/asm/arch-aspeed/timer.h line 28

Quoted 1 line

arch/arm/include/asm/arch-aspeed/timer.h new file +1 -0

+ * separate devices. Since only one timer is needed at the moment,

making

Quoted 31 lines

arch/arm/include/asm/arch-aspeed/timer.h new file +26 -0

+ * it this just one device.
+ */
+
+struct ast_timer_counter {
+       u32 status;
+       u32 reload_val;
+       u32 match1;
+       u32 match2;
+};
+
+struct ast_timer {
+       struct ast_timer_counter timers1[3];
+       u32 ctrl1;
+       u32 ctrl2;
+#ifdef CONFIG_ASPEED_AST2500
+       u32 ctrl3;
+       u32 ctrl1_clr;
+#else
+       u32 reserved[2];
+#endif
+       struct ast_timer_counter timers2[5];
+};
+
+#endif  /* __ASSEMBLY__ */
+
+#endif  /* _ASM_ARCH_TIMER_H */

drivers/timer/Kconfig +0 -0

index cb18f12fc9..9c5f98bb88 100644

drivers/timer/Kconfig config OMAP_TIMER · line 54

Quoted 9 lines

drivers/timer/Kconfig +6 -0

@@ -46,4 +46,11 @@ config OMAP_TIMER
        help
          Select this to enable an timer for Omap devices.
 
+config AST_TIMER
+       bool "Aspeed ast2400/ast2500 timer support"
+       depends on TIMER
+       default y if ARCH_ASPEED
+       help
+         Select this to enable timer for Aspeed ast2400/ast2500 devices.

Can you add more detail? How many channels? What features are supported by the driver?

Quoted 30 lines

drivers/timer/Kconfig +1 -0

+
 endmenu

drivers/timer/Makefile +1 -0

index f351fbb4e0..a4b1a486b0 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_ALTERA_TIMER)      += altera_timer.o
 obj-$(CONFIG_SANDBOX_TIMER)    += sandbox_timer.o
 obj-$(CONFIG_X86_TSC_TIMER)    += tsc_timer.o
 obj-$(CONFIG_OMAP_TIMER)       += omap-timer.o
+obj-$(CONFIG_AST_TIMER)        += ast_timer.o

drivers/timer/ast_timer.c new file +13 -0

new file mode 100644
index 0000000000..f644882f40
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <timer.h>
+#include <asm/io.h>
+#include <asm/arch/timer.h>
+

drivers/timer/ast_timer.c line 23

Quoted 10 lines

drivers/timer/ast_timer.c new file +10 -0

+DECLARE_GLOBAL_DATA_PTR;
+
+#define AST_TICK_TIMER  1
+#define AST_TMC_RELOAD_VAL  0xffffffff
+
+struct ast_timer_priv {
+       struct ast_timer *regs;
+};
+
+static struct ast_timer_counter *ast_get_timer_counter(struct ast_timer

*timer,

drivers/timer/ast_timer.c

Quoted 12 lines

drivers/timer/ast_timer.c new file +12 -0

+                                                      int n)
+{
+       if (n > 3)
+               return &timer->timers2[n - 4];
+       else
+               return &timer->timers1[n - 1];
+}
+
+static int ast_timer_probe(struct udevice *dev)
+{
+       struct ast_timer_priv *priv = dev_get_priv(dev);
+       struct ast_timer_counter *tmc = ast_get_timer_counter(priv->

regs,

drivers/timer/ast_timer.c line 36

Quoted 1 line

drivers/timer/ast_timer.c new file +1 -0

+

AST_TICK_TIMER);

I suppose tmc could be in your struct ast_timer_priv to save you doing this each time?

Quoted 14 lines

drivers/timer/ast_timer.c new file +14 -0

+       struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+       writel(AST_TMC_RELOAD_VAL, &tmc->reload_val);
+
+       /*
+        * Stop the timer. This will also load reload_val into
+        * the status register.
+        */
+       clrbits_le32(&priv->regs->ctrl1,
+                    AST_TMC_EN << AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));
+       /* Start the timer from the fixed 1MHz clock. */
+       setbits_le32(&priv->regs->ctrl1,
+                    (AST_TMC_EN | AST_TMC_1MHZ) <<
+                    AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));

drivers/timer/ast_timer.c

Quoted 10 lines

drivers/timer/ast_timer.c new file +10 -0

+
+       uc_priv->clock_rate = AST_TMC_RATE;
+
+       return 0;
+}
+
+static int ast_timer_get_count(struct udevice *dev, u64 *count)
+{
+       struct ast_timer_priv *priv = dev_get_priv(dev);
+       struct ast_timer_counter *tmc = ast_get_timer_counter(priv->

regs,

drivers/timer/ast_timer.c line 61

Quoted 1 line

drivers/timer/ast_timer.c new file +1 -0

+

AST_TICK_TIMER);

drivers/timer/ast_timer.c line 72

Quoted 11 lines

drivers/timer/ast_timer.c new file +11 -0

+
+       *count = AST_TMC_RELOAD_VAL - readl(&tmc->status);
+
+       return 0;
+}
+
+static int ast_timer_ofdata_to_platdata(struct udevice *dev)
+{
+       struct ast_timer_priv *priv = dev_get_priv(dev);
+
+       priv->regs = (struct ast_timer *)dev_get_addr(dev);

You can use dev_get_addr_ptr() if you like.

Quoted 5 lines

drivers/timer/ast_timer.c new file +5 -0

+
+       return 0;
+}
+
+static const struct timer_ops ast_timer_ops = {

drivers/timer/ast_timer.c line 87

Quoted 10 lines

drivers/timer/ast_timer.c new file +10 -0

+       .get_count = ast_timer_get_count,
+};
+
+static const struct udevice_id ast_timer_ids[] = {
+       { .compatible = "aspeed,ast2500-timer" },
+       { .compatible = "aspeed,ast2400-timer" },
+       { }
+};
+
+U_BOOT_DRIVER(sandbox_timer) = {

s/sandbox/ast/ or something, as Tom, mentioned.

Quoted 11 lines

drivers/timer/ast_timer.c new file +9 -1

+       .name = "ast_timer",
+       .id = UCLASS_TIMER,
+       .of_match = ast_timer_ids,
+       .probe = ast_timer_probe,
+       .priv_auto_alloc_size = sizeof(struct ast_timer_priv),
+       .ofdata_to_platdata = ast_timer_ofdata_to_platdata,
+       .ops = &ast_timer_ops,
+       .flags = DM_FLAG_PRE_RELOC,
+};
--

2.11.0.390.gc69c2f50cf-goog

Regards,
Simon

Quoted 1 line

Hi Simon,

This is older series, that I merged into a smaller number of patches and already addressed all of Tom's comments: http://lists.denx.de/pipermail/u-boot/2017-January/277883.html

Most of your comments still apply, just want to make sure we are on the same page.

Hi Maxim,

Quoted 8 lines · Maxim Sloyko

Add support for timer for Aspeed ast2400/ast2500 devices. The driver actually controls several devices, but because all devices share the same Control Register, it is somewhat difficult to completely decouple them. Since only one timer is needed at the moment, this should be OK.

The timer uses fixed clock, so does not rely on a clock driver.

Quoted 7 lines

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/include/asm/arch-aspeed/timer.h | 54 ++++++++++++++++++
 drivers/timer/Kconfig                    |  7 +++
 drivers/timer/Makefile                   |  1 +
 drivers/timer/ast_timer.c                | 96

++++++++++++++++++++++++++++++++

Quoted 3 lines

 4 files changed, 158 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-aspeed/timer.h
 create mode 100644 drivers/timer/ast_timer.c

Reviewed-by Simon Glass <[email protected]>

nits below.

Review

Quoted 2 lines

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-aspeed/timer.h

Quoted 22 lines

new file mode 100644
index 0000000000..87c5b354ec

arch/arm/include/asm/arch-aspeed/timer.h new file +17 -0

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+#ifndef _ASM_ARCH_TIMER_H
+#define _ASM_ARCH_TIMER_H
+
+/* Each timer has 4 control bits in ctrl1 register.
+ * Timer1 uses bits 0:3, Timer2 uses bits 4:7 and so on,
+ * such that timer X uses bits (4 * X - 4):(4 * X - 1)
+ * If the timer does not support PWM, bit 4 is reserved.
+ */
+#define AST_TMC_EN                     (1 << 0)
+#define AST_TMC_1MHZ                   (1 << 1)
+#define AST_TMC_OVFINTR                        (1 << 2)
+#define AST_TMC_PWM                    (1 << 3)

arch/arm/include/asm/arch-aspeed/timer.h line 27

Quoted 10 lines

arch/arm/include/asm/arch-aspeed/timer.h new file +10 -0

+
+/* Timers are counted from 1 in the datasheet. */
+#define AST_TMC_CTRL1_SHIFT(n)                 (4 * ((n) - 1))
+
+#define AST_TMC_RATE  (1000*1000)
+
+#ifndef __ASSEMBLY__
+
+/*
+ * All timers share control registers, which makes it harder to make

them

arch/arm/include/asm/arch-aspeed/timer.h line 28

Quoted 1 line

arch/arm/include/asm/arch-aspeed/timer.h new file +1 -0

+ * separate devices. Since only one timer is needed at the moment,

making

Quoted 34 lines

arch/arm/include/asm/arch-aspeed/timer.h new file +26 -0

+ * it this just one device.
+ */
+
+struct ast_timer_counter {
+       u32 status;
+       u32 reload_val;
+       u32 match1;
+       u32 match2;
+};
+
+struct ast_timer {
+       struct ast_timer_counter timers1[3];
+       u32 ctrl1;
+       u32 ctrl2;
+#ifdef CONFIG_ASPEED_AST2500
+       u32 ctrl3;
+       u32 ctrl1_clr;
+#else
+       u32 reserved[2];
+#endif
+       struct ast_timer_counter timers2[5];
+};
+
+#endif  /* __ASSEMBLY__ */
+
+#endif  /* _ASM_ARCH_TIMER_H */

drivers/timer/Kconfig +0 -0

index cb18f12fc9..9c5f98bb88 100644
@@ -46,4 +46,11 @@ config OMAP_TIMER
        help
          Select this to enable an timer for Omap devices.
 

drivers/timer/Kconfig config OMAP_TIMER · line 54

Quoted 6 lines

drivers/timer/Kconfig +6 -0

+config AST_TIMER
+       bool "Aspeed ast2400/ast2500 timer support"
+       depends on TIMER
+       default y if ARCH_ASPEED
+       help
+         Select this to enable timer for Aspeed ast2400/ast2500 devices.

Can you add more detail? How many channels? What features are supported by the driver?

Quoted 31 lines

drivers/timer/Kconfig +1 -0

+
 endmenu

drivers/timer/Makefile +1 -0

index f351fbb4e0..a4b1a486b0 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_ALTERA_TIMER)      += altera_timer.o
 obj-$(CONFIG_SANDBOX_TIMER)    += sandbox_timer.o
 obj-$(CONFIG_X86_TSC_TIMER)    += tsc_timer.o
 obj-$(CONFIG_OMAP_TIMER)       += omap-timer.o
+obj-$(CONFIG_AST_TIMER)        += ast_timer.o

drivers/timer/ast_timer.c new file +13 -0

new file mode 100644
index 0000000000..f644882f40
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <timer.h>
+#include <asm/io.h>
+#include <asm/arch/timer.h>
+

drivers/timer/ast_timer.c line 23

Quoted 10 lines

drivers/timer/ast_timer.c new file +10 -0

+DECLARE_GLOBAL_DATA_PTR;
+
+#define AST_TICK_TIMER  1
+#define AST_TMC_RELOAD_VAL  0xffffffff
+
+struct ast_timer_priv {
+       struct ast_timer *regs;
+};
+
+static struct ast_timer_counter *ast_get_timer_counter(struct ast_timer

*timer,

drivers/timer/ast_timer.c

Quoted 12 lines

drivers/timer/ast_timer.c new file +12 -0

+                                                      int n)
+{
+       if (n > 3)
+               return &timer->timers2[n - 4];
+       else
+               return &timer->timers1[n - 1];
+}
+
+static int ast_timer_probe(struct udevice *dev)
+{
+       struct ast_timer_priv *priv = dev_get_priv(dev);
+       struct ast_timer_counter *tmc = ast_get_timer_counter(priv->

regs,

drivers/timer/ast_timer.c line 36

Quoted 1 line

drivers/timer/ast_timer.c new file +1 -0

+

AST_TICK_TIMER);

I suppose tmc could be in your struct ast_timer_priv to save you doing this each time?

Quoted 15 lines

drivers/timer/ast_timer.c new file +14 -0

+       struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+       writel(AST_TMC_RELOAD_VAL, &tmc->reload_val);
+
+       /*
+        * Stop the timer. This will also load reload_val into
+        * the status register.
+        */
+       clrbits_le32(&priv->regs->ctrl1,
+                    AST_TMC_EN << AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));
+       /* Start the timer from the fixed 1MHz clock. */
+       setbits_le32(&priv->regs->ctrl1,
+                    (AST_TMC_EN | AST_TMC_1MHZ) <<
+                    AST_TMC_CTRL1_SHIFT(AST_TICK_TIMER));

drivers/timer/ast_timer.c

Quoted 10 lines

drivers/timer/ast_timer.c new file +10 -0

+
+       uc_priv->clock_rate = AST_TMC_RATE;
+
+       return 0;
+}
+
+static int ast_timer_get_count(struct udevice *dev, u64 *count)
+{
+       struct ast_timer_priv *priv = dev_get_priv(dev);
+       struct ast_timer_counter *tmc = ast_get_timer_counter(priv->

regs,

drivers/timer/ast_timer.c line 61

Quoted 1 line

drivers/timer/ast_timer.c new file +1 -0

+

AST_TICK_TIMER);

drivers/timer/ast_timer.c line 72

Quoted 11 lines

drivers/timer/ast_timer.c new file +11 -0

+
+       *count = AST_TMC_RELOAD_VAL - readl(&tmc->status);
+
+       return 0;
+}
+
+static int ast_timer_ofdata_to_platdata(struct udevice *dev)
+{
+       struct ast_timer_priv *priv = dev_get_priv(dev);
+
+       priv->regs = (struct ast_timer *)dev_get_addr(dev);

You can use dev_get_addr_ptr() if you like.

Quoted 10 lines

drivers/timer/ast_timer.c new file +9 -0

+
+       return 0;
+}
+
+static const struct timer_ops ast_timer_ops = {
+       .get_count = ast_timer_get_count,
+};
+
+static const struct udevice_id ast_timer_ids[] = {

drivers/timer/ast_timer.c line 87

Quoted 6 lines

drivers/timer/ast_timer.c new file +6 -0

+       { .compatible = "aspeed,ast2500-timer" },
+       { .compatible = "aspeed,ast2400-timer" },
+       { }
+};
+
+U_BOOT_DRIVER(sandbox_timer) = {

s/sandbox/ast/ or something, as Tom, mentioned.

Quoted 11 lines

drivers/timer/ast_timer.c new file +9 -1

+       .name = "ast_timer",
+       .id = UCLASS_TIMER,
+       .of_match = ast_timer_ids,
+       .probe = ast_timer_probe,
+       .priv_auto_alloc_size = sizeof(struct ast_timer_priv),
+       .ofdata_to_platdata = ast_timer_ofdata_to_platdata,
+       .ops = &ast_timer_ops,
+       .flags = DM_FLAG_PRE_RELOC,
+};
--

Quoted 1 line

2.11.0.390.gc69c2f50cf-goog

Regards,
Simon

Quoted 1 line

The driver uses watchdog timer to do resets and particular watchdog device to use is hardcoded (0)

Signed-off-by Maxim Sloyko <[email protected]>

---

 drivers/sysreset/Makefile       |  1 +
 drivers/sysreset/sysreset_ast.c | 55 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 drivers/sysreset/sysreset_ast.c

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423 ++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

arch/arm/dts/ast2500.dtsi new file +0 -0

new file mode 100644
index 0000000000..1a2a3f7ee3

Quoted 9 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423 ++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

arch/arm/dts/ast2500.dtsi line 2

Quoted 8 lines

arch/arm/dts/ast2500.dtsi new file +2 -0

new file mode 100644
index 0000000000..1a2a3f7ee3
@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ * https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Is this also found in the Linux kernel or not yet? Thanks!

--
Tom

Quoted 11 lines · Tom Rini

Quoted 9 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423 ++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

Quoted 10 lines

arch/arm/dts/ast2500.dtsi line 2

Quoted 8 lines

arch/arm/dts/ast2500.dtsi new file +2 -0

new file mode 100644
index 0000000000..1a2a3f7ee3
@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ * https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Is this also found in the Linux kernel or not yet? Thanks!

Yes, this is also in in main Linux kernel now, as I've found out, but actually there is a number of differences, most notably there is no pin configuration in this device tree, because there is no pinctrl driver.

Should I remove this reference or modify it?

Quoted 23 lines · Maxim Sloyko

Quoted 21 lines · Tom Rini

Quoted 15 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423 ++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

arch/arm/dts/ast2500.dtsi new file +0 -0

new file mode 100644
index 0000000000..1a2a3f7ee3

arch/arm/dts/ast2500.dtsi line 2

Quoted 2 lines

arch/arm/dts/ast2500.dtsi new file +2 -0

@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ * https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Is this also found in the Linux kernel or not yet? Thanks!

Quoted 4 lines

Yes, this is also in in main Linux kernel now, as I've found out, but actually there is a number of differences, most notably there is no pin configuration in this device tree, because there is no pinctrl driver.

Quoted 2 lines

Should I remove this reference or modify it?

Ideally, we will take the kernel dts files and then add what we need on top of that in one of CONFIG_SYS_CPU/CONFIG_SYS_SOC/CONFIG_SYS_VENDOR -u-boot.dtsi files, see for example arch/arm/dts/sunxi-u-boot.dtsi or arch/arm/dts/tegra124-nyan-big-u-boot.dtsi

--
Tom

Quoted 24 lines · Tom Rini

Quoted 23 lines · Maxim Sloyko

Quoted 21 lines · Tom Rini

Quoted 15 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423 ++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

arch/arm/dts/ast2500.dtsi new file +0 -0

new file mode 100644
index 0000000000..1a2a3f7ee3

arch/arm/dts/ast2500.dtsi line 2

Quoted 2 lines

arch/arm/dts/ast2500.dtsi new file +2 -0

@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ * https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Is this also found in the Linux kernel or not yet? Thanks!

Quoted 4 lines

Quoted 4 lines

Yes, this is also in in main Linux kernel now, as I've found out, but actually there is a number of differences, most notably there is no pin configuration in this device tree, because there is no pinctrl driver.

Actually, I take that back, I was looking at the wrong linux Linux kernel tree still... Only basic version of device tree has made it to mainline kernel, but it's enough at the moment, so I used that instead.

Quoted 7 lines

Quoted 2 lines

Should I remove this reference or modify it?

Ideally, we will take the kernel dts files and then add what we need on top of that in one of CONFIG_SYS_CPU/CONFIG_SYS_SOC/CONFIG_SYS_VENDOR -u-boot.dtsi files, see for example arch/arm/dts/sunxi-u-boot.dtsi or arch/arm/dts/tegra124-nyan-big-u-boot.dtsi

OK, so I took the device tree from the Linux kernel, (ast2500.dtsi), added modifications in ast2500-u-boot.dtsi and now include ast2500-u-boot.dtsi in ast2500-evb.dts. Let me know if I misunderstood you.

Hi Maxim,

Quoted 9 lines · Maxim Sloyko

Quoted 8 lines · Tom Rini

Quoted 7 lines · Maxim Sloyko

Quoted 6 lines · Tom Rini

Quoted 4 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423

++++++++++++++++++++++++++++++++

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

arch/arm/dts/ast2500.dtsi line 2

Quoted 8 lines

arch/arm/dts/ast2500.dtsi new file +2 -0

new file mode 100644
index 0000000000..1a2a3f7ee3
@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ * https://github.com/openbmc/linux/blob/c5682cb/arch/arm/

boot/dts/aspeed-g5.dtsi

Quoted 21 lines

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

Is this also found in the Linux kernel or not yet? Thanks!

Yes, this is also in in main Linux kernel now, as I've found out, but actually there is a number of differences, most notably there is no pin configuration in this device tree, because there is no pinctrl driver.

Actually, I take that back, I was looking at the wrong linux Linux kernel tree still... Only basic version of device tree has made it to mainline kernel, but it's enough at the moment, so I used that instead.

Quoted 7 lines

Quoted 2 lines

Should I remove this reference or modify it?

Ideally, we will take the kernel dts files and then add what we need on top of that in one of CONFIG_SYS_CPU/CONFIG_SYS_SOC/CONFIG_SYS_VENDOR -u-boot.dtsi files, see for example arch/arm/dts/sunxi-u-boot.dtsi or arch/arm/dts/tegra124-nyan-big-u-boot.dtsi

Quoted 4 lines

OK, so I took the device tree from the Linux kernel, (ast2500.dtsi), added modifications in ast2500-u-boot.dtsi and now include ast2500-u-boot.dtsi in ast2500-evb.dts. Let me know if I misunderstood you.

There is some magic in the Makefile which automatically includes the .dtsi if you name it correctly:

# Try these files in order to find the U-Boot-specific .dtsi include file u_boot_dtsi_options = $(wildcard $(dts_dir)/$(basename $(notdir $<))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \
$(wildcard $(dts_dir)/u-boot.dtsi)

So you should not need to include it explicitly.

Regards,
Simon

Quoted 66 lines · Simon Glass

Hi Maxim,

Quoted 45 lines · Maxim Sloyko

Quoted 26 lines · Tom Rini

Quoted 25 lines · Maxim Sloyko

Quoted 23 lines · Tom Rini

Quoted 18 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423
++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

arch/arm/dts/ast2500.dtsi new file +2 -0

new file mode 100644
index 0000000000..1a2a3f7ee3
@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ *

Quoted 1 line

https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Is this also found in the Linux kernel or not yet? Thanks!

Quoted 4 lines

Quoted 4 lines

Yes, this is also in in main Linux kernel now, as I've found out, but actually there is a number of differences, most notably there is no pin configuration in this device tree, because there is no pinctrl driver.

Actually, I take that back, I was looking at the wrong linux Linux kernel tree still... Only basic version of device tree has made it to mainline kernel, but it's enough at the moment, so I used that instead.

Quoted 7 lines

Quoted 2 lines

Should I remove this reference or modify it?

Ideally, we will take the kernel dts files and then add what we need on top of that in one of CONFIG_SYS_CPU/CONFIG_SYS_SOC/CONFIG_SYS_VENDOR -u-boot.dtsi files, see for example arch/arm/dts/sunxi-u-boot.dtsi or arch/arm/dts/tegra124-nyan-big-u-boot.dtsi

Quoted 4 lines

OK, so I took the device tree from the Linux kernel, (ast2500.dtsi), added modifications in ast2500-u-boot.dtsi and now include ast2500-u-boot.dtsi in ast2500-evb.dts. Let me know if I misunderstood you.

There is some magic in the Makefile which automatically includes the .dtsi if you name it correctly:

# Try these files in order to find the U-Boot-specific .dtsi include file u_boot_dtsi_options = $(wildcard $(dts_dir)/$(basename $(notdir $<))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \
$(wildcard $(dts_dir)/u-boot.dtsi)

Quoted 1 line

So you should not need to include it explicitly.

Well, it looks like the only applicable for this case would be CONFIG_SYS_SOC, but it looks like this setting also affects the mach- and arch- directories used. In this case I want different aspeed SoCs to share those directories, but
they will have different DTs.

Hi Maxim,

Quoted 72 lines · Maxim Sloyko

Quoted 67 lines · Simon Glass

Hi Maxim,

Quoted 45 lines · Maxim Sloyko

Quoted 26 lines · Tom Rini

Quoted 25 lines · Maxim Sloyko

Quoted 23 lines · Tom Rini

Quoted 18 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423
++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

arch/arm/dts/ast2500.dtsi new file +2 -0

new file mode 100644
index 0000000000..1a2a3f7ee3
@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ *

Quoted 1 line

https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Is this also found in the Linux kernel or not yet? Thanks!

Quoted 4 lines

Quoted 4 lines

Yes, this is also in in main Linux kernel now, as I've found out, but actually there is a number of differences, most notably there is no pin configuration in this device tree, because there is no pinctrl driver.

Actually, I take that back, I was looking at the wrong linux Linux kernel tree still... Only basic version of device tree has made it to mainline kernel, but it's enough at the moment, so I used that instead.

Quoted 7 lines

Quoted 2 lines

Should I remove this reference or modify it?

Ideally, we will take the kernel dts files and then add what we need on top of that in one of CONFIG_SYS_CPU/CONFIG_SYS_SOC/CONFIG_SYS_VENDOR -u-boot.dtsi files, see for example arch/arm/dts/sunxi-u-boot.dtsi or arch/arm/dts/tegra124-nyan-big-u-boot.dtsi

Quoted 4 lines

OK, so I took the device tree from the Linux kernel, (ast2500.dtsi), added modifications in ast2500-u-boot.dtsi and now include ast2500-u-boot.dtsi in ast2500-evb.dts. Let me know if I misunderstood you.

There is some magic in the Makefile which automatically includes the .dtsi if you name it correctly:

# Try these files in order to find the U-Boot-specific .dtsi include file u_boot_dtsi_options = $(wildcard $(dts_dir)/$(basename $(notdir $<))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \
$(wildcard $(dts_dir)/u-boot.dtsi)

So you should not need to include it explicitly.

Quoted 5 lines

Well, it looks like the only applicable for this case would be CONFIG_SYS_SOC, but it looks like this setting also affects the mach- and arch- directories used. In this case I want different aspeed SoCs to share those directories, but
they will have different DTs.

The first option is to a .dtsi derived from the .dts filename. Does that help?

Regards,
Simon

Quoted 76 lines · Simon Glass

Hi Maxim,

Quoted 72 lines · Maxim Sloyko

Quoted 67 lines · Simon Glass

Hi Maxim,

Quoted 45 lines · Maxim Sloyko

Quoted 26 lines · Tom Rini

Quoted 25 lines · Maxim Sloyko

Quoted 23 lines · Tom Rini

Quoted 18 lines · Maxim Sloyko

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/dts/ast2500.dtsi               | 423
++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/ast2500-scu.h |  29 +++
 2 files changed, 452 insertions(+)
 create mode 100644 arch/arm/dts/ast2500.dtsi
 create mode 100644 include/dt-bindings/clock/ast2500-scu.h

arch/arm/dts/ast2500.dtsi new file +2 -0

new file mode 100644
index 0000000000..1a2a3f7ee3
@@ -0,0 +1,423 @@
+/* This device tree is copied from
+ *

Quoted 1 line

https://github.com/openbmc/linux/blob/c5682cb/arch/arm/boot/dts/aspeed-g5.dtsi

Is this also found in the Linux kernel or not yet? Thanks!

Quoted 4 lines

Quoted 4 lines

Yes, this is also in in main Linux kernel now, as I've found out, but actually there is a number of differences, most notably there is no pin configuration in this device tree, because there is no pinctrl driver.

Actually, I take that back, I was looking at the wrong linux Linux kernel tree still... Only basic version of device tree has made it to mainline kernel, but it's enough at the moment, so I used that instead.

Quoted 7 lines

Quoted 2 lines

Should I remove this reference or modify it?

Ideally, we will take the kernel dts files and then add what we need on top of that in one of CONFIG_SYS_CPU/CONFIG_SYS_SOC/CONFIG_SYS_VENDOR -u-boot.dtsi files, see for example arch/arm/dts/sunxi-u-boot.dtsi or arch/arm/dts/tegra124-nyan-big-u-boot.dtsi

Quoted 4 lines

OK, so I took the device tree from the Linux kernel, (ast2500.dtsi), added modifications in ast2500-u-boot.dtsi and now include ast2500-u-boot.dtsi in ast2500-evb.dts. Let me know if I misunderstood you.

There is some magic in the Makefile which automatically includes the .dtsi if you name it correctly:

# Try these files in order to find the U-Boot-specific .dtsi include file u_boot_dtsi_options = $(wildcard $(dts_dir)/$(basename $(notdir $<))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_SOC))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_CPU))-u-boot.dtsi) \
$(wildcard $(dts_dir)/$(subst $\",,$(CONFIG_SYS_VENDOR))-u-boot.dtsi) \
$(wildcard $(dts_dir)/u-boot.dtsi)

So you should not need to include it explicitly.

Quoted 7 lines

Quoted 5 lines

Well, it looks like the only applicable for this case would be CONFIG_SYS_SOC, but it looks like this setting also affects the mach- and arch- directories used. In this case I want different aspeed SoCs to share those directories, but
they will have different DTs.

The first option is to a .dtsi derived from the .dts filename. Does that help?

Hmm, probably not... if I understand that correctly.

This is what I have right now:

ast2500.dtsi -- device tree from the linux kernel, used by all ast2500-based boards.
ast2500-u-boot.dtsi -- these are u-boot specific additions, with u-boot,dm-pre-reloc labels, clock specification and SDRAM driver node. These are still need to be shared by all ast2500-based boards. ast2500-evb.dts -- Eval board specific stuff, right now basically just chosen node with stdout-path and clock frequency specification for SDRAM.

I don't this it would make sense to have ast2500-evb-u-boot.dtsi just to avoid include, because when I later create ast2500-zaius-bmc.dts or something like that, I would have the same problem.

This driver is ast2500 specific and is not compatible with earlier

versions of this chip. The differences are not that large, but they are in somewhat random places, so making it compatible with ast2400 is not worth the effort at the moment.

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/include/asm/arch-aspeed/scu_ast2500.h | 108 +++++++++++
 drivers/clk/Makefile                           |   2 +
 drivers/clk/aspeed/Makefile                    |   7 +
 drivers/clk/aspeed/clk_ast2500.c               | 255 +++++++++++++++++++++++++
 4 files changed, 372 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-aspeed/scu_ast2500.h
 create mode 100644 drivers/clk/aspeed/Makefile
 create mode 100644 drivers/clk/aspeed/clk_ast2500.c

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +66 -0

new file mode 100644
index 0000000000..febff9d2d3
@@ -0,0 +1,108 @@
+#ifndef _ASM_ARCH_SCU_AST2500_H
+#define _ASM_ARCH_SCU_AST2500_H
+
+#define SCU_UNLOCK_VALUE               0x1688a8a8
+
+#define SCU_HWSTRAP_VGAMEM_MASK                3
+#define SCU_HWSTRAP_VGAMEM_SHIFT       2
+#define SCU_HWSTRAP_DDR4               (1 << 24)
+#define SCU_HWSTRAP_CLKIN_25MHZ                (1 << 23)
+
+#define SCU_MPLL_DENUM_SHIFT           0
+#define SCU_MPLL_DENUM_MASK            0x1f
+#define SCU_MPLL_NUM_SHIFT             5
+#define SCU_MPLL_NUM_MASK              0xff
+#define SCU_MPLL_POST_SHIFT            13
+#define SCU_MPLL_POST_MASK             0x3f
+
+#define SCU_HPLL_DENUM_SHIFT           0
+#define SCU_HPLL_DENUM_MASK            0x1f
+#define SCU_HPLL_NUM_SHIFT             5
+#define SCU_HPLL_NUM_MASK              0xff
+#define SCU_HPLL_POST_SHIFT            13
+#define SCU_HPLL_POST_MASK             0x3f
+
+#define SCU_MISC2_UARTCLK_SHIFT                24
+
+#define SCU_MISC_UARTCLK_DIV13         (1 << 12)
+
+#ifndef __ASSEMBLY__
+
+struct ast2500_clk_priv {
+       struct ast2500_scu *scu;
+};
+
+struct ast2500_scu {
+       u32 protection_key;
+       u32 sysreset_ctrl1;
+       u32 clk_sel1;
+       u32 clk_stop_ctrl1;
+       u32 freq_counter_ctrl;
+       u32 freq_counter_cmp;
+       u32 intr_ctrl;
+       u32 d2_pll_param;
+       u32 m_pll_param;
+       u32 h_pll_param;
+       u32 d_pll_param;
+       u32 misc_ctrl1;
+       u32 pci_config[3];
+       u32 sysreset_status;
+       u32 vga_handshake[2];
+       u32 mac_clk_delay;
+       u32 misc_ctrl2;
+       u32 vga_scratch[8];
+       u32 hwstrap;
+       u32 rng_ctrl;
+       u32 rng_data;
+       u32 rev_id;
+       u32 pinmux_ctrl[6];
+       u32 reserved0;
+       u32 extrst_sel;
+       u32 pinmux_ctrl1[4];
+       u32 reserved1[2];
+       u32 mac_clk_delay_100M;
+       u32 mac_clk_delay_10M;
+       u32 wakeup_enable;
+       u32 wakeup_control;

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +18 -0

+       struct ast2500_cpu2 {
+               u32 ctrl;
+               u32 base_addr[9];
+               u32 cache_ctrl;
+       } cpu2;
+       u32 reserved4;
+       u32 d_pll_ext_param[3];
+       u32 d2_pll_ext_param[3];
+       u32 mh_pll_ext_param;
+       u32 reserved5;
+       u32 chip_id[2];
+       u32 reserved6[2];
+       u32 uart_clk_ctrl;
+       u32 reserved7[7];
+       u32 pcie_config;
+       u32 mmio_decode;
+       u32 reloc_ctrl_decode[2];
+       u32 mailbox_addr;

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +4 -0

+
+#endif  /* __ASSEMBLY__ */
+
+#endif  /* _ASM_ARCH_SCU_AST2500_H */

drivers/clk/Makefile +2 -0

index 40a5e8cae8..625513789c 100644
@@ -16,3 +16,5 @@ obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
 obj-$(CONFIG_CLK_EXYNOS) += exynos/
 obj-$(CONFIG_CLK_AT91) += at91/
 obj-$(CONFIG_CLK_BOSTON) += clk_boston.o
+
+obj-$(CONFIG_ARCH_ASPEED) += aspeed/

drivers/clk/aspeed/Makefile new file +7 -0

new file mode 100644
index 0000000000..65d1cd6e29
@@ -0,0 +1,7 @@
+#
+# Copyright (c) 2016 Google, Inc
+#
+# SPDX-License-Identifier:      GPL-2.0+
+#
+
+obj-$(CONFIG_ASPEED_AST2500) += clk_ast2500.o

drivers/clk/aspeed/clk_ast2500.c new file +49 -0

new file mode 100644
index 0000000000..c888a6d35b
@@ -0,0 +1,255 @@
+/*
+ * (C) Copyright 2016 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0
+ */
+
+#include <common.h>
+#include <asm/arch/scu_ast2500.h>
+#include <asm/io.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <dm/lists.h>
+#include <dt-bindings/clock/ast2500-scu.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * For H-PLL and M-PLL the formula is
+ * (Output Frequency) = CLKIN * ((M + 1) / (N + 1)) / (P + 1)
+ * M - Numerator
+ * N - Denumerator
+ * P - Post Divider
+ * They have the same layout in their control register.
+ */
+
+/*
+ * Get the rate of the M-PLL clock from input clock frequency and
+ * the value of the M-PLL Parameter Register.
+ */
+static ulong ast2500_get_mpll_rate(ulong clkin, u32 mpll_reg)
+{
+       const ulong num = (mpll_reg >> SCU_MPLL_NUM_SHIFT) & SCU_MPLL_NUM_MASK;
+       const ulong denum = (mpll_reg >> SCU_MPLL_DENUM_SHIFT)
+                       & SCU_MPLL_DENUM_MASK;
+       const ulong post_div = (mpll_reg >> SCU_MPLL_POST_SHIFT)
+                       & SCU_MPLL_POST_MASK;
+
+       return (clkin * ((num + 1) / (denum + 1))) / post_div;
+}
+
+/*
+ * Get the rate of the H-PLL clock from input clock frequency and
+ * the value of the H-PLL Parameter Register.
+ */
+static ulong ast2500_get_hpll_rate(ulong clkin, u32 hpll_reg)
+{
+       const ulong num = (hpll_reg >> SCU_HPLL_NUM_SHIFT) & SCU_HPLL_NUM_MASK;
+       const ulong denum = (hpll_reg >> SCU_HPLL_DENUM_SHIFT)
+                       & SCU_HPLL_DENUM_MASK;

drivers/clk/aspeed/clk_ast2500.c new file +51 -0

+        */
+       ulong uart_clkin;
+
+       if (readl(&scu->misc_ctrl2) & (1 << (uart + SCU_MISC2_UARTCLK_SHIFT)))
+               uart_clkin = 192 * 1000 * 1000;
+       else
+               uart_clkin = 24 * 1000 * 1000;
+
+       if (readl(&scu->misc_ctrl1) & SCU_MISC_UARTCLK_DIV13)
+               uart_clkin /= 13;
+
+       return uart_clkin;
+}
+
+static ulong ast2500_clk_get_rate(struct clk *clk)
+{
+       struct ast2500_clk_priv *priv = dev_get_priv(clk->dev);
+       ulong clkin = ast2500_get_clkin(priv->scu);
+       ulong rate;
+
+       switch (clk->id) {
+       case PLL_HPLL:
+       case ARMCLK:
+               /*
+                * This ignores dynamic/static slowdown of ARMCLK and may
+                * be inaccurate.
+                */
+               rate = ast2500_get_hpll_rate(clkin,
+                                            readl(&priv->scu->h_pll_param));
+               break;
+       case MCLK_DDR:
+               rate = ast2500_get_mpll_rate(clkin,
+                                            readl(&priv->scu->m_pll_param));
+               break;
+       case PCLK_UART1:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 1);
+               break;
+       case PCLK_UART2:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 2);
+               break;
+       case PCLK_UART3:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 3);
+               break;
+       case PCLK_UART4:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 4);
+               break;
+       case PCLK_UART5:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 5);
+               break;
+       default:
+               return -ENOENT;

drivers/clk/aspeed/clk_ast2500.c new file +12 -0

+}
+
+static void ast2500_scu_lock(struct ast2500_scu *scu)
+{
+       writel(~SCU_UNLOCK_VALUE, &scu->protection_key);
+       while (readl(&scu->protection_key))
+               ;
+}
+
+static ulong ast2500_configure_ddr(struct ast2500_scu *scu, ulong rate)
+{
+       ulong clkin = ast2500_get_clkin(scu);

drivers/clk/aspeed/clk_ast2500.c new file +15 -0

+       for (denum = 0; denum <= SCU_MPLL_DENUM_MASK; ++denum) {
+               for (post = 0; post <= SCU_MPLL_POST_MASK; ++post) {
+                       num = (rate_khz * (post + 1) / clkin_khz) * (denum + 1);
+                       ulong new_rate_khz = (clkin_khz
+                                             * ((num + 1) / (denum + 1)))
+                                            / (post + 1);
+
+                       /* Keep the rate below requested one. */
+                       if (new_rate_khz > rate_khz)
+                               continue;
+
+                       if (new_rate_khz - rate_khz < delta) {
+                               delta = new_rate_khz - rate_khz;
+
+                               best_num = num;

drivers/clk/aspeed/clk_ast2500.c new file +31 -0

+
+       mpll_reg = readl(&scu->m_pll_param);
+       mpll_reg &= ~((SCU_MPLL_POST_MASK << SCU_MPLL_POST_SHIFT)
+                     | (SCU_MPLL_NUM_MASK << SCU_MPLL_NUM_SHIFT)
+                     | (SCU_MPLL_DENUM_MASK << SCU_MPLL_DENUM_SHIFT));
+       mpll_reg |= (best_post << SCU_MPLL_POST_SHIFT)
+           | (best_num << SCU_MPLL_NUM_SHIFT)
+           | (best_denum << SCU_MPLL_DENUM_SHIFT);
+
+       ast2500_scu_unlock(scu);
+       writel(mpll_reg, &scu->m_pll_param);
+       ast2500_scu_lock(scu);
+
+       return ast2500_get_mpll_rate(clkin, mpll_reg);
+}
+
+static ulong ast2500_clk_set_rate(struct clk *clk, ulong rate)
+{
+       struct ast2500_clk_priv *priv = dev_get_priv(clk->dev);
+
+       ulong new_rate;
+       switch (clk->id) {
+       case PLL_MPLL:
+       case MCLK_DDR:
+               new_rate = ast2500_configure_ddr(priv->scu, rate);
+               break;
+       default:
+               return -ENOENT;
+       }
+
+       return new_rate;

drivers/clk/aspeed/clk_ast2500.c new file +30 -1

+
+       return 0;
+}
+
+static int ast2500_clk_bind(struct udevice *dev)
+{
+       int ret;
+
+       /* The reset driver does not have a device node, so bind it here */
+       ret = device_bind_driver(gd->dm_root, "ast_sysreset", "reset", &dev);
+       if (ret)
+               debug("Warning: No reset driver: ret=%d\n", ret);
+
+       return 0;
+}
+
+static const struct udevice_id ast2500_clk_ids[] = {
+       { .compatible = "aspeed,ast2500-scu" },
+       { }
+};
+
+U_BOOT_DRIVER(aspeed_ast2500_scu) = {
+       .name           = "aspeed_ast2500_scu",
+       .id             = UCLASS_CLK,
+       .of_match       = ast2500_clk_ids,
+       .priv_auto_alloc_size = sizeof(struct ast2500_clk_priv),
+       .ops            = &ast2500_clk_ops,
+       .bind           = ast2500_clk_bind,
+       .probe          = ast2500_clk_probe,
+};
--

2.11.0.390.gc69c2f50cf-goog

Hi Maxim,

Quoted 1 line · Maxim Sloyko

This driver is ast2500 specific and is not compatible with earlier

ast2500-specific

Quoted 11 lines

versions of this chip. The differences are not that large, but they are in somewhat random places, so making it compatible with ast2400 is not worth the effort at the moment.

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/include/asm/arch-aspeed/scu_ast2500.h | 108 +++++++++++
 drivers/clk/Makefile                           |   2 +
 drivers/clk/aspeed/Makefile                    |   7 +
 drivers/clk/aspeed/clk_ast2500.c               | 255

+++++++++++++++++++++++++

Review

Quoted 6 lines

 4 files changed, 372 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-aspeed/scu_ast2500.h
 create mode 100644 drivers/clk/aspeed/Makefile
 create mode 100644 drivers/clk/aspeed/clk_ast2500.c

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-aspeed/scu_ast2500.h

Quoted 75 lines

new file mode 100644
index 0000000000..febff9d2d3

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +70 -0

@@ -0,0 +1,108 @@
+#ifndef _ASM_ARCH_SCU_AST2500_H
+#define _ASM_ARCH_SCU_AST2500_H
+
+#define SCU_UNLOCK_VALUE               0x1688a8a8
+
+#define SCU_HWSTRAP_VGAMEM_MASK                3
+#define SCU_HWSTRAP_VGAMEM_SHIFT       2
+#define SCU_HWSTRAP_DDR4               (1 << 24)
+#define SCU_HWSTRAP_CLKIN_25MHZ                (1 << 23)
+
+#define SCU_MPLL_DENUM_SHIFT           0
+#define SCU_MPLL_DENUM_MASK            0x1f
+#define SCU_MPLL_NUM_SHIFT             5
+#define SCU_MPLL_NUM_MASK              0xff
+#define SCU_MPLL_POST_SHIFT            13
+#define SCU_MPLL_POST_MASK             0x3f
+
+#define SCU_HPLL_DENUM_SHIFT           0
+#define SCU_HPLL_DENUM_MASK            0x1f
+#define SCU_HPLL_NUM_SHIFT             5
+#define SCU_HPLL_NUM_MASK              0xff
+#define SCU_HPLL_POST_SHIFT            13
+#define SCU_HPLL_POST_MASK             0x3f
+
+#define SCU_MISC2_UARTCLK_SHIFT                24
+
+#define SCU_MISC_UARTCLK_DIV13         (1 << 12)
+
+#ifndef __ASSEMBLY__
+
+struct ast2500_clk_priv {
+       struct ast2500_scu *scu;
+};
+
+struct ast2500_scu {
+       u32 protection_key;
+       u32 sysreset_ctrl1;
+       u32 clk_sel1;
+       u32 clk_stop_ctrl1;
+       u32 freq_counter_ctrl;
+       u32 freq_counter_cmp;
+       u32 intr_ctrl;
+       u32 d2_pll_param;
+       u32 m_pll_param;
+       u32 h_pll_param;
+       u32 d_pll_param;
+       u32 misc_ctrl1;
+       u32 pci_config[3];
+       u32 sysreset_status;
+       u32 vga_handshake[2];
+       u32 mac_clk_delay;
+       u32 misc_ctrl2;
+       u32 vga_scratch[8];
+       u32 hwstrap;
+       u32 rng_ctrl;
+       u32 rng_data;
+       u32 rev_id;
+       u32 pinmux_ctrl[6];
+       u32 reserved0;
+       u32 extrst_sel;
+       u32 pinmux_ctrl1[4];
+       u32 reserved1[2];
+       u32 mac_clk_delay_100M;
+       u32 mac_clk_delay_10M;
+       u32 wakeup_enable;
+       u32 wakeup_control;
+       u32 reserved2[3];
+       u32 sysreset_ctrl2;
+       u32 clk_sel2;
+       u32 clk_stop_ctrl2;

arch/arm/include/asm/arch-aspeed/scu_ast2500.h line 76

Quoted 6 lines

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +6 -0

+       u32 freerun_counter;
+       u32 freerun_counter_ext;
+       u32 clk_duty_meas_ctrl;
+       u32 clk_duty_meas_res;
+       u32 reserved3[4];
+       /* The next registers are not key protected */

key-protected

Quoted 23 lines

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +22 -0

+       struct ast2500_cpu2 {
+               u32 ctrl;
+               u32 base_addr[9];
+               u32 cache_ctrl;
+       } cpu2;
+       u32 reserved4;
+       u32 d_pll_ext_param[3];
+       u32 d2_pll_ext_param[3];
+       u32 mh_pll_ext_param;
+       u32 reserved5;
+       u32 chip_id[2];
+       u32 reserved6[2];
+       u32 uart_clk_ctrl;
+       u32 reserved7[7];
+       u32 pcie_config;
+       u32 mmio_decode;
+       u32 reloc_ctrl_decode[2];
+       u32 mailbox_addr;
+       u32 shared_sram_decode[2];
+       u32 bmc_rev_id;
+       u32 reserved8;
+       u32 bmc_device_id;

arch/arm/include/asm/arch-aspeed/scu_ast2500.h line 104

Quoted 6 lines

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +6 -0

+       u32 reserved9[13];
+       u32 clk_duty_sel;
+};
+
+int ast_get_clk(struct udevice **devp);
+void *ast_get_scu(void);

Function comments please.

drivers/clk/aspeed/clk_

Quoted 29 lines

arch/arm/include/asm/arch-aspeed/scu_ast2500.h new file +4 -0

+
+#endif  /* __ASSEMBLY__ */
+
+#endif  /* _ASM_ARCH_SCU_AST2500_H */

drivers/clk/Makefile +2 -0

index 40a5e8cae8..625513789c 100644
@@ -16,3 +16,5 @@ obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
 obj-$(CONFIG_CLK_EXYNOS) += exynos/
 obj-$(CONFIG_CLK_AT91) += at91/
 obj-$(CONFIG_CLK_BOSTON) += clk_boston.o
+
+obj-$(CONFIG_ARCH_ASPEED) += aspeed/

drivers/clk/aspeed/Makefile new file +7 -0

new file mode 100644
index 0000000000..65d1cd6e29
@@ -0,0 +1,7 @@
+#
+# Copyright (c) 2016 Google, Inc
+#
+# SPDX-License-Identifier:      GPL-2.0+
+#
+
+obj-$(CONFIG_ASPEED_AST2500) += clk_ast2500.o

drivers/clk/aspeed/clk_ renamed from drivers/clk/aspeed/clk_ast2500.c +0 -0

ast2500.c

Quoted 27 lines

new file mode 100644
index 0000000000..c888a6d35b

drivers/clk/aspeed/clk_ast2500.c new file +22 -0

@@ -0,0 +1,255 @@
+/*
+ * (C) Copyright 2016 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0
+ */
+
+#include <common.h>
+#include <asm/arch/scu_ast2500.h>
+#include <asm/io.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <dm/lists.h>
+#include <dt-bindings/clock/ast2500-scu.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * For H-PLL and M-PLL the formula is
+ * (Output Frequency) = CLKIN * ((M + 1) / (N + 1)) / (P + 1)
+ * M - Numerator
+ * N - Denumerator
+ * P - Post Divider

drivers/clk/aspeed/clk_ast2500.c line 32

Quoted 10 lines

drivers/clk/aspeed/clk_ast2500.c new file +10 -0

+ * They have the same layout in their control register.
+ */
+
+/*
+ * Get the rate of the M-PLL clock from input clock frequency and
+ * the value of the M-PLL Parameter Register.
+ */
+static ulong ast2500_get_mpll_rate(ulong clkin, u32 mpll_reg)
+{
+       const ulong num = (mpll_reg >> SCU_MPLL_NUM_SHIFT) &

SCU_MPLL_NUM_MASK;

Quoted 5 lines

drivers/clk/aspeed/clk_ast2500.c new file +5 -0

+       const ulong denum = (mpll_reg >> SCU_MPLL_DENUM_SHIFT)
+                       & SCU_MPLL_DENUM_MASK;
+       const ulong post_div = (mpll_reg >> SCU_MPLL_POST_SHIFT)
+                       & SCU_MPLL_POST_MASK;
+

drivers/clk/aspeed/clk_ast2500.c

Quoted 10 lines

drivers/clk/aspeed/clk_ast2500.c new file +10 -0

+       return (clkin * ((num + 1) / (denum + 1))) / post_div;
+}
+
+/*
+ * Get the rate of the H-PLL clock from input clock frequency and
+ * the value of the H-PLL Parameter Register.
+ */
+static ulong ast2500_get_hpll_rate(ulong clkin, u32 hpll_reg)
+{
+       const ulong num = (hpll_reg >> SCU_HPLL_NUM_SHIFT) &

SCU_HPLL_NUM_MASK;

drivers/clk/aspeed/clk_ast2500.c line 59

Quoted 12 lines

drivers/clk/aspeed/clk_ast2500.c new file +12 -0

+       const ulong denum = (hpll_reg >> SCU_HPLL_DENUM_SHIFT)
+                       & SCU_HPLL_DENUM_MASK;
+       const ulong post_div = (hpll_reg >> SCU_HPLL_POST_SHIFT)
+                       & SCU_HPLL_POST_MASK;
+
+       return (clkin * ((num + 1) / (denum + 1))) / post_div;
+}
+
+static ulong ast2500_get_clkin(struct ast2500_scu *scu)
+{
+       return readl(&scu->hwstrap) & SCU_HWSTRAP_CLKIN_25MHZ
+                       ? 25*1000*1000 : 24*1000*1000;

Spaces around *

drivers/clk/aspeed/clk_ast2500.c

Quoted 4 lines

drivers/clk/aspeed/clk_ast2500.c new file +3 -0

+}
+
+static ulong ast2500_get_uart_clk_rate(struct ast2500_scu *scu, int

uart)

Function comment. What is 'uart' for?

drivers/clk/aspeed/clk_ast2500.c

Quoted 4 lines

drivers/clk/aspeed/clk_ast2500.c new file +3 -0

+{
+       /*
+        * ast2500 datasheet is very confusing when it comes to UART

clocks,

drivers/clk/aspeed/clk_ast2500.c

Quoted 4 lines

drivers/clk/aspeed/clk_ast2500.c new file +4 -0

+        * especially when CLKIN = 25 MHz. The settings are in
+        * different registers and it is unclear how they interact.
+        *
+        * This has only been tested with default settings and CLKIN =

24 MHz.

Can you detect this and return -EINVAL if it can't work? If not, that's fine.

drivers/clk/aspeed/clk_ast2500.c

Quoted 5 lines

drivers/clk/aspeed/clk_ast2500.c new file +4 -0

+        */
+       ulong uart_clkin;
+
+       if (readl(&scu->misc_ctrl2) & (1 << (uart +

SCU_MISC2_UARTCLK_SHIFT)))

Quoted 15 lines

drivers/clk/aspeed/clk_ast2500.c new file +15 -0

+               uart_clkin = 192 * 1000 * 1000;
+       else
+               uart_clkin = 24 * 1000 * 1000;
+
+       if (readl(&scu->misc_ctrl1) & SCU_MISC_UARTCLK_DIV13)
+               uart_clkin /= 13;
+
+       return uart_clkin;
+}
+
+static ulong ast2500_clk_get_rate(struct clk *clk)
+{
+       struct ast2500_clk_priv *priv = dev_get_priv(clk->dev);
+       ulong clkin = ast2500_get_clkin(priv->scu);
+       ulong rate;

drivers/clk/aspeed/clk_ast2500.c line 122

Quoted 10 lines

drivers/clk/aspeed/clk_ast2500.c new file +10 -0

+
+       switch (clk->id) {
+       case PLL_HPLL:
+       case ARMCLK:
+               /*
+                * This ignores dynamic/static slowdown of ARMCLK and may
+                * be inaccurate.
+                */
+               rate = ast2500_get_hpll_rate(clkin,
+

readl(&priv->scu->h_pll_param));

drivers/clk/aspeed/clk_ast2500.c line 122

Quoted 4 lines

drivers/clk/aspeed/clk_ast2500.c new file +4 -0

+               break;
+       case MCLK_DDR:
+               rate = ast2500_get_mpll_rate(clkin,
+

readl(&priv->scu->m_pll_param));

Quoted 22 lines

drivers/clk/aspeed/clk_ast2500.c new file +22 -0

+               break;
+       case PCLK_UART1:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 1);
+               break;
+       case PCLK_UART2:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 2);
+               break;
+       case PCLK_UART3:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 3);
+               break;
+       case PCLK_UART4:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 4);
+               break;
+       case PCLK_UART5:
+               rate = ast2500_get_uart_clk_rate(priv->scu, 5);
+               break;
+       default:
+               return -ENOENT;
+       }
+
+       return rate;
+}

drivers/clk/aspeed/clk_ast2500.c line 130

Quoted 6 lines

drivers/clk/aspeed/clk_ast2500.c new file +6 -0

+
+static void ast2500_scu_unlock(struct ast2500_scu *scu)
+{
+       writel(SCU_UNLOCK_VALUE, &scu->protection_key);
+       while (!readl(&scu->protection_key))
+               ;

Can this ever timeout?

Quoted 13 lines

drivers/clk/aspeed/clk_ast2500.c new file +12 -0

+}
+
+static void ast2500_scu_lock(struct ast2500_scu *scu)
+{
+       writel(~SCU_UNLOCK_VALUE, &scu->protection_key);
+       while (readl(&scu->protection_key))
+               ;
+}
+
+static ulong ast2500_configure_ddr(struct ast2500_scu *scu, ulong rate)
+{
+       ulong clkin = ast2500_get_clkin(scu);

drivers/clk/aspeed/clk_ast2500.c line 152

Quoted 10 lines

drivers/clk/aspeed/clk_ast2500.c new file +10 -0

+       u32 mpll_reg;
+
+       /*
+        * There are not that many combinations of numerator, denumerator
+        * and post divider, so just brute force the best combination.
+        * However, to avoid overflow when multiplying, use kHz.
+        */
+       const ulong clkin_khz = clkin / 1000;
+       const ulong rate_khz = rate / 1000;
+

drop blank line

drivers/clk/aspeed/clk_ast2500.c line 157

Quoted 5 lines

drivers/clk/aspeed/clk_ast2500.c new file +5 -0

+       ulong best_num = 0;
+       ulong best_denum = 0;
+       ulong best_post = 0;
+       ulong delta = rate;
+

drop blank line

drivers/clk/aspeed/clk_ast2500.c line 158

Quoted 1 line

drivers/clk/aspeed/clk_ast2500.c new file +1 -0

+       ulong num, denum, post;

add blank line

drivers/clk/aspeed/clk_ast2500.c

Quoted 3 lines

drivers/clk/aspeed/clk_ast2500.c new file +3 -0

+       for (denum = 0; denum <= SCU_MPLL_DENUM_MASK; ++denum) {
+               for (post = 0; post <= SCU_MPLL_POST_MASK; ++post) {
+                       num = (rate_khz * (post + 1) / clkin_khz) *

(denum + 1);

drivers/clk/aspeed/clk_ast2500.c line 163

Quoted 2 lines

drivers/clk/aspeed/clk_ast2500.c new file +2 -0

+                       ulong new_rate_khz = (clkin_khz
+                                             * ((num + 1) / (denum +

1)))

Quoted 10 lines

drivers/clk/aspeed/clk_ast2500.c new file +10 -0

+                                            / (post + 1);
+
+                       /* Keep the rate below requested one. */
+                       if (new_rate_khz > rate_khz)
+                               continue;
+
+                       if (new_rate_khz - rate_khz < delta) {
+                               delta = new_rate_khz - rate_khz;
+
+                               best_num = num;

drivers/clk/aspeed/clk_ast2500.c line 183

Quoted 10 lines

drivers/clk/aspeed/clk_ast2500.c new file +10 -0

+                               best_denum = denum;
+                               best_post = post;
+
+                               if (delta == 0)
+                                       goto rate_calc_done;
+                       }
+               }
+       }
+
+ rate_calc_done:

Drop blank line

Can it fail? Do you need to check delta?

Quoted 36 lines

drivers/clk/aspeed/clk_ast2500.c new file +35 -0

+
+       mpll_reg = readl(&scu->m_pll_param);
+       mpll_reg &= ~((SCU_MPLL_POST_MASK << SCU_MPLL_POST_SHIFT)
+                     | (SCU_MPLL_NUM_MASK << SCU_MPLL_NUM_SHIFT)
+                     | (SCU_MPLL_DENUM_MASK << SCU_MPLL_DENUM_SHIFT));
+       mpll_reg |= (best_post << SCU_MPLL_POST_SHIFT)
+           | (best_num << SCU_MPLL_NUM_SHIFT)
+           | (best_denum << SCU_MPLL_DENUM_SHIFT);
+
+       ast2500_scu_unlock(scu);
+       writel(mpll_reg, &scu->m_pll_param);
+       ast2500_scu_lock(scu);
+
+       return ast2500_get_mpll_rate(clkin, mpll_reg);
+}
+
+static ulong ast2500_clk_set_rate(struct clk *clk, ulong rate)
+{
+       struct ast2500_clk_priv *priv = dev_get_priv(clk->dev);
+
+       ulong new_rate;
+       switch (clk->id) {
+       case PLL_MPLL:
+       case MCLK_DDR:
+               new_rate = ast2500_configure_ddr(priv->scu, rate);
+               break;
+       default:
+               return -ENOENT;
+       }
+
+       return new_rate;
+}
+
+struct clk_ops ast2500_clk_ops = {
+       .get_rate = ast2500_clk_get_rate,

drivers/clk/aspeed/clk_ast2500.c line 224

Quoted 6 lines

drivers/clk/aspeed/clk_ast2500.c new file +6 -0

+       .set_rate = ast2500_clk_set_rate,
+};
+
+static int ast2500_clk_probe(struct udevice *dev)
+{
+       struct ast2500_clk_priv *priv = dev_get_priv(dev);

blank line

drivers/clk/aspeed/clk_ast2500.c line 225

Quoted 2 lines

drivers/clk/aspeed/clk_ast2500.c new file +1 -0

+       priv->scu = (struct ast2500_scu *)dev_get_addr(dev);

Maybe dev_get_addr_ptr()

Also should check that it is not FDT_ADDR_NONE.

drivers/clk/aspeed/clk_ast2500.c

Quoted 10 lines

drivers/clk/aspeed/clk_ast2500.c new file +9 -0

+
+       return 0;
+}
+
+static int ast2500_clk_bind(struct udevice *dev)
+{
+       int ret;
+
+       /* The reset driver does not have a device node, so bind it here

*/

drivers/clk/aspeed/clk_ast2500.c line 235

Quoted 1 line

drivers/clk/aspeed/clk_ast2500.c new file +1 -0

+       ret = device_bind_driver(gd->dm_root, "ast_sysreset", "reset",

&dev);

Quoted 21 lines

drivers/clk/aspeed/clk_ast2500.c new file +20 -1

+       if (ret)
+               debug("Warning: No reset driver: ret=%d\n", ret);
+
+       return 0;
+}
+
+static const struct udevice_id ast2500_clk_ids[] = {
+       { .compatible = "aspeed,ast2500-scu" },
+       { }
+};
+
+U_BOOT_DRIVER(aspeed_ast2500_scu) = {
+       .name           = "aspeed_ast2500_scu",
+       .id             = UCLASS_CLK,
+       .of_match       = ast2500_clk_ids,
+       .priv_auto_alloc_size = sizeof(struct ast2500_clk_priv),
+       .ops            = &ast2500_clk_ops,
+       .bind           = ast2500_clk_bind,
+       .probe          = ast2500_clk_probe,
+};
--

Quoted 2 lines

2.11.0.390.gc69c2f50cf-goog

Regards,
Simon

Helper function to get access to SCU (System Control Unit) through Clock driver. This is similar to rockchip_get_cru function, which was used as an example.

It will be used by other drivers to get access to SCU.

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/mach-aspeed/Kconfig               |  2 ++
 arch/arm/mach-aspeed/Makefile              |  1 +
 arch/arm/mach-aspeed/ast2500/Kconfig       |  6 ++++++
 arch/arm/mach-aspeed/ast2500/Makefile      |  1 +
 arch/arm/mach-aspeed/ast2500/clk_ast2500.c | 31 ++++++++++++++++++++++++++++++
 5 files changed, 41 insertions(+)
 create mode 100644 arch/arm/mach-aspeed/ast2500/Kconfig
 create mode 100644 arch/arm/mach-aspeed/ast2500/Makefile
 create mode 100644 arch/arm/mach-aspeed/ast2500/clk_ast2500.c

Hi Maxim,

Quoted 6 lines · Maxim Sloyko

Helper function to get access to SCU (System Control Unit) through Clock driver. This is similar to rockchip_get_cru function, which was used as an example.

It will be used by other drivers to get access to SCU.

Quoted 8 lines

Signed-off-by Maxim Sloyko <[email protected]>

---

 arch/arm/mach-aspeed/Kconfig               |  2 ++
 arch/arm/mach-aspeed/Makefile              |  1 +
 arch/arm/mach-aspeed/ast2500/Kconfig       |  6 ++++++
 arch/arm/mach-aspeed/ast2500/Makefile      |  1 +
 arch/arm/mach-aspeed/ast2500/clk_ast2500.c | 31

++++++++++++++++++++++++++++++