[U-Boot] [PATCH 2/2] Tegra: MMC: Add DT support to MMC driver for all T20 boards

[Stephen Warren]

On 02/04/2013 04:48 PM, Tom Warren wrote:
> tegra_mmc_init() now uses DT info for bus width, WP/CD GPIOs, etc.
> Tested on Seaboard, fully functional.

> diff --git a/board/compal/paz00/paz00.c b/board/compal/paz00/paz00.c
> index 1447f47..5cee91a 100644
> --- a/board/compal/paz00/paz00.c
> +++ b/board/compal/paz00/paz00.c
> @@ -55,18 +55,18 @@ static void pin_mux_mmc(void)
>  /* this is a weak define that we are overriding */
>  int board_mmc_init(bd_t *bd)
>  {
> -	debug("board_mmc_init called\n");
> +	debug("%s called\n", __func__);
>  
>  	/* Enable muxes, etc. for SDMMC controllers */
>  	pin_mux_mmc();
>  
> -	debug("board_mmc_init: init eMMC\n");
> -	/* init dev 0, eMMC chip, with 8-bit bus */
> -	tegra_mmc_init(0, 8, -1, -1);
> +	debug("%s: init eMMC\n", __func__);
> +	/* init dev 0, eMMC chip */
> +	tegra_mmc_init(0);
>  
> -	debug("board_mmc_init: init SD slot\n");
> -	/* init dev 3, SD slot, with 4-bit bus */
> -	tegra_mmc_init(3, 4, GPIO_PV1, GPIO_PV5);
> +	debug("%s: init SD slot\n", __func__);
> +	/* init dev 3, SD slot */
> +	tegra_mmc_init(3);
>  
>  	return 0;
>  }

That doesn't look right. The board code still has knowledge of which
SDHCI controllers are in use by the board. Instead, the board should
just call tegra_mmc_init() with no parameters at all, and the MMC driver
should scan the device tree for all present-and-enabled SDHCI nodes, and
instantiate a U-Boot SDHCI device. Without this, the device tree isn't
in control of the whole process, so there's little point doing the
conversion; a new board couldn't be supported /just/ by creating a new
device tree file.

> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c

> +#ifndef CONFIG_OF_CONTROL
> +#error "Please enable device tree support to use this driver"
> +#endif

So CONFIG_OF_CONTROL must be enabled ...

>  
>  static void tegra_get_setup(struct mmc_host *host, int dev_index)
>  {
> -	debug("tegra_get_setup: dev_index = %d\n", dev_index);
> +	debug("%s: dev_index = %d\n", __func__, dev_index);
> +
> +#ifdef CONFIG_OF_CONTROL

... so there's no need for that ifdef

> +	int count, node = 0;
> +	int node_list[MAX_HOSTS];
> +
> +	count = fdtdec_find_aliases_for_id(gd->fdt_blob, "sdmmc",
> +		COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);
> +	debug("%s: count of nodes is %d\n", __func__, count);
> +
> +	if (count < dev_index) {
> +		printf("%s: device index %d exceeds node count (%d)!\n",
> +			__func__, dev_index, count);
> +		return;
> +	}

This requires that an alias exist in order for the SDHCI node to be
found/processed. This isn't correct; the SDHCI nodes must be enumerated
themselves, and then the aliases (if any are present) provide a naming
hint for them, but even without aliases, the SDHCI nodes must be processed.

> +	/*
> +	 * NOTE: mmc->bus_width is determined by mmc.c dynamically.
> +	 * Should we override it with this value?
> +	 */
> +	host->width = fdtdec_get_int(gd->fdt_blob, node, "bus-width", 0);
> +	if (!host->width)
> +		debug("%s: no sdmmc width found\n", __func__);

It should be possible to inform the MMC core of the width of the bus in
terms of wires on the PCB. It should only probe the connected device up
to that width. If that feature is missing, it can be added later though,
outside the scope of this patch set.

You didn't Cc the MMC maintainer; they should be Cc'd since this code is
in drivers/mmc/.