diff mbox

[U-Boot,09/12] aspeed/ast2500: Common board init functions for ast2500 based boards

Message ID 20170104194656.124368-10-maxims@google.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Maxim Sloyko Jan. 4, 2017, 7:46 p.m. UTC
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

Comments

Simon Glass Jan. 14, 2017, 5:14 p.m. UTC | #1
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
Maxim Sloyko Jan. 17, 2017, 8:17 p.m. UTC | #2
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 mbox

Patch

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;
+}
+