[U-Boot] [PATCH v2 2/2] mmc: tangier: Add Intel Tangier eMMC/SDHCI driver

[Andy Shevchenko]

On Sat, Feb 18, 2017 at 6:34 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Andy,
>
> On Fri, Feb 17, 2017 at 9:02 PM, Andy Shevchenko
> <andriy.shevchenko at linux.intel.com> wrote:
>> From: Felipe Balbi <felipe.balbi at linux.intel.com>
>>
>> This patch adds Intel Tangier eMMC/SDHCI driver.
>
> So this eMMC/SDHC controller is not on PCI bus, correct?

Yes and no.

It's so called "fake PCI". Intel MID have couple of real PCI devices
and many "fake" ones.
They have (semi-)complaint programming interface, but do not fully
follow PCI spec.

To implement it as a PCI controller we need a PCI bus quirk. I dunno
if on this stage it makes sense for U-Boot.

Our goal is to have minimum support of Intel Edison board in U-Boot
upstream from which we (or anybody else) might continue to enhance.

> If so, can
> you add more comments to the commit message?

I would add something as per this discussion.

>
>>
>> Signed-off-by: Vincent Tinelli <vincent.tinelli at intel.com>
>> Signed-off-by: Felipe Balbi <felipe.balbi at linux.intel.com>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
>> ---
>>  drivers/mmc/Kconfig         |  5 ++++
>>  drivers/mmc/Makefile        |  1 +
>>  drivers/mmc/tangier_sdhci.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 78 insertions(+)
>>  create mode 100644 drivers/mmc/tangier_sdhci.c
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index af8436f337..e060547b1d 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -318,6 +318,11 @@ config MMC_SDHCI_XENON
>>
>>           If unsure, say N.
>>
>> +config MMC_SDHCI_TANGIER
>> +       bool "Tangier SDHCI controller support"
>> +       depends on DM_MMC && BLK
>> +       depends on MMC_SDHCI
>
> Please add a "help" here, like mentioning it's not on PCI bus (if I am correct)?
>
>> +
>>  config MMC_SDHCI_TEGRA
>>         bool "SDHCI platform support for the Tegra SD/MMC Controller"
>>         depends on TEGRA
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 9fccac8ead..b210d2c3b3 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -61,6 +61,7 @@ obj-$(CONFIG_MMC_SDHCI_PIC32)         += pic32_sdhci.o
>>  obj-$(CONFIG_MMC_SDHCI_ROCKCHIP)       += rockchip_sdhci.o
>>  obj-$(CONFIG_MMC_SDHCI_S5P)            += s5p_sdhci.o
>>  obj-$(CONFIG_MMC_SDHCI_SPEAR)          += spear_sdhci.o
>> +obj-$(CONFIG_MMC_SDHCI_TANGIER)                += tangier_sdhci.o
>>  obj-$(CONFIG_MMC_SDHCI_TEGRA)          += tegra_mmc.o
>>  obj-$(CONFIG_MMC_SDHCI_XENON)          += xenon_sdhci.o
>>  obj-$(CONFIG_MMC_SDHCI_ZYNQ)           += zynq_sdhci.o
>> diff --git a/drivers/mmc/tangier_sdhci.c b/drivers/mmc/tangier_sdhci.c
>> new file mode 100644
>> index 0000000000..5ab2afa8b5
>> --- /dev/null
>> +++ b/drivers/mmc/tangier_sdhci.c
>> @@ -0,0 +1,72 @@
>

> Please add a file header with author and license information here.

I will ask Felipe.

>> +       ret = sdhci_setup_cfg(&plat->cfg, host, 200000000, 400000);
>
> Can we use some defines for 200000000 and 400000?

Ditto.

-- 
With Best Regards,
Andy Shevchenko