Simon,
On Tue, Feb 12, 2013 at 11:07 AM, Simon Glass sjg@chromium.org wrote:
Hi,
On Tue, Feb 5, 2013 at 1:02 PM, Tom Warren twarren.nvidia@gmail.com wrote:
Stephen,
On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren swarren@wwwdotorg.org wrote:
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
I took Allen's recent SPI/SLINK driver(s) as an example, but as you point out, if CONFIG_OF_CONTROL isn't enabled, you get build errors anyway.
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.
Again, I used Allen's SLINK driver for as a template here. In fact, it looks like our I2C and SPI/SLINK drivers do this, as well as the Exynos SPI and S3C24x0 I2C driver all do this. Can you point out a U-Boot driver that does it the right way (preferably with more than 1 node, like MMC)? I can take a look at that code to use as an example of what you're proposing above.
You have it correct already. Stephen please take another look and let me know what problem you see with this approach. I'm very sorry that I am so late to this discussion.
PTAL at the current patchset (v2) - I changed it to look for 'sdhci', which names the node and the aliases, and seem to work fine. I also have one function that parses all the nodes at once, so only one call to tegra_mmc_init() is needed from the board level.
/** 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/.
Thanks, added Andy Fleming to CC.
Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon