[U-Boot] [PATCH 0/5] Tegra30: MMC: Add DT-based MMC driver for Tegra30/Cardhu

[U-Boot] [PATCH 0/5] Tegra30: MMC: Add DT-based MMC driver for Tegra30/Cardhu

[Tom Warren]

This patchset adds SDMMC device-tree support to the Tegra30 dts files,
and enables the Tegra MMC driver on Tegra30 Cardhu.

I've tested this on my Cardhu-A04 and everything works fine,
including card detect. All Tegra boards also build w/o error, and
Seaboard MMC functionality is unchanged.

Tom Warren (5):
  Tegra30: fdt: Add SDMMC (sdhci) nodes for T30 boards (Cardhu for now)
  Tegra: MMC: Added/update SDMMC registers/base addresses for T20/T30
  Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines
  mmc: Tegra: Add SD bus power/voltage function and MMC pad init call.
  Tegra30: MMC: Enable DT MMC driver support for Tegra30 Cardhu boards

 arch/arm/dts/tegra30.dtsi                      |   32 +++++++++++++++
 arch/arm/include/asm/arch-tegra/tegra_mmc.h    |   35 ++++++++++++++---
 arch/arm/include/asm/arch-tegra20/tegra.h      |    5 ++
 arch/arm/include/asm/arch-tegra30/gp_padctrl.h |    6 +++
 arch/arm/include/asm/arch-tegra30/tegra.h      |    5 ++
 board/nvidia/cardhu/cardhu.c                   |   49 +++++++++++++++++++++++
 board/nvidia/common/board.c                    |   50 +++++++++++++++++++++++-
 board/nvidia/dts/tegra30-cardhu.dts            |   15 +++++++
 drivers/mmc/tegra_mmc.c                        |   48 ++++++++++++++++++++--
 include/configs/cardhu.h                       |   20 +++++++++-
 include/configs/tegra30-common.h               |    3 +
 11 files changed, 255 insertions(+), 13 deletions(-)

[U-Boot] [PATCH 1/5] Tegra30: fdt: Add SDMMC (sdhci) nodes for T30 boards (Cardhu for now)

[Tom Warren]

Took these values directly from the kernel dts files.

Signed-off-by: Tom Warren <twarren@nvidia.com>
---
 arch/arm/dts/tegra30.dtsi           |   32 ++++++++++++++++++++++++++++++++
 board/nvidia/dts/tegra30-cardhu.dts |   15 +++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
index 9483e80..834d617 100644
--- a/arch/arm/dts/tegra30.dtsi
+++ b/arch/arm/dts/tegra30.dtsi
@@ -184,4 +184,36 @@
 		clocks = <&tegra_car 105>;
 		status = "disabled";
 	};
+
+	sdhci at 78000000 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000000 0x200>;
+		interrupts = <0 14 0x04>;
+		clocks = <&tegra_car 14>;
+		status = "disabled";
+	};
+
+	sdhci at 78000200 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000200 0x200>;
+		interrupts = <0 15 0x04>;
+		clocks = <&tegra_car 9>;
+		status = "disabled";
+	};
+
+	sdhci at 78000400 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000400 0x200>;
+		interrupts = <0 19 0x04>;
+		clocks = <&tegra_car 69>;
+		status = "disabled";
+	};
+
+	sdhci at 78000600 {
+		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
+		reg = <0x78000600 0x200>;
+		interrupts = <0 31 0x04>;
+		clocks = <&tegra_car 15>;
+		status = "disabled";
+	};
 };
diff --git a/board/nvidia/dts/tegra30-cardhu.dts b/board/nvidia/dts/tegra30-cardhu.dts
index 48039c9..4d22b48 100644
--- a/board/nvidia/dts/tegra30-cardhu.dts
+++ b/board/nvidia/dts/tegra30-cardhu.dts
@@ -12,6 +12,8 @@
 		i2c2 = "/i2c at 7000c400";
 		i2c3 = "/i2c at 7000c500";
 		i2c4 = "/i2c at 7000c700";
+		sdhci0 = "/sdhci at 78000600";
+		sdhci1 = "/sdhci at 78000000";
 	};
 
 	memory {
@@ -48,4 +50,17 @@
 		status = "okay";
 		spi-max-frequency = <25000000>;
 	};
+
+	sdhci at 78000000 {
+		status = "okay";
+		cd-gpios = <&gpio 69 1>; /* gpio PI5 */
+		wp-gpios = <&gpio 155 0>; /* gpio PT3 */
+		power-gpios = <&gpio 31 0>; /* gpio PD7 */
+		bus-width = <4>;
+	};
+
+	sdhci at 78000600 {
+		status = "okay";
+		bus-width = <8>;
+	};
 };
-- 
1.7.0.4

[U-Boot] [PATCH 2/5] Tegra: MMC: Added/update SDMMC registers/base addresses for T20/T30

[Tom Warren]

Moved SDMMC base addresses into each SoC's main header, since they differ.
Added pad control settings for T30 from the TRM, and added additional
vendor-specific SD/MMC registers and bus power defines.

Signed-off-by: Tom Warren <twarren@nvidia.com>
---
 arch/arm/include/asm/arch-tegra/tegra_mmc.h    |   35 +++++++++++++++++++----
 arch/arm/include/asm/arch-tegra20/tegra.h      |    5 +++
 arch/arm/include/asm/arch-tegra30/gp_padctrl.h |    6 ++++
 arch/arm/include/asm/arch-tegra30/tegra.h      |    5 +++
 4 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
index bd18f5f..cd896a5 100644
--- a/arch/arm/include/asm/arch-tegra/tegra_mmc.h
+++ b/arch/arm/include/asm/arch-tegra/tegra_mmc.h
@@ -22,10 +22,7 @@
 #ifndef __TEGRA_MMC_H_
 #define __TEGRA_MMC_H_
 
-#define TEGRA_SDMMC1_BASE	0xC8000000
-#define TEGRA_SDMMC2_BASE	0xC8000200
-#define TEGRA_SDMMC3_BASE	0xC8000400
-#define TEGRA_SDMMC4_BASE	0xC8000600
+#include <fdtdec.h>
 
 #define MAX_HOSTS		4	/* Max number of 'hosts'/controllers */
 
@@ -64,12 +61,30 @@ struct tegra_mmc {
 	unsigned char	admaerr;	/* offset 54h */
 	unsigned char	res4[3];	/* RESERVED, offset 55h-57h */
 	unsigned long	admaaddr;	/* offset 58h-5Fh */
-	unsigned char	res5[0x9c];	/* RESERVED, offset 60h-FBh */
+	unsigned char	res5[0xa0];	/* RESERVED, offset 60h-FBh */
 	unsigned short	slotintstatus;	/* offset FCh */
 	unsigned short	hcver;		/* HOST Version */
-	unsigned char	res6[0x100];	/* RESERVED, offset 100h-1FFh */
+	unsigned int	venclkctl;	/* _VENDOR_CLOCK_CNTRL_0,    100h */
+	unsigned int	venspictl;	/* _VENDOR_SPI_CNTRL_0,      104h */
+	unsigned int	venspiintsts;	/* _VENDOR_SPI_INT_STATUS_0, 108h */
+	unsigned int	venceatactl;	/* _VENDOR_CEATA_CNTRL_0,    10Ch */
+	unsigned int	venbootctl;	/* _VENDOR_BOOT_CNTRL_0,     110h */
+	unsigned int	venbootacktout;	/* _VENDOR_BOOT_ACK_TIMEOUT, 114h */
+	unsigned int	venbootdattout;	/* _VENDOR_BOOT_DAT_TIMEOUT, 118h */
+	unsigned int	vendebouncecnt;	/* _VENDOR_DEBOUNCE_COUNT_0, 11Ch */
+	unsigned int	venmiscctl;	/* _VENDOR_MISC_CNTRL_0,     120h */
+	unsigned int	res6[47];	/* 0x124 ~ 0x1DC */
+	unsigned int	sdmemcmppadctl;	/* _SDMEMCOMPPADCTRL_0,      1E0h */
+	unsigned int	autocalcfg;	/* _AUTO_CAL_CONFIG_0,       1E4h */
+	unsigned int	autocalintval;	/* _AUTO_CAL_INTERVAL_0,     1E8h */
+	unsigned int	autocalsts;	/* _AUTO_CAL_STATUS_0,       1ECh */
 };
 
+#define TEGRA_MMC_PWRCTL_SD_BUS_POWER				(1 << 0)
+#define TEGRA_MMC_PWRCTL_SD_BUS_VOLTAGE_V1_8			(5 << 1)
+#define TEGRA_MMC_PWRCTL_SD_BUS_VOLTAGE_V3_0			(6 << 1)
+#define TEGRA_MMC_PWRCTL_SD_BUS_VOLTAGE_V3_3			(7 << 1)
+
 #define TEGRA_MMC_HOSTCTL_DMASEL_MASK				(3 << 3)
 #define TEGRA_MMC_HOSTCTL_DMASEL_SDMA				(0 << 3)
 #define TEGRA_MMC_HOSTCTL_DMASEL_ADMA2_32BIT			(2 << 3)
@@ -119,6 +134,14 @@ struct tegra_mmc {
 
 #define TEGRA_MMC_NORINTSIGEN_XFER_COMPLETE			(1 << 1)
 
+/* SDMMC1/3 settings from section 24.6 of T30 TRM */
+#define MEMCOMP_PADCTRL_VREF	7
+#define AUTO_CAL_ENABLED	(1 << 29)
+#define AUTO_CAL_PD_OFFSET	(0x70 << 8)
+#define AUTO_CAL_PU_OFFSET	(0x62 << 0)
+
+void pad_init_mmc(struct tegra_mmc *reg);
+
 struct mmc_host {
 	struct tegra_mmc *reg;
 	int id;			/* device id/number, 0-3 */
diff --git a/arch/arm/include/asm/arch-tegra20/tegra.h b/arch/arm/include/asm/arch-tegra20/tegra.h
index ad5c01d..ba0084e 100644
--- a/arch/arm/include/asm/arch-tegra20/tegra.h
+++ b/arch/arm/include/asm/arch-tegra20/tegra.h
@@ -30,6 +30,11 @@
 
 #define TEGRA_USB1_BASE		0xC5000000
 
+#define TEGRA_SDMMC1_BASE	0xC8000000
+#define TEGRA_SDMMC2_BASE	0xC8000200
+#define TEGRA_SDMMC3_BASE	0xC8000400
+#define TEGRA_SDMMC4_BASE	0xC8000600
+
 #define BCT_ODMDATA_OFFSET	4068	/* 12 bytes from end of BCT */
 
 #define MAX_NUM_CPU		2
diff --git a/arch/arm/include/asm/arch-tegra30/gp_padctrl.h b/arch/arm/include/asm/arch-tegra30/gp_padctrl.h
index 9b383d0..48b9a3b 100644
--- a/arch/arm/include/asm/arch-tegra30/gp_padctrl.h
+++ b/arch/arm/include/asm/arch-tegra30/gp_padctrl.h
@@ -56,4 +56,10 @@ struct apb_misc_gp_ctlr {
 	u32	sdio1cfg;	/* 0xEC: APB_MISC_GP_SDIO1CFGPADCTRL */
 };
 
+/* SDMMC1/3 settings from section 24.6 of T30 TRM */
+#define GP_SDIOCFG_DRVUP_SLWF	(1 << 30)
+#define GP_SDIOCFG_DRVDN_SLWR	(1 << 28)
+#define GP_SDIOCFG_DRVUP	(0x2E << 20)
+#define GP_SDIOCFG_DRVDN	(0x2A << 12)
+
 #endif	/* _TEGRA30_GP_PADCTRL_H_ */
diff --git a/arch/arm/include/asm/arch-tegra30/tegra.h b/arch/arm/include/asm/arch-tegra30/tegra.h
index c02c5d8..b33ea79 100644
--- a/arch/arm/include/asm/arch-tegra30/tegra.h
+++ b/arch/arm/include/asm/arch-tegra30/tegra.h
@@ -23,6 +23,11 @@
 
 #define TEGRA_USB1_BASE		0x7D000000
 
+#define TEGRA_SDMMC1_BASE	0x78000000
+#define TEGRA_SDMMC2_BASE	0x78000200
+#define TEGRA_SDMMC3_BASE	0x78000400
+#define TEGRA_SDMMC4_BASE	0x78000600
+
 #define BCT_ODMDATA_OFFSET	6116	/* 12 bytes from end of BCT */
 
 #define MAX_NUM_CPU		4
-- 
1.7.0.4

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Tom Warren]

T30 requires specific SDMMC pad programming, and bus power-rail bringup.

Signed-off-by: Tom Warren <twarren@nvidia.com>
---
 board/nvidia/cardhu/cardhu.c |   49 +++++++++++++++++++++++++++++++++++++++++
 board/nvidia/common/board.c  |   50 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 98 insertions(+), 1 deletions(-)

diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c
index df4cb6b..363bddc 100644
--- a/board/nvidia/cardhu/cardhu.c
+++ b/board/nvidia/cardhu/cardhu.c
@@ -24,6 +24,10 @@
 #include <common.h>
 #include <asm/arch/pinmux.h>
 #include "pinmux-config-cardhu.h"
+#include <i2c.h>
+
+#define PMU_I2C_ADDRESS		0x2D
+#define MAX_I2C_RETRY		3
 
 /*
  * Routine: pinmux_init
@@ -37,3 +41,48 @@ void pinmux_init(void)
 	pinmux_config_table(unused_pins_lowpower,
 		ARRAY_SIZE(unused_pins_lowpower));
 }
+
+#if defined(CONFIG_TEGRA_MMC)
+/*
+ * Do I2C/PMU writes to bring up SD card bus power
+ *
+ */
+void board_sdmmc_voltage_init(void)
+{
+	uchar reg, data_buffer[1];
+	int i;
+
+	i2c_set_bus_num(0);	/* PMU is on bus 0 */
+
+	data_buffer[0] = 0x65;
+	reg = 0x32;
+
+	for (i = 0; i < MAX_I2C_RETRY; ++i) {
+		if (i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1))
+			udelay(100);
+	}
+
+	data_buffer[0] = 0x09;
+	reg = 0x67;
+
+	for (i = 0; i < MAX_I2C_RETRY; ++i) {
+		if (i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1))
+			udelay(100);
+	}
+}
+
+/*
+ * Routine: pin_mux_mmc
+ * Description: setup the MMC muxes, power rails, etc.
+ */
+void pin_mux_mmc(void)
+{
+	/*
+	 * NOTE: We don't do mmc-specific pin muxes here.
+	 * They were done globally in pinmux_init().
+	 */
+
+	/* Bring up the SDIO1 power rail */
+	board_sdmmc_voltage_init();
+}
+#endif	/* MMC */
diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index babbe08..4ca6b29 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -49,6 +49,8 @@
 #include <asm/arch-tegra/usb.h>
 #endif
 #ifdef CONFIG_TEGRA_MMC
+#include <asm/arch/gp_padctrl.h>
+#include <asm/arch-tegra/tegra_mmc.h>
 #include <asm/arch-tegra/mmc.h>
 #endif
 #include <i2c.h>
@@ -245,4 +247,50 @@ int board_mmc_init(bd_t *bd)
 
 	return 0;
 }
-#endif
+
+void pad_init_mmc(struct tegra_mmc *reg)
+{
+#if defined(CONFIG_TEGRA30)
+	struct apb_misc_gp_ctlr *const gpc =
+		(struct apb_misc_gp_ctlr *)NV_PA_APB_MISC_GP_BASE;
+	struct tegra_mmc *const sdmmc = (struct tegra_mmc *)reg;
+	u32 val, padcfg, padmask, offset = (unsigned int)reg;
+
+	debug("%s: sdmmc address = %08x\n", __func__, (unsigned int)sdmmc);
+
+	/* Set the pad drive strength for SDMMC1 or 3 only */
+	if (offset != TEGRA_SDMMC1_BASE && offset != TEGRA_SDMMC3_BASE) {
+		debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
+			__func__);
+		return;
+	}
+
+	/* Set pads as per T30 TRM, section 24.6.1.2 */
+	padcfg = (GP_SDIOCFG_DRVUP_SLWF | GP_SDIOCFG_DRVDN_SLWR | \
+		GP_SDIOCFG_DRVUP | GP_SDIOCFG_DRVDN);
+	padmask = 0x00000FFF;
+
+	if (offset == TEGRA_SDMMC1_BASE) {
+		val = readl(&gpc->sdio1cfg);
+		val &= padmask;
+		val |= padcfg;
+		writel(val, &gpc->sdio1cfg);
+	} else {/* SDMMC3 */
+		val = readl(&gpc->sdio3cfg);
+		val &= padmask;
+		val |= padcfg;
+		writel(val, &gpc->sdio3cfg);
+	}
+
+	val = readl(&sdmmc->sdmemcmppadctl);
+	val &= 0xFFFFFFF0;
+	val |= MEMCOMP_PADCTRL_VREF;
+	writel(val, &sdmmc->sdmemcmppadctl);
+
+	val = readl(&sdmmc->autocalcfg);
+	val &= 0xFFFF0000;
+	val |= AUTO_CAL_PU_OFFSET | AUTO_CAL_PD_OFFSET | AUTO_CAL_ENABLED;
+	writel(val, &sdmmc->autocalcfg);
+#endif	/* T30 */
+}
+#endif	/* MMC */
-- 
1.7.0.4

[U-Boot] [PATCH 4/5] mmc: Tegra: Add SD bus power/voltage function and MMC pad init call.

[Tom Warren]

Tegra30 requires the SD Bus Voltage & Power bits be set in the SD
Power Control register. Tegra20 works w/o them set, but do it anyway
for those SoCs as it's part of the SD spec. Also call a common
board pad init routine (pad_init_mmc) in mmc_reset(), used by
Tegra30 only for now.

Signed-off-by: Tom Warren <twarren@nvidia.com>
---
 drivers/mmc/tegra_mmc.c |   48 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
index 6063d08..9e44771 100644
--- a/drivers/mmc/tegra_mmc.c
+++ b/drivers/mmc/tegra_mmc.c
@@ -21,7 +21,6 @@
 
 #include <bouncebuf.h>
 #include <common.h>
-#include <fdtdec.h>
 #include <asm/gpio.h>
 #include <asm/io.h>
 #include <asm/arch/clock.h>
@@ -38,6 +37,38 @@ struct mmc_host mmc_host[MAX_HOSTS];
 #error "Please enable device tree support to use this driver"
 #endif
 
+static void mmc_set_power(struct mmc_host *host, unsigned short power)
+{
+	u8 pwr = 0;
+	debug("%s: power = %x\n", __func__, power);
+
+	if (power != (unsigned short)-1) {
+		switch (1 << power) {
+		case MMC_VDD_165_195:
+			pwr = TEGRA_MMC_PWRCTL_SD_BUS_VOLTAGE_V1_8;
+			break;
+		case MMC_VDD_29_30:
+		case MMC_VDD_30_31:
+			pwr = TEGRA_MMC_PWRCTL_SD_BUS_VOLTAGE_V3_0;
+			break;
+		case MMC_VDD_32_33:
+		case MMC_VDD_33_34:
+			pwr = TEGRA_MMC_PWRCTL_SD_BUS_VOLTAGE_V3_3;
+			break;
+		}
+	}
+	debug("%s: pwr = %X\n", __func__, pwr);
+
+	/* Set the bus voltage first (if any) */
+	writeb(pwr, &host->reg->pwrcon);
+	if (pwr == 0)
+		return;
+
+	/* Now enable bus power */
+	pwr |= TEGRA_MMC_PWRCTL_SD_BUS_POWER;
+	writeb(pwr, &host->reg->pwrcon);
+}
+
 static void mmc_prepare_data(struct mmc_host *host, struct mmc_data *data,
 				struct bounce_buffer *bbstate)
 {
@@ -334,8 +365,7 @@ static void mmc_change_clock(struct mmc_host *host, uint clock)
 	debug(" mmc_change_clock called\n");
 
 	/*
-	 * Change Tegra SDMMCx clock divisor here. Source is 216MHz,
-	 * PLLP_OUT0
+	 * Change Tegra SDMMCx clock divisor here. Source is PLLP_OUT0
 	 */
 	if (clock == 0)
 		goto out;
@@ -410,7 +440,7 @@ static void mmc_set_ios(struct mmc *mmc)
 	debug("mmc_set_ios: hostctl = %08X\n", ctrl);
 }
 
-static void mmc_reset(struct mmc_host *host)
+static void mmc_reset(struct mmc_host *host, struct mmc *mmc)
 {
 	unsigned int timeout;
 	debug(" mmc_reset called\n");
@@ -436,6 +466,14 @@ static void mmc_reset(struct mmc_host *host)
 		timeout--;
 		udelay(1000);
 	}
+
+	/* Set SD bus voltage & enable bus power */
+	mmc_set_power(host, fls(mmc->voltages) - 1);
+	debug("%s: power control = %02X, host control = %02X\n", __func__,
+		readb(&host->reg->pwrcon), readb(&host->reg->hostctl));
+
+	/* Make sure SDIO pads are set up */
+	pad_init_mmc(host->reg);
 }
 
 static int mmc_core_init(struct mmc *mmc)
@@ -444,7 +482,7 @@ static int mmc_core_init(struct mmc *mmc)
 	unsigned int mask;
 	debug(" mmc_core_init called\n");
 
-	mmc_reset(host);
+	mmc_reset(host, mmc);
 
 	host->version = readw(&host->reg->hcver);
 	debug("host version = %x\n", host->version);
-- 
1.7.0.4

[U-Boot] [PATCH 5/5] Tegra30: MMC: Enable DT MMC driver support for Tegra30 Cardhu boards

[Tom Warren]

Tested on my Cardhu-A04 tablet, eMMC and SD-Card work fine, can load
a kernel off of an SD card OK, card detect works, and the env is now
stored in eMMC (end of the 2nd 'boot' sector, same as Tegra20).

Signed-off-by: Tom Warren <twarren@nvidia.com>
---
 include/configs/cardhu.h         |   20 +++++++++++++++++++-
 include/configs/tegra30-common.h |    3 +++
 2 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/include/configs/cardhu.h b/include/configs/cardhu.h
index 1616b39..18c7eb8 100644
--- a/include/configs/cardhu.h
+++ b/include/configs/cardhu.h
@@ -47,7 +47,25 @@
 #define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
 
-#define CONFIG_ENV_IS_NOWHERE
+/* SD/MMC */
+#define CONFIG_MMC
+#define CONFIG_GENERIC_MMC
+#define CONFIG_TEGRA_MMC
+#define CONFIG_CMD_MMC
+
+#define CONFIG_DOS_PARTITION
+#define CONFIG_EFI_PARTITION
+#define CONFIG_FS_EXT4
+#define CONFIG_FS_FAT
+#define CONFIG_CMD_EXT2
+#define CONFIG_CMD_FAT
+#define CONFIG_CMD_FS_GENERIC
+
+/* Environment in eMMC, at the end of 2nd "boot sector" */
+#define CONFIG_ENV_IS_IN_MMC
+#define CONFIG_ENV_OFFSET		((512 * 1024) - CONFIG_ENV_SIZE)
+#define CONFIG_SYS_MMC_ENV_DEV		0
+#define CONFIG_SYS_MMC_ENV_PART		2
 
 /* SPI */
 #define CONFIG_TEGRA_SLINK
diff --git a/include/configs/tegra30-common.h b/include/configs/tegra30-common.h
index 04517e1..bd1dfe8 100644
--- a/include/configs/tegra30-common.h
+++ b/include/configs/tegra30-common.h
@@ -86,4 +86,7 @@
 /* Total I2C ports on Tegra30 */
 #define TEGRA_I2C_NUM_CONTROLLERS	5
 
+/* Misc utility code */
+#define CONFIG_BOUNCE_BUFFER
+
 #endif /* _TEGRA30_COMMON_H_ */
-- 
1.7.0.4

[U-Boot] [PATCH 0/5] Tegra30: MMC: Add DT-based MMC driver for Tegra30/Cardhu

[Stephen Warren]

On 02/26/2013 01:46 PM, Tom Warren wrote:
> This patchset adds SDMMC device-tree support to the Tegra30 dts files,
> and enables the Tegra MMC driver on Tegra30 Cardhu.
> 
> I've tested this on my Cardhu-A04 and everything works fine,
> including card detect. All Tegra boards also build w/o error, and
> Seaboard MMC functionality is unchanged.

The series,
Tested-by: Stephen Warren <swarren@nvidia.com>
(also on a Cardhu A04)

Now I can re-write our internal wiki to make use of commands like
"bootz", "part" and "fsload" on all our boards :-) :-) :-)

[U-Boot] [PATCH 1/5] Tegra30: fdt: Add SDMMC (sdhci) nodes for T30 boards (Cardhu for now)

[Stephen Warren]

On 02/26/2013 01:46 PM, Tom Warren wrote:
> Took these values directly from the kernel dts files.

> diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi

> +	sdhci at 78000000 {
> +		compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";

Looking at this more, I /think/ this should only include the Tegra30
compatible value, since there are new quirks that are required to be
enabled on Tegra30 relative to Tegra20 or the HW won't work. The kernel
DT file is no doubt buggy here.

Cc'ing Rhyland and Pavan to confirm this. (Note: this is
Tegra30-vs-Tegra20, not Tegra114-vs-Tegra30 that we just discussed
downstream).

[U-Boot] [PATCH 2/5] Tegra: MMC: Added/update SDMMC registers/base addresses for T20/T30

[Stephen Warren]

On 02/26/2013 01:46 PM, Tom Warren wrote:
> Moved SDMMC base addresses into each SoC's main header, since they differ.
> Added pad control settings for T30 from the TRM, and added additional
> vendor-specific SD/MMC registers and bus power defines.

> diff --git a/arch/arm/include/asm/arch-tegra/tegra_mmc.h b/arch/arm/include/asm/arch-tegra/tegra_mmc.h

> -#define TEGRA_SDMMC1_BASE	0xC8000000
> -#define TEGRA_SDMMC2_BASE	0xC8000200
> -#define TEGRA_SDMMC3_BASE	0xC8000400
> -#define TEGRA_SDMMC4_BASE	0xC8000600

This is odd; are these values even used at all now that the registers
are described in device tree? Kinda the whole point of DT is that we
don't need to SoC-specific register addresses or board-specific data in
the code any more.

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Stephen Warren]

On 02/26/2013 01:46 PM, Tom Warren wrote:
> T30 requires specific SDMMC pad programming, and bus power-rail bringup.

> diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c

> +/*
> + * Do I2C/PMU writes to bring up SD card bus power
> + *
> + */
> +void board_sdmmc_voltage_init(void)

We really shouldn't be adding to board files if we're remotely serious
about device tree; the whole point of DT is to remove code from the
board files, and describe the desired configuration as data in DT instead.

This function should be replaced by regulator nodes/properties in the
device tree, and the MMC (core?) driver calling into the board-specific
regulator driver to request the desired voltages.

But so long as we file a bug to replace this code with an explicit
regulator driver in the future, I guess it's fine for now.

> +{
> +	uchar reg, data_buffer[1];
> +	int i;
> +
> +	i2c_set_bus_num(0);	/* PMU is on bus 0 */
> +
> +	data_buffer[0] = 0x65;
> +	reg = 0x32;

We should at least comment what those register numbers and values mean.

BTW, I just noticed that commit f01b631 "Tegra30: Add/enable Cardhu
build (T30 reference board)" adds a file called
board/nvidia/cardhu/cardhu.c.mmc. That's a mistake, right?

> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c

Hmm. This seems like SoC code, not board code...

> +void pad_init_mmc(struct tegra_mmc *reg)
> +{
> +#if defined(CONFIG_TEGRA30)

> +	/* Set the pad drive strength for SDMMC1 or 3 only */
> +	if (offset != TEGRA_SDMMC1_BASE && offset != TEGRA_SDMMC3_BASE) {
> +		debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
> +			__func__);
> +		return;
> +	}

Perhaps pass in the MMC instance ID instead of the base address. That'd
avoid having to know the base addresses in this code.

In fact, just putting this code into the pinmux driver (which owns these
registers) seems like a better idea; there's no need to only do this
when the SD controller is enabled, is there?

[U-Boot] [PATCH 1/5] Tegra30: fdt: Add SDMMC (sdhci) nodes for T30 boards (Cardhu for now)

[Tom Warren]

Stephen/Rhyland,

On Tue, Feb 26, 2013 at 4:10 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 02/26/2013 01:46 PM, Tom Warren wrote:
>> Took these values directly from the kernel dts files.
>
>> diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
>
>> +     sdhci at 78000000 {
>> +             compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
>
> Looking at this more, I /think/ this should only include the Tegra30
> compatible value, since there are new quirks that are required to be
> enabled on Tegra30 relative to Tegra20 or the HW won't work. The kernel
> DT file is no doubt buggy here.
Looking at the SDMMC reg space in the T20 and T30 TRMs, I don't see
anything major that would make the MMC driver not work on T30 as is
(in fact, I know it works just fine w/o modification). Looking at the
sdhci-tegra.c driver source, the only quirk difference is
DATA_TIMEOUT_USES_SDCLK. The U-Boot Tegra MMC driver doesn't reference
the caps Timeout Clock Frequency bits, so this quirk/difference
doesn't matter.

>
> Cc'ing Rhyland and Pavan to confirm this. (Note: this is
> Tegra30-vs-Tegra20, not Tegra114-vs-Tegra30 that we just discussed
> downstream).
Let's see what Rhyland/Pavan have to say before I change this.

Tom

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Tom Warren]

Stephen,

On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 02/26/2013 01:46 PM, Tom Warren wrote:
>> T30 requires specific SDMMC pad programming, and bus power-rail bringup.
>
>> diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c
>
>> +/*
>> + * Do I2C/PMU writes to bring up SD card bus power
>> + *
>> + */
>> +void board_sdmmc_voltage_init(void)
>
> We really shouldn't be adding to board files if we're remotely serious
> about device tree; the whole point of DT is to remove code from the
> board files, and describe the desired configuration as data in DT instead.
>
> This function should be replaced by regulator nodes/properties in the
> device tree, and the MMC (core?) driver calling into the board-specific
> regulator driver to request the desired voltages.
>
> But so long as we file a bug to replace this code with an explicit
> regulator driver in the future, I guess it's fine for now.
I'll file a bug for doing all PMU/power rail work from DT. I think it
makes much more sense as a separate (non-MMC) patch.

>
>> +{
>> +     uchar reg, data_buffer[1];
>> +     int i;
>> +
>> +     i2c_set_bus_num(0);     /* PMU is on bus 0 */
>> +
>> +     data_buffer[0] = 0x65;
>> +     reg = 0x32;
>
> We should at least comment what those register numbers and values mean.
Unfortunately, that's not documented in any downstream/NV-generated
code that I've seen. I'll have to find the PMU spec and decode what
these writes are doing.

>
> BTW, I just noticed that commit f01b631 "Tegra30: Add/enable Cardhu
> build (T30 reference board)" adds a file called
> board/nvidia/cardhu/cardhu.c.mmc. That's a mistake, right?
Yep, that's the Cardhu file I was hacking on for MMC support way back
when. I can remove it in V2 of these patches, or would you prefer a
single, separate patch to do that?

>
>> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
>
> Hmm. This seems like SoC code, not board code...
>
>> +void pad_init_mmc(struct tegra_mmc *reg)
>> +{
>> +#if defined(CONFIG_TEGRA30)
>
>> +     /* Set the pad drive strength for SDMMC1 or 3 only */
>> +     if (offset != TEGRA_SDMMC1_BASE && offset != TEGRA_SDMMC3_BASE) {
>> +             debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
>> +                     __func__);
>> +             return;
>> +     }
>
> Perhaps pass in the MMC instance ID instead of the base address. That'd
> avoid having to know the base addresses in this code.
I still need to know if I've got a SDIO or eMMC ID, though, and I
don't think the flags for that in the mmc/host structs (mmc->version,
etc.) get populated until the mmc driver has done some I/O with the
device (eMMC or SD-card), and I need to set up the pads before that.

>
> In fact, just putting this code into the pinmux driver (which owns these
> registers) seems like a better idea; there's no need to only do this
> when the SD controller is enabled, is there?
Half of these regs are in the SD controller register space
(sdmemcmpctl and autocalcfg), and get reset when the controller gets
reset (mmc_reset). So they need to be set each time a reset occurs. It
makes sense to keep the GP SDIOCFG writes here, too.

As to where the pad_init_mmc function belongs, it is SoC-specific,
yes. T20 doesn't need it (no sdmemcmpctl or autocalcfg regs on T20
SDMMC), and T30 and T114 use slightly different bits/values for GP
SDIOCFG and sdmemcmpctl/autocalcfg. But the differences are small
enough that I thought this routine should be placed in a common area,
and not duplicated for each SoC, so I put it here.

Do you have a suggestion of where it would be better placed?

Tom

[U-Boot] [PATCH 1/5] Tegra30: fdt: Add SDMMC (sdhci) nodes for T30 boards (Cardhu for now)

[Stephen Warren]

On 02/27/2013 09:20 AM, Tom Warren wrote:
> Stephen/Rhyland,
> 
> On Tue, Feb 26, 2013 at 4:10 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/26/2013 01:46 PM, Tom Warren wrote:
>>> Took these values directly from the kernel dts files.
>>
>>> diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
>>
>>> +     sdhci at 78000000 {
>>> +             compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
>>
>> Looking at this more, I /think/ this should only include the Tegra30
>> compatible value, since there are new quirks that are required to be
>> enabled on Tegra30 relative to Tegra20 or the HW won't work. The kernel
>> DT file is no doubt buggy here.
>
> Looking at the SDMMC reg space in the T20 and T30 TRMs, I don't see
> anything major that would make the MMC driver not work on T30 as is
> (in fact, I know it works just fine w/o modification). Looking at the
> sdhci-tegra.c driver source, the only quirk difference is
> DATA_TIMEOUT_USES_SDCLK. The U-Boot Tegra MMC driver doesn't reference
> the caps Timeout Clock Frequency bits, so this quirk/difference
> doesn't matter.

The compatible value does not document whether the U-Boot driver will
operate on the HW without changes, but rather whether there are
incompatible HW changes.

Now, those changes might only affect some theoretical driver that
doesn't actually exist. However, that is not relevant; compatible is
purely about describing HW compatibility or not.

I'll note that both the current upstream U-Boot MMC driver and likely
the current upstream Linux kernel MMC driver probably don't take
advantage of many of the performance or power-saving features present in
the HW, so it's likely pretty easy for their to be a HW difference that
could affect a driver, but not actually yet affect either of these two
particular drivers.

In particular, the difference in DATA_TIMEOUT_USES_SDCLK would probably
be enough on its own to merit not marking Tegra30 as
backwards-compatible with Tegra20, since it's a bug WAR or issue that a
Tegra30 driver apparently would need to be aware of but not a Tegra20
driver (if that feature is used by the driver).

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Stephen Warren]

On 02/27/2013 09:59 AM, Tom Warren wrote:
> Stephen,
> 
> On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/26/2013 01:46 PM, Tom Warren wrote:
>>> T30 requires specific SDMMC pad programming, and bus power-rail bringup.
>>
>>> diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c
>>
>>> +/*
>>> + * Do I2C/PMU writes to bring up SD card bus power
>>> + *
>>> + */
>>> +void board_sdmmc_voltage_init(void)
>>
>> We really shouldn't be adding to board files if we're remotely serious
>> about device tree; the whole point of DT is to remove code from the
>> board files, and describe the desired configuration as data in DT instead.
>>
>> This function should be replaced by regulator nodes/properties in the
>> device tree, and the MMC (core?) driver calling into the board-specific
>> regulator driver to request the desired voltages.
>>
>> But so long as we file a bug to replace this code with an explicit
>> regulator driver in the future, I guess it's fine for now.
>
> I'll file a bug for doing all PMU/power rail work from DT. I think it
> makes much more sense as a separate (non-MMC) patch.

Yes, certainly a separate patch. Ideally it'd be implemented before
other code that relies on it. That's why I think we need to take a
higher-level look at DT support in U-Boot, rather than simply finding
these issue accidentally while implementing the features we already know
we need.

>> BTW, I just noticed that commit f01b631 "Tegra30: Add/enable Cardhu
>> build (T30 reference board)" adds a file called
>> board/nvidia/cardhu/cardhu.c.mmc. That's a mistake, right?
>
> Yep, that's the Cardhu file I was hacking on for MMC support way back
> when. I can remove it in V2 of these patches, or would you prefer a
> single, separate patch to do that?

Is the commit that adds that file already pulled into higher-level
repos? If not, I would simply rebase it to remove that file. If it has,
then a separate patch to delete it before this series would make sense.

>>> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
>>
>> Hmm. This seems like SoC code, not board code...
>>
>>> +void pad_init_mmc(struct tegra_mmc *reg)
>>> +{
>>> +#if defined(CONFIG_TEGRA30)
>>
>>> +     /* Set the pad drive strength for SDMMC1 or 3 only */
>>> +     if (offset != TEGRA_SDMMC1_BASE && offset != TEGRA_SDMMC3_BASE) {
>>> +             debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
>>> +                     __func__);
>>> +             return;
>>> +     }
>>
>> Perhaps pass in the MMC instance ID instead of the base address. That'd
>> avoid having to know the base addresses in this code.
>
> I still need to know if I've got a SDIO or eMMC ID, though, and I
> don't think the flags for that in the mmc/host structs (mmc->version,
> etc.) get populated until the mmc driver has done some I/O with the
> device (eMMC or SD-card), and I need to set up the pads before that.
> 
>> In fact, just putting this code into the pinmux driver (which owns these
>> registers) seems like a better idea; there's no need to only do this
>> when the SD controller is enabled, is there?
>
> Half of these regs are in the SD controller register space
> (sdmemcmpctl and autocalcfg), and get reset when the controller gets
> reset (mmc_reset). So they need to be set each time a reset occurs. It
> makes sense to keep the GP SDIOCFG writes here, too.
> 
> As to where the pad_init_mmc function belongs, it is SoC-specific,
> yes. T20 doesn't need it (no sdmemcmpctl or autocalcfg regs on T20
> SDMMC), and T30 and T114 use slightly different bits/values for GP
> SDIOCFG and sdmemcmpctl/autocalcfg. But the differences are small
> enough that I thought this routine should be placed in a common area,
> and not duplicated for each SoC, so I put it here.
> 
> Do you have a suggestion of where it would be better placed?

For the pinmux registers, I think they should be programmed by the
pinmux driver at the same time as the rest of the pinmux is programmed.

For the SD registers, I guess we need to program them from the SD driver
as you say, but need to add some more properties to the DT in order to
parameterize this.

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Tom Warren]

Stephen,

On Wed, Feb 27, 2013 at 11:08 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 02/27/2013 09:59 AM, Tom Warren wrote:
>> Stephen,
>>
>> On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 02/26/2013 01:46 PM, Tom Warren wrote:
>>>> T30 requires specific SDMMC pad programming, and bus power-rail bringup.
>>>
>>>> diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c
>>>
>>>> +/*
>>>> + * Do I2C/PMU writes to bring up SD card bus power
>>>> + *
>>>> + */
>>>> +void board_sdmmc_voltage_init(void)
>>>
>>> We really shouldn't be adding to board files if we're remotely serious
>>> about device tree; the whole point of DT is to remove code from the
>>> board files, and describe the desired configuration as data in DT instead.
>>>
>>> This function should be replaced by regulator nodes/properties in the
>>> device tree, and the MMC (core?) driver calling into the board-specific
>>> regulator driver to request the desired voltages.
>>>
>>> But so long as we file a bug to replace this code with an explicit
>>> regulator driver in the future, I guess it's fine for now.
>>
>> I'll file a bug for doing all PMU/power rail work from DT. I think it
>> makes much more sense as a separate (non-MMC) patch.
>
> Yes, certainly a separate patch. Ideally it'd be implemented before
> other code that relies on it. That's why I think we need to take a
> higher-level look at DT support in U-Boot, rather than simply finding
> these issue accidentally while implementing the features we already know
> we need.
>
>>> BTW, I just noticed that commit f01b631 "Tegra30: Add/enable Cardhu
>>> build (T30 reference board)" adds a file called
>>> board/nvidia/cardhu/cardhu.c.mmc. That's a mistake, right?
>>
>> Yep, that's the Cardhu file I was hacking on for MMC support way back
>> when. I can remove it in V2 of these patches, or would you prefer a
>> single, separate patch to do that?
>
> Is the commit that adds that file already pulled into higher-level
> repos? If not, I would simply rebase it to remove that file. If it has,
> then a separate patch to delete it before this series would make sense.
>
>>>> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
>>>
>>> Hmm. This seems like SoC code, not board code...
>>>
>>>> +void pad_init_mmc(struct tegra_mmc *reg)
>>>> +{
>>>> +#if defined(CONFIG_TEGRA30)
>>>
>>>> +     /* Set the pad drive strength for SDMMC1 or 3 only */
>>>> +     if (offset != TEGRA_SDMMC1_BASE && offset != TEGRA_SDMMC3_BASE) {
>>>> +             debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
>>>> +                     __func__);
>>>> +             return;
>>>> +     }
>>>
>>> Perhaps pass in the MMC instance ID instead of the base address. That'd
>>> avoid having to know the base addresses in this code.
>>
>> I still need to know if I've got a SDIO or eMMC ID, though, and I
>> don't think the flags for that in the mmc/host structs (mmc->version,
>> etc.) get populated until the mmc driver has done some I/O with the
>> device (eMMC or SD-card), and I need to set up the pads before that.
>>
>>> In fact, just putting this code into the pinmux driver (which owns these
>>> registers) seems like a better idea; there's no need to only do this
>>> when the SD controller is enabled, is there?
>>
>> Half of these regs are in the SD controller register space
>> (sdmemcmpctl and autocalcfg), and get reset when the controller gets
>> reset (mmc_reset). So they need to be set each time a reset occurs. It
>> makes sense to keep the GP SDIOCFG writes here, too.
>>
>> As to where the pad_init_mmc function belongs, it is SoC-specific,
>> yes. T20 doesn't need it (no sdmemcmpctl or autocalcfg regs on T20
>> SDMMC), and T30 and T114 use slightly different bits/values for GP
>> SDIOCFG and sdmemcmpctl/autocalcfg. But the differences are small
>> enough that I thought this routine should be placed in a common area,
>> and not duplicated for each SoC, so I put it here.
>>
>> Do you have a suggestion of where it would be better placed?
>
> For the pinmux registers, I think they should be programmed by the
> pinmux driver at the same time as the rest of the pinmux is programmed.
Technically, they're not pinmux registers (PINMUX_AUX_ space), but GP
regs (APB_MISC_GP_ space). Since the pinmux _code_ (no pinmux driver
is used in Tegra U-Boot) for T30 is just a large table slam, I don't
think it makes sense to add GP pad reg writes there. These pads need
to be tuned when you've got a board w/an SD-card device hanging off of
them. So it makes sense to have these 2 register writes here in
pad_init_mmc(). I can take out the SDMMC1/3 test and just write both
SDIO1CFG and SDIO3CFG, since the values are the same.
>
> For the SD registers, I guess we need to program them from the SD driver
> as you say, but need to add some more properties to the DT in order to
> parameterize this.
Why don't we get this in so we can move forward, and when there's a
kernel version of these params in the DT files, I can port it over.

I've got the compatible string changed to just 'nvidia,tegra30-sdhci'
in the dtsi file, and I've changed the pad_init_mmc() code to use the
mmc_id (PERIPH_ID_SDMMCx) instead of the base addresses to check for
an SDIO port before setting the pad config regs. I'll send that up as
V2 of the patchset, probably tomorrow.

Tom

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Stephen Warren]

On 03/04/2013 04:11 PM, Tom Warren wrote:
> Stephen,
> 
> On Wed, Feb 27, 2013 at 11:08 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/27/2013 09:59 AM, Tom Warren wrote:
>>> Stephen,
>>>
>>> On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 02/26/2013 01:46 PM, Tom Warren wrote:
>>>>> T30 requires specific SDMMC pad programming, and bus power-rail bringup.
>>>>
>>>>> diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c
>>>>
>>>>> +/*
>>>>> + * Do I2C/PMU writes to bring up SD card bus power
>>>>> + *
>>>>> + */
>>>>> +void board_sdmmc_voltage_init(void)
>>>>
>>>> We really shouldn't be adding to board files if we're remotely serious
>>>> about device tree; the whole point of DT is to remove code from the
>>>> board files, and describe the desired configuration as data in DT instead.
>>>>
>>>> This function should be replaced by regulator nodes/properties in the
>>>> device tree, and the MMC (core?) driver calling into the board-specific
>>>> regulator driver to request the desired voltages.
>>>>
>>>> But so long as we file a bug to replace this code with an explicit
>>>> regulator driver in the future, I guess it's fine for now.
>>>
>>> I'll file a bug for doing all PMU/power rail work from DT. I think it
>>> makes much more sense as a separate (non-MMC) patch.
>>
>> Yes, certainly a separate patch. Ideally it'd be implemented before
>> other code that relies on it. That's why I think we need to take a
>> higher-level look at DT support in U-Boot, rather than simply finding
>> these issue accidentally while implementing the features we already know
>> we need.
>>
>>>> BTW, I just noticed that commit f01b631 "Tegra30: Add/enable Cardhu
>>>> build (T30 reference board)" adds a file called
>>>> board/nvidia/cardhu/cardhu.c.mmc. That's a mistake, right?
>>>
>>> Yep, that's the Cardhu file I was hacking on for MMC support way back
>>> when. I can remove it in V2 of these patches, or would you prefer a
>>> single, separate patch to do that?
>>
>> Is the commit that adds that file already pulled into higher-level
>> repos? If not, I would simply rebase it to remove that file. If it has,
>> then a separate patch to delete it before this series would make sense.
>>
>>>>> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
>>>>
>>>> Hmm. This seems like SoC code, not board code...
>>>>
>>>>> +void pad_init_mmc(struct tegra_mmc *reg)
>>>>> +{
>>>>> +#if defined(CONFIG_TEGRA30)
>>>>
>>>>> +     /* Set the pad drive strength for SDMMC1 or 3 only */
>>>>> +     if (offset != TEGRA_SDMMC1_BASE && offset != TEGRA_SDMMC3_BASE) {
>>>>> +             debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
>>>>> +                     __func__);
>>>>> +             return;
>>>>> +     }
>>>>
>>>> Perhaps pass in the MMC instance ID instead of the base address. That'd
>>>> avoid having to know the base addresses in this code.
>>>
>>> I still need to know if I've got a SDIO or eMMC ID, though, and I
>>> don't think the flags for that in the mmc/host structs (mmc->version,
>>> etc.) get populated until the mmc driver has done some I/O with the
>>> device (eMMC or SD-card), and I need to set up the pads before that.
>>>
>>>> In fact, just putting this code into the pinmux driver (which owns these
>>>> registers) seems like a better idea; there's no need to only do this
>>>> when the SD controller is enabled, is there?
>>>
>>> Half of these regs are in the SD controller register space
>>> (sdmemcmpctl and autocalcfg), and get reset when the controller gets
>>> reset (mmc_reset). So they need to be set each time a reset occurs. It
>>> makes sense to keep the GP SDIOCFG writes here, too.
>>>
>>> As to where the pad_init_mmc function belongs, it is SoC-specific,
>>> yes. T20 doesn't need it (no sdmemcmpctl or autocalcfg regs on T20
>>> SDMMC), and T30 and T114 use slightly different bits/values for GP
>>> SDIOCFG and sdmemcmpctl/autocalcfg. But the differences are small
>>> enough that I thought this routine should be placed in a common area,
>>> and not duplicated for each SoC, so I put it here.
>>>
>>> Do you have a suggestion of where it would be better placed?
>>
>> For the pinmux registers, I think they should be programmed by the
>> pinmux driver at the same time as the rest of the pinmux is programmed.
>
> Technically, they're not pinmux registers (PINMUX_AUX_ space), but GP
> regs (APB_MISC_GP_ space). Since the pinmux _code_ (no pinmux driver
> is used in Tegra U-Boot)

The distinction isn't relevant for this discussion. Anyway, there really
is a driver...

> for T30 is just a large table slam, I don't
> think it makes sense to add GP pad reg writes there. These pads need
> to be tuned when you've got a board w/an SD-card device hanging off of
> them. So it makes sense to have these 2 register writes here in
> pad_init_mmc(). I can take out the SDMMC1/3 test and just write both
> SDIO1CFG and SDIO3CFG, since the values are the same.

Are these value board-specific or SoC-specific? I thought that the
values came from the TRM and simply had to be set as described.

If the values are SoC-specific, I think we can just key off the
compatible value to determine whether to apply them.

If the values are board-specific, I really think we must put them into
device tree. Otherwise, we cannot claim that we have actually supported
DT in the driver - we're only doing half the job, and can't support new
boards simply by providing a new DT.

The one out might be that we could claim the current values are the
default if the DT provides no other values, even once we do define a way
for DT to provide those values. That would at least be
backwards-compatible. However, that's again going down the path of
initially defining the minimal DT binding required to get some system
running, but planning to incrementally enhance the DT binding later.
While we aren't getting strong pushback on this right now from upstream,
I can guarantee we will do very soon, so we really ought to just get
into the habit of completely defining DT bindings from the start. Still,
I suppose I won't nak this patch because of that yet.

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Tom Warren]

On Mon, Mar 4, 2013 at 5:28 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/04/2013 04:11 PM, Tom Warren wrote:
>> Stephen,
>>
>> On Wed, Feb 27, 2013 at 11:08 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 02/27/2013 09:59 AM, Tom Warren wrote:
>>>> Stephen,
>>>>
>>>> On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 02/26/2013 01:46 PM, Tom Warren wrote:
>>>>>> T30 requires specific SDMMC pad programming, and bus power-rail bringup.
>>>>>
>>>>>> diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c
>>>>>
>>>>>> +/*
>>>>>> + * Do I2C/PMU writes to bring up SD card bus power
>>>>>> + *
>>>>>> + */
>>>>>> +void board_sdmmc_voltage_init(void)
>>>>>
>>>>> We really shouldn't be adding to board files if we're remotely serious
>>>>> about device tree; the whole point of DT is to remove code from the
>>>>> board files, and describe the desired configuration as data in DT instead.
>>>>>
>>>>> This function should be replaced by regulator nodes/properties in the
>>>>> device tree, and the MMC (core?) driver calling into the board-specific
>>>>> regulator driver to request the desired voltages.
>>>>>
>>>>> But so long as we file a bug to replace this code with an explicit
>>>>> regulator driver in the future, I guess it's fine for now.
>>>>
>>>> I'll file a bug for doing all PMU/power rail work from DT. I think it
>>>> makes much more sense as a separate (non-MMC) patch.
>>>
>>> Yes, certainly a separate patch. Ideally it'd be implemented before
>>> other code that relies on it. That's why I think we need to take a
>>> higher-level look at DT support in U-Boot, rather than simply finding
>>> these issue accidentally while implementing the features we already know
>>> we need.
>>>
>>>>> BTW, I just noticed that commit f01b631 "Tegra30: Add/enable Cardhu
>>>>> build (T30 reference board)" adds a file called
>>>>> board/nvidia/cardhu/cardhu.c.mmc. That's a mistake, right?
>>>>
>>>> Yep, that's the Cardhu file I was hacking on for MMC support way back
>>>> when. I can remove it in V2 of these patches, or would you prefer a
>>>> single, separate patch to do that?
>>>
>>> Is the commit that adds that file already pulled into higher-level
>>> repos? If not, I would simply rebase it to remove that file. If it has,
>>> then a separate patch to delete it before this series would make sense.
>>>
>>>>>> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
>>>>>
>>>>> Hmm. This seems like SoC code, not board code...
>>>>>
>>>>>> +void pad_init_mmc(struct tegra_mmc *reg)
>>>>>> +{
>>>>>> +#if defined(CONFIG_TEGRA30)
>>>>>
>>>>>> +     /* Set the pad drive strength for SDMMC1 or 3 only */
>>>>>> +     if (offset != TEGRA_SDMMC1_BASE && offset != TEGRA_SDMMC3_BASE) {
>>>>>> +             debug("%s: settings are only valid for SDMMC1/SDMMC3!\n",
>>>>>> +                     __func__);
>>>>>> +             return;
>>>>>> +     }
>>>>>
>>>>> Perhaps pass in the MMC instance ID instead of the base address. That'd
>>>>> avoid having to know the base addresses in this code.
>>>>
>>>> I still need to know if I've got a SDIO or eMMC ID, though, and I
>>>> don't think the flags for that in the mmc/host structs (mmc->version,
>>>> etc.) get populated until the mmc driver has done some I/O with the
>>>> device (eMMC or SD-card), and I need to set up the pads before that.
>>>>
>>>>> In fact, just putting this code into the pinmux driver (which owns these
>>>>> registers) seems like a better idea; there's no need to only do this
>>>>> when the SD controller is enabled, is there?
>>>>
>>>> Half of these regs are in the SD controller register space
>>>> (sdmemcmpctl and autocalcfg), and get reset when the controller gets
>>>> reset (mmc_reset). So they need to be set each time a reset occurs. It
>>>> makes sense to keep the GP SDIOCFG writes here, too.
>>>>
>>>> As to where the pad_init_mmc function belongs, it is SoC-specific,
>>>> yes. T20 doesn't need it (no sdmemcmpctl or autocalcfg regs on T20
>>>> SDMMC), and T30 and T114 use slightly different bits/values for GP
>>>> SDIOCFG and sdmemcmpctl/autocalcfg. But the differences are small
>>>> enough that I thought this routine should be placed in a common area,
>>>> and not duplicated for each SoC, so I put it here.
>>>>
>>>> Do you have a suggestion of where it would be better placed?
>>>
>>> For the pinmux registers, I think they should be programmed by the
>>> pinmux driver at the same time as the rest of the pinmux is programmed.
>>
>> Technically, they're not pinmux registers (PINMUX_AUX_ space), but GP
>> regs (APB_MISC_GP_ space). Since the pinmux _code_ (no pinmux driver
>> is used in Tegra U-Boot)
>
> The distinction isn't relevant for this discussion. Anyway, there really
> is a driver...
I must be dense. Please point it out to me. It is relevant, because
you've asked to put the GP register writes in the pinmux driver.

>
>> for T30 is just a large table slam, I don't
>> think it makes sense to add GP pad reg writes there. These pads need
>> to be tuned when you've got a board w/an SD-card device hanging off of
>> them. So it makes sense to have these 2 register writes here in
>> pad_init_mmc(). I can take out the SDMMC1/3 test and just write both
>> SDIO1CFG and SDIO3CFG, since the values are the same.
>
> Are these value board-specific or SoC-specific? I thought that the
> values came from the TRM and simply had to be set as described.
>
> If the values are SoC-specific, I think we can just key off the
> compatible value to determine whether to apply them.
Correct. They are SoC-specific as per the TRM. I am using the mmc_id
(periph_id value) to determine whether to write SDIO1CFG or SDIO3CFG
in V2, going up this morning.

>
> If the values are board-specific, I really think we must put them into
> device tree. Otherwise, we cannot claim that we have actually supported
> DT in the driver - we're only doing half the job, and can't support new
> boards simply by providing a new DT.
>
> The one out might be that we could claim the current values are the
> default if the DT provides no other values, even once we do define a way
> for DT to provide those values. That would at least be
> backwards-compatible. However, that's again going down the path of
> initially defining the minimal DT binding required to get some system
> running, but planning to incrementally enhance the DT binding later.
> While we aren't getting strong pushback on this right now from upstream,
> I can guarantee we will do very soon, so we really ought to just get
> into the habit of completely defining DT bindings from the start. Still,
> I suppose I won't nak this patch because of that yet.

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Stephen Warren]

On 03/05/2013 08:28 AM, Tom Warren wrote:
> On Mon, Mar 4, 2013 at 5:28 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 03/04/2013 04:11 PM, Tom Warren wrote:
>>> On Wed, Feb 27, 2013 at 11:08 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 02/27/2013 09:59 AM, Tom Warren wrote:
>>>>> On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
...
>>>> For the pinmux registers, I think they should be programmed by the
>>>> pinmux driver at the same time as the rest of the pinmux is programmed.
>>>
>>> Technically, they're not pinmux registers (PINMUX_AUX_ space), but GP
>>> regs (APB_MISC_GP_ space). Since the pinmux _code_ (no pinmux driver
>>> is used in Tegra U-Boot)
>>
>> The distinction isn't relevant for this discussion. Anyway, there really
>> is a driver...
>
> I must be dense. Please point it out to me. It is relevant, because
> you've asked to put the GP register writes in the pinmux driver.

[swarren at swarren-lx1 u-boot]$ find|grep pinmux|grep tegra|grep \\.c$
./arch/arm/cpu/tegra30-common/pinmux.c
./arch/arm/cpu/tegra114-common/pinmux.c
./arch/arm/cpu/tegra20-common/pinmux.c

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Tom Warren]

On Tue, Mar 5, 2013 at 10:03 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/05/2013 08:28 AM, Tom Warren wrote:
>> On Mon, Mar 4, 2013 at 5:28 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 03/04/2013 04:11 PM, Tom Warren wrote:
>>>> On Wed, Feb 27, 2013 at 11:08 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 02/27/2013 09:59 AM, Tom Warren wrote:
>>>>>> On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> ...
>>>>> For the pinmux registers, I think they should be programmed by the
>>>>> pinmux driver at the same time as the rest of the pinmux is programmed.
>>>>
>>>> Technically, they're not pinmux registers (PINMUX_AUX_ space), but GP
>>>> regs (APB_MISC_GP_ space). Since the pinmux _code_ (no pinmux driver
>>>> is used in Tegra U-Boot)
>>>
>>> The distinction isn't relevant for this discussion. Anyway, there really
>>> is a driver...
>>
>> I must be dense. Please point it out to me. It is relevant, because
>> you've asked to put the GP register writes in the pinmux driver.
>
> [swarren at swarren-lx1 u-boot]$ find|grep pinmux|grep tegra|grep \\.c$
> ./arch/arm/cpu/tegra30-common/pinmux.c
> ./arch/arm/cpu/tegra114-common/pinmux.c
> ./arch/arm/cpu/tegra20-common/pinmux.c
OK, there's a semantic chasm here. This is not a 'driver' in my mind -
it's just the pinmux code to set the bits in the PINMUX_AUX regs, and
over half of it's tables & structs. Again, T30 reads in a few tables
of pinmux/pingroup settings, and slams 'em out to the regs using the
functions in pinmux.c (set_tristate, set_pullupdown, etc.).

So would you be satisfied if I removed the GP sdio pad cfg writes from
the common board-level code (common/board.c) and put them in a
function that's called during pinmux_init(), early on in
board_early_init_f()? That'll preclude being able to parse the mmc_id
to see if I need to write SDIO1CFG or SDIO3CFG, so I'll have to slam
'em both, but that should be OK.

[U-Boot] [PATCH 3/5] Tegra30: MMC: Add SD bus power-rail and SDMMC pad init routines

[Stephen Warren]

On 03/05/2013 10:21 AM, Tom Warren wrote:
> On Tue, Mar 5, 2013 at 10:03 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 03/05/2013 08:28 AM, Tom Warren wrote:
>>> On Mon, Mar 4, 2013 at 5:28 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 03/04/2013 04:11 PM, Tom Warren wrote:
>>>>> On Wed, Feb 27, 2013 at 11:08 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>> On 02/27/2013 09:59 AM, Tom Warren wrote:
>>>>>>> On Tue, Feb 26, 2013 at 4:26 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> ...
>>>>>> For the pinmux registers, I think they should be programmed by the
>>>>>> pinmux driver at the same time as the rest of the pinmux is programmed.
>>>>>
>>>>> Technically, they're not pinmux registers (PINMUX_AUX_ space), but GP
>>>>> regs (APB_MISC_GP_ space). Since the pinmux _code_ (no pinmux driver
>>>>> is used in Tegra U-Boot)

We're discussing struct apb_misc_gp_ctrl fields sdio1cfg and sdio3cfg,
right?

Those /are/ pinmux registers. The pinmux HW has two sets of registers
that feed into it; the pin mux selects (see 17.1.6 in the Tegra30 TRM)
and the pad control registers (see 17.1.4 in the Tegra30 TRM).

Both sets of registers should fully controlled by the pinmux driver, the
values/tables being provided by a board-specific file. Perhaps a common
table could be provided if all/many boards use the same value for some
settings, i.e. in pinmux_init():

pinmux_config_padctrl(tegra_padctrl_sdio1_common, ARRAY_SIZE(...));
pinmux_config_padctrl(tegra_padctrl_sdio3_common, ...);
pinmux_config_padctrl(tegra_padctrl_cardhu_specific, ...);

or:

pinmux_config_padctrl(cardhu_padctrl, ...);

... where cardhu_padctrl[] is probably defined in
pinmux-config-cardhu.h, and could use some centralized macros to create
the appropriate SDIO1CFG/... entries.