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

[Stephen Warren <swarren@wwwdotorg.org>]

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?