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

[Andy Fleming]

On Thu, Feb 14, 2013 at 3:04 PM, Tom Warren <twarren.nvidia at gmail.com>wrote:

> tegra_mmc_init() now parses the DT info for bus width, WP/CD GPIOs, etc.
> Tested on Seaboard, fully functional.
>
> Tamonten boards (medcom-wide, plutux, and tec) use a different/new
> dtsi file w/common settings.
>
> Signed-off-by: Tom Warren <twarren at nvidia.com>
> Signed-off-by: Thierry Reding <thierry.reding at avionic-design.de>
> ---
> v2:
> - all boards now call tegra_mmc_init once, w/no params
> - count MMC controllers, not aliases
> - AD boards (medcom/plutux/tec) use common tegra20-tamonten.dtsi
> v3:
> - move any power init inside board's pin_mux_mmc function, and/or
>  create pin_mux_mmc function if necessary.
> - move board_mmc_init out of each board file and into ../common/board.c
> v4:
> - remove #ifdef CONFIG_TEGRA_MMC from trimslice.c
> - fix minor whitespace issue in board/nvidia/common/board.c
> - remove marking of used node_list entries in MMC driver, not needed
>
>


> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
> index d749ab0..918a98d 100644
> --- a/drivers/mmc/tegra_mmc.c
> +++ b/drivers/mmc/tegra_mmc.c
> @@ -21,6 +21,7 @@
>
>  #include <bouncebuf.h>
>  #include <common.h>
> +#include <fdtdec.h>
>  #include <asm/gpio.h>
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
> @@ -28,54 +29,23 @@
>  #include <asm/arch-tegra/tegra_mmc.h>
>  #include <mmc.h>
>
> -/* support 4 mmc hosts */
> -struct mmc mmc_dev[4];
> -struct mmc_host mmc_host[4];
> +DECLARE_GLOBAL_DATA_PTR;
>
> +struct mmc mmc_dev[MAX_HOSTS];
> +struct mmc_host mmc_host[MAX_HOSTS];
>
> -/**
> - * Get the host address and peripheral ID for a device. Devices are
> numbered
> - * from 0 to 3.
> - *
> - * @param host         Structure to fill in (base, reg, mmc_id)
> - * @param dev_index    Device index (0-3)
> - */
> -static void tegra_get_setup(struct mmc_host *host, int dev_index)
> -{
> -       debug("tegra_get_setup: dev_index = %d\n", dev_index);
> -
> -       switch (dev_index) {
> -       case 1:
> -               host->base = TEGRA_SDMMC3_BASE;
> -               host->mmc_id = PERIPH_ID_SDMMC3;
> -               break;
> -       case 2:
> -               host->base = TEGRA_SDMMC2_BASE;
> -               host->mmc_id = PERIPH_ID_SDMMC2;
> -               break;
> -       case 3:
> -               host->base = TEGRA_SDMMC1_BASE;
> -               host->mmc_id = PERIPH_ID_SDMMC1;
> -               break;
> -       case 0:
> -       default:
> -               host->base = TEGRA_SDMMC4_BASE;
> -               host->mmc_id = PERIPH_ID_SDMMC4;
> -               break;
> -       }
> -
> -       host->reg = (struct tegra_mmc *)host->base;
> -}
> +#ifndef CONFIG_OF_CONTROL
> +#error "Please enable device tree support to use this driver"
> +#endif
>
>  static void mmc_prepare_data(struct mmc_host *host, struct mmc_data *data,
>                                 struct bounce_buffer *bbstate)
>  {
>         unsigned char ctrl;
>
> -
> -       debug("buf: %p (%p), data->blocks: %u, data->blocksize: %u\n",
> -               bbstate->bounce_buffer, bbstate->user_buffer, data->blocks,
> -               data->blocksize);
> +       debug("%s: buf: %p (%p), data->blocks: %u, data->blocksize: %u\n",
> +               __func__, bbstate->bounce_buffer, bbstate->user_buffer,
> +               data->blocks, data->blocksize);
>


This patch is FULL of these changes. It makes it almost impossible to
identify the substantive changes to this code. In the future, please put
changes to debug output in a separate patch, unless it directly applies to
the relevant change. Also, I note that you didn't mention the fact that you
reworked all the debug() calls to prefix with the function name in your
patch description.


> +/*
> + * Process a list of nodes, adding them to our list of SDMMC ports.
> + *
> + * @param blob          fdt blob
> + * @param node_list     list of nodes to process (any <=0 are ignored)
> + * @param count         number of nodes to process
> + * @return 0 if ok, -1 on error
> + */
> +static int process_nodes(const void *blob, int node_list[], int count)
> +{
> +       struct mmc_host *host;
> +       int i, node;
> +
> +       debug("%s: count = %d\n", __func__, count);
> +
> +       /* build mmc_host[] for each controller */
> +       for (i = 0; i < count; i++) {
> +               node = node_list[i];
> +               if (node <= 0)
> +                       continue;
> +
> +               host = &mmc_host[i];
> +               host->id = i;
> +
> +               if (mmc_get_config(blob, node, host)) {
> +                       printf("%s: failed to decode dev %d\n", __func__,
> i);
> +                       return -1;
> +               }
> +               do_mmc_init(i);
> +       }
> +       return 0;
> +}
> +
> +void tegra_mmc_init(void)
> +{
> +       int node_list[MAX_HOSTS], count;
> +       const void *blob = gd->fdt_blob;
> +       debug("%s entry\n", __func__);
> +
> +       count = fdtdec_find_aliases_for_id(blob, "sdhci",
> +               COMPAT_NVIDIA_TEGRA20_SDMMC, node_list, MAX_HOSTS);
> +       debug("%s: count of sdhci nodes is %d\n", __func__, count);
> +
> +       if (process_nodes(blob, node_list, count)) {
> +               printf("%s: Error processing mmc node(s)!\n", __func__);
> +               return;
> +       }
> +}
>


Hmmm.... what does fdtdec_find_aliases_for_id() do? It looks like you are
attempting to go through all of the sdhci nodes, and I can't help but
wonder why you wouldn't use the approach the kernel uses --
for_each_compatible_node()? I'm a little worried that "aliases" are being
overused, here.

Andy