[U-Boot] [PATCH 1/3] arm: Tegra2: Add generic Tegra2 SoC support Signed-off-by: Tom Warren <twarren@nvidia.com>

Peter Tyser
2011-01-12T23:56:44Z

Hi,

Your Signed-off-by seems to have made its way into the subject line. It
should be moved into the email contents. A brief description of what
this patch does, what peripherals are supported, etc would be nice too.

<snip>

> create mode 100644 board/tegra2/common/board.c
> create mode 100644 board/tegra2/common/board.h

The format of the board directory is generally board/<vendor>/<board> or
board/<board> if the vendor only plans on maintaining 1 board. If these
are nvidia-supported boards I'd think board/nvidia is the proper place
to add files board-specific files.

<snip>

> +/*
> + * Boot ROM initializes the odmdata in APBDEV_PMC_SCRATCH20_0,
> + * so we are using this value to identify memory size.
> + */
> +
> +static unsigned int query_sdram_size(void)
> +{
> + pmc_ctlr *const pmc = (pmc_ctlr *)NV_PA_PMC_BASE;
> + u32 reg;
> +
> + reg = pmc->pmc_scratch20;

In powerpc-land we use io access functions to access registers, eg
in_be32(). I'm not familiar with ARM, but I'd guess they should be used
in general. Someone with ARM experience feel free to chime in here.

> + debug("pmc->pmc_scratch20 (ODMData) = 0x%08lX\n", reg);
> +
> + /* bits 31:28 in OdmData are used for RAM size */
> + switch ((reg) >> 28) {
> + case 1:
> + return 0x10000000; /* 256 MB */
> + case 2:
> + return 0x20000000; /* 512 MB */
> + case 3:
> + default:
> + return 0x40000000; /* 1GB */
> + }
> +}

<snip>

> +#ifndef _CLK_RST_H_
> +#define _CLK_RST_H_
> +
> +/* Clock/Reset Controller (CLK_RST_CONTROLLER_) regs */
> +

Instead of adding a new vol_uint typedef and making each field of this
struct vol_uint, you can make each one uint, then instantiate the
structure as volatile.

There are a mixture of lower and upper case hex digits in the comments
below too - it'd be good to be consistent.

> +typedef struct clk_rst_ctlr {
> + vol_uint crc_rst_src; /* _RST_SOURCE_0, 0x00*/
> + vol_uint crc_rst_dev_l; /* _RST_DEVICES_L_0, 0x04*/
> + vol_uint crc_rst_dev_h; /* _RST_DEVICES_H_0, 0x08*/
> + vol_uint crc_rst_dev_u; /* _RST_DEVICES_U_0, 0x0C*/
> + vol_uint crc_clk_out_enb_l; /* _CLK_OUT_ENB_L_0, 0x10*/
> + vol_uint crc_clk_out_enb_h; /* _CLK_OUT_ENB_H_0, 0x14*/
> + vol_uint crc_clk_out_enb_u; /* _CLK_OUT_ENB_U_0, 0x18*/
> + vol_uint crc_reserved0; /* reserved_0, 0x1c*/
> + vol_uint crc_cclk_brst_pol; /* _CCLK_BURST_POLICY_0,0x20*/
> + vol_uint crc_super_cclk_div; /* _SUPER_CCLK_DIVIDER_0,0x24*/
> + vol_uint crc_sclk_brst_pol; /* _SCLK_BURST_POLICY_0, 0x28*/
> + vol_uint crc_super_sclk_div; /* _SUPER_SCLK_DIVIDER_0,0x2C*/
> + vol_uint crc_clk_sys_rate; /* _CLK_SYSTEM_RATE_0, 0x30*/
> + vol_uint crc_prog_dly_clk; /* _PROG_DLY_CLK_0, 0x34*/
> + vol_uint crc_aud_sync_clk_rate; /* _AUDIO_SYNC_CLK_RATE_0,0x38*/
> + vol_uint crc_reserved1; /* reserved_1, 0x3c*/
> + vol_uint crc_cop_clk_skip_plcy; /* _COP_CLK_SKIP_POLICY_0,0x40*/
> + vol_uint crc_clk_mask_arm; /* _CLK_MASK_ARM_0, 0x44*/
> + vol_uint crc_misc_clk_enb; /* _MISC_CLK_ENB_0, 0x48*/
> + vol_uint crc_clk_cpu_cmplx; /* _CLK_CPU_CMPLX_0, 0x4C*/
> + vol_uint crc_osc_ctrl; /* _OSC_CTRL_0, 0x50*/
> + vol_uint crc_pll_lfsr; /* _PLL_LFSR_0, 0x54*/
> + vol_uint crc_osc_freq_det; /* _OSC_FREQ_DET_0, 0x58*/
> + vol_uint crc_osc_freq_det_stat; /* _OSC_FREQ_DET_STATUS_0,0x5C*/
> + vol_uint crc_reserved2[8]; /* reserved_2[8], 0x60-7C*/
> +
> + vol_uint crc_pllc_base; /* _PLLC_BASE_0, 0x80*/
> + vol_uint crc_pllc_out; /* _PLLC_OUT_0, 0x84*/
> + vol_uint crc_reserved3; /* reserved_3, 0x88*/
> + vol_uint crc_pllc_misc; /* _PLLC_MISC_0, 0x8C*/
> +
> + vol_uint crc_pllm_base; /* _PLLM_BASE_0, 0x90*/
> + vol_uint crc_pllm_out; /* _PLLM_OUT_0, 0x94*/
> + vol_uint crc_reserved4; /* reserved_4, 0x98*/
> + vol_uint crc_pllm_misc; /* _PLLM_MISC_0, 0x9C*/
> +
> + vol_uint crc_pllp_base; /* _PLLP_BASE_0, 0xA0*/
> + vol_uint crc_pllp_outa; /* _PLLP_OUTA_0, 0xA4*/
> + vol_uint crc_pllp_outb; /* _PLLP_OUTB_0, 0xA8*/
> + vol_uint crc_pllp_misc; /* _PLLP_MISC_0, 0xAC*/
> +
> + vol_uint crc_plla_base; /* _PLLA_BASE_0, 0xB0*/
> + vol_uint crc_plla_out; /* _PLLA_OUT_0, 0xB4*/
> + vol_uint crc_reserved5; /* reserved_5, 0xB8*/
> + vol_uint crc_plla_misc; /* _PLLA_MISC_0, 0xBC*/
> +
> + vol_uint crc_pllu_base; /* _PLLU_BASE_0, 0xC0*/
> + vol_uint crc_reserved6; /* _reserved_6, 0xC4*/
> + vol_uint crc_reserved7; /* _reserved_7, 0xC8*/
> + vol_uint crc_pllu_misc; /* _PLLU_MISC_0, 0xCC*/
> +
> + vol_uint crc_plld_base; /* _PLLD_BASE_0, 0xD0*/
> + vol_uint crc_reserved8; /* _reserved_8, 0xD4*/
> + vol_uint crc_reserved9; /* _reserved_9, 0xD8*/
> + vol_uint crc_plld_misc; /* _PLLD_MISC_0, 0xDC*/
> +
> + vol_uint crc_pllx_base; /* _PLLX_BASE_0, 0xE0*/
> + vol_uint crc_pllx_misc; /* _PLLX_MISC_0, 0xE4*/
> +
> + vol_uint crc_plle_base; /* _PLLE_BASE_0, 0xE8*/
> + vol_uint crc_plle_misc; /* _PLLE_MISC_0, 0xEC*/
> +
> + vol_uint crc_plls_base; /* _PLLS_BASE_0, 0xF0*/
> + vol_uint crc_plls_misc; /* _PLLS_MISC_0, 0xF4*/
> + vol_uint crc_reserved10; /* _reserved_10, 0xF8*/
> + vol_uint crc_reserved11; /* _reserved_11, 0xFC*/
> +
> + vol_uint crc_clk_src_i2s1; /*_I2S1_0, 0x100*/
> + vol_uint crc_clk_src_i2s2; /*_I2S2_0, 0x104*/
> + vol_uint crc_clk_src_spdif_out; /*_SPDIF_OUT_0, 0x108*/
> + vol_uint crc_clk_src_spdif_in; /*_SPDIF_IN_0, 0x10C*/
> + vol_uint crc_clk_src_pwm; /*_PWM_0, 0x110*/
> + vol_uint crc_clk_src_spi1; /*_SPI1_0, 0x114*/
> + vol_uint crc_clk_src_sbc2; /*_SBC2_0, 0x118*/
> + vol_uint crc_clk_src_sbc3; /*_SBC3_0, 0x11C*/
> + vol_uint crc_clk_src_xio; /*_XIO_0, 0x120*/
> + vol_uint crc_clk_src_i2c1; /*_I2C1_0, 0x124*/
> + vol_uint crc_clk_src_dvc_i2c; /*_DVC_I2C_0, 0x128*/
> + vol_uint crc_clk_src_twc; /*_TWC_0, 0x12C*/
> + vol_uint crc_reserved12; /* 0x130*/
> + vol_uint crc_clk_src_sbc1; /*_SBC1_0, 0x134*/
> + vol_uint crc_clk_src_disp1; /*_DISP1_0, 0x138*/
> + vol_uint crc_clk_src_disp2; /*_DISP2_0, 0x13C*/
> + vol_uint crc_clk_src_cve; /*_CVE_0, 0x140*/
> + vol_uint crc_clk_src_ide; /*_IDE_0, 0x144*/
> + vol_uint crc_clk_src_vi; /*_VI_0, 0x148*/
> + vol_uint crc_reserved13; /* 0x14c*/
> + vol_uint crc_clk_src_sdmmc1; /*_SDMMC1_0, 0x150*/
> + vol_uint crc_clk_src_sdmmc2; /*_SDMMC2_0, 0x154*/
> + vol_uint crc_clk_src_g3d; /*_G3D_0, 0x158*/
> + vol_uint crc_clk_src_g2d; /*_G2D_0, 0x15C*/
> + vol_uint crc_clk_src_ndflash; /*_NDFLASH_0, 0x160*/
> + vol_uint crc_clk_src_sdmmc4; /*_SDMMC4_0, 0x164*/
> + vol_uint crc_clk_src_vfir; /*_VFIR_0, 0x168*/
> + vol_uint crc_clk_src_epp; /*_EPP_0, 0x16C*/
> + vol_uint crc_clk_src_mp3; /*_MPE_0, 0x170*/
> + vol_uint crc_clk_src_mipi; /*_MIPI_0, 0x174*/
> + vol_uint crc_clk_src_uarta; /*_UARTA_0, 0x178*/
> + vol_uint crc_clk_src_uartb; /*_UARTB_0, 0x17C*/
> + vol_uint crc_clk_src_host1x; /*_HOST1X_0, 0x180*/
> + vol_uint crc_reserved14; /* 0x184*/
> + vol_uint crc_clk_src_tvo; /*_TVO_0, 0x188*/
> + vol_uint crc_clk_src_hdmi; /*_HDMI_0, 0x18C*/
> + vol_uint crc_reserved15; /* 0x190*/
> + vol_uint crc_clk_src_tvdac; /*_TVDAC_0, 0x194*/
> + vol_uint crc_clk_src_i2c2; /*_I2C2_0, 0x198*/
> + vol_uint crc_clk_src_emc; /*_EMC_0, 0x19C*/
> + vol_uint crc_clk_src_uartc; /*_UARTC_0, 0x1A0*/
> + vol_uint crc_reserved16; /* 0x1a4*/
> + vol_uint crc_clk_src_vi_sensor; /*_VI_SENSOR_0, 0x1A8*/
> + vol_uint crc_reserved17; /* 0x1ac*/
> + vol_uint crc_reserved18; /* 0x1b0*/
> + vol_uint crc_clk_src_sbc4; /*_SBC4_0, 0x1B4*/
> + vol_uint crc_clk_src_i2c3; /*_I2C3_0, 0x1B8*/
> + vol_uint crc_clk_src_sdmmc3; /*_SDMMC3_0, 0x1BC*/
> + vol_uint crc_clk_src_uartd; /*_UARTD_0, 0x1C0*/
> + vol_uint crc_clk_src_uarte; /*_UARTE_0, 0x1C4*/
> + vol_uint crc_clk_src_vde; /*_VDE_0, 0x1C8*/
> + vol_uint crc_clk_src_owr; /*_OWR_0, 0x1CC*/
> + vol_uint crc_clk_src_nor; /*_NOR_0, 0x1D0*/
> + vol_uint crc_clk_src_csite; /*_CSITE_0, 0x1D4*/
> + vol_uint crc_reserved19[9]; /* 0x1d8-1F8*/
> + vol_uint crc_clk_src_osc; /*_OSC_0, 0x1FC*/
> +} clk_rst_ctlr;

<snip>

> +#ifndef _PMC_H_
> +#define _PMC_H_
> +typedef unsigned int volatile vol_uint;

Same volatile comment applies here. Probably applies to the remainder
of the patch.

> +/* Power Management Controller (APBDEV_PMC_) registers */
> +
> +typedef struct pmc_ctlr {
> + vol_uint pmc_cntrl; /* _CNTRL_0, offset 00 */
> + vol_uint pmc_sec_disable; /* _SEC_DISABLE_0, offset 04 */
> + vol_uint pmc_pmc_swrst; /* _PMC_SWRST_0, offset 08 */
> + vol_uint pmc_wake_mask; /* _WAKE_MASK_0, offset 0C */
> + vol_uint pmc_wake_lvl; /* _WAKE_LVL_0, offset 10 */
> + vol_uint pmc_wake_status; /* _WAKE_STATUS_0, offset 14 */
> + vol_uint pmc_sw_wake_status; /* _SW_WAKE_STATUS_0, offset 18 */
> + vol_uint pmc_dpd_pads_oride; /* _DPD_PADS_ORIDE_0, offset 1C */
> + vol_uint pmc_dpd_sample; /* _DPD_PADS_SAMPLE_0, offset 20 */
> + vol_uint pmc_dpd_enable; /* _DPD_PADS_ENABLE_0, offset 24 */
> + vol_uint pmc_pwrgate_timer_off; /* _PWRGATE_TIMER_OFF_0, offset 28 */
> + vol_uint pmc_pwrgate_timer_on; /* _PWRGATE_TIMER_ON_0, offset 2C */
> + vol_uint pmc_pwrgate_toggle; /* _PWRGATE_TOGGLE_0, offset 30 */
> + vol_uint pmc_remove_clamping; /* _REMOVE_CLAMPING_CMD_0, offset 34 */
> + vol_uint pmc_pwrgate_status; /* _PWRGATE_STATUS_0, offset 38 */
> + vol_uint pmc_pwrgood_timer; /* _PWRGOOD_TIMER_0, offset 3C */
> + vol_uint pmc_blink_timer; /* _BLINK_TIMER_0, offset 40 */
> + vol_uint pmc_no_iopower; /* _NO_IOPOWER_0, offset 44 */
> + vol_uint pmc_pwr_det; /* _PWR_DET_0, offset 48 */
> + vol_uint pmc_pwr_det_latch; /* _PWR_DET_LATCH_0, offset 4C */
> +
> + vol_uint pmc_scratch0; /* _SCRATCH0_0, offset 50 */
> + vol_uint pmc_scratch1; /* _SCRATCH1_0, offset 54 */
> + vol_uint pmc_scratch2; /* _SCRATCH2_0, offset 58 */
> + vol_uint pmc_scratch3; /* _SCRATCH3_0, offset 5C */
> + vol_uint pmc_scratch4; /* _SCRATCH4_0, offset 60 */
> + vol_uint pmc_scratch5; /* _SCRATCH5_0, offset 64 */
> + vol_uint pmc_scratch6; /* _SCRATCH6_0, offset 68 */
> + vol_uint pmc_scratch7; /* _SCRATCH7_0, offset 6C */
> + vol_uint pmc_scratch8; /* _SCRATCH8_0, offset 70 */
> + vol_uint pmc_scratch9; /* _SCRATCH9_0, offset 74 */
> + vol_uint pmc_scratch10; /* _SCRATCH10_0, offset 78 */
> + vol_uint pmc_scratch11; /* _SCRATCH11_0, offset 7C */
> + vol_uint pmc_scratch12; /* _SCRATCH12_0, offset 80 */
> + vol_uint pmc_scratch13; /* _SCRATCH13_0, offset 84 */
> + vol_uint pmc_scratch14; /* _SCRATCH14_0, offset 88 */
> + vol_uint pmc_scratch15; /* _SCRATCH15_0, offset 8C */
> + vol_uint pmc_scratch16; /* _SCRATCH16_0, offset 90 */
> + vol_uint pmc_scratch17; /* _SCRATCH17_0, offset 94 */
> + vol_uint pmc_scratch18; /* _SCRATCH18_0, offset 98 */
> + vol_uint pmc_scratch19; /* _SCRATCH19_0, offset 9C */
> + vol_uint pmc_scratch20; /* _SCRATCH20_0, offset A0 */
> + vol_uint pmc_scratch21; /* _SCRATCH21_0, offset A4 */
> + vol_uint pmc_scratch22; /* _SCRATCH22_0, offset A8 */
> + vol_uint pmc_scratch23; /* _SCRATCH23_0, offset AC */
> +
> + vol_uint pmc_secure_scratch0; /* _SECURE_SCRATCH0_0, offset B0 */
> + vol_uint pmc_secure_scratch1; /* _SECURE_SCRATCH1_0, offset B4 */
> + vol_uint pmc_secure_scratch2; /* _SECURE_SCRATCH2_0, offset B8 */
> + vol_uint pmc_secure_scratch3; /* _SECURE_SCRATCH3_0, offset BC */
> + vol_uint pmc_secure_scratch4; /* _SECURE_SCRATCH4_0, offset C0 */
> + vol_uint pmc_secure_scratch5; /* _SECURE_SCRATCH5_0, offset C4 */
> +
> + vol_uint pmc_cpupwrgood_timer; /* _CPUPWRGOOD_TIMER_0, offset C8 */
> + vol_uint pmc_cpupwroff_timer; /* _CPUPWROFF_TIMER_0, offset CC */
> + vol_uint pmc_pg_mask; /* _PG_MASK_0, offset D0 */
> + vol_uint pmc_pg_mask_1; /* _PG_MASK_1_0, offset D4 */
> + vol_uint pmc_auto_wake_lvl; /* _AUTO_WAKE_LVL_0, offset D8 */
> + vol_uint pmc_auto_wake_lvl_mask; /* _AUTO_WAKE_LVL_MASK_0, offset DC */
> + vol_uint pmc_wake_delay; /* _WAKE_DELAY_0, offset E0 */
> + vol_uint pmc_pwr_det_val; /* _PWR_DET_VAL_0, offset E4 */
> + vol_uint pmc_ddr_pwr; /* _DDR_PWR_0, offset E8 */
> + vol_uint pmc_usb_debounce_del; /* _USB_DEBOUNCE_DEL_0, offset EC */
> + vol_uint pmc_usb_ao; /* _USB_AO_0, offset F0 */
> + vol_uint pmc_crypto_op; /* _CRYPTO_OP__0, offset F4 */
> + vol_uint pmc_pllp_wb0_override; /* _PLLP_WB0_OVERRIDE_0, offset F8 */
> +
> + vol_uint pmc_scratch24; /* _SCRATCH24_0, offset FC */
> + vol_uint pmc_scratch25; /* _SCRATCH24_0, offset 100 */
> + vol_uint pmc_scratch26; /* _SCRATCH24_0, offset 104 */
> + vol_uint pmc_scratch27; /* _SCRATCH24_0, offset 108 */
> + vol_uint pmc_scratch28; /* _SCRATCH24_0, offset 10C */
> + vol_uint pmc_scratch29; /* _SCRATCH24_0, offset 110 */
> + vol_uint pmc_scratch30; /* _SCRATCH24_0, offset 114 */
> + vol_uint pmc_scratch31; /* _SCRATCH24_0, offset 118 */
> + vol_uint pmc_scratch32; /* _SCRATCH24_0, offset 11C */
> + vol_uint pmc_scratch33; /* _SCRATCH24_0, offset 120 */
> + vol_uint pmc_scratch34; /* _SCRATCH24_0, offset 124 */
> + vol_uint pmc_scratch35; /* _SCRATCH24_0, offset 128 */
> + vol_uint pmc_scratch36; /* _SCRATCH24_0, offset 12C */
> + vol_uint pmc_scratch37; /* _SCRATCH24_0, offset 130 */
> + vol_uint pmc_scratch38; /* _SCRATCH24_0, offset 134 */
> + vol_uint pmc_scratch39; /* _SCRATCH24_0, offset 138 */
> + vol_uint pmc_scratch40; /* _SCRATCH24_0, offset 13C */
> + vol_uint pmc_scratch41; /* _SCRATCH24_0, offset 140 */
> + vol_uint pmc_scratch42; /* _SCRATCH24_0, offset 144 */
> +
> + vol_uint pmc_bo_mirror0; /* _BOUNDOUT_MIRROR0_0, offset 148 */
> + vol_uint pmc_bo_mirror1; /* _BOUNDOUT_MIRROR1_0, offset 14C */
> + vol_uint pmc_bo_mirror2; /* _BOUNDOUT_MIRROR2_0, offset 150 */
> + vol_uint pmc_sys_33v_en; /* _SYS_33V_EN_0, offset 154 */
> + vol_uint pmc_bo_mirror_access; /* _BOUNDOUT_MIRROR_ACCESS_0, off158 */
> + vol_uint pmc_gate; /* _GATE_0, offset 15C */
> +} pmc_ctlr;
> +
> +#endif /* PMC_H */
> diff --git a/arch/arm/include/asm/arch-tegra2/sys_proto.h b/arch/arm/include/asm/arch-tegra2/sys_proto.h
> new file mode 100644
> index 0000000..70e63a8
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-tegra2/sys_proto.h
> @@ -0,0 +1,33 @@
> +/*
> + * (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
> + */
> +
> +#ifndef _SYS_PROTO_H_
> +#define _SYS_PROTO_H_
> +
> +typedef struct {
> + char *board_string;
> +} tegra2_sysinfo;
> +
> +void invalidate_dcache(void);
> +
> +#endif
> diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h b/arch/arm/include/asm/arch-tegra2/tegra2.h
> new file mode 100644
> index 0000000..ebe56f2
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
> @@ -0,0 +1,56 @@
> +/*
> + * (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
> + */
> +
> +#ifndef _TEGRA2_H_
> +#define _TEGRA2_H_
> +
> +#ifndef FALSE
> +#define FALSE 0
> +#endif
> +#ifndef TRUE
> +#define TRUE 1
> +#endif

It'd be better to either consolidate the existing definitions of
TRUE/FALSE to some place in a common header, or just use 0 and 1.

<snip>

> +typedef struct uart_ctlr {
> + vol_uint uart_thr_dlab_0; /* UART_THR_DLAB_0_0, offset 00 */
> + vol_uint uart_ier_dlab_0; /* UART_IER_DLAB_0_0, offset 04 */
> + vol_uint uart_iir_fcr; /* UART_IIR_FCR_0, offset 08 */
> + vol_uint uart_lcr; /* UART_LCR_0, offset 0C */
> + vol_uint uart_mcr; /* UART_MCR_0, offset 10 */
> + vol_uint uart_lsr; /* UART_LSR_0, offset 14 */
> + vol_uint uart_msr; /* UART_MSR_0, offset 18 */
> + vol_uint uart_spr; /* UART_SPR_0, offset 1C */
> + vol_uint uart_irda_csr; /* UART_IRDA_CSR_0, offset 20 */
> + vol_uint uart_reserved[6]; /* Reserved, unused */
> + vol_uint uart_asr; /* UART_ASR_0, offset 3C */
> +} uart_ctlr;
> +
> +#define LSR_RDR (1 << 0)

The tab after the define should be a space. Same with defines
throughout the patch.

<snip>

> +/*
> + * 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);
> + clkrst->crc_pllp_base = reg;
> +
> + reg |= PLL_ENABLE;
> + clkrst->crc_pllp_base = reg;
> +
> + reg &= ~PLL_BYPASS;
> + clkrst->crc_pllp_base = reg;
> +
> + pllp_init_done++;
> + }
> +
> + /* Now do the UART reset/clock enable based on uart_num */
> +

Above empty newline should be removed.

> + if (uart_num == UART_D) {
> +

Remove empty newline after if() globally.

> + /* Assert Reset to UART */
> + reg = clkrst->crc_rst_dev_u;
> + reg |= SWR_UARTD_RST; /* SWR_UARTD_RST = 1 */
> + clkrst->crc_rst_dev_u = reg;
> +
> + /* Enable clk to UART */
> + reg = clkrst->crc_clk_out_enb_u;
> + reg |= CLK_ENB_UARTD; /* CLK_ENB_UARTD = 1 */
> + clkrst->crc_clk_out_enb_u = reg;
> +
> + /* Enable pllp_out0 to UART */
> + reg = clkrst->crc_clk_src_uartd;
> + reg &= 0x3FFFFFFF; /* UARTD_CLK_SRC = 00, PLLP_OUT0 */
> + clkrst->crc_clk_src_uartd = reg;
> +
> + /* wait for 2us */
> + udelay(2);
> +
> + /* De-assert reset to UART */
> + reg = clkrst->crc_rst_dev_u;
> + reg &= ~SWR_UARTD_RST; /* SWR_UARTD_RST = 0 */
> + clkrst->crc_rst_dev_u = reg;
> +

Same with empty newlines before the end of an if().

> + } else if (uart_num == UART_A) {
> +
> + /* Assert Reset to UART */
> + reg = clkrst->crc_rst_dev_l;
> + reg |= SWR_UARTA_RST; /* SWR_UARTA_RST = 1 */
> + clkrst->crc_rst_dev_l = reg;
> +
> + /* Enable clk to UART */
> + reg = clkrst->crc_clk_out_enb_l;
> + reg |= CLK_ENB_UARTA; /* CLK_ENB_UARTA = 1 */
> + clkrst->crc_clk_out_enb_l = reg;
> +
> + /* Enable pllp_out0 to UART */
> + reg = clkrst->crc_clk_src_uarta;
> + reg &= 0x3FFFFFFF; /* UARTA_CLK_SRC = 00, PLLP_OUT0 */
> + clkrst->crc_clk_src_uarta = reg;
> +
> + /* wait for 2us */
> + udelay(2);
> +
> + /* De-assert reset to UART */
> + reg = clkrst->crc_rst_dev_l;
> + reg &= ~SWR_UARTA_RST; /* SWR_UARTA_RST = 0 */
> + clkrst->crc_rst_dev_l = reg;
> +
> + }
> +}
> +
> +/*
> + * Routine: pin_mux_uart
> + * Description: setup the pin muxes/tristate values for UART based on uart_num
> + */
> +void pin_mux_uart(int uart_num)
> +{
> + pinmux_tri_ctlr *const pmt = (pinmux_tri_ctlr *)NV_PA_APB_MISC_BASE;
> + u32 reg;
> +
> + if (uart_num == UART_D) {
> +
> + reg = pmt->pmt_ctl_b;
> + reg &= 0xFFFFFFF3; /* GMC_SEL [3:2] = 00, UARTD */
> + pmt->pmt_ctl_b = reg;
> +
> + reg = pmt->pmt_tri_a;
> + reg &= ~Z_GMC; /* Z_GMC = normal (0) */
> + pmt->pmt_tri_a = reg;
> +
> + } else if (uart_num == UART_A) {
> +
> + reg = pmt->pmt_ctl_c;
> + reg &= 0xFFF0FFFF; /* IRRX_/IRTX_SEL [19:16] = 00 UARTA */
> + pmt->pmt_ctl_c = reg;
> +
> + reg = pmt->pmt_tri_a;
> + reg &= ~Z_IRRX; /* Z_IRRX = normal (0) */
> + reg &= ~Z_IRTX; /* Z_IRTX = normal (0) */
> + pmt->pmt_tri_a = reg;
> + }
> +}
> +
> +void do_uart(uart_ctlr *u)
> +{

U-Boot command implementations are generally are prefixed with do_ (eg
do_md5sum). It'd be nice to pick a more descriptive function name that
doesn't start with do_.

> + u32 reg;
> +
> + /* Prepare the divisor value */
> + reg = NVRM_PLLP_FIXED_FREQ_KHZ * 1000 / NV_DEFAULT_DEBUG_BAUD / 16;
> +
> + /* Set up UART parameters */
> + u->uart_lcr = 0x80;
> + u->uart_thr_dlab_0 = reg;
> + u->uart_ier_dlab_0 = 0;
> + u->uart_lcr = 0;
> + u->uart_iir_fcr = 0x37;
> + u->uart_ier_dlab_0 = 0;
> + u->uart_lcr = 3; /* 8N1 */
> + u->uart_mcr = 2;
> + u->uart_msr = 0;
> + u->uart_spr = 0;
> + u->uart_irda_csr = 0;
> + u->uart_asr = 0;
> + u->uart_iir_fcr = 0x31;

There should be defines for the serial port registers in ns16550. Using
them would make it more clear what is being set up.

> + /* Flush any old characters out of the RX FIFO */
> + reg = u->uart_lsr;
> +
> + while (reg & LSR_RDR) {
> + reg = u->uart_thr_dlab_0;
> + reg = u->uart_lsr;
> + }
> +}
> +
> +/*
> + * Routine: init_uart
> + * Description: init the UART clocks, muxes, and baudrate/parity/etc.
> + */
> +void init_uart(int uart_num)
> +{
> + if (uart_num == UART_A) {
> + uart_ctlr *const uart = (uart_ctlr *)NV_PA_APB_UARTA_BASE;
> +
> + uart_clock_init(UART_A);
> +
> + /* Enable UARTA - uses config 0 */
> + pin_mux_uart(UART_A);
> +
> + do_uart(uart);
> +
> + } else if (uart_num == UART_D) {
> + uart_ctlr *const uart = (uart_ctlr *)NV_PA_APB_UARTD_BASE;
> +
> + uart_clock_init(UART_D);
> +
> + /* Enable UARTD - uses config 0 */
> + pin_mux_uart(UART_D);
> +
> + do_uart(uart);
> + }

This block could be surrounded by #ifdef CONFIG_TEGRA2_ENABLE_UARTD to
save some space. Same with UARTA if its ever disabled. Same comment
with other areas that choose between UART A/D.

> +}
> +
> +void uart_init(void)
> +{
> +#if (CONFIG_TEGRA2_ENABLE_UARTA)
> + init_uart(UART_A);
> +#endif
> +#if (CONFIG_TEGRA2_ENABLE_UARTD)
> + init_uart(UART_D);
> +#endif
> +}
> diff --git a/board/tegra2/common/board.h b/board/tegra2/common/board.h
> new file mode 100644
> index 0000000..60525a8
> --- /dev/null
> +++ b/board/tegra2/common/board.h
> @@ -0,0 +1,59 @@
> +/*
> + * (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
> + */
> +
> +#ifndef _COMMON_BOARD_H_
> +#define _COMMON_BOARD_H_
> +
> +typedef unsigned int volatile vol_uint;

If you do need to add this typedef (which hopefully you don't), it'd
probably be better somewhere global so every board vendor doesn't have
the same chunk of code. Eg along with vu_long.

> +#include <asm/arch/clk_rst.h>
> +#include <asm/arch/pinmux.h>
> +#include <asm/arch/uart.h>
> +
> +#define NVRM_PLLP_FIXED_FREQ_KHZ 216000
> +#define NV_DEFAULT_DEBUG_BAUD 115200
> +
> +#define PLL_BYPASS (1 << 31)
> +#define PLL_ENABLE (1 << 30)
> +#define PLL_BASE_OVRRIDE (1 << 28)
> +#define PLL_DIVP (1 << 20) /* post divider, b22:20 */
> +#define PLL_DIVM 0x0C /* input divider, b4:0 */
> +
> +#define SWR_UARTD_RST (1 << 2)
> +#define CLK_ENB_UARTD (1 << 2)
> +#define SWR_UARTA_RST (1 << 6)
> +#define CLK_ENB_UARTA (1 << 6)

Tabs should be removed. Are the above CPU-specifc defines? If so, they
should be moved somewhere cpu-specific so other board vendors don't need
to define them too. Actually, looking over it again, perhaps the entire
contents of board/tegra2/ should be moved somewhere in arch/arm?

Regards,
Peter