On 18 January 2017 at 14:44, Maxim Sloyko <[email protected]> wrote: > Add support for Watchdog Timer, which 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. > > 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. > > Add sysreset driver, which uses watchdog timer to do resets and particular > watchdog device to use is hardcoded (0) > > --- > > Changes in v4: > - Expanded AST2500 description in Kconfig > - Expanded ast_timer description in Kconfig > - Added struct ast_timer_counter to timer private data > - Use dev_get_addr_ptr in timer's of_platdata > - Added helper function to wdt for resetting peripherals > > Changes in v3: > - Added SYS_TEXT_BASE as Kconfig option > > Changes in v2: > - Moved number of WDTs to a Kconfig option > > Changes in v1: > - Merged together the patches related to aspeed common drivers and > configuration > - Fixed timer driver name (was sandbox_timer) > - Removed yet nonexistent files from mach-aspeed/Makefile > > > Signed-off-by: Maxim Sloyko <[email protected]> > --- > arch/arm/Kconfig | 7 +++ > arch/arm/Makefile | 1 + > arch/arm/include/asm/arch-aspeed/timer.h | 54 +++++++++++++++++ > arch/arm/include/asm/arch-aspeed/wdt.h | 99 > ++++++++++++++++++++++++++++++++ > arch/arm/mach-aspeed/Kconfig | 27 +++++++++ > arch/arm/mach-aspeed/Makefile | 7 +++ > arch/arm/mach-aspeed/ast_wdt.c | 59 +++++++++++++++++++ > drivers/sysreset/Makefile | 1 + > drivers/sysreset/sysreset_ast.c | 55 ++++++++++++++++++ > drivers/timer/Kconfig | 12 ++++ > drivers/timer/Makefile | 1 + > drivers/timer/ast_timer.c | 97 +++++++++++++++++++++++++++++++ > 12 files changed, 420 insertions(+) > create mode 100644 arch/arm/include/asm/arch-aspeed/timer.h > create mode 100644 arch/arm/include/asm/arch-aspeed/wdt.h > create mode 100644 arch/arm/mach-aspeed/Kconfig > create mode 100644 arch/arm/mach-aspeed/Makefile > create mode 100644 arch/arm/mach-aspeed/ast_wdt.c > create mode 100644 drivers/sysreset/sysreset_ast.c > create mode 100644 drivers/timer/ast_timer.c
Reviewed-by: Simon Glass <[email protected]> Some comments below which you could do as part of your clean-up if you like. > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 714dd8b514..135c544335 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -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" > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index 236debb452..cc73e1038e 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -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 > diff --git a/arch/arm/include/asm/arch-aspeed/timer.h > b/arch/arm/include/asm/arch-aspeed/timer.h > new file mode 100644 > index 0000000000..87c5b354ec > --- /dev/null > +++ b/arch/arm/include/asm/arch-aspeed/timer.h > @@ -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 We don't want have to #ifdefs in drivers. The driver should support both devices and use the compatible string to determine which is used. So here I think you can get rid of 'reserved'. Perhaps add a comment that these two registers don't exist on one device? > + struct ast_timer_counter timers2[5]; > +}; > + > +#endif /* __ASSEMBLY__ */ > + > +#endif /* _ASM_ARCH_TIMER_H */ > diff --git a/arch/arm/include/asm/arch-aspeed/wdt.h > b/arch/arm/include/asm/arch-aspeed/wdt.h > new file mode 100644 > index 0000000000..b292a0e67b > --- /dev/null > +++ b/arch/arm/include/asm/arch-aspeed/wdt.h > @@ -0,0 +1,99 @@ > +/* > + * (C) Copyright 2016 Google, Inc > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _ASM_ARCH_WDT_H > +#define _ASM_ARCH_WDT_H > + > +#define WDT_BASE 0x1e785000 TODO(email): Use device tree to get this value > + > +/* > + * Special value that needs to be written to counter_restart register to > + * (re)start the timer > + */ > +#define WDT_COUNTER_RESTART_VAL 0x4755 > + > +/* Control register */ > +#define WDT_CTRL_RESET_MODE_SHIFT 5 > +#define WDT_CTRL_RESET_MODE_MASK 3 > + > +#define WDT_CTRL_EN (1 << 0) > +#define WDT_CTRL_RESET (1 << 1) > +#define WDT_CTRL_CLK1MHZ (1 << 4) > +#define WDT_CTRL_2ND_BOOT (1 << 7) > + > +/* Values for Reset Mode */ > +#define WDT_CTRL_RESET_SOC 0 > +#define WDT_CTRL_RESET_CHIP 1 > +#define WDT_CTRL_RESET_CPU 2 > +#define WDT_CTRL_RESET_MASK 3 > + > +/* Reset Mask register */ > +#define WDT_RESET_ARM (1 << 0) > +#define WDT_RESET_COPROC (1 << 1) > +#define WDT_RESET_SDRAM (1 << 2) > +#define WDT_RESET_AHB (1 << 3) > +#define WDT_RESET_I2C (1 << 4) > +#define WDT_RESET_MAC1 (1 << 5) > +#define WDT_RESET_MAC2 (1 << 6) > +#define WDT_RESET_GCRT (1 << 7) > +#define WDT_RESET_USB20 (1 << 8) > +#define WDT_RESET_USB11_HOST (1 << 9) > +#define WDT_RESET_USB11_EHCI2 (1 << 10) > +#define WDT_RESET_VIDEO (1 << 11) > +#define WDT_RESET_HAC (1 << 12) > +#define WDT_RESET_LPC (1 << 13) > +#define WDT_RESET_SDSDIO (1 << 14) > +#define WDT_RESET_MIC (1 << 15) > +#define WDT_RESET_CRT2C (1 << 16) > +#define WDT_RESET_PWM (1 << 17) > +#define WDT_RESET_PECI (1 << 18) > +#define WDT_RESET_JTAG (1 << 19) > +#define WDT_RESET_ADC (1 << 20) > +#define WDT_RESET_GPIO (1 << 21) > +#define WDT_RESET_MCTP (1 << 22) > +#define WDT_RESET_XDMA (1 << 23) > +#define WDT_RESET_SPI (1 << 24) > +#define WDT_RESET_MISC (1 << 25) > + > +#ifndef __ASSEMBLY__ > +struct ast_wdt { > + u32 counter_status; > + u32 counter_reload_val; > + u32 counter_restart; > + u32 ctrl; > + u32 timeout_status; > + u32 clr_timeout_status; > + u32 reset_width; > +#ifdef CONFIG_ASPEED_AST2500 > + u32 reset_mask; > +#else > + u32 reserved0; > +#endif Same here > +}; > + > +void wdt_stop(struct ast_wdt *wdt); > +void wdt_start(struct ast_wdt *wdt, u32 timeout); > + > +/** > + * Reset peripherals specified by mask > + * > + * Note, that this is only supported by ast2500 SoC > + * > + * @wdt: watchdog to use for this reset > + * @mask: reset mask. @return? > + */ > +int ast_wdt_reset_masked(struct ast_wdt *wdt, u32 mask); > + > +/** > + * ast_get_wdt() - get a pointer to watchdog registers > + * > + * @wdt_number: 0-based WDT peripheral number > + * @return pointer to registers or -ve error on error > + */ > +struct ast_wdt *ast_get_wdt(u8 wdt_number); > +#endif /* __ASSEMBLY__ */ > + > +#endif /* _ASM_ARCH_WDT_H */ > diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig > new file mode 100644 > index 0000000000..b72ed89af7 > --- /dev/null > +++ b/arch/arm/mach-aspeed/Kconfig > @@ -0,0 +1,27 @@ > +if ARCH_ASPEED > + > +config SYS_ARCH > + default "arm" > + > +config SYS_SOC > + default "aspeed" > + > +config SYS_TEXT_BASE > + default 0x00000000 > + > +config ASPEED_AST2500 > + bool "Support Aspeed AST2500 SoC" > + select CPU_ARM1176 > + help > + The Aspeed AST2500 is a ARM-based SoC with arm1176 CPU. > + It is used as Board Management Controller on many server boards, > + which is enabled by support of LPC and eSPI peripherals. > + > +config WDT_NUM > + int "Number of Watchdog Timers" > + default 3 if ASPEED_AST2500 > + help > + The number of Watchdot Timers on a SoC. > + AST2500 has three WDTsk earlier versions have two or fewer. > + > +endif > diff --git a/arch/arm/mach-aspeed/Makefile b/arch/arm/mach-aspeed/Makefile > new file mode 100644 > index 0000000000..a14b8f751d > --- /dev/null > +++ b/arch/arm/mach-aspeed/Makefile > @@ -0,0 +1,7 @@ > +# > +# Copyright (c) 2016 Google, Inc > +# > +# SPDX-License-Identifier: GPL-2.0+ > +# > + > +obj-$(CONFIG_ARCH_ASPEED) += ast_wdt.o > diff --git a/arch/arm/mach-aspeed/ast_wdt.c b/arch/arm/mach-aspeed/ast_wdt.c > new file mode 100644 > index 0000000000..22481ab7ea > --- /dev/null > +++ b/arch/arm/mach-aspeed/ast_wdt.c > @@ -0,0 +1,59 @@ > +/* > + * (C) Copyright 2016 Google, Inc > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <asm/io.h> > +#include <asm/arch/wdt.h> > +#include <linux/err.h> > + > +void wdt_stop(struct ast_wdt *wdt) > +{ > + clrbits_le32(&wdt->ctrl, WDT_CTRL_EN); > +} > + > +void wdt_start(struct ast_wdt *wdt, u32 timeout) > +{ > + writel(timeout, &wdt->counter_reload_val); > + writel(WDT_COUNTER_RESTART_VAL, &wdt->counter_restart); > + /* > + * Setting CLK1MHZ bit is just for compatibility with ast2400 part. > + * On ast2500 watchdog timer clock is fixed at 1MHz and the bit is > + * read-only > + */ > + setbits_le32(&wdt->ctrl, > + WDT_CTRL_EN | WDT_CTRL_RESET | WDT_CTRL_CLK1MHZ); > +} > + > +struct ast_wdt *ast_get_wdt(u8 wdt_number) > +{ > + if (wdt_number > CONFIG_WDT_NUM - 1) > + return ERR_PTR(-EINVAL); > + > + return (struct ast_wdt *)(WDT_BASE + > + sizeof(struct ast_wdt) * wdt_number); > +} > + > +int ast_wdt_reset_masked(struct ast_wdt *wdt, u32 mask) > +{ > +#ifdef CONFIG_ASPEED_AST2500 > + if (!mask) > + return -EINVAL; > + > + writel(mask, &wdt->reset_mask); > + clrbits_le32(&wdt->ctrl, > + WDT_CTRL_RESET_MASK << WDT_CTRL_RESET_MODE_SHIFT); > + wdt_start(wdt, 1); > + > + /* Wait for WDT to reset */ > + while (readl(&wdt->ctrl) & WDT_CTRL_EN) > + ; > + wdt_stop(wdt); > + > + return 0; > +#else > + return -EINVAL; > +#endif > +} > diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile > index fa75cc52de..37638a8eea 100644 > --- a/drivers/sysreset/Makefile > +++ b/drivers/sysreset/Makefile > @@ -14,3 +14,4 @@ obj-$(CONFIG_ROCKCHIP_RK3399) += sysreset_rk3399.o > obj-$(CONFIG_SANDBOX) += sysreset_sandbox.o > obj-$(CONFIG_ARCH_SNAPDRAGON) += sysreset_snapdragon.o > obj-$(CONFIG_TARGET_XTFPGA) += sysreset_xtfpga.o > +obj-$(CONFIG_ARCH_ASPEED) += sysreset_ast.o > diff --git a/drivers/sysreset/sysreset_ast.c b/drivers/sysreset/sysreset_ast.c > new file mode 100644 > index 0000000000..a0ab12851d > --- /dev/null > +++ b/drivers/sysreset/sysreset_ast.c > @@ -0,0 +1,55 @@ > +/* > + * (C) Copyright 2016 Google, Inc > + * > + * SPDX-License-Identifier: GPL-2.0 > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <errno.h> > +#include <sysreset.h> > +#include <asm/io.h> > +#include <asm/arch/wdt.h> > +#include <linux/err.h> > + > +/* Number of Watchdog Timer ticks before reset */ > +#define AST_WDT_RESET_TIMEOUT 10 > +#define AST_WDT_FOR_RESET 0 > + > +static int ast_sysreset_request(struct udevice *dev, enum sysreset_t type) > +{ > + struct ast_wdt *wdt = ast_get_wdt(AST_WDT_FOR_RESET); > + u32 reset_mode = 0; > + > + if (IS_ERR(wdt)) > + return PTR_ERR(wdt); > + > + switch (type) { > + case SYSRESET_WARM: > + reset_mode = WDT_CTRL_RESET_CPU; > + break; > + case SYSRESET_COLD: > + reset_mode = WDT_CTRL_RESET_CHIP; > + break; > + default: > + return -EPROTONOSUPPORT; > + } > + > + /* Clear reset mode bits */ > + clrsetbits_le32(&wdt->ctrl, > + (WDT_CTRL_RESET_MODE_MASK << > WDT_CTRL_RESET_MODE_SHIFT), > + (reset_mode << WDT_CTRL_RESET_MODE_SHIFT)); > + wdt_start(wdt, AST_WDT_RESET_TIMEOUT); > + > + return -EINPROGRESS; > +} > + > +static struct sysreset_ops ast_sysreset = { > + .request = ast_sysreset_request, > +}; > + > +U_BOOT_DRIVER(sysreset_ast) = { > + .name = "ast_sysreset", > + .id = UCLASS_SYSRESET, > + .ops = &ast_sysreset, > +}; > diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig > index cb18f12fc9..cd38a6d4bd 100644 > --- a/drivers/timer/Kconfig > +++ b/drivers/timer/Kconfig > @@ -46,4 +46,16 @@ 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. > + This is a simple sys timer driver, it is compatible with lib/time.c, > + but does not support any interrupts. Even though SoC has 8 hardware > + counters, they are all treated as a single device by this driver. > + This is mostly because they all share several registers which > + makes it difficult to completely separate them. > + > endmenu > diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile > index f351fbb4e0..a4b1a486b0 100644 > --- a/drivers/timer/Makefile > +++ b/drivers/timer/Makefile > @@ -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 > diff --git a/drivers/timer/ast_timer.c b/drivers/timer/ast_timer.c > new file mode 100644 > index 0000000000..d7c5460cd3 > --- /dev/null > +++ b/drivers/timer/ast_timer.c > @@ -0,0 +1,97 @@ > +/* > + * 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; > + struct ast_timer_counter *tmc; > +}; > + > +static struct ast_timer_counter *ast_get_timer_counter(struct ast_timer > *timer, > + 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 timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); > + > + writel(AST_TMC_RELOAD_VAL, &priv->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); > + > + *count = AST_TMC_RELOAD_VAL - readl(&priv->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 = dev_get_addr_ptr(dev); > + if (IS_ERR(priv->regs)) > + return PTR_ERR(priv->regs); > + > + priv->tmc = ast_get_timer_counter(priv->regs, AST_TICK_TIMER); > + > + return 0; > +} > + > +static const struct timer_ops ast_timer_ops = { > + .get_count = ast_timer_get_count, > +}; > + > +static const struct udevice_id ast_timer_ids[] = { > + { .compatible = "aspeed,ast2500-timer" }, > + { .compatible = "aspeed,ast2400-timer" }, Here you can put .data = ... and use an enum to select it. Then your driver can operate with either at runtime. Having said that I cannot see where ast_wdt_reset_masked() is called. > + { } > +}; > + > +U_BOOT_DRIVER(ast_timer) = { > + .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.483.g087da7b7c-goog > Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
