tegra: Enhance funcmux to support I2C and MMC

PATCH New Simon Glass 3 participants 14 messages Jan 9, 2012 – Jan 11, 2012

Subsystems (2)

Arm (32-bit) Tegra architecture support

lore

Share & tools

lore

Simon Glass

author

2012-01-09 22:53 UTC #0 lore raw

cover tegra: Enhance funcmux to support I2C and MMC

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(-)

Signature · 1 line

1.7.3.1

Simon Glass

author

2012-01-09 22:53 UTC #1 lore raw

PATCH 1/7 tegra: Adjust funcmux config test to permit expansion

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 <[email protected]>

---
 arch/arm/cpu/armv7/tegra2/funcmux.c        |   35 +++++++++++++++++----------
 arch/arm/include/asm/arch-tegra2/funcmux.h |    3 ++
 2 files changed, 25 insertions(+), 13 deletions(-)

arch/arm/cpu/armv7/tegra2/funcmux.c +22 ~~-13~~

index 0878f51..0f03b9f 100644
@@ -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;
 }

arch/arm/include/asm/arch-tegra2/funcmux.h +3 -0

index 2d724a2..d4f9cfb 100644
@@ -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)

Signature · 1 line

1.7.3.1

Simon Glass

author

2012-01-09 22:53 UTC #2 lore raw

PATCH 2/7 tegra: Add I2C support to funcmux

In reply to Simon Glass

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

Signed-off-by Simon Glass <[email protected]>

---
 arch/arm/cpu/armv7/tegra2/funcmux.c |   40 +++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

arch/arm/cpu/armv7/tegra2/funcmux.c +40 -0

index 0f03b9f..82d994a 100644
@@ -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;

Signature · 1 line

1.7.3.1

Simon Glass

author

2012-01-09 22:53 UTC #3 lore raw

PATCH 3/7 tegra: Enhance funcmux to support options

In reply to 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 <[email protected]>

---
 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(-)

arch/arm/cpu/armv7/tegra2/board.c +1 -1

index ea06570..8b9e895 100644
@@ -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);
 		}
 	}

arch/arm/cpu/armv7/tegra2/funcmux.c +13 -1

index 82d994a..ea40f1a 100644
@@ -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);

arch/arm/include/asm/arch-tegra2/funcmux.h +2 -1

index d4f9cfb..a3b315b 100644
@@ -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

Signature · 1 line

1.7.3.1

Simon Glass

author

2012-01-09 22:53 UTC #4 lore raw

PATCH 4/7 tegra: Add SDMMC support to funcmux

In reply to 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 <[email protected]>

---
 arch/arm/cpu/armv7/tegra2/funcmux.c        |   70 ++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-tegra2/funcmux.h |    5 ++
 2 files changed, 75 insertions(+), 0 deletions(-)

arch/arm/cpu/armv7/tegra2/funcmux.c +70 -0

index ea40f1a..accae5c 100644
@@ -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;

arch/arm/include/asm/arch-tegra2/funcmux.h +5 -0

index a3b315b..2c48433 100644
@@ -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.
  *

Signature · 1 line

1.7.3.1

Simon Glass

author

2012-01-09 22:53 UTC #5 lore raw

PATCH 5/7 tegra: Use funcmux for MMC on tamonten

In reply to Simon Glass

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

Signed-off-by Simon Glass <[email protected]>

---
 board/avionic-design/common/tamonten.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

board/avionic-design/common/tamonten.c +2 -8

index 97e59fb..c5fd988 100644
@@ -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

Signature · 1 line

1.7.3.1

Thierry Reding

2012-01-10 07:31 UTC #6 lore raw

PATCH 5/7 tegra: Use funcmux for MMC on tamonten

Quoted 3 lines · Simon Glass

Use the new funcmux_select() feature to set up the MMC pin mux.
Signed-off-by Simon Glass <[email protected]>

Tested on Plutux and Medcom.

Acked-by Thierry Reding <[email protected]>

Tested-by Thierry Reding <[email protected]>

Quoted 35 lines

---
 board/avionic-design/common/tamonten.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

board/avionic-design/common/tamonten.c +2 -8

index 97e59fb..c5fd988 100644
@@ -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

Signature · 2 lines

-- 
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>

Simon Glass

author

2012-01-11 21:54 UTC #7 lore raw

PATCH 5/7 tegra: Use funcmux for MMC on tamonten

Quoted 9 lines · Thierry Reding

Quoted 3 lines · Simon Glass
Use the new funcmux_select() feature to set up the MMC pin mux.
Signed-off-by Simon Glass <[email protected]>
Tested on Plutux and Medcom.
Acked-by Thierry Reding <[email protected]>
Tested-by Thierry Reding <[email protected]>

Thanks!

Quoted 36 lines

Quoted 36 lines
---
?board/avionic-design/common/tamonten.c | ? 10 ++-------- ?1 files changed, 2 insertions(+), 8 deletions(-)
board/avionic-design/common/tamonten.c +0 -0
index 97e59fb..c5fd988 100644
@@ -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>
board/avionic-design/common/tamonten.c +0 -0
@@ -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

Simon Glass

author

2012-01-09 22:53 UTC #8 lore raw

PATCH 6/7 tegra: Use funcmux for MMC on harmony

In reply to Simon Glass

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

Signed-off-by Simon Glass <[email protected]>

---
 board/nvidia/harmony/harmony.c |   19 ++++---------------
 1 files changed, 4 insertions(+), 15 deletions(-)

board/nvidia/harmony/harmony.c +4 ~~-15~~

index d5e147d..bb8aeb4 100644
@@ -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 */

Signature · 1 line

1.7.3.1

Simon Glass

author

2012-01-09 22:53 UTC #9 lore raw

PATCH 7/7 tegra: Use funcmux for MMC on seaboard

In reply to Simon Glass

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

Signed-off-by Simon Glass <[email protected]>

---
 board/nvidia/seaboard/seaboard.c |   21 ++++-----------------
 1 files changed, 4 insertions(+), 17 deletions(-)

board/nvidia/seaboard/seaboard.c +4 ~~-17~~

index 56acd61..9349f29 100644
@@ -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);

Signature · 1 line

1.7.3.1

Stephen Warren

2012-01-09 23:11 UTC #10 lore raw

cover tegra: Enhance funcmux to support I2C and MMC

In reply to Simon Glass

Quoted 7 lines · 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.

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.

Signature · 1 line

nvpublic

Simon Glass

author

2012-01-09 23:36 UTC #11 lore raw

cover tegra: Enhance funcmux to support I2C and MMC

Hi Stephen,

Quoted 15 lines · Stephen Warren

Quoted 7 lines · 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.
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:

Quoted 5 lines

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).

Quoted 4 lines

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?

Quoted 5 lines

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

Quoted 3 lines

--
nvpublic

Stephen Warren

2012-01-09 23:46 UTC #12 lore raw

cover tegra: Enhance funcmux to support I2C and MMC

Quoted 22 lines · Simon Glass

Hi Stephen,
Quoted 19 lines · Stephen Warren
Quoted 7 lines · 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.
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

Quoted 6 lines

Quoted 1 line
... 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.

Quoted 3 lines

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.

Quoted 8 lines

Quoted 3 lines
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:

Quoted 7 lines

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.

Signature · 1 line

nvpublic

Simon Glass

author

2012-01-11 22:41 UTC #13 lore raw

cover tegra: Enhance funcmux to support I2C and MMC

Hi Stephen,

Quoted 30 lines · Stephen Warren

Quoted 22 lines · Simon Glass
Hi Stephen,
Quoted 19 lines · Stephen Warren
Quoted 7 lines · 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.
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
Quoted 6 lines
Quoted 1 line
... 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.

Quoted 4 lines

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:

Quoted 7 lines

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 :-)

Quoted 10 lines

Quoted 4 lines
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!

Quoted 15 lines

Quoted 15 lines
Quoted 4 lines
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,
...
};

Quoted 7 lines

Quoted 1 line
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

Quoted 3 lines

--
nvpublic