[U-Boot] [PATCH 3/7] Tegra30: Add common CPU (shared) files

[Stephen Warren]

On 10/02/2012 04:45 PM, Tom Warren wrote:
> These files are used by both SPL and main U-Boot.
> Also made minor changes to shared Tegra code to support
> T30 differences.

> diff --git a/arch/arm/cpu/tegra-common/ap.c b/arch/arm/cpu/tegra-common/ap.c

> @@ -58,6 +62,20 @@ int tegra_get_chip_type(void)
>  			return TEGRA_SOC_T25;
>  		}
>  		break;
> +	case CHIPID_TEGRA30:
> +		switch (tegra_sku_id) {
> +		case SKU_ID_T30:
> +			/*
> +			 * T30 has two options. We will return TEGRA_SOC_T30
> +			 * until we have the fdt set up when it may change to
> +			 * TEGRA_SOC_T30_408MHZ depending on the PLLP freq.
> +			 */
> +			if (clock_get_rate(CLOCK_ID_PERIPH) == 408000000)
> +				return TEGRA_SOC_T30_408MHZ;
> +			else
> +				return TEGRA_SOC_T30;

Hmmm. I wonder why this doesn't return just the actual chip type; isn't
the different in PLL_P rates a SW option and not a facet of the HW? The
whole TEGRA_SOC_T30_408MHZ thing seems strange to me.

> diff --git a/arch/arm/cpu/tegra-common/board.c b/arch/arm/cpu/tegra-common/board.c

> @@ -59,9 +59,12 @@ unsigned int query_sdram_size(void)
>  	case 1:
>  		return 0x10000000;	/* 256 MB */
>  	case 2:
> -	default:
>  		return 0x20000000;	/* 512 MB */
> -	case 3:
> +	case 4:
> +		return 0x40000000;	/* 1GB */
> +	case 8:
> +		return 0x7ff00000;	/* 2GB - 1MB */
> +	default:
>  		return 0x40000000;	/* 1GB */
>  	}
>  }

I think we should be a little more explicit about the Tegra20/Tegra30
differences here. At least for case 0, there's a difference in what this
field means for the two chips.

Value	Tegra20	Tegra30
0	512	256
1	256	256
2	512	512
3	1024	768
4	undef	1024
5	undef	undef
6	undef	undef
7	undef	undef
8	n/a	2048-1

At least the Toshiba AC100 uses value "0" in this field and has 512MB
not 1GB of RAM.

I also note that for Tegra30, this field is bits 31:28, whereas for
Tegra20 it's bits 30:28; we aren't masking off the top bit as we should
in the current code. Again, the Toshiba AC100 appears to set this extra
top bit, and only succeeds in getting 512M as the result of this
function because of the default case.

> diff --git a/arch/arm/cpu/tegra30-common/pinmux.c b/arch/arm/cpu/tegra30-common/pinmux.c

Hmmm. The header file this relies on doesn't exist yet. But perhaps this
is OK since there's no boards.cfg entry for Tegra30 yet.

> +const struct tegra_pingroup_desc tegra_soc_pingroups[PINGRP_COUNT] = {
> +	/*  NAME	VDD	f0	f1	f2	f3	fSafe	io */

"fSafe	io" should be deleted; those columns don't exist in the table.

I compared this table to that in the upstream kernel, which I spent a
lot of time on correlating with the various documentation sources and
our downstream kernel. Some changes should be made:

> +	PINI(DAP3_FS,	  BB,	   I2S2,	RSVD1,	   DISPA,   DISPB),
> +	PINI(DAP3_DIN,	  BB,	   I2S2,	RSVD1,	   DISPA,   DISPB),
> +	PINI(DAP3_DOUT,	  BB,	   I2S2,	RSVD1,	   DISPA,   DISPB),
> +	PINI(DAP3_SCLK,	  BB,	   I2S2,	RSVD1,	   DISPA,   DISPB),

In the kernel at least, the RSVD values are only used in particular
slots, i.e. RSVD1 always yields function 0, RSVD2==func1, RSVD3==func2,
RSVD4==func3. That way, when representing the "safe" value, one can use
RSVDn to guarantee to get func(n-1). I suspect, it'd be a good idea for
U-Boot to adopt the same policy. So, s/RSVD1/RSVD2/ above, and make
similar changes throughout this table.

> +	PINI(GPIO_PV0,	  BB,	   RSVD,	RSVD,	   RSVD,    RSVD),
> +	PINI(GPIO_PV1,	  BB,	   RSVD,	RSVD,	   RSVD,    RSVD),

And likewise, plain "RSVD" doesn't exist. If you duplicate RSVD into
each column of the table, there's no way to know which function
pinmux_set_func(foo, RSVD) might pick, since it matches them all.

> +	PINI(SDMMC1_CLK,  SDMMC1,  SDMMC1,	RSVD1,	   RSVD2,   BAD),
> +	PINI(SDMMC1_CMD,  SDMMC1,  SDMMC1,	RSVD1,	   RSVD2,   BAD),
> +	PINI(SDMMC1_DAT3, SDMMC1,  SDMMC1,	RSVD1,	   UARTE,   BAD),
> +	PINI(SDMMC1_DAT2, SDMMC1,  SDMMC1,	RSVD1,	   UARTE,   BAD),
> +	PINI(SDMMC1_DAT1, SDMMC1,  SDMMC1,	RSVD1,	   UARTE,   BAD),
> +	PINI(SDMMC1_DAT0, SDMMC1,  SDMMC1,	RSVD1,	   UARTE,   BAD),

I believe the final column for all those entries should be UARTA.

> +	PINI(GPIO_PV3,	  SDMMC1,  BAD,		RSVD1,	   RSVD2,   RSVD3),

BAD should be CLK_12M_OUT.

> +	PINO(LCD_PWR2,	  LCD,	   DISPA,	DISPB,	  SPI5,     BAD),
> +	PINO(LCD_SDOUT,	  LCD,	   DISPA,	DISPB,	  SPI5,     BAD),
> +	PINO(LCD_WR_N,	  LCD,	   DISPA,	DISPB,	  SPI5,     BAD),
> +	PINO(LCD_SCK,	  LCD,	   DISPA,	DISPB,	  SPI5,     BAD),
> +	PINO(LCD_PWR0,	  LCD,	   DISPA,	DISPB,	  SPI5,     BAD),

BAD should be HDCP for all of those.

> +	PINI(HDMI_INT,	  LCD,	   RSVD,	RSVD,	   RSVD,    RSVD),

The first reserved should be HDMI.

> +	PINI(VI_D0,	  VI,	   BAD,		RSVD1,	   VI,      RSVD2),
> +	PINI(VI_D1,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D2,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D3,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D4,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D5,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D6,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D7,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D8,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
> +	PINI(VI_D9,	  VI,	   BAD,		SDMMC2,	   VI,      RSVD1),
 +	PINI(VI_D10,	  VI,	   BAD,		RSVD1,	   VI,      RSVD2),
> +	PINI(VI_D11,	  VI,	   BAD,		RSVD1,	   VI,      RSVD2),

BAD should be DDR for all of those.

> +	PINI(VI_MCLK,	  VI,	   VI,		BAD,	   BAD,     BAD),

All the functions for this pin select VI.

> +	PINI(VI_VSYNC,	  VI,	   BAD,		RSVD1,	   VI,      RSVD2),
> +	PINI(VI_HSYNC,	  VI,	   BAD,		RSVD1,	   VI,      RSVD2),

BAD should be DDR for both of those.

> +	PINI(UART2_RXD,	  UART,	   IRDA,	SPDIF,	   UARTA,   SPI4),
> +	PINI(UART2_TXD,	  UART,	   IRDA,	SPDIF,	   UARTA,   SPI4),

IRDA just means UARTB, so I think remove IRDA, and replace all usage
with UARTB everywhere.

> +	PINI(GMI_CS0_N,	  GMI,	   RSVD1,	NAND,	   GMI,     BAD),
> +	PINI(GMI_A17,	  GMI,	   UARTD,	SPI4,	   GMI,     BAD),
> +	PINI(GMI_A18,	  GMI,	   UARTD,	SPI4,	   GMI,     BAD),

BAD should be DTV in all of those.

> +	PINI(GEN2_I2C_SCL, GMI,	   I2C2,	BAD,	   GMI,     RSVD3),
> +	PINI(GEN2_I2C_SDA, GMI,	   I2C2,	BAD,	   GMI,     RSVD3),

BAD should be HDCP in both of those.

> +	PINI(CAM_MCLK,	  CAM,	   VI,		BAD,	   VI_ALT2, POPSDMMC4),

We should rename POPSDMMC4 to just SDMMC4 throughout; there's just a
single SDMMC4 controller; no a separate POPSDMMC4 controller.

> +	PINI(CAM_I2C_SCL, CAM,	   BAD,		I2C3,	   RSVD2,   POPSDMMC4),
> +	PINI(CAM_I2C_SDA, CAM,	   BAD,		I2C3,	   RSVD2,   POPSDMMC4),

BAD should be VGP1 and VGP2 in those two lines.

> +	PINI(KB_ROW3,	  SYS,	   KBC,		BAD,	   RSVD2,   BAD),

For all of KBC_ROW* and KBC_COL*, the BAD in func1 (i.e. the first BAD)
should be NAND.

The second BAD there should be RSVD4.

> +	PINI(KB_ROW6,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW7,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW8,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW9,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW10,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW11,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW12,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW13,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW14,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),
> +	PINI(KB_ROW15,	  SYS,	   KBC,		BAD,	   SDMMC2,  BAD),

The second bad (final column) for all of those should be MIO.

> +	PINI(KB_COL0,	  SYS,	   KBC,		BAD,	   TRACE,   BAD),
> +	PINI(KB_COL1,	  SYS,	   KBC,		BAD,	   TRACE,   BAD),

The second bad (final column) for both of those should be TEST.

> +	PINI(KB_COL6,	  SYS,	   KBC,		BAD,	   TRACE,   BAD),
> +	PINI(KB_COL7,	  SYS,	   KBC,		BAD,	   TRACE,   BAD),

The second bad (final column) for both of those should be MIO.

> +	PINI(CORE_PWR_REQ, SYS,	   RSVD,	RSVD,	   RSVD,    RSVD),

func0 (the first RSVD) should be CORE_PWR_REQ.

> +	PINI(CPU_PWR_REQ, SYS,	   RSVD,	RSVD,	   RSVD,    RSVD),

func0 (the first RSVD) should be CPU_PWR_REQ.

> +	PINI(PWR_INT_N,	  SYS,	   RSVD,	RSVD,	   RSVD,    RSVD),

func0 (the first RSVD) should be PWR_INT_N.

> +	PINI(CLK_32K_IN,  SYS,	   RSVD,	RSVD,	   RSVD,    RSVD),

func0 (the first RSVD) should be CLK_32K_IN.

> +	PINI(OWR,	  SYS,	   OWR,		RSVD,	   RSVD,    RSVD),

func1 (the first RSVD) should be CEC.

> +	PINI(SPDIF_IN,	  AUDIO,   SPDIF,	HDA,	   BAD,     DAPSDMMC2),
> +	PINI(SPDIF_OUT,	  AUDIO,   SPDIF,	RSVD1,	   BAD,     DAPSDMMC2),

BAD should be I2C1 for both of those.

Rename DAPSDMMC2 to SDMMC2.

> +	PINI(SPI2_MOSI,	  AUDIO,   SPI6,	SPI2,	   BAD,     GMI),
> +	PINI(SPI2_MISO,	  AUDIO,   SPI6,	SPI2,	   BAD,     GMI),
> +	PINI(SPI2_CS0_N,  AUDIO,   SPI6,	SPI2,	   BAD,     GMI),
> +	PINI(SPI2_SCK,	  AUDIO,   SPI6,	SPI2,	   BAD,     GMI),

BAD should be GMI for all of those.

> +	PINI(SPI1_MOSI,	  AUDIO,   SPI2,	SPI1,	   BAD,     GMI),
> +	PINI(SPI1_SCK,	  AUDIO,   SPI2,	SPI1,	   BAD,     GMI),
> +	PINI(SPI1_CS0_N,  AUDIO,   SPI2,	SPI1,	   BAD,     GMI),
> +	PINI(SPI1_MISO,	  AUDIO,   BAD,		SPI1,	   BAD,     RSVD3),

SPI1_MISO's first BAD should be SPI3.

For all of those 4, func2's BAD should be SPI2 or rather SPI2_ALT.

> +	PINI(SPI2_CS1_N,  AUDIO,   BAD,		SPI2,	   BAD,     BAD),
> +	PINI(SPI2_CS2_N,  AUDIO,   BAD,		SPI2,	   BAD,     BAD),

In both of those, the first BAD should be SPI3, the second BAD SPI2_ALT,
and the third BAD I2C1.

> +	PINI(SDMMC3_CLK,  SDMMC3,  UARTA,	PWM2,	   SDMMC3,  BAD),

BAD should be SPI3.

> +	PINI(SDMMC3_CMD,  SDMMC3,  UARTA,	PWM3,	   SDMMC3,  BAD),

BAD should be SPI2.

> +	PINI(SDMMC3_DAT0, SDMMC3,  RSVD0,	RSVD1,	   SDMMC3,  BAD),
> +	PINI(SDMMC3_DAT1, SDMMC3,  RSVD0,	RSVD1,	   SDMMC3,  BAD),
> +	PINI(SDMMC3_DAT2, SDMMC3,  RSVD0,	PWM1,	   SDMMC3,  BAD),
> +	PINI(SDMMC3_DAT3, SDMMC3,  RSVD0,	PWM0,	   SDMMC3,  BAD),

BAD should be SPI3 in all of those.

> +	PINI(SDMMC3_DAT4, SDMMC3,  PWM1,	BAD,	   SDMMC3,  BAD),
> +	PINI(SDMMC3_DAT5, SDMMC3,  PWM0,	BAD,	   SDMMC3,  BAD),
> +	PINI(SDMMC3_DAT6, SDMMC3,  SPDIF,	BAD,	   SDMMC3,  BAD),
> +	PINI(SDMMC3_DAT7, SDMMC3,  SPDIF,	BAD,	   SDMMC3,  BAD),

func1's BAD should be SPI4 in all of those.

func3's BAD should be SPI2 in all of those.

> +void pinmux_set_func(enum pmux_pingrp pin, enum pmux_func func)

> +	/* Handle special values */
> +	if (func == PMUX_FUNC_SAFE)
> +		func = tegra_soc_pingroups[pin].func_safe;
> +
> +	if (func & PMUX_FUNC_RSVD) {
> +		mux = func & 0x3;

Indeed, this is the way the code should work, but in order for this code
to work correctly, RSVD1 must only be used in the func0 column in the
data, RSVD2 in the func1 column, etc.

> +static int pinmux_set_lock(enum pmux_pingrp pin, enum pmux_pin_lock lock)

> +	reg = readl(pin_lock);
> +	reg &= ~(0x1 << PMUX_LOCK_SHIFT);
> +	if (lock == PMUX_PIN_LOCK_ENABLE)
> +		reg |= (0x1 << PMUX_LOCK_SHIFT);
> +	writel(reg, pin_lock);

Note that the LOCK bit, once set, can never be set back to zero. Perhaps
a diagnostic should be issued and/or an error returned if this is
attempted? The kernel does that.