Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
On 01/19/11 23:19, Tom Warren wrote:
Signed-off-by: Tom Warren twar...@nvidia.com
---
Changes for V2:
- Coding style cleanup
- Move serial driver changes to separate patch
- Use board/nvidia/ instead of /board/tegra
- Remove TRUE/FALSE defines
- Use standard NS16550 register/bit defines in UART init
Changes for V3:
- Use I/O accessors for Tegra2 HW MMIO register access
- Allow conditional compile of UARTA/UARTD code to save space
arch/arm/cpu/armv7/tegra2/Makefile | 48 +
arch/arm/cpu/armv7/tegra2/board.c| 91 ++
arch/arm/cpu/armv7/tegra2/config.mk | 28 +++
arch/arm/cpu/armv7/tegra2/lowlevel_init.S| 66 +++
arch/arm/cpu/armv7/tegra2/sys_info.c | 35
arch/arm/cpu/armv7/tegra2/timer.c| 122 +
arch/arm/include/asm/arch-tegra2/clk_rst.h | 155
arch/arm/include/asm/arch-tegra2/pinmux.h| 52 ++
arch/arm/include/asm/arch-tegra2/pmc.h | 125 +
arch/arm/include/asm/arch-tegra2/sys_proto.h | 33
arch/arm/include/asm/arch-tegra2/tegra2.h| 49 +
arch/arm/include/asm/arch-tegra2/uart.h | 45 +
board/nvidia/common/board.c | 249
++
board/nvidia/common/board.h | 57 ++
14 files changed, 1155 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/cpu/armv7/tegra2/Makefile
create mode 100644 arch/arm/cpu/armv7/tegra2/board.c
create mode 100644 arch/arm/cpu/armv7/tegra2/config.mk
create mode 100644 arch/arm/cpu/armv7/tegra2/lowlevel_init.S
create mode 100644 arch/arm/cpu/armv7/tegra2/sys_info.c
create mode 100644 arch/arm/cpu/armv7/tegra2/timer.c
create mode 100644 arch/arm/include/asm/arch-tegra2/clk_rst.h
create mode 100644 arch/arm/include/asm/arch-tegra2/pinmux.h
create mode 100644 arch/arm/include/asm/arch-tegra2/pmc.h
create mode 100644 arch/arm/include/asm/arch-tegra2/sys_proto.h
create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2.h
create mode 100644 arch/arm/include/asm/arch-tegra2/uart.h
create mode 100644 board/nvidia/common/board.c
create mode 100644 board/nvidia/common/board.h
[ snip ]
+ */
+
+#ifndef _CLK_RST_H_
+#define _CLK_RST_H_
+
+/* Clock/Reset Controller (CLK_RST_CONTROLLER_) regs */
+
+typedef volatile struct clk_rst_ctlr {
Is it necessary to use the structure to map the clocks and reset controller?
Wouldn't be better to port Linux implementation of Tegra2 clocks to U-Boot as
well?
Besides, since you're using I/O accessors anyway, the struct can replaces with
base address and offset definitions.
+ uint crc_rst_src; /* _RST_SOURCE_0, 0x00*/
+ uint crc_rst_dev_l; /* _RST_DEVICES_L_0,0x04*/
+ uint crc_rst_dev_h; /* _RST_DEVICES_H_0,0x08*/
+ uint crc_rst_dev_u; /* _RST_DEVICES_U_0,0x0C*/
+ uint crc_clk_out_enb_l; /* _CLK_OUT_ENB_L_0,0x10*/
+ uint crc_clk_out_enb_h; /* _CLK_OUT_ENB_H_0,0x14*/
[ snip ]
+
+#ifndef _PINMUX_H_
+#define _PINMUX_H_
+
+/* APB MISC Pin Mux and Tristate (APB_MISC_PP_) registers */
+
+typedef volatile struct pinmux_tri_ctlr {
The same comment is valid also for the pin multiplexing registers...
+ uint pmt_reserved0; /* ABP_MISC_PP_ reserved offset 00 */
+ uint pmt_reserved1; /* ABP_MISC_PP_ reserved offset 04 */
+ uint pmt_strap_opt_a; /* _STRAPPING_OPT_A_0, offset 08 */
+
+#ifndef _PMC_H_
+#define _PMC_H_
+
+/* Power Management Controller (APBDEV_PMC_) registers */
+
+typedef volatile struct pmc_ctlr {
And for the PMC registers as well.
+ uint pmc_cntrl; /* _CNTRL_0, offset 00 */
+ uint pmc_sec_disable; /* _SEC_DISABLE_0, offset 04 */
+ uint pmc_pmc_swrst; /* _PMC_SWRST_0, offset 08 */
+ uint pmc_wake_mask; /* _WAKE_MASK_0, offset 0C */
+ uint pmc_wake_lvl; /* _WAKE_LVL_0, offset 10 */
[ snip ]
+#ifndef _TEGRA2_H_
+#define _TEGRA2_H_
+
+#define NV_PA_SDRAM_BASE 0x
+#define NV_PA_TMRUS_BASE 0x60005010
+#define NV_PA_CLK_RST_BASE 0x60006000
+#define NV_PA_APB_MISC_BASE 0x7000
+#define NV_PA_APB_UARTA_BASE (NV_PA_APB_MISC_BASE + 0x6000)
+#define NV_PA_APB_UARTB_BASE (NV_PA_APB_MISC_BASE + 0x6040)
+#define NV_PA_APB_UARTC_BASE (NV_PA_APB_MISC_BASE + 0x6200)
+#define NV_PA_APB_UARTD_BASE (NV_PA_APB_MISC_BASE + 0x6300)
+#define NV_PA_APB_UARTE_BASE (NV_PA_APB_MISC_BASE + 0x6400)
+#define NV_PA_PMC_BASE 0x7000E400
what is the purpose of NV_PA prefix here?
+#define TEGRA2_SDRC_CS0 NV_PA_SDRAM_BASE
+#define LOW_LEVEL_SRAM_STACK 0x4000FFFC
+
+#ifndef __ASSEMBLY__
+typedef volatile struct timerus {
+ unsigned int cntr_1us;
[ snip ]
+#ifndef _UART_H_
+#define
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Mike,
On Mon, Jan 24, 2011 at 4:55 AM, Mike Rapoport m...@compulab.co.il wrote:
On 01/19/11 23:19, Tom Warren wrote:
Signed-off-by: Tom Warren twar...@nvidia.com
---
Changes for V2:
- Coding style cleanup
- Move serial driver changes to separate patch
- Use board/nvidia/ instead of /board/tegra
- Remove TRUE/FALSE defines
- Use standard NS16550 register/bit defines in UART init
Changes for V3:
- Use I/O accessors for Tegra2 HW MMIO register access
- Allow conditional compile of UARTA/UARTD code to save space
arch/arm/cpu/armv7/tegra2/Makefile | 48 +
arch/arm/cpu/armv7/tegra2/board.c | 91 ++
arch/arm/cpu/armv7/tegra2/config.mk | 28 +++
arch/arm/cpu/armv7/tegra2/lowlevel_init.S | 66 +++
arch/arm/cpu/armv7/tegra2/sys_info.c | 35
arch/arm/cpu/armv7/tegra2/timer.c | 122 +
arch/arm/include/asm/arch-tegra2/clk_rst.h | 155
arch/arm/include/asm/arch-tegra2/pinmux.h | 52 ++
arch/arm/include/asm/arch-tegra2/pmc.h | 125 +
arch/arm/include/asm/arch-tegra2/sys_proto.h | 33
arch/arm/include/asm/arch-tegra2/tegra2.h | 49 +
arch/arm/include/asm/arch-tegra2/uart.h | 45 +
board/nvidia/common/board.c | 249
++
board/nvidia/common/board.h | 57 ++
14 files changed, 1155 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/cpu/armv7/tegra2/Makefile
create mode 100644 arch/arm/cpu/armv7/tegra2/board.c
create mode 100644 arch/arm/cpu/armv7/tegra2/config.mk
create mode 100644 arch/arm/cpu/armv7/tegra2/lowlevel_init.S
create mode 100644 arch/arm/cpu/armv7/tegra2/sys_info.c
create mode 100644 arch/arm/cpu/armv7/tegra2/timer.c
create mode 100644 arch/arm/include/asm/arch-tegra2/clk_rst.h
create mode 100644 arch/arm/include/asm/arch-tegra2/pinmux.h
create mode 100644 arch/arm/include/asm/arch-tegra2/pmc.h
create mode 100644 arch/arm/include/asm/arch-tegra2/sys_proto.h
create mode 100644 arch/arm/include/asm/arch-tegra2/tegra2.h
create mode 100644 arch/arm/include/asm/arch-tegra2/uart.h
create mode 100644 board/nvidia/common/board.c
create mode 100644 board/nvidia/common/board.h
[ snip ]
+ */
+
+#ifndef _CLK_RST_H_
+#define _CLK_RST_H_
+
+/* Clock/Reset Controller (CLK_RST_CONTROLLER_) regs */
+
+typedef volatile struct clk_rst_ctlr {
Is it necessary to use the structure to map the clocks and reset controller?
Wouldn't be better to port Linux implementation of Tegra2 clocks to U-Boot as
well?
Besides, since you're using I/O accessors anyway, the struct can replaces with
base address and offset definitions.
I asked Wolfgang to pre-review the original patch, and this is what he
said about original
base+offset register access code:
Wolfgang We do not allow this in U-Boot. Please turn all offset
tables into C structs, and
Wolfgang create a set of I/O accessor functions (or macros) as needed
to provide the needed
Wolfgang memory barriers on your architecture.
Using structs seems like a natural way to map HW MMIO regs, and is
done throughout U-Boot.
The structs are already written, contain just the members needed for
U-Boot (to a large degree),
and as Wolfgang has said in the past, U-Boot is not Linux, so I see no
reason to bring in the
Linux Tegra2 structs for any of these HW blocks. When I start posting
the drivers (SPI, USB,
etc.), then it might make sense to use (copy w/edits) the Linux data
structs, etc.
+ uint crc_rst_src; /* _RST_SOURCE_0, 0x00*/
+ uint crc_rst_dev_l; /* _RST_DEVICES_L_0, 0x04*/
+ uint crc_rst_dev_h; /* _RST_DEVICES_H_0, 0x08*/
+ uint crc_rst_dev_u; /* _RST_DEVICES_U_0, 0x0C*/
+ uint crc_clk_out_enb_l; /* _CLK_OUT_ENB_L_0, 0x10*/
+ uint crc_clk_out_enb_h; /* _CLK_OUT_ENB_H_0, 0x14*/
[ snip ]
+
+#ifndef _PINMUX_H_
+#define _PINMUX_H_
+
+/* APB MISC Pin Mux and Tristate (APB_MISC_PP_) registers */
+
+typedef volatile struct pinmux_tri_ctlr {
The same comment is valid also for the pin multiplexing registers...
+ uint pmt_reserved0; /* ABP_MISC_PP_ reserved offset 00 */
+ uint pmt_reserved1; /* ABP_MISC_PP_ reserved offset 04 */
+ uint pmt_strap_opt_a; /* _STRAPPING_OPT_A_0, offset 08 */
+
+#ifndef _PMC_H_
+#define _PMC_H_
+
+/* Power Management Controller (APBDEV_PMC_) registers */
+
+typedef volatile struct pmc_ctlr {
And for the PMC registers as well.
+ uint pmc_cntrl; /* _CNTRL_0, offset 00 */
+ uint pmc_sec_disable; /* _SEC_DISABLE_0, offset 04 */
+ uint pmc_pmc_swrst; /* _PMC_SWRST_0, offset 08 */
+ uint pmc_wake_mask; /* _WAKE_MASK_0, offset 0C */
+ uint pmc_wake_lvl;
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Dear Mike Rapoport, In message 4d3d68a9.4040...@compulab.co.il you wrote: Besides, since you're using I/O accessors anyway, the struct can replaces with base address and offset definitions. We do not allow such construtcs in U-Boot. With C structs, you can have proper type checking by the compiler (well, at least assuming you have proper I/O accessors in place). +#define NV_PA_APB_UARTC_BASE (NV_PA_APB_MISC_BASE + 0x6200) +#define NV_PA_APB_UARTD_BASE (NV_PA_APB_MISC_BASE + 0x6300) +#define NV_PA_APB_UARTE_BASE (NV_PA_APB_MISC_BASE + 0x6400) +#define NV_PA_PMC_BASE 0x7000E400 what is the purpose of NV_PA prefix here? Good catch. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de When the bosses talk about improving productivity, they are never talking about themselves. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Dear Tom Warren, In message AANLkTinoK+s5OivHAyLg10Z=gwzkccujwjdykssfg...@mail.gmail.com you wrote: ... +#define NV_PA_APB_UARTD_BASE (NV_PA_APB_MISC_BASE + 0x6300) +#define NV_PA_APB_UARTE_BASE (NV_PA_APB_MISC_BASE + 0x6400) +#define NV_PA_PMC_BASE 0x7000E400 what is the purpose of NV_PA prefix here? NV_Physical_Address - a base address of a HW block (Power Management Cntrlr, etc.) Well, the NV_ part is not needed, right? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Writing a book is like washing an elephant: there's no good place to begin or end, and it's hard to keep track of what you've already covered. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Wolfgang ( Mike), On Mon, Jan 24, 2011 at 12:00 PM, Wolfgang Denk w...@denx.de wrote: Dear Tom Warren, In message AANLkTinoK+s5OivHAyLg10Z=gwzkccujwjdykssfg...@mail.gmail.com you wrote: ... +#define NV_PA_APB_UARTD_BASE (NV_PA_APB_MISC_BASE + 0x6300) +#define NV_PA_APB_UARTE_BASE (NV_PA_APB_MISC_BASE + 0x6400) +#define NV_PA_PMC_BASE 0x7000E400 what is the purpose of NV_PA prefix here? NV_Physical_Address - a base address of a HW block (Power Management Cntrlr, etc.) Well, the NV_ part is not needed, right? True. I can remove it, but why? It designates this as a NVIDIA-specific define. I see the same thing in AT91, OMAP, NetARM, DaVinci, IMX files, etc. etc. Best regards, Wolfgang Denk Thanks, Tom -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Writing a book is like washing an elephant: there's no good place to begin or end, and it's hard to keep track of what you've already covered. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Dear Tom Warren, In message aanlktimn6c5dufxwte4z8u6rrab84kgc30stwj8ne...@mail.gmail.com you wrote: +#define NV_PA_APB_UARTD_BASE (NV_PA_APB_MISC_BASE + 0x6300) +#define NV_PA_APB_UARTE_BASE (NV_PA_APB_MISC_BASE + 0x6400) +#define NV_PA_PMC_BASE 0x7000E400 what is the purpose of NV_PA prefix here? NV_Physical_Address - a base address of a HW block (Power Management Cntrlr, etc.) Well, the NV_ part is not needed, right? True. I can remove it, but why? It designates this as a NVIDIA-specific define. I see the same thing in AT91, OMAP, NetARM, DaVinci, IMX files, etc. etc. OK, I don't insist. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de A weak mind is like a microscope, which magnifies trifling things, but cannot receive great ones. -- Philip Earl of Chesterfield ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Dear Tom Warren, In message 1295471986-2395-2-git-send-email-twar...@nvidia.com you wrote: Signed-off-by: Tom Warren twar...@nvidia.com checkpatch.pl reports: total: 6 errors, 12 warnings, 1155 lines checked /tmp/patch has style problems, please review. Please clean up. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Where shall I begin, please your Majesty? he asked. Begin at the beginning, the King said, gravely, and go on till you come to the end: then stop.- Alice's Adventures in Wonderland, Lewis Carroll ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
On Wed, Jan 19, 2011 at 5:04 PM, Peter Tyser pty...@xes-inc.com wrote:
Hi Tom,
Some last minutes nits:
It looks like some of the new functions can be declared statically.
It'd be nice to do so where possible.
Which functions, Peter? Please point them out specifically, thanks.
snip
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra2/lowlevel_init.S
@@ -0,0 +1,66 @@
+/*
+ * Board specific setup info
This is CPU-specific code, correct?
Yes - I'll change the comment.
+ *
+ * (C) Copyright 2010,2011
+ * NVIDIA Corporation www.nvidia.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include config.h
+#include version.h
+
+_TEXT_BASE:
+ .word CONFIG_SYS_TEXT_BASE @ sdram load addr from config file
+
+.global invalidate_dcache
+invalidate_dcache:
+ mov pc, lr
+
+
+ .align 5
+.global reset_cpu
+reset_cpu:
+ ldr r1, rstctl @ get addr for global reset
+ @ reg
+ ldr r3, [r1]
+ orr r3, r3, #0x10
+ str r3, [r1] @ force reset
+ mov r0, r0
+_loop_forever:
+ b _loop_forever
+rstctl:
+ .word PRM_RSTCTRL
+
+.globl lowlevel_init
+lowlevel_init:
+ ldr sp, SRAM_STACK
+ str ip, [sp]
+ mov ip, lr
+ bl s_init @ go setup pll, mux memory
+ ldr ip, [sp]
+ mov lr, ip
+
+ mov pc, lr @ back to arch calling code
+
+ @ the literal pools origin
+ .ltorg
+
+SRAM_STACK:
+ .word LOW_LEVEL_SRAM_STACK
diff --git a/arch/arm/cpu/armv7/tegra2/sys_info.c
b/arch/arm/cpu/armv7/tegra2/sys_info.c
new file mode 100644
index 000..6d11dc1
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra2/sys_info.c
@@ -0,0 +1,35 @@
+/*
+ * (C) Copyright 2010,2011
+ * NVIDIA Corporation www.nvidia.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include common.h
+
+#ifdef CONFIG_DISPLAY_CPUINFO
+/* Print CPU information */
+int print_cpuinfo(void)
+{
+ puts(TEGRA2\n);
+
+ /* TBD: Add printf of major/minor rev info, stepping, etc. */
+ return 0;
+}
+#endif /* CONFIG_DISPLAY_CPUINFO */
diff --git a/arch/arm/cpu/armv7/tegra2/timer.c
b/arch/arm/cpu/armv7/tegra2/timer.c
new file mode 100644
index 000..858af0f
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra2/timer.c
@@ -0,0 +1,122 @@
+/*
+ * (C) Copyright 2010,2011
+ * NVIDIA Corporation www.nvidia.com
+ *
+ * (C) Copyright 2008
+ * Texas Instruments
+ *
+ * Richard Woodruff r-woodru...@ti.com
+ * Syed Moahmmed Khasim kha...@ti.com
+ *
+ * (C) Copyright 2002
+ * Sysgo Real-Time Solutions, GmbH www.elinos.com
+ * Marius Groeger mgroe...@sysgo.de
+ * Alex Zuepke a...@sysgo.de
+ *
+ * (C) Copyright 2002
+ * Gary Jennejohn, DENX Software Engineering, ga...@denx.de
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Graeme,
On Wed, Jan 19, 2011 at 5:20 PM, Graeme Russ graeme.r...@gmail.com wrote:
On Thu, Jan 20, 2011 at 8:19 AM, Tom Warren twarren.nvi...@gmail.com wrote:
+
+/*
+ * Routine: uart_clock_init
+ * Description: init the PLL and clock for the UART in uart_num
+ */
+void uart_clock_init(int uart_num)
+{
+ clk_rst_ctlr *const clkrst = (clk_rst_ctlr *)NV_PA_CLK_RST_BASE;
+ static int pllp_init_done;
+ u32 reg;
+
+ if (!pllp_init_done) {
+
+ /* Override pllp setup for 216MHz operation. */
+ reg = (PLL_BYPASS | PLL_BASE_OVRRIDE | PLL_DIVP);
+ reg |= (((NVRM_PLLP_FIXED_FREQ_KHZ/500) 8) | PLL_DIVM);
+ writel(reg, clkrst-crc_pllp_base);
+
+ reg |= PLL_ENABLE;
+ writel(reg, clkrst-crc_pllp_base);
Is this correct? Should it not be writel(reg, clkrst-crc_pllp_base);
Well, the PLLs, UART and device clocks that I'm writing all seem to work OK.
I'll take a look at the ARM asm code generated, but you are probably right.
But shouldn't the compiler have complained if I wasn't passing the
struct address?
Similarly for other readl()'s and writel()'s
Regards,
Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Wolfgang, On Thu, Jan 20, 2011 at 1:40 AM, Wolfgang Denk w...@denx.de wrote: Dear Tom Warren, In message 1295471986-2395-2-git-send-email-twar...@nvidia.com you wrote: Signed-off-by: Tom Warren twar...@nvidia.com checkpatch.pl reports: total: 6 errors, 12 warnings, 1155 lines checked /tmp/patch has style problems, please review. Please clean up. I run checkpatch.pl (v 0.31) on every patch before I submit it, and I did see 12 warnings but no errors. The warnings were minor - new typedefs and volatile structs. Could you please provide the text of the checkpatch.pl output so I can see what the errors might be? Thanks. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Where shall I begin, please your Majesty? he asked. Begin at the beginning, the King said, gravely, and go on till you come to the end: then stop. - Alice's Adventures in Wonderland, Lewis Carroll ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
On Thu, 2011-01-20 at 09:41 -0700, Tom Warren wrote:
On Wed, Jan 19, 2011 at 5:04 PM, Peter Tyser pty...@xes-inc.com wrote:
Hi Tom,
Some last minutes nits:
It looks like some of the new functions can be declared statically.
It'd be nice to do so where possible.
Which functions, Peter? Please point them out specifically, thanks.
Any function that won't be called from outside the scope of the file.
Eg it looks like init_uart() and setup_uart() are local functions and
should be static. Those are the 2 that jumped out initially, but you
should review to see if there are others.
snip
+/*
+ * Routine: uart_clock_init
+ * Description: init the PLL and clock for the UART in uart_num
+ */
+void uart_clock_init(int uart_num)
+{
Are all these uart functions board-specific? They look more
CPU-specific. If that's the case they should be moved somewhere in
arch/arm/*. Other boards that use the Tegra2 don't want to duplicate
this code or link into Nvidia's board/nvidia directory.
It's Tegra2 SoC-specific - that's not the CPU, per se. I guess I could move
it to arch/arm/cpu/armv7/tegra2, if you think it's important enough.
I think they should be moved. If they aren't, the next board vendor (eg
my company) that uses the Tegra2 will copy your board.[ch] into their
board/vendor directory and use them as a starting point, which is a
large duplication of code. Moving it somewhere in arch/arm is the
right thing to do and will make every future tegra2 board port cleaner
and easier.
Best,
Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Dear Tom Warren, In message AANLkTi=yMzEztQ5p=dosj4ukkfvx8gmrzoaxjbwbj...@mail.gmail.com you wrote: ... Are all these uart functions board-specific? They look more CPU-specific. If that's the case they should be moved somewhere in arch/arm/*. Other boards that use the Tegra2 don't want to duplicate this code or link into Nvidia's board/nvidia directory. It's Tegra2 SoC-specific - that's not the CPU, per se. I guess I could move it to arch/arm/cpu/armv7/tegra2, if you think it's important enough. Yes, please do. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Q: How many DEC repairman does it take to fix a flat ? A: Five; four to hold the car up and one to swap tires. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Dear Tom Warren, In message AANLkTimcTq74=jz0hjcmsumye3tf2zeuy9r9f7qvt...@mail.gmail.com you wrote: I'll take a look at the ARM asm code generated, but you are probably right. But shouldn't the compiler have complained if I wasn't passing the struct address? I'm surprised about this, too. But then, current mainline code still has the horrible (*(volatile unsigned int *)(a) = (v)) definition, so the cast will eat all potential warnings :-( Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de ...all the good computer designs are bootlegged; the formally planned products, if they are built at all, are dogs! - David E. Lundstrom, A Few Good Men From Univac, MIT Press, 1987 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Dear Tom Warren, In message AANLkTi=ujj5teeyqq8q4rw7qaicjvav4xs83pdcxh...@mail.gmail.com you wrote: I run checkpatch.pl (v 0.31) on every patch before I submit it, and I did see 12 warnings but no errors. The warnings were minor - new typedefs and volatile structs. Could you please provide the text of the checkpatch.pl output so I can see what the errors might be? Sorry for the false alarms, I was using an older version of checkpatch. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de It took him several minutes to understand any new idea put to him, and this is a very valuable trait in a leader, because anything any- one is still trying to explain to you after two minutes is probably important and anything they give up after a mere minute or so is al- most certainly something they shouldn't have been bothering you with in the first place. - Terry Pratchett, _Reaper Man_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
On Fri, Jan 21, 2011 at 9:50 AM, Wolfgang Denk w...@denx.de wrote:
Dear Tom Warren,
In message AANLkTimcTq74=jz0hjcmsumye3tf2zeuy9r9f7qvt...@mail.gmail.com you
wrote:
I'll take a look at the ARM asm code generated, but you are probably right.
But shouldn't the compiler have complained if I wasn't passing the
struct address?
I'm surprised about this, too. But then, current mainline code still
has the horrible (*(volatile unsigned int *)(a) = (v)) definition,
so the cast will eat all potential warnings :-(
Yes, I noticed this with x86 - I can do something like the following
without the compiler warning me:
typdef struct blah {
u32 foo;
u16 bar;
} blah_t;
blah_t *fred = 0x1000;
writel(1, fred-foo);
writel(1, fred-bar);
writew(1, fred-foo);
writew(1, fred-bar);
This is particularly nasty with the sc520's Memory Mapped Control
Registers - I have found a few here and there where longs were being
written to words and visa-versa
Regards,
Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
Hi Tom,
Some last minutes nits:
It looks like some of the new functions can be declared statically.
It'd be nice to do so where possible.
snip
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra2/lowlevel_init.S
@@ -0,0 +1,66 @@
+/*
+ * Board specific setup info
This is CPU-specific code, correct?
+ *
+ * (C) Copyright 2010,2011
+ * NVIDIA Corporation www.nvidia.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include config.h
+#include version.h
+
+_TEXT_BASE:
+ .word CONFIG_SYS_TEXT_BASE@ sdram load addr from config file
+
+.global invalidate_dcache
+invalidate_dcache:
+ mov pc, lr
+
+
+ .align 5
+.global reset_cpu
+reset_cpu:
+ ldr r1, rstctl @ get addr for global reset
+ @ reg
+ ldr r3, [r1]
+ orr r3, r3, #0x10
+ str r3, [r1]@ force reset
+ mov r0, r0
+_loop_forever:
+ b _loop_forever
+rstctl:
+ .word PRM_RSTCTRL
+
+.globl lowlevel_init
+lowlevel_init:
+ ldr sp, SRAM_STACK
+ str ip, [sp]
+ mov ip, lr
+ bl s_init @ go setup pll, mux memory
+ ldr ip, [sp]
+ mov lr, ip
+
+ mov pc, lr @ back to arch calling code
+
+ @ the literal pools origin
+ .ltorg
+
+SRAM_STACK:
+ .word LOW_LEVEL_SRAM_STACK
diff --git a/arch/arm/cpu/armv7/tegra2/sys_info.c
b/arch/arm/cpu/armv7/tegra2/sys_info.c
new file mode 100644
index 000..6d11dc1
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra2/sys_info.c
@@ -0,0 +1,35 @@
+/*
+ * (C) Copyright 2010,2011
+ * NVIDIA Corporation www.nvidia.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include common.h
+
+#ifdef CONFIG_DISPLAY_CPUINFO
+/* Print CPU information */
+int print_cpuinfo(void)
+{
+ puts(TEGRA2\n);
+
+ /* TBD: Add printf of major/minor rev info, stepping, etc. */
+ return 0;
+}
+#endif /* CONFIG_DISPLAY_CPUINFO */
diff --git a/arch/arm/cpu/armv7/tegra2/timer.c
b/arch/arm/cpu/armv7/tegra2/timer.c
new file mode 100644
index 000..858af0f
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra2/timer.c
@@ -0,0 +1,122 @@
+/*
+ * (C) Copyright 2010,2011
+ * NVIDIA Corporation www.nvidia.com
+ *
+ * (C) Copyright 2008
+ * Texas Instruments
+ *
+ * Richard Woodruff r-woodru...@ti.com
+ * Syed Moahmmed Khasim kha...@ti.com
+ *
+ * (C) Copyright 2002
+ * Sysgo Real-Time Solutions, GmbH www.elinos.com
+ * Marius Groeger mgroe...@sysgo.de
+ * Alex Zuepke a...@sysgo.de
+ *
+ * (C) Copyright 2002
+ * Gary Jennejohn, DENX Software Engineering, ga...@denx.de
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along
Re: [U-Boot] [PATCH v3 1/4] arm: Tegra2: Add basic NVIDIA Tegra2 SoC support
On Thu, Jan 20, 2011 at 8:19 AM, Tom Warren twarren.nvi...@gmail.com wrote:
+
+/*
+ * Routine: uart_clock_init
+ * Description: init the PLL and clock for the UART in uart_num
+ */
+void uart_clock_init(int uart_num)
+{
+ clk_rst_ctlr *const clkrst = (clk_rst_ctlr *)NV_PA_CLK_RST_BASE;
+ static int pllp_init_done;
+ u32 reg;
+
+ if (!pllp_init_done) {
+
+ /* Override pllp setup for 216MHz operation. */
+ reg = (PLL_BYPASS | PLL_BASE_OVRRIDE | PLL_DIVP);
+ reg |= (((NVRM_PLLP_FIXED_FREQ_KHZ/500) 8) | PLL_DIVM);
+ writel(reg, clkrst-crc_pllp_base);
+
+ reg |= PLL_ENABLE;
+ writel(reg, clkrst-crc_pllp_base);
Is this correct? Should it not be writel(reg, clkrst-crc_pllp_base);
Similarly for other readl()'s and writel()'s
Regards,
Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
