Re: [U-Boot] [PATCH 4/9] Tegra: T30: Add common Tegra30 CPU files

On 09/12/2012 04:10 PM, Tom Warren wrote:

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

+unsigned int query_sdram_size(void) +{

• struct pmc_ctlr *const pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE;
• u32 reg;
• reg = readl(&pmc->pmc_scratch20);
• debug("pmc->pmc_scratch20 (ODMData) = 0x%08x\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 4:

• return 0x40000000;	/* 1GB */
  

• case 8:

• return 0x7ff00000;	/* 2GB - 1MB */
  

• default:

• return 0x40000000;	/* 1GB */
  

• }

+}

According to our wiki, there's also a 3=768MB option here, although I have no idea if that's really true. I guess we can add it if we need it.

+static int uart_configs[] = {

• FUNCMUX_UART1_ULPI_UART2, /* UARTA */
• FUNCMUX_UART2_IRDA,
• -1,
• FUNCMUX_UART4_GMC,
• -1,

+};

I guess I'm repeating what I wrote for the funcmux.[ch] change, but those FUNCMUX_* enum names don't make sense on Tegra30.

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

+/*

• • This array translates a periph_id to a periphc_internal_id
• • Not present/matched up:
• • uint vi_sensor; _VI_SENSOR_0, 0x1A8
• • SPDIF - which is both 0x08 and 0x0c
• */

+#define NONE(name) (-1) +#define OFFSET(name, value) PERIPHC_ ## name +static s8 periph_id_to_internal_id[PERIPH_ID_COUNT] = {

• /* Low word: 31:0 */
• NONE(CPU),
• NONE(COP),
• NONE(TRIGSYS),
• NONE(RESERVED3),
• NONE(RESERVED4),
• NONE(TMR),
• PERIPHC_UART1,
• PERIPHC_UART2, /* and vfir 0x68 */

Oh, there's a mapping table anyway. In that case, I would definitely recommend aligning the enum order with the kernel to allow easier DT handling in the future.

Still, as you mentioned in another email, this is something we can do later since it doesn't change any interface to anything outside U-Boot.

+#ifdef CONFIG_OF_CONTROL +/*

• • Convert a device tree clock ID to our peripheral ID. They are mostly
• • the same but we are very cautious so we check that a valid clock ID is
• • provided.
• • @param clk_id Clock ID according to tegra20 device tree binding
• • @return peripheral ID, or PERIPH_ID_NONE if the clock ID is invalid
• */

+static enum periph_id clk_id_to_periph_id(int clk_id) +{

• if (clk_id > 95)

• return PERIPH_ID_NONE;
  

• switch (clk_id) {
• case 1:
• case 2:
• case 7:
• case 10:
• case 20:
• case 30:
• case 35:
• case 49:
• case 56:
• case 74:
• case 76:
• case 77:
• case 78:
• case 79:
• case 80:
• case 81:
• case 82:
• case 83:
• case 91:
• case 95:

• return PERIPH_ID_NONE;
  

• default:

• return clk_id;
  

• }

+}

This is something that would need to be validated against the Tegra30 device tree bindings for the clock module. I guess since CONFIG_OF_CONTROL isn't enabled now, it's somewhat OK to defer this until later, but a comment here indicating that fact would be useful.

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

+int funcmux_select(enum periph_id id, int config) +{

• int bad_config = config != FUNCMUX_DEFAULT;
• switch (id) {
• case PERIPH_ID_UART1:

• switch (config) {
  

• case FUNCMUX_UART1_ULPI_UART2:
  

• 	pinmux_set_func(PINGRP_ULPI_DATA0, PMUX_FUNC_UARTA); txd
  

• 	pinmux_set_func(PINGRP_ULPI_DATA1, PMUX_FUNC_UARTA); rxd
  

• 	pinmux_set_func(PINGRP_ULPI_DATA2, PMUX_FUNC_UARTA); cts
  

• 	pinmux_set_func(PINGRP_ULPI_DATA3, PMUX_FUNC_UARTA); rts
  

• 	pinmux_set_func(PINGRP_ULPI_DATA4, PMUX_FUNC_UARTA); ri
  

• 	pinmux_set_func(PINGRP_ULPI_DATA5, PMUX_FUNC_UARTA); dcd
  

• 	pinmux_set_func(PINGRP_ULPI_DATA6, PMUX_FUNC_UARTA); dsr
  

• 	pinmux_set_func(PINGRP_UART2_RTS_N, PMUX_FUNC_UARTA); txd
  

• 	pinmux_set_func(PINGRP_UART2_CTS_N, PMUX_FUNC_UARTA); rxd
  

I've augmented the lines above with comments re: what signals are selected onto each line when those functions are selected.

Note that RXD/TXD are duplicated. I suspect you're just getting lucky that when rxd is selected on both ULPI_DATA1 and UART2_CTS_N, that it happens to take the input from the correct one of those two pins.

Looking at the schematic for Cardhu A02, you only want to configure ULPI_DATA[3:0] for UARTA. All those other pins are used for purposes other than UART.

And somewhat as an aside, this is why I wonder if a funcmux-style API is still a good idea; you could easily have the following sets of UARTA signals in use:

RXD+TXD RXD+TXD+RTS+CTS RXD+TXD+RTS+CTS+RI+DCD+DSR+DTR (or perhaps subsets of those last 3?)

... and for each of those 3 sets of signals, any of the individual signals can actually be muxed onto one of n different pins, where n is at least 2, and up to 4 in many cases. That's perhaps thousands of combinations just for UARTA!

Calling pinmux.c APIs directly from the board file seems much simpler, and avoids a level of abstraction that doesn't really exist in HW.