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

[Stephen Warren]

On 02/12/2013 11:07 AM, Simon Glass wrote:
> Hi,
> 
> On Tue, Feb 5, 2013 at 1:02 PM, Tom Warren <twarren.nvidia at gmail.com> wrote:
>> Stephen,
>>
>> On Tue, Feb 5, 2013 at 1:03 PM, Stephen Warren <swarren at 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.

Could you explain how this works then? The code I looked at (a) was
driven by board files not DT enumerationk (b) errored out if no alias ID
was present.

But given there's a V2, I'll go look at that and check again.