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

[Tom Warren <twarren.nvidia@gmail.com>]

Marc,

On Tue, Feb 5, 2013 at 1:54 PM, Marc Dietrich <marvin24@gmx.de> wrote:
> Tom,
>
> On Tuesday 05 February 2013 13:41:21 Tom Warren wrote:
>> Marc,
>>
>> On Tue, Feb 5, 2013 at 1:06 PM, Marc Dietrich <marvin24@gmx.de> wrote:
>> > On Tuesday 05 February 2013 08:31:03 Tom Warren wrote:
>> >> Marc,
>> >>
>> >> On Tue, Feb 5, 2013 at 2:28 AM, Marc Dietrich <marvin24@gmx.de> wrote:
>> >> >> [...]
>> >> >>
>> >> >> 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);
>> >> >
>> >> > This looks wrong because the sd is on sdmmc0
>> >> >
>> >> >> -     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);
>> >> >
>> >> > and the emmc on sdmmc3. The DTS is correct.
>> >> >
>> >> > Not your fault as it seems to be wrong in the original code already.
>> >> > I guess it didn't made large difference but may in the future. I wonder
>> >> > how to test this though.
>> >> >
>> >> > Marc
>> >>
>> >> OK, so just the comments are wrong in paz00.c - I can fix that if I
>> >> have to do a V2 patchset, or when I apply the patches to u-boot-tegra.
>> >
>> > ah no, this is weird!
>> >
>> >         index 3 maps to sdmmc1
>> >         index 2 maps to sdmmc2
>> >         index 1 maps to sdmmc3
>> >         index 0 maps to sdmmc4
>> >
>> > so all is fine, nearly ...
>> >
>> >> As to testing, just stop at the command prompt and select each device
>> >> (mmc dev 0, etc.) and run mmcinfo. You should be able to tell from the
>> >> data displayed whether you are on an SD-card or eMMC chip. You can
>> >> also eject the SD-card and you should get a warning about card
>> >> presence due to the CD GPIO.
>> >
>> > the sd card is not detected because:
>> >
>> > TEGRA20
>> > Board: Compal Paz00
>> > DRAM:  512 MiB
>> > MMC:   tegra_get_setup: dev_index = 0
>> > tegra_get_setup: count of nodes is 2
>> > tegra_get_setup: found controller at c8000600, width = 8, periph_id = 15
>> > tegra_mmc_init: index 0, bus width 8 pwr_gpio -1 cd_gpio -1
>> > tegra_mmc_init: bus width = 8
>> > tegra_get_setup: dev_index = 3
>> > tegra_get_setup: count of nodes is 2
>> > tegra_get_setup: device index 3 exceeds node count (2)!
>> >
>> > If I understand correctly, you are counting the aliases only, not the
>> > controllers..., so index 3 (the sdcard) is not initialized at all. Arrr,
>> > debugging stole all of my time, but I guess this needs fixing.
>>
>> Yep, I am checking the aliases to get a node count (just like the
>> Tegra SPI, SLINK, and I2C drivers, and the Exynos SPI and S3C I2C
>> drivers).
>>
>> I left the Paz00 tegra_mmc_init(3) call the same as originally (w/o
>> the width and GPIO params, of course). The device numbering is kind of
>> arbitrary - if there are only 2 MMC devices present, I'd expect dev 0
>> to be eMMC and dev 1 to be SD (again, like my T20 reference board,
>> Seaboard).  I don't see that Paz uses mmc anywhere in its config files
>> for a boot script - does it have to have mmc dev 3 be SD? or would dev
>> 1 work? Note that the tegra20-common-post.h file that all T20 boards
>> inherit uses dev 0 and dev 1.
>
> U-boot scans all devices during boot, so no need to specify a specific one. I
> think what Stephen is suggesting is the right way. Forget all this dev ids and
> let the device tree control everything. The aliases can be used for enumbering
> if someone really needs it.

Yeah, I just now realized the the tegra_mmc driver used this code to
assign base addresses to dev IDs:

        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;

        }

which puts SDMMC4 at dev 0 (eMMC, I knew that) and the other 3
(usually SDIO) in reverse order, SDMMC1 = dev 3, etc. I didn't know
that.

As you say, letting the DT control it all, with only a single
'mmc_init' call from the board file is the way to go. I'll redo and
submit a V2 later in the week.

Thanks again!

>
>> Let me look into it - wish I had a Paz00 board here to debug with.
>> I'll try to simulate this on my Seaboard, maybe with all 4 MMC device
>> addresses in the alias.
>
> He, if you ever come to old Europe, there are still some shops selling
> them ;-)

Unfortunately, no plans to travel to Europe in the near future. :(
>
> Marc
>
>