On 28/07/16 13:06, Marek Vasut wrote:
+++ b/board/imgtec/boston/boston-regs.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef __BOARD_BOSTON_REGS_H__
+#define __BOARD_BOSTON_REGS_H__
+
+#define BOSTON_PLAT_BASE 0x17ffd000
+#define BOSTON_LCD_BASE 0x17fff000
+
+/*
+ * Platform Register Definitions
+ */
+#define BOSTON_PLAT_CORE_CL 0x04
+
+#define BOSTON_PLAT_DDR3STAT 0x14
+# define BOSTON_PLAT_DDR3STAT_CALIB (1 << 2)
+
+#define BOSTON_PLAT_MMCMDIV 0x30
+# define BOSTON_PLAT_MMCMDIV_CLK0DIV (0xff << 0)
+# define BOSTON_PLAT_MMCMDIV_INPUT (0xff << 8)
+# define BOSTON_PLAT_MMCMDIV_MUL (0xff << 16)
+# define BOSTON_PLAT_MMCMDIV_CLK1DIV (0xff << 24)
+
+#define BOSTON_PLAT_DDRCONF0 0x38
+# define BOSTON_PLAT_DDRCONF0_SIZE (0xf << 0)
+
+#ifndef __ASSEMBLY__
+
+#include <asm/io.h>
+
+#define BUILD_PLAT_ACCESSORS(offset, name) \
+static inline uint32_t read_boston_##name(void)
\
+{ \
+ uint32_t *reg = (void *)CKSEG1ADDR(BOSTON_PLAT_BASE) + (offset);\
+ return __raw_readl(reg); \
+}
Don't we have enough standard accessors to confuse people ?
Why do you add another custom ones ? Remove this and just use
standard accessors throughout the code.
Hi Marek,
These accessors are simple wrappers around __raw_readl, I'd hardly say
they can be considered confusing. The alternative is lots of:
val = __raw_readl((void *)CKSEG1ADDR(BOSTON_PLAT_BASE) + OFFSET);
...and that is just plain ugly. Invoking readl on a field of a struct
representing these registers would be nice, but some of them need to be
accessed from assembly so that would involve duplication which isn't
nice. I think this way is the best option, where if you want to read the
Boston core_cl register you call read_boston_core_cl() - it's hardly
confusing what that does.
+BUILD_PLAT_ACCESSORS(BOSTON_PLAT_CORE_CL, core_cl)
+BUILD_PLAT_ACCESSORS(BOSTON_PLAT_MMCMDIV, mmcmdiv)
+BUILD_PLAT_ACCESSORS(BOSTON_PLAT_DDRCONF0, ddrconf0)
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __BOARD_BOSTON_REGS_H__ */
diff --git a/board/imgtec/boston/checkboard.c b/board/imgtec/boston/checkboard.c
new file mode 100644
index 0000000..417ac4e
--- /dev/null
+++ b/board/imgtec/boston/checkboard.c
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <common.h>
+
+#include <asm/mipsregs.h>
+
+#include "boston-lcd.h"
+#include "boston-regs.h"
+int checkboard(void)
+{
+ u32 changelist;
+
+ lowlevel_display("U-boot ");
+
+ printf("Board: MIPS Boston\n");
+
+ printf("CPU: 0x%08x", read_c0_prid());
This should be in print_cpuinfo()
I don't agree. This goes on to read a board-specific register to
determine information about the CPU (the revision of its RTL) and that
should not be done in arch-level code, which is what every other
implementation of print_cpuinfo is. Perhaps it would be better if we had
a nice DM-using CPU driver which Boston could have an extension of, but
we don't have that for MIPS right now & this series is not the place to
add it.
+ changelist = read_boston_core_cl();
+ if (changelist > 1)
+ printf(" cl%x", changelist);
+ putc('\n');
+
+ return 0;
+}
diff --git a/board/imgtec/boston/ddr.c b/board/imgtec/boston/ddr.c
new file mode 100644
index 0000000..7caed4b
--- /dev/null
+++ b/board/imgtec/boston/ddr.c
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <common.h>
+
+#include <asm/addrspace.h>
+
+#include "boston-regs.h"
+
+phys_size_t initdram(int board_type)
+{
+ u32 ddrconf0 = read_boston_ddrconf0();
+
+ return (phys_size_t)(ddrconf0 & BOSTON_PLAT_DDRCONF0_SIZE) << 30;
+}
+
+ulong board_get_usable_ram_top(ulong total_size)
+{
+ DECLARE_GLOBAL_DATA_PTR;
+
+ if (gd->ram_top < CONFIG_SYS_SDRAM_BASE) {
+ /* 2GB wrapped around to 0 */
+ return CKSEG0ADDR(256 << 20);
+ }
+
+ return min_t(unsigned long, gd->ram_top, CKSEG0ADDR(256 << 20));
+}
diff --git a/board/imgtec/boston/lowlevel_init.S
b/board/imgtec/boston/lowlevel_init.S
new file mode 100644
index 0000000..8928172
--- /dev/null
+++ b/board/imgtec/boston/lowlevel_init.S
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <config.h>
+
+#include <asm/addrspace.h>
+#include <asm/asm.h>
+#include <asm/mipsregs.h>
+#include <asm/regdef.h>
+
+#include "boston-regs.h"
+
+.data
+
+msg_ddr_cal: .ascii "DDR Cal "
+msg_ddr_ok: .ascii "DDR OK "
+
+.text
+
+LEAF(lowlevel_init)
+ move s0, ra
+
+ PTR_LA a0, msg_ddr_cal
+ bal lowlevel_display
Don't you need nop after branch on mips ?
No. Branch delay slots are filled automatically by the assembler unless
you explicitly tell it not to with a ".set noreorder" directive, and
they're not just filled with NOPs - they can contain essentially any
non-control-flow-affecting instruction that it's safe to execute
regardless of the outcome of the branch. They're also somewhat optional
in MIPSr6 where "compact" branches don't have delay slots, and gone
entirely in microMIPSr6.
+
+ PTR_LI t0, CKSEG1ADDR(BOSTON_PLAT_BASE)
+1: lw t1, BOSTON_PLAT_DDR3STAT(t0)
+ andi t1, t1, BOSTON_PLAT_DDR3STAT_CALIB
+ beqz t1, 1b
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
