[U-Boot] [PATCH v2 1/1] mmc: Add MMC support for stm32h7 Socs

Jaehoon Chung jh80.chung at samsung.com
Thu Jul 20 04:22:14 UTC 2017


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.chotard at st.com wrote:
>>> From: Patrice Chotard <patrice.chotard at 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.kerello at st.com>
>>> Signed-off-by: Patrice Chotard <patrice.chotard at 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.

> 
> 
>>
>>> +
>>> +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? :)

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),
>>> +};
>>>
>>
> 
> 



More information about the U-Boot mailing list