Message ID | 20170104194656.124368-10-maxims@google.com |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
Hi Maxim, On 4 January 2017 at 12:46, Maxim Sloyko <maxims@google.com> wrote: > Signed-off-by: Maxim Sloyko <maxims@google.com> > --- > > arch/arm/mach-aspeed/Makefile | 2 +- > arch/arm/mach-aspeed/ast2500-board.c | 74 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 75 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/mach-aspeed/ast2500-board.c > > diff --git a/arch/arm/mach-aspeed/Makefile b/arch/arm/mach-aspeed/Makefile > index 1f7af71b03..9d29ff7f6f 100644 > --- a/arch/arm/mach-aspeed/Makefile > +++ b/arch/arm/mach-aspeed/Makefile > @@ -5,4 +5,4 @@ > # > > obj-$(CONFIG_ARCH_ASPEED) += ast_wdt.o > -obj-$(CONFIG_ASPEED_AST2500) += ast2500/ > +obj-$(CONFIG_ASPEED_AST2500) += ast2500/ ast2500-board.o > diff --git a/arch/arm/mach-aspeed/ast2500-board.c b/arch/arm/mach-aspeed/ast2500-board.c > new file mode 100644 > index 0000000000..5b2b0ce132 > --- /dev/null > +++ b/arch/arm/mach-aspeed/ast2500-board.c > @@ -0,0 +1,74 @@ > +#include <common.h> > +#include <asm/arch/timer.h> > +#include <asm/arch/wdt.h> > +#include <asm/io.h> > +#include <dm.h> > +#include <dm/uclass.h> > +#include <linux/err.h> > +#include <ram.h> > +#include <timer.h> Check include file order. > + > +/* Second Watchdog Timer by default is configured /* * Second watchdog... * ... */ Please check globally. Try to use more columns if you can. > + * to trigger secondary boot source. > + */ > +#define AST_2ND_BOOT_WDT (1) Remove () around simple constants. > + > +/* Third Watchdog Timer by default is configured > + * to toggle Flash address mode switch before reset. > + */ > +#define AST_FLASH_ADDR_DETECT_WDT (2) > + > +DECLARE_GLOBAL_DATA_PTR; > + > +void lowlevel_init(void) > +{ > + /* > + * These two watchdogs need to be stopped as soon as possible, > + * otherwise the board might hang. By default they are set to > + * a very short timeout and even simple debug write to serial > + * console early in the init process might cause them to fire. > + */ > + struct ast_wdt *flash_addr_wdt = > + (struct ast_wdt *)(WDT_BASE + > + sizeof(struct ast_wdt) * > + AST_FLASH_ADDR_DETECT_WDT); > + > + clrbits_le32(&flash_addr_wdt->ctrl, WDT_CTRL_EN); > + > +#ifndef CONFIG_FIRMWARE_2ND_BOOT > + struct ast_wdt *sec_boot_wdt = > + (struct ast_wdt *)(WDT_BASE + > + sizeof(struct ast_wdt) * > + AST_2ND_BOOT_WDT); > + > + clrbits_le32(&sec_boot_wdt->ctrl, WDT_CTRL_EN); > +#endif Seems this should call a helper function in ast_wdt.c or similar. > +} > + > +int board_init(void) > +{ > + gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100; blank line > + return 0; > +} > + > +int dram_init(void) > +{ > + struct udevice *dev; > + int ret = uclass_get_device(UCLASS_RAM, 0, &dev); blank line, and better to separate decl and code in this case, since you check it below. > + if (ret) { > + debug("DRAM FAIL1\r\n"); > + return ret; > + } > + > + struct ram_info ram; move to top of function > + ret = ram_get_info(dev, &ram); > + if (ret) { > + debug("DRAM FAIL2\r\n"); > + return ret; > + } > + drop extra blank line > + > + gd->ram_size = ram.size; blank line > + return 0; > +} > + > -- > 2.11.0.390.gc69c2f50cf-goog > REgards, Simon
On Sat, Jan 14, 2017 at 9:14 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Maxim, > > On 4 January 2017 at 12:46, Maxim Sloyko <maxims@google.com> wrote: > > Signed-off-by: Maxim Sloyko <maxims@google.com> > > --- > > > > arch/arm/mach-aspeed/Makefile | 2 +- > > arch/arm/mach-aspeed/ast2500-board.c | 74 > ++++++++++++++++++++++++++++++++++++ > > 2 files changed, 75 insertions(+), 1 deletion(-) > > create mode 100644 arch/arm/mach-aspeed/ast2500-board.c > > > > diff --git a/arch/arm/mach-aspeed/Makefile b/arch/arm/mach-aspeed/ > Makefile > > index 1f7af71b03..9d29ff7f6f 100644 > > --- a/arch/arm/mach-aspeed/Makefile > > +++ b/arch/arm/mach-aspeed/Makefile > > @@ -5,4 +5,4 @@ > > # > > > > obj-$(CONFIG_ARCH_ASPEED) += ast_wdt.o > > -obj-$(CONFIG_ASPEED_AST2500) += ast2500/ > > +obj-$(CONFIG_ASPEED_AST2500) += ast2500/ ast2500-board.o > > diff --git a/arch/arm/mach-aspeed/ast2500-board.c > b/arch/arm/mach-aspeed/ast2500-board.c > > new file mode 100644 > > index 0000000000..5b2b0ce132 > > --- /dev/null > > +++ b/arch/arm/mach-aspeed/ast2500-board.c > > @@ -0,0 +1,74 @@ > > +#include <common.h> > > +#include <asm/arch/timer.h> > > +#include <asm/arch/wdt.h> > > +#include <asm/io.h> > > +#include <dm.h> > > +#include <dm/uclass.h> > > +#include <linux/err.h> > > +#include <ram.h> > > +#include <timer.h> > > Check include file order. > > > + > > +/* Second Watchdog Timer by default is configured > > /* > * Second watchdog... > * ... > */ > > Please check globally. Try to use more columns if you can. > > > + * to trigger secondary boot source. > > + */ > > +#define AST_2ND_BOOT_WDT (1) > > Remove () around simple constants. > > > + > > +/* Third Watchdog Timer by default is configured > > + * to toggle Flash address mode switch before reset. > > + */ > > +#define AST_FLASH_ADDR_DETECT_WDT (2) > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +void lowlevel_init(void) > > +{ > > + /* > > + * These two watchdogs need to be stopped as soon as possible, > > + * otherwise the board might hang. By default they are set to > > + * a very short timeout and even simple debug write to serial > > + * console early in the init process might cause them to fire. > > + */ > > + struct ast_wdt *flash_addr_wdt = > > + (struct ast_wdt *)(WDT_BASE + > > + sizeof(struct ast_wdt) * > > + AST_FLASH_ADDR_DETECT_WDT); > > + > > + clrbits_le32(&flash_addr_wdt->ctrl, WDT_CTRL_EN); > > + > > +#ifndef CONFIG_FIRMWARE_2ND_BOOT > > + struct ast_wdt *sec_boot_wdt = > > + (struct ast_wdt *)(WDT_BASE + > > + sizeof(struct ast_wdt) * > > + AST_2ND_BOOT_WDT); > > + > > + clrbits_le32(&sec_boot_wdt->ctrl, WDT_CTRL_EN); > > +#endif > > Seems this should call a helper function in ast_wdt.c or similar. > I can't call wdt_stop() here, because that would require stack, which is not yet available in lowlevel_init. Other comments will be addressed for v4. > > > +} > > + > > +int board_init(void) > > +{ > > + gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100; > > blank line > > > + return 0; > > +} > > + > > +int dram_init(void) > > +{ > > + struct udevice *dev; > > + int ret = uclass_get_device(UCLASS_RAM, 0, &dev); > > blank line, and better to separate decl and code in this case, since > you check it below. > > > + if (ret) { > > + debug("DRAM FAIL1\r\n"); > > + return ret; > > + } > > + > > + struct ram_info ram; > > move to top of function > > > + ret = ram_get_info(dev, &ram); > > + if (ret) { > > + debug("DRAM FAIL2\r\n"); > > + return ret; > > + } > > + > > drop extra blank line > > > + > > + gd->ram_size = ram.size; > > blank line > > > + return 0; > > +} > > + > > -- > > 2.11.0.390.gc69c2f50cf-goog > > > > REgards, > Simon >
diff --git a/arch/arm/mach-aspeed/Makefile b/arch/arm/mach-aspeed/Makefile index 1f7af71b03..9d29ff7f6f 100644 --- a/arch/arm/mach-aspeed/Makefile +++ b/arch/arm/mach-aspeed/Makefile @@ -5,4 +5,4 @@ # obj-$(CONFIG_ARCH_ASPEED) += ast_wdt.o -obj-$(CONFIG_ASPEED_AST2500) += ast2500/ +obj-$(CONFIG_ASPEED_AST2500) += ast2500/ ast2500-board.o diff --git a/arch/arm/mach-aspeed/ast2500-board.c b/arch/arm/mach-aspeed/ast2500-board.c new file mode 100644 index 0000000000..5b2b0ce132 --- /dev/null +++ b/arch/arm/mach-aspeed/ast2500-board.c @@ -0,0 +1,74 @@ +#include <common.h> +#include <asm/arch/timer.h> +#include <asm/arch/wdt.h> +#include <asm/io.h> +#include <dm.h> +#include <dm/uclass.h> +#include <linux/err.h> +#include <ram.h> +#include <timer.h> + +/* Second Watchdog Timer by default is configured + * to trigger secondary boot source. + */ +#define AST_2ND_BOOT_WDT (1) + +/* Third Watchdog Timer by default is configured + * to toggle Flash address mode switch before reset. + */ +#define AST_FLASH_ADDR_DETECT_WDT (2) + +DECLARE_GLOBAL_DATA_PTR; + +void lowlevel_init(void) +{ + /* + * These two watchdogs need to be stopped as soon as possible, + * otherwise the board might hang. By default they are set to + * a very short timeout and even simple debug write to serial + * console early in the init process might cause them to fire. + */ + struct ast_wdt *flash_addr_wdt = + (struct ast_wdt *)(WDT_BASE + + sizeof(struct ast_wdt) * + AST_FLASH_ADDR_DETECT_WDT); + + clrbits_le32(&flash_addr_wdt->ctrl, WDT_CTRL_EN); + +#ifndef CONFIG_FIRMWARE_2ND_BOOT + struct ast_wdt *sec_boot_wdt = + (struct ast_wdt *)(WDT_BASE + + sizeof(struct ast_wdt) * + AST_2ND_BOOT_WDT); + + clrbits_le32(&sec_boot_wdt->ctrl, WDT_CTRL_EN); +#endif +} + +int board_init(void) +{ + gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100; + return 0; +} + +int dram_init(void) +{ + struct udevice *dev; + int ret = uclass_get_device(UCLASS_RAM, 0, &dev); + if (ret) { + debug("DRAM FAIL1\r\n"); + return ret; + } + + struct ram_info ram; + ret = ram_get_info(dev, &ram); + if (ret) { + debug("DRAM FAIL2\r\n"); + return ret; + } + + + gd->ram_size = ram.size; + return 0; +} +
Signed-off-by: Maxim Sloyko <maxims@google.com> --- arch/arm/mach-aspeed/Makefile | 2 +- arch/arm/mach-aspeed/ast2500-board.c | 74 ++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-aspeed/ast2500-board.c