Hi Jaehoon
On 07/20/2017 06:22 AM, Jaehoon Chung wrote:
> Hi Patrice
>
> On 07/19/2017 09:20 PM, Patrice CHOTARD wrote:
>> Hi Jaehoon
>>
>> On 07/19/2017 12:11 PM, Jaehoon Chung wrote:
>>> Hi,
>>>
>>> On 07/19/2017 04:27 PM, patrice.chot...@st.com wrote:
>>>> From: Patrice Chotard <patrice.chot...@st.com>
>>>>
>>>> This patch adds SD/MMC support for STM32H7 SoCs.
>>>>
>>>> Here is an extraction of SDMMC main features, embedded in
>>>> STM32H7 SoCs.
>>>> The SD/MMC block include the following:
>>>> _ Full compliance with MultiMediaCard System Specification
>>>> Version 4.51. Card support for three different databus modes:
>>>> 1-bit (default), 4-bit and 8-bit.
>>>> _ Full compatibility with previous versions of MultiMediaCards
>>>> (backward compatibility).
>>>> _ Full compliance with SD memory card specifications version 4.1.
>>>> (SDR104 SDMMC_CK speed limited to maximum allowed IO speed,
>>>> SPI mode and UHS-II mode not supported).
>>>> _ Full compliance with SDIO card specification version 4.0.
>>>> Card support for two different databus modes: 1-bit (default)
>>>> and 4-bit. (SDR104 SDMMC_CK speed limited to maximum allowed IO
>>>> speed, SPI mode and UHS-II mode not supported).
>>>> _ Data transfer up to 208 Mbyte/s for the 8 bit mode.
>>>> (depending maximum allowed IO speed).
>>>> _ Data and command output enable signals to control external
>>>> bidirectional drivers.
>>>>
>>>> The current version of the SDMMC supports only one SD/SDIO/MMC card
>>>> at any one time and a stack of MMC Version 4.51 or previous.
>>>>
>>>> Signed-off-by: Christophe Kerello <christophe.kere...@st.com>
>>>> Signed-off-by: Patrice Chotard <patrice.chot...@st.com>
>>>> ---
>>>>
>>>> v2: _ add .get_cd() callback support
>>>>
>>>> drivers/mmc/Kconfig | 8 +
>>>> drivers/mmc/Makefile | 1 +
>>>> drivers/mmc/stm32_sdmmc2.c | 619
>>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 628 insertions(+)
>>>> create mode 100644 drivers/mmc/stm32_sdmmc2.c
>>>>
>>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>>> index 82b8d75..f2e4c26 100644
>>>> --- a/drivers/mmc/Kconfig
>>>> +++ b/drivers/mmc/Kconfig
>>>> @@ -377,6 +377,14 @@ config GENERIC_ATMEL_MCI
>>>> the SD Memory Card Specification V2.0, the SDIO V2.0
>>>> specification
>>>> and CE-ATA V1.1.
>>>>
>>>> +config STM32_SDMMC2
>>>
>>> Why add the SDMMC2? not SDMMC?
>>> Is there a special reason?
>>
>> It's simply the IP name
>>
>>>
>>>> + bool "STMicroelectronics STM32H7 SD/MMC Host Controller support"
>>>> + depends on DM_MMC && OF_CONTROL && DM_MMC_OPS
>>>> + help
>>>> + This selects support for the SD/MMC controller on STM32H7 SoCs.
>>>> + If you have a board based on such a SoC and with a SD/MMC slot,
>>>> + say Y or M here.
>>>> +
>>>> endif
>>>>
>>>> config TEGRA124_MMC_DISABLE_EXT_LOOPBACK
>>>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>>>> index 2d781c3..2584663 100644
>>>> --- a/drivers/mmc/Makefile
>>>> +++ b/drivers/mmc/Makefile
>>>> @@ -43,6 +43,7 @@ obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
>>>> obj-$(CONFIG_MMC_SANDBOX) += sandbox_mmc.o
>>>> obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>>>> obj-$(CONFIG_SH_SDHI) += sh_sdhi.o
>>>> +obj-$(CONFIG_STM32_SDMMC2) += stm32_sdmmc2.o
>>>>
>>>> # SDHCI
>>>> obj-$(CONFIG_MMC_SDHCI) += sdhci.o
>>>> diff --git a/drivers/mmc/stm32_sdmmc2.c b/drivers/mmc/stm32_sdmmc2.c
>>>> new file mode 100644
>>>> index 0000000..84f96e5
>>>> --- /dev/null
>>>> +++ b/drivers/mmc/stm32_sdmmc2.c
>>>> @@ -0,0 +1,619 @@
>>>> +/*
>>>> + * Copyright (c) 2017 STMicrelectronics
>>>> + *
>>>> + * SPDX-License-Identifier: GPL-2.0
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <clk.h>
>>>> +#include <dm.h>
>>>> +#include <fdtdec.h>
>>>> +#include <libfdt.h>
>>>> +#include <mmc.h>
>>>> +#include <reset.h>
>>>> +#include <asm/io.h>
>>>> +#include <asm/gpio.h>
>>>> +
>>>> +struct stm32_sdmmc2 {
>>>> + u32 power; /* SDMMC power control [0x00] */
>>>> + u32 clkcr; /* SDMMC clock control [0x04] */
>>>> + u32 arg; /* SDMMC argument [0x08] */
>>>> + u32 cmd; /* SDMMC command [0x0C] */
>>>> + u32 respcmd; /* SDMMC command response [0x10] */
>>>> + u32 resp1; /* SDMMC response 1 [0x14] */
>>>> + u32 resp2; /* SDMMC response 2 [0x18] */
>>>> + u32 resp3; /* SDMMC response 3 [0x1C] */
>>>> + u32 resp4; /* SDMMC response 4 [0x20] */
>>>> + u32 dtimer; /* SDMMC data timer [0x24] */
>>>> + u32 dlen; /* SDMMC data length [0x28] */
>>>> + u32 dctrl; /* SDMMC data control [0x2C] */
>>>> + u32 dcount; /* SDMMC data counter [0x30] */
>>>> + u32 sta; /* SDMMC status [0x34] */
>>>> + u32 icr; /* SDMMC interrupt clear [0x38] */
>>>> + u32 mask; /* SDMMC mask [0x3C] */
>>>> + u32 acktime; /* SDMMC Acknowledgment timer [0x40] */
>>>> + u32 reserved0[3]; /* Reserved, 0x44 - 0x4C - 0x4C */
>>>> + u32 idmactrl; /* SDMMC DMA control [0x50] */
>>>> + u32 idmabsize; /* SDMMC DMA buffer size [0x54] */
>>>> + u32 idmabase0; /* SDMMC DMA buffer 0 base address [0x58] */
>>>> + u32 idmabase1; /* SDMMC DMA buffer 1 base address [0x5C] */
>>>> + u32 reserved1[8]; /* Reserved, 0x4C-0x7C */
>>>> + u32 fifo; /* SDMMC data FIFO [0x80] */
>>>> +};
>>>
>>> This is for register offset, right?
>>
>> yes
>>
>>>
>>> I want to use the defined value..likes "#define SDMMC_POWER_CONTROL 0x00"
>>> (It's my preference.)
>>> I'm not sure but i have remembered that was difficult to debug.
>>
>> But on http://www.denx.de/wiki/U-Boot/CodingStyle, it's recommended to
>> use structures for I/O access, see "Use structures for I/O access " chapter.
>
> It's recommended, not mandatory.
> Already used the "define" style in mmc subsystem on u-boot.
> e.g) sdhci/dwmmc controller etc...
>
> I think it's no problem to use "define" style..
> In my opinion, it's more complex than define..
>
> For example, some controllers can be changed the register offset according to
> IP version.
> Then we needs to make new structure for it.
>
> And if someone calculated wrong offset, then it's possible that all register
> should be accessed to wrong offset.
> That's why it's my preference.
Ok i will convert this driver with registers offset.
>
>>
>>
>>>
>>>> +
>>>> +struct stm32_sdmmc2_host {
>>>> + struct stm32_sdmmc2 *base;
>>>> + struct mmc_config cfg;
>>>> + struct clk clk;
>>>> + struct reset_ctl reset_ctl;
>>>> + struct gpio_desc cd_gpio;
>>>> + u32 clk_reg_add;
>>>> + u32 pwr_reg_add;
>>>> +};
>>>> +
>
> [..snip..]
>
>>>> + cfg->host_caps |= MMC_MODE_4BIT;
>>>> + break;
>>>> + case 1:
>>>> + break;
>>>> + default:
>>>> + printf("%s: invalid \"bus-width\" property\n", __func__);
>>>> + ret = -ENOENT;
>>>> + goto reset_free;
>>>
>>> Maybe default value can be 1..
>>
>> Default value is already set to 1 in case the property "bus-width" is
>> not present in DT.
>>
>> The "default" case is just a protection in case of "bus-width" property
>> is set with other values than 1, 4 or 8.
>
> Then i think it can be just displayed wrong dt property value, not returned
> error
> and host controller and card can be worked with 1bit buswidth..
> how about? :)
Agree, i will fix it
Thanks
Patrice
>
> Best Regards,
> Jaehoon Chung
>
>>
>>>
>>>> + }
>>>> +
>>>> + mmc = mmc_create(cfg, host);
>>>> + if (!mmc) {
>>>> + ret = -ENOMEM;
>>>> + goto reset_free;
>>>> + }
>>>> +
>>>> + mmc->block_dev.removable = !dev_read_bool(dev, "non-removable");
>>>> + mmc->dev = dev;
>>>> + upriv->mmc = mmc;
>>>> +
>>>> + return 0;
>>>> +
>>>> +reset_free:
>>>> + reset_free(&host->reset_ctl);
>>>> +clk_disable:
>>>> + clk_disable(&host->clk);
>>>> +clk_free:
>>>> + clk_free(&host->clk);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static const struct udevice_id stm32_sdmmc2_ids[] = {
>>>> + { .compatible = "st,stm32-sdmmc2" },
>>>> + { }
>>>> +};
>>>> +
>>>> +U_BOOT_DRIVER(stm32_sdmmc2) = {
>>>> + .name = "stm32_sdmmc2",
>>>> + .id = UCLASS_MMC,
>>>> + .of_match = stm32_sdmmc2_ids,
>>>> + .ops = &stm32_sdmmc2_ops,
>>>> + .probe = stm32_sdmmc2_probe,
>>>> + .priv_auto_alloc_size = sizeof(struct stm32_sdmmc2_host),
>>>> +};
>>>>
>>>
>>
>>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
