[U-Boot] [PATCH 0/7] tegra: Enhance funcmux to support I2C and MMC

[U-Boot] [PATCH 0/7] tegra: Enhance funcmux to support I2C and MMC

[Simon Glass]

This series expands funcmux_select() to support configs other than 0, and
to support options associated with a config.

This permits introduction of I2C support using multiple config options.

The options parameter is used by MMC to select standard (4-bit) or 8-bit
operation.


Simon Glass (7):
  tegra: Adjust funcmux config test to permit expansion
  tegra: Add I2C support to funcmux
  tegra: Enhance funcmux to support options
  tegra: Add SDMMC support to funcmux
  tegra: Use funcmux for MMC on tamonten
  tegra: Use funcmux for MMC on harmony
  tegra: Use funcmux for MMC on seaboard

 arch/arm/cpu/armv7/tegra2/board.c          |    2 +-
 arch/arm/cpu/armv7/tegra2/funcmux.c        |  159 +++++++++++++++++++++++++---
 arch/arm/include/asm/arch-tegra2/funcmux.h |   11 ++-
 board/avionic-design/common/tamonten.c     |   10 +--
 board/nvidia/harmony/harmony.c             |   19 +---
 board/nvidia/seaboard/seaboard.c           |   21 +---
 6 files changed, 166 insertions(+), 56 deletions(-)

-- 
1.7.3.1

[U-Boot] [PATCH 1/7] tegra: Adjust funcmux config test to permit expansion

[Simon Glass]

We want to support config options other than zero, so move the test to the
end to allow intermediate code to OK such a config.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/funcmux.c        |   35 +++++++++++++++++----------
 arch/arm/include/asm/arch-tegra2/funcmux.h |    3 ++
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
index 0878f51..0f03b9f 100644
--- a/arch/arm/cpu/armv7/tegra2/funcmux.c
+++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
@@ -26,27 +26,30 @@
 
 int funcmux_select(enum periph_id id, int config)
 {
-	if (config != 0) {
-		debug("%s: invalid config %d for periph_id %d", __func__,
-		      config, id);
-		return -1;
-	}
+	int bad_config = config != 0;
+
 	switch (id) {
 	case PERIPH_ID_UART1:
-		pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
-		pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
-		pinmux_tristate_disable(PINGRP_IRRX);
-		pinmux_tristate_disable(PINGRP_IRTX);
+		if (config == 0) {
+			pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
+			pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
+			pinmux_tristate_disable(PINGRP_IRRX);
+			pinmux_tristate_disable(PINGRP_IRTX);
+		}
 		break;
 
 	case PERIPH_ID_UART2:
-		pinmux_set_func(PINGRP_UAD, PMUX_FUNC_IRDA);
-		pinmux_tristate_disable(PINGRP_UAD);
+		if (config == 0) {
+			pinmux_set_func(PINGRP_UAD, PMUX_FUNC_IRDA);
+			pinmux_tristate_disable(PINGRP_UAD);
+		}
 		break;
 
 	case PERIPH_ID_UART4:
-		pinmux_set_func(PINGRP_GMC, PMUX_FUNC_UARTD);
-		pinmux_tristate_disable(PINGRP_GMC);
+		if (config == 0) {
+			pinmux_set_func(PINGRP_GMC, PMUX_FUNC_UARTD);
+			pinmux_tristate_disable(PINGRP_GMC);
+		}
 		break;
 
 	default:
@@ -54,5 +57,11 @@ int funcmux_select(enum periph_id id, int config)
 		return -1;
 	}
 
+	if (bad_config) {
+		debug("%s: invalid config %d for periph_id %d", __func__,
+		      config, id);
+		return -1;
+	}
+
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h b/arch/arm/include/asm/arch-tegra2/funcmux.h
index 2d724a2..d4f9cfb 100644
--- a/arch/arm/include/asm/arch-tegra2/funcmux.h
+++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
@@ -32,6 +32,9 @@
  * The basic config is 0, and higher numbers indicate different
  * pinmux settings to bring the peripheral out on other pins,
  *
+ * This function also disables tristate for the function's pins,
+ * so that they operate in normal mode.
+ *
  * @param id		Peripheral id
  * @param config	Configuration to use (generally 0)
  * @return 0 if ok, -1 on error (e.g. incorrect id or config)
-- 
1.7.3.1

[U-Boot] [PATCH 2/7] tegra: Add I2C support to funcmux

[Simon Glass]

Add support to funcmux for selecting I2C functions and programming
the pinmux appropriately.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/funcmux.c |   40 +++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
index 0f03b9f..82d994a 100644
--- a/arch/arm/cpu/armv7/tegra2/funcmux.c
+++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
@@ -52,6 +52,46 @@ int funcmux_select(enum periph_id id, int config)
 		}
 		break;
 
+	case PERIPH_ID_DVC_I2C:
+		/* there is only one selection, pinmux_config is ignored */
+		if (config == 0) {
+			pinmux_set_func(PINGRP_I2CP, PMUX_FUNC_I2C);
+			pinmux_tristate_disable(PINGRP_I2CP);
+		}
+		break;
+
+	case PERIPH_ID_I2C1:
+		/* support pinmux_config of 0 for now, */
+		if (config == 0) {
+			pinmux_set_func(PINGRP_RM, PMUX_FUNC_I2C);
+			pinmux_tristate_disable(PINGRP_RM);
+		}
+		break;
+	case PERIPH_ID_I2C2: /* I2C2 */
+		switch (config) {
+		case 0:	/* DDC pin group, select I2C2 */
+			pinmux_set_func(PINGRP_DDC, PMUX_FUNC_I2C2);
+			/* PTA to HDMI */
+			pinmux_set_func(PINGRP_PTA, PMUX_FUNC_HDMI);
+			pinmux_tristate_disable(PINGRP_DDC);
+			break;
+		case 1:	/* PTA pin group, select I2C2 */
+			pinmux_set_func(PINGRP_PTA, PMUX_FUNC_I2C2);
+			/* set DDC_SEL to RSVDx (RSVD2 works for now) */
+			pinmux_set_func(PINGRP_DDC, PMUX_FUNC_RSVD2);
+			pinmux_tristate_disable(PINGRP_PTA);
+			bad_config = 0;
+			break;
+		}
+		break;
+	case PERIPH_ID_I2C3: /* I2C3 */
+		/* support pinmux_config of 0 for now */
+		if (config == 0) {
+			pinmux_set_func(PINGRP_DTF, PMUX_FUNC_I2C3);
+			pinmux_tristate_disable(PINGRP_DTF);
+		}
+		break;
+
 	default:
 		debug("%s: invalid periph_id %d", __func__, id);
 		return -1;
-- 
1.7.3.1

[U-Boot] [PATCH 3/7] tegra: Enhance funcmux to support options

[Simon Glass]

Add support for an options parameter to funcmux_select() which allows
different pinmux options to be selected. An example of where this might
be used is a UART which supports 2-wire and 4-wire operation. The option
parameter in this case would specify which is required.

Invalid options cause failure with debug messages if enabled. Some
peripheral configs will only support certain options, and if these
required options are not provided, an error is also flagged.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/board.c          |    2 +-
 arch/arm/cpu/armv7/tegra2/funcmux.c        |   14 +++++++++++++-
 arch/arm/include/asm/arch-tegra2/funcmux.h |    3 ++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index ea06570..8b9e895 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -120,7 +120,7 @@ static void setup_uarts(int uart_ids)
 		if (uart_ids & (1 << i)) {
 			enum periph_id id = id_for_uart[i];
 
-			funcmux_select(id, 0);
+			funcmux_select(id, 0, 0);
 			clock_ll_start_uart(id);
 		}
 	}
diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
index 82d994a..ea40f1a 100644
--- a/arch/arm/cpu/armv7/tegra2/funcmux.c
+++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
@@ -22,11 +22,13 @@
 /* Tegra2 high-level function multiplexing */
 #include <common.h>
 #include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
 
-int funcmux_select(enum periph_id id, int config)
+int funcmux_select(enum periph_id id, int config, int options)
 {
 	int bad_config = config != 0;
+	int required_options = 0;  /* options that should have been given */
 
 	switch (id) {
 	case PERIPH_ID_UART1:
@@ -97,6 +99,16 @@ int funcmux_select(enum periph_id id, int config)
 		return -1;
 	}
 
+	if (options) {
+		debug("%s: invalid options %#x for periph_id %d", __func__,
+		      options, id);
+		return -1;
+	}
+	if (required_options) {
+		debug("%s: mising required options %#x for periph_id %d",
+		      __func__, required_options, id);
+		return -1;
+	}
 	if (bad_config) {
 		debug("%s: invalid config %d for periph_id %d", __func__,
 		      config, id);
diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h b/arch/arm/include/asm/arch-tegra2/funcmux.h
index d4f9cfb..a3b315b 100644
--- a/arch/arm/include/asm/arch-tegra2/funcmux.h
+++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
@@ -37,8 +37,9 @@
  *
  * @param id		Peripheral id
  * @param config	Configuration to use (generally 0)
+ * @param options	Options to enable (device-specific)
  * @return 0 if ok, -1 on error (e.g. incorrect id or config)
  */
-int funcmux_select(enum periph_id id, int config);
+int funcmux_select(enum periph_id id, int config, int options);
 
 #endif
-- 
1.7.3.1

[U-Boot] [PATCH 4/7] tegra: Add SDMMC support to funcmux

[Simon Glass]

This adds support for SDMMC ports to the funcmux. Only one
option is supported: FUNCMUXO_SDMMC_8BIT which selects an 8-bit
wide SDIO interface where available.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/cpu/armv7/tegra2/funcmux.c        |   70 ++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-tegra2/funcmux.h |    5 ++
 2 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/tegra2/funcmux.c b/arch/arm/cpu/armv7/tegra2/funcmux.c
index ea40f1a..accae5c 100644
--- a/arch/arm/cpu/armv7/tegra2/funcmux.c
+++ b/arch/arm/cpu/armv7/tegra2/funcmux.c
@@ -94,6 +94,76 @@ int funcmux_select(enum periph_id id, int config, int options)
 		}
 		break;
 
+	case PERIPH_ID_SDMMC2:
+		if (config == 0) {
+			if (options & FUNCMUXO_SDMMC_8BIT) {
+				pinmux_set_func(PINGRP_DTA, PMUX_FUNC_SDIO2);
+				pinmux_set_func(PINGRP_DTD, PMUX_FUNC_SDIO2);
+
+				pinmux_tristate_disable(PINGRP_DTA);
+				pinmux_tristate_disable(PINGRP_DTD);
+				options &= ~FUNCMUXO_SDMMC_8BIT;
+			} else {
+				required_options = FUNCMUXO_SDMMC_8BIT;
+			}
+		}
+		break;
+
+	case PERIPH_ID_SDMMC3:
+		if (config == 0) {
+			pinmux_set_func(PINGRP_SDB, PMUX_FUNC_SDIO3);
+			pinmux_set_func(PINGRP_SDC, PMUX_FUNC_SDIO3);
+			pinmux_set_func(PINGRP_SDD, PMUX_FUNC_SDIO3);
+
+			pinmux_tristate_disable(PINGRP_SDC);
+			pinmux_tristate_disable(PINGRP_SDD);
+			pinmux_tristate_disable(PINGRP_SDB);
+
+			if (options & FUNCMUXO_SDMMC_8BIT) {
+				pinmux_set_func(PINGRP_SLXA, PMUX_FUNC_SDIO3);
+				pinmux_set_func(PINGRP_SLXC, PMUX_FUNC_SDIO3);
+				pinmux_set_func(PINGRP_SLXD, PMUX_FUNC_SDIO3);
+				pinmux_set_func(PINGRP_SLXK, PMUX_FUNC_SDIO3);
+
+				pinmux_tristate_disable(PINGRP_SLXA);
+				pinmux_tristate_disable(PINGRP_SLXC);
+				pinmux_tristate_disable(PINGRP_SLXD);
+				pinmux_tristate_disable(PINGRP_SLXK);
+				options &= ~FUNCMUXO_SDMMC_8BIT;
+			}
+		}
+		break;
+
+	case PERIPH_ID_SDMMC4:
+		switch (config) {
+		case 0:
+			if (options & FUNCMUXO_SDMMC_8BIT) {
+				pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
+				pinmux_set_func(PINGRP_ATC, PMUX_FUNC_SDIO4);
+				pinmux_set_func(PINGRP_ATD, PMUX_FUNC_SDIO4);
+
+				pinmux_tristate_disable(PINGRP_ATB);
+				pinmux_tristate_disable(PINGRP_ATC);
+				pinmux_tristate_disable(PINGRP_ATD);
+				options &= ~FUNCMUXO_SDMMC_8BIT;
+			} else {
+				required_options = FUNCMUXO_SDMMC_8BIT;
+			}
+			break;
+
+		case 1:
+			pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
+			pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4);
+			pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4);
+
+			pinmux_tristate_disable(PINGRP_ATB);
+			pinmux_tristate_disable(PINGRP_GMA);
+			pinmux_tristate_disable(PINGRP_GME);
+			bad_config = 0;
+			break;
+		}
+		break;
+
 	default:
 		debug("%s: invalid periph_id %d", __func__, id);
 		return -1;
diff --git a/arch/arm/include/asm/arch-tegra2/funcmux.h b/arch/arm/include/asm/arch-tegra2/funcmux.h
index a3b315b..2c48433 100644
--- a/arch/arm/include/asm/arch-tegra2/funcmux.h
+++ b/arch/arm/include/asm/arch-tegra2/funcmux.h
@@ -24,6 +24,11 @@
 #ifndef __FUNCMUX_H
 #define __FUNCMUX_H
 
+/* Options supported by the func mux */
+enum {
+	FUNCMUXO_SDMMC_8BIT	= 1 << 0,	/* 8-pin SDMMC interface */
+};
+
 /**
  * Select a config for a particular peripheral.
  *
-- 
1.7.3.1

[U-Boot] [PATCH 5/7] tegra: Use funcmux for MMC on tamonten

[Simon Glass]

Use the new funcmux_select() feature to set up the MMC pin mux.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/avionic-design/common/tamonten.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/board/avionic-design/common/tamonten.c b/board/avionic-design/common/tamonten.c
index 97e59fb..c5fd988 100644
--- a/board/avionic-design/common/tamonten.c
+++ b/board/avionic-design/common/tamonten.c
@@ -32,6 +32,7 @@
 #include <asm/arch/sys_proto.h>
 #include <asm/arch/clk_rst.h>
 #include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/uart.h>
 #include <asm/arch/mmc.h>
@@ -63,14 +64,7 @@ int timer_init(void)
  */
 static void pin_mux_mmc(void)
 {
-	/* SDMMC4: config 3, x8 on 2nd set of pins */
-	pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
-	pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4);
-	pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4);
-
-	pinmux_tristate_disable(PINGRP_ATB);
-	pinmux_tristate_disable(PINGRP_GMA);
-	pinmux_tristate_disable(PINGRP_GME);
+	funcmux_select(PERIPH_ID_SDMMC4, 1, FUNCMUXO_SDMMC_8BIT);
 }
 #endif
 
-- 
1.7.3.1

[U-Boot] [PATCH 6/7] tegra: Use funcmux for MMC on harmony

[Simon Glass]

Use the new funcmux_select() feature to set up the MMC pin mux.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/nvidia/harmony/harmony.c |   19 ++++---------------
 1 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/board/nvidia/harmony/harmony.c b/board/nvidia/harmony/harmony.c
index d5e147d..bb8aeb4 100644
--- a/board/nvidia/harmony/harmony.c
+++ b/board/nvidia/harmony/harmony.c
@@ -24,6 +24,8 @@
 #include <common.h>
 #include <asm/io.h>
 #include <asm/arch/tegra2.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/mmc.h>
 #include <asm/gpio.h>
@@ -46,27 +48,14 @@ void gpio_config_uart(void)
  */
 static void pin_mux_mmc(void)
 {
-	/* SDMMC4: config 3, x8 on 2nd set of pins */
-	pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
-	pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4);
-	pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4);
-
-	pinmux_tristate_disable(PINGRP_ATB);
-	pinmux_tristate_disable(PINGRP_GMA);
-	pinmux_tristate_disable(PINGRP_GME);
+	funcmux_select(PERIPH_ID_SDMMC4, 1, FUNCMUXO_SDMMC_8BIT);
+	funcmux_select(PERIPH_ID_SDMMC2, 0, FUNCMUXO_SDMMC_8BIT);
 
 	/* For power GPIO PI6 */
 	pinmux_tristate_disable(PINGRP_ATA);
 	/* For CD GPIO PH2 */
 	pinmux_tristate_disable(PINGRP_ATD);
 
-	/* SDMMC2: SDIO2_CLK, SDIO2_CMD, SDIO2_DAT[7:0] */
-	pinmux_set_func(PINGRP_DTA, PMUX_FUNC_SDIO2);
-	pinmux_set_func(PINGRP_DTD, PMUX_FUNC_SDIO2);
-
-	pinmux_tristate_disable(PINGRP_DTA);
-	pinmux_tristate_disable(PINGRP_DTD);
-
 	/* For power GPIO PT3 */
 	pinmux_tristate_disable(PINGRP_DTB);
 	/* For CD GPIO PI5 */
-- 
1.7.3.1

[U-Boot] [PATCH 7/7] tegra: Use funcmux for MMC on seaboard

[Simon Glass]

Use the new funcmux_select() feature to set up the MMC pin mux.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/nvidia/seaboard/seaboard.c |   21 ++++-----------------
 1 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c
index 56acd61..9349f29 100644
--- a/board/nvidia/seaboard/seaboard.c
+++ b/board/nvidia/seaboard/seaboard.c
@@ -24,6 +24,8 @@
 #include <common.h>
 #include <asm/io.h>
 #include <asm/arch/tegra2.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/mmc.h>
 #include <asm/gpio.h>
@@ -59,23 +61,8 @@ void gpio_config_uart(void)
  */
 static void pin_mux_mmc(void)
 {
-	/* SDMMC4: config 3, x8 on 2nd set of pins */
-	pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
-	pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4);
-	pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4);
-
-	pinmux_tristate_disable(PINGRP_ATB);
-	pinmux_tristate_disable(PINGRP_GMA);
-	pinmux_tristate_disable(PINGRP_GME);
-
-	/* SDMMC3: SDIO3_CLK, SDIO3_CMD, SDIO3_DAT[3:0] */
-	pinmux_set_func(PINGRP_SDB, PMUX_FUNC_SDIO3);
-	pinmux_set_func(PINGRP_SDC, PMUX_FUNC_SDIO3);
-	pinmux_set_func(PINGRP_SDD, PMUX_FUNC_SDIO3);
-
-	pinmux_tristate_disable(PINGRP_SDC);
-	pinmux_tristate_disable(PINGRP_SDD);
-	pinmux_tristate_disable(PINGRP_SDB);
+	funcmux_select(PERIPH_ID_SDMMC4, 1, FUNCMUXO_SDMMC_8BIT);
+	funcmux_select(PERIPH_ID_SDMMC3, 0, 0);
 
 	/* For power GPIO PI6 */
 	pinmux_tristate_disable(PINGRP_ATA);
-- 
1.7.3.1

[U-Boot] [PATCH 0/7] tegra: Enhance funcmux to support I2C and MMC

[Stephen Warren]

On 01/09/2012 03:53 PM, Simon Glass wrote:
> This series expands funcmux_select() to support configs other than 0, and
> to support options associated with a config.
> 
> This permits introduction of I2C support using multiple config options.
> 
> The options parameter is used by MMC to select standard (4-bit) or 8-bit
> operation.

The unification in this series basically seems fine.

Why not consider bus width part of the "config" though, rather the
complicating things with an extra parameter? As an example, for SDMMC4,
you'd have say:

0: ATC + ATD 8 bit
1: ATB + GMA 4 bit
2: ATB + GMA + GME 8 bit

... and no option values.

Also, we should probably define names for the config values, at least in
the cases where 0 isn't the only option. Hard-coding 0 or 1 at the call
sites isn't very meaningful.

Oh, and SDMMC4's option 0 isn't correct: I think pin group ATB should be
removed, since it contains signals SDIO4_CLK and SDIO4_CMD which are
also part of pin group ATC, when those pin groups are set to function
SDIO4 at least. I didn't check to see if any other similar problems exist.

-- 
nvpublic

[U-Boot] [PATCH 0/7] tegra: Enhance funcmux to support I2C and MMC

[Simon Glass]

Hi Stephen,

On Mon, Jan 9, 2012 at 3:11 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/09/2012 03:53 PM, Simon Glass wrote:
>> This series expands funcmux_select() to support configs other than 0, and
>> to support options associated with a config.
>>
>> This permits introduction of I2C support using multiple config options.
>>
>> The options parameter is used by MMC to select standard (4-bit) or 8-bit
>> operation.
>
> The unification in this series basically seems fine.
>
> Why not consider bus width part of the "config" though, rather the
> complicating things with an extra parameter? As an example, for SDMMC4,
> you'd have say:
>
> 0: ATC + ATD 8 bit
> 1: ATB + GMA 4 bit
> 2: ATB + GMA + GME 8 bit
>
> ... and no option values.

I am thinking ahead a little to where we have more peripherals with
several options. If we imagine a situation where the SOC has 3
different pin configs each of which can be 1-bit, 4-bit or 8-bit, then
it is nice to have the options broken out separately.

Also, we can also use the options for something else, like Tegra 3's
drive strength and slew rate control (and perhaps other things I
understand even less).

>
> Also, we should probably define names for the config values, at least in
> the cases where 0 isn't the only option. Hard-coding 0 or 1 at the call
> sites isn't very meaningful.

I can certainly do that - it was in the back of my mind. But the only
thing I could think of was to create an enum with the pingroup
assignments, like:

enum {
    UART1_IRRX_IRTX   = 0,
    UART2_UAD             = 0,
...
};

Seems a bit ugly?

>
> Oh, and SDMMC4's option 0 isn't correct: I think pin group ATB should be
> removed, since it contains signals SDIO4_CLK and SDIO4_CMD which are
> also part of pin group ATC, when those pin groups are set to function
> SDIO4 at least. I didn't check to see if any other similar problems exist.

Yes, thanks for finding that, will fix once I hear back about the above.

Regards,
SImon

>
> --
> nvpublic

[U-Boot] [PATCH 0/7] tegra: Enhance funcmux to support I2C and MMC

[Stephen Warren]

On 01/09/2012 04:36 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Jan 9, 2012 at 3:11 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> On 01/09/2012 03:53 PM, Simon Glass wrote:
>>> This series expands funcmux_select() to support configs other than 0, and
>>> to support options associated with a config.
>>>
>>> This permits introduction of I2C support using multiple config options.
>>>
>>> The options parameter is used by MMC to select standard (4-bit) or 8-bit
>>> operation.
>>
>> The unification in this series basically seems fine.
>>
>> Why not consider bus width part of the "config" though, rather the
>> complicating things with an extra parameter? As an example, for SDMMC4,
>> you'd have say:
>>
>> 0: ATC + ATD 8 bit
>> 1: ATB + GMA 4 bit
>> 2: ATB + GMA + GME 8 bit
>>
>> ... and no option values.
> 
> I am thinking ahead a little to where we have more peripherals with
> several options. If we imagine a situation where the SOC has 3
> different pin configs each of which can be 1-bit, 4-bit or 8-bit, then
> it is nice to have the options broken out separately.

On Tegra20, the pin mux is controlled in groups, so you're mostly
picking which groups to use for the function, which then determines the
bus width. In other words, its often unlikely that you can pick bus
width as an independent option from the set of pins/groups used.

On Tegra30, the situation is about the same except that the mux function
is picked on a per-pin basis instead of in groups of pins, which takes
the same argument even further; for a 4-bit bus you'd simply remove 4
pins from the list of pins being used, so it doesn't make sense to refer
to 4-bit as an option of an 8-bit setup with some pins unused, because
the unused pins are set to some other mux function.

> Also, we can also use the options for something else, like Tegra 3's
> drive strength and slew rate control (and perhaps other things I
> understand even less).

There are far far too many options for that to be represented by a
single U32, or even a small number of them. When boards start needing to
set up drive strengths etc., we'll probably need individual API calls
for each config option, since each board's characterization will trigger
a combination of options that's extremely likely to be unique.

>> Also, we should probably define names for the config values, at least in
>> the cases where 0 isn't the only option. Hard-coding 0 or 1 at the call
>> sites isn't very meaningful.
> 
> I can certainly do that - it was in the back of my mind. But the only
> thing I could think of was to create an enum with the pingroup
> assignments, like:
> 
> enum {
>     UART1_IRRX_IRTX   = 0,
>     UART2_UAD             = 0,
> ...
> };
> 
> Seems a bit ugly?

I don't agree. The options are just completely arbitrary IDs for a
particular set of pins/groups that are being used. Well, arbitrary
within the set of possibilities given the wiring of Tegra's pinmux HW
anyway. Hence, giving those IDs names based on which pins/groups are
being used makes sense to me.

-- 
nvpublic

[U-Boot] [PATCH 5/7] tegra: Use funcmux for MMC on tamonten

[Thierry Reding]

* Simon Glass wrote:
> Use the new funcmux_select() feature to set up the MMC pin mux.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Tested on Plutux and Medcom.

Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>

> ---
>  board/avionic-design/common/tamonten.c |   10 ++--------
>  1 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/board/avionic-design/common/tamonten.c b/board/avionic-design/common/tamonten.c
> index 97e59fb..c5fd988 100644
> --- a/board/avionic-design/common/tamonten.c
> +++ b/board/avionic-design/common/tamonten.c
> @@ -32,6 +32,7 @@
>  #include <asm/arch/sys_proto.h>
>  #include <asm/arch/clk_rst.h>
>  #include <asm/arch/clock.h>
> +#include <asm/arch/funcmux.h>
>  #include <asm/arch/pinmux.h>
>  #include <asm/arch/uart.h>
>  #include <asm/arch/mmc.h>
> @@ -63,14 +64,7 @@ int timer_init(void)
>   */
>  static void pin_mux_mmc(void)
>  {
> -	/* SDMMC4: config 3, x8 on 2nd set of pins */
> -	pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
> -	pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4);
> -	pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4);
> -
> -	pinmux_tristate_disable(PINGRP_ATB);
> -	pinmux_tristate_disable(PINGRP_GMA);
> -	pinmux_tristate_disable(PINGRP_GME);
> +	funcmux_select(PERIPH_ID_SDMMC4, 1, FUNCMUXO_SDMMC_8BIT);
>  }
>  #endif

>  
> -- 
> 1.7.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120110/e5bc4e4f/attachment.pgp>

[U-Boot] [PATCH 5/7] tegra: Use funcmux for MMC on tamonten

[Simon Glass]

On Mon, Jan 9, 2012 at 11:31 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> * Simon Glass wrote:
>> Use the new funcmux_select() feature to set up the MMC pin mux.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Tested on Plutux and Medcom.
>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>

Thanks!

>
>> ---
>> ?board/avionic-design/common/tamonten.c | ? 10 ++--------
>> ?1 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/board/avionic-design/common/tamonten.c b/board/avionic-design/common/tamonten.c
>> index 97e59fb..c5fd988 100644
>> --- a/board/avionic-design/common/tamonten.c
>> +++ b/board/avionic-design/common/tamonten.c
>> @@ -32,6 +32,7 @@
>> ?#include <asm/arch/sys_proto.h>
>> ?#include <asm/arch/clk_rst.h>
>> ?#include <asm/arch/clock.h>
>> +#include <asm/arch/funcmux.h>
>> ?#include <asm/arch/pinmux.h>
>> ?#include <asm/arch/uart.h>
>> ?#include <asm/arch/mmc.h>
>> @@ -63,14 +64,7 @@ int timer_init(void)
>> ? */
>> ?static void pin_mux_mmc(void)
>> ?{
>> - ? ? /* SDMMC4: config 3, x8 on 2nd set of pins */
>> - ? ? pinmux_set_func(PINGRP_ATB, PMUX_FUNC_SDIO4);
>> - ? ? pinmux_set_func(PINGRP_GMA, PMUX_FUNC_SDIO4);
>> - ? ? pinmux_set_func(PINGRP_GME, PMUX_FUNC_SDIO4);
>> -
>> - ? ? pinmux_tristate_disable(PINGRP_ATB);
>> - ? ? pinmux_tristate_disable(PINGRP_GMA);
>> - ? ? pinmux_tristate_disable(PINGRP_GME);
>> + ? ? funcmux_select(PERIPH_ID_SDMMC4, 1, FUNCMUXO_SDMMC_8BIT);
>> ?}
>> ?#endif
>
>>
>> --
>> 1.7.3.1

[U-Boot] [PATCH 0/7] tegra: Enhance funcmux to support I2C and MMC

[Simon Glass]

Hi Stephen,

On Mon, Jan 9, 2012 at 3:46 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/09/2012 04:36 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Mon, Jan 9, 2012 at 3:11 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>> On 01/09/2012 03:53 PM, Simon Glass wrote:
>>>> This series expands funcmux_select() to support configs other than 0, and
>>>> to support options associated with a config.
>>>>
>>>> This permits introduction of I2C support using multiple config options.
>>>>
>>>> The options parameter is used by MMC to select standard (4-bit) or 8-bit
>>>> operation.
>>>
>>> The unification in this series basically seems fine.
>>>
>>> Why not consider bus width part of the "config" though, rather the
>>> complicating things with an extra parameter? As an example, for SDMMC4,
>>> you'd have say:
>>>
>>> 0: ATC + ATD 8 bit
>>> 1: ATB + GMA 4 bit
>>> 2: ATB + GMA + GME 8 bit
>>>
>>> ... and no option values.
>>
>> I am thinking ahead a little to where we have more peripherals with
>> several options. If we imagine a situation where the SOC has 3
>> different pin configs each of which can be 1-bit, 4-bit or 8-bit, then
>> it is nice to have the options broken out separately.
>
> On Tegra20, the pin mux is controlled in groups, so you're mostly
> picking which groups to use for the function, which then determines the
> bus width. In other words, its often unlikely that you can pick bus
> width as an independent option from the set of pins/groups used.

I wasn't really saying it was independent, just that some configs
offer different variations and it would be nice to make this explicit
rather than flattening the config + options information into a single
value. For now the benefit is marginal so I will remove it,
particularly as you say it will be no use on Tegra30:

>
> On Tegra30, the situation is about the same except that the mux function
> is picked on a per-pin basis instead of in groups of pins, which takes
> the same argument even further; for a 4-bit bus you'd simply remove 4
> pins from the list of pins being used, so it doesn't make sense to refer
> to 4-bit as an option of an 8-bit setup with some pins unused, because
> the unused pins are set to some other mux function.

IMO it does make sense for the user - remember I am trying to simplify
pinmux use for boards. Exposing all the different pins is almost
exactly what I am trying to avoid :-)

>
>> Also, we can also use the options for something else, like Tegra 3's
>> drive strength and slew rate control (and perhaps other things I
>> understand even less).
>
> There are far far too many options for that to be represented by a
> single U32, or even a small number of them. When boards start needing to
> set up drive strengths etc., we'll probably need individual API calls
> for each config option, since each board's characterization will trigger
> a combination of options that's extremely likely to be unique.

My reading of the Tegra30 registers suggested this was possible, but
it seems I assumed too much!

>
>>> Also, we should probably define names for the config values, at least in
>>> the cases where 0 isn't the only option. Hard-coding 0 or 1 at the call
>>> sites isn't very meaningful.
>>
>> I can certainly do that - it was in the back of my mind. But the only
>> thing I could think of was to create an enum with the pingroup
>> assignments, like:
>>
>> enum {
>> ? ? UART1_IRRX_IRTX ? = 0,
>> ? ? UART2_UAD ? ? ? ? ? ? = 0,
>> ...
>> };
>>
>> Seems a bit ugly?
>
> I don't agree. The options are just completely arbitrary IDs for a
> particular set of pins/groups that are being used. Well, arbitrary
> within the set of possibilities given the wiring of Tegra's pinmux HW
> anyway. Hence, giving those IDs names based on which pins/groups are
> being used makes sense to me.

OK ugly long enums are on their way :-) - will send a new series.
Thanks again for the helpful review.

Regards,
Simon

>
> --
> nvpublic