[U-Boot] [PATCH 4/7] STiH410: Add STi SDHCI driver

Jaehoon Chung jh80.chung at samsung.com
Thu Feb 2 08:00:01 UTC 2017


Hi,

On 01/21/2017 01:05 AM, patrice.chotard at st.com wrote:
> From: Patrice Chotard <patrice.chotard at st.com>

I want to put minimum commit message at here.
You need to rework this on latest u-boot.

Before sending patch, run checkpatch.pl at least. Also add the Maintainers.
After that, i will review more.

> 
> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
> ---
>  arch/arm/Kconfig                          |   2 +
>  arch/arm/include/asm/arch-stih410/sdhci.h |  69 +++++++++++++++
>  arch/arm/include/asm/arch-stih410/sti.h   |   5 ++
>  board/st/stih410-b2260/board.c            |   3 +
>  drivers/mmc/Kconfig                       |   7 ++
>  drivers/mmc/Makefile                      |   1 +
>  drivers/mmc/sti_sdhci.c                   | 137 ++++++++++++++++++++++++++++++
>  include/configs/stih410-b2260.h           |   4 +
>  include/dm/platform_data/sti_sdhci.h      |  17 ++++
>  9 files changed, 245 insertions(+)
>  create mode 100644 arch/arm/include/asm/arch-stih410/sdhci.h
>  create mode 100644 drivers/mmc/sti_sdhci.c
>  create mode 100644 include/dm/platform_data/sti_sdhci.h
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 98546ae..9a472e6 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -897,6 +897,8 @@ config ARCH_STI
>  	select CPU_V7
>  	select DM
>  	select DM_SERIAL
> +	select BLK
> +	select DM_MMC
>  	help
>  	  Support for STMicroelectronics STiH407/10 SoC family.
>  	  This SoC is used on Linaro 96Board STiH410-B2260
> diff --git a/arch/arm/include/asm/arch-stih410/sdhci.h b/arch/arm/include/asm/arch-stih410/sdhci.h
> new file mode 100644
> index 0000000..f45b961
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-stih410/sdhci.h
> @@ -0,0 +1,69 @@
> +/*
> + * (C) Copyright 2017 Patrice Chotard <patrice.chotard at st.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __STI_SDHCI_H__
> +#define __STI_SDHCI_H__
> +
> +#define FLASHSS_MMC_CORE_CONFIG_1			0x400
> +#define FLASHSS_MMC_CORECFG_TIMEOUT_CLK_UNIT_MHZ	BIT(24)
> +#define FLASHSS_MMC_CORECFG_TIMEOUT_CLK_FREQ_MIN	BIT(12)
> +
> +#define STI_FLASHSS_MMC_CORE_CONFIG_1			\
> +	(FLASHSS_MMC_CORECFG_TIMEOUT_CLK_UNIT_MHZ	| \
> +	 FLASHSS_MMC_CORECFG_TIMEOUT_CLK_FREQ_MIN)
> +
> +#define FLASHSS_MMC_CORE_CONFIG_2			0x404
> +#define FLASHSS_MMC_CORECFG_HIGH_SPEED			BIT(28)
> +#define FLASHSS_MMC_CORECFG_8BIT_EMMC			BIT(20)
> +#define MAX_BLK_LENGTH_1024				BIT(16)
> +#define BASE_CLK_FREQ_200				0xc8
> +
> +
> +#define STI_FLASHSS_MMC_CORE_CONFIG2	\
> +	(FLASHSS_MMC_CORECFG_HIGH_SPEED	| \
> +	 FLASHSS_MMC_CORECFG_8BIT_EMMC	| \
> +	 MAX_BLK_LENGTH_1024		| \
> +	 BASE_CLK_FREQ_200 << 0)
> +
> +#define STI_FLASHSS_SDCARD_CORE_CONFIG2			\
> +	(FLASHSS_MMC_CORECFG_HIGH_SPEED			| \
> +	 MAX_BLK_LENGTH_1024				| \
> +	 BASE_CLK_FREQ_200)
> +
> +#define FLASHSS_MMC_CORE_CONFIG_3			0x408
> +#define FLASHSS_MMC_CORECFG_SLOT_TYPE_EMMC		BIT(28)
> +#define FLASHSS_MMC_CORECFG_ASYNCH_INTR_SUPPORT		BIT(20)
> +#define FLASHSS_MMC_CORECFG_3P3_VOLT			BIT(8)
> +#define FLASHSS_MMC_CORECFG_SUSP_RES_SUPPORT		BIT(4)
> +#define FLASHSS_MMC_CORECFG_SDMA			BIT(0)
> +
> +#define STI_FLASHSS_MMC_CORE_CONFIG3			\
> +	 (FLASHSS_MMC_CORECFG_SLOT_TYPE_EMMC		| \
> +	 FLASHSS_MMC_CORECFG_ASYNCH_INTR_SUPPORT	| \
> +	 FLASHSS_MMC_CORECFG_3P3_VOLT			| \
> +	 FLASHSS_MMC_CORECFG_SUSP_RES_SUPPORT		| \
> +	 FLASHSS_MMC_CORECFG_SDMA)
> +
> +#define STI_FLASHSS_SDCARD_CORE_CONFIG3			\
> +	 (FLASHSS_MMC_CORECFG_ASYNCH_INTR_SUPPORT	| \
> +	 FLASHSS_MMC_CORECFG_3P3_VOLT			| \
> +	 FLASHSS_MMC_CORECFG_SUSP_RES_SUPPORT		| \
> +	 FLASHSS_MMC_CORECFG_SDMA)
> +
> +#define FLASHSS_MMC_CORE_CONFIG_4			0x40c
> +#define FLASHSS_MMC_CORECFG_D_DRIVER_SUPPORT		BIT(20)
> +#define FLASHSS_MMC_CORECFG_C_DRIVER_SUPPORT		BIT(16)
> +#define FLASHSS_MMC_CORECFG_A_DRIVER_SUPPORT		BIT(12)
> +
> +#define STI_FLASHSS_MMC_CORE_CONFIG4			\
> +	(FLASHSS_MMC_CORECFG_D_DRIVER_SUPPORT		| \
> +	 FLASHSS_MMC_CORECFG_C_DRIVER_SUPPORT		| \
> +	 FLASHSS_MMC_CORECFG_A_DRIVER_SUPPORT)
> +
> +#define ST_MMC_CCONFIG_REG_5		0x210
> +#define SYSCONF_MMC1_ENABLE_BIT		3
> +
> +#endif	/* _STI_SDHCI_H_ */
> diff --git a/arch/arm/include/asm/arch-stih410/sti.h b/arch/arm/include/asm/arch-stih410/sti.h
> index f167560..d9e6f03 100644
> --- a/arch/arm/include/asm/arch-stih410/sti.h
> +++ b/arch/arm/include/asm/arch-stih410/sti.h
> @@ -17,4 +17,9 @@
>  /* ASC UART located in the main "COMMs" block */
>  #define STIH410_ASC1_BASE (STIH410_COMMS_BASE + 0x00031000)
>  
> +/* MMC Controller Base (in the FlashSS) */
> +#define STIH410_FLASH_IF_REG1_BASE		0x09060000
> +#define STIH410_CONFIG_SYS_FLASHSS_PORT1_BASE	STIH410_FLASH_IF_REG1_BASE
> +#define STIH410_CONFIG_SYS_MMC0_BASE		STIH410_CONFIG_SYS_FLASHSS_PORT1_BASE 	/* MMC #0 */
> +
>  #endif /* _STI_H_ */
> diff --git a/board/st/stih410-b2260/board.c b/board/st/stih410-b2260/board.c
> index 72b0042..607a52f 100644
> --- a/board/st/stih410-b2260/board.c
> +++ b/board/st/stih410-b2260/board.c
> @@ -7,9 +7,12 @@
>   */
>  
>  #include <common.h>
> +#include <mmc.h>
> +#include <sdhci.h>
>  #include <asm/arch/sti.h>
>  #include <dm/platdata.h>
>  #include <dm/platform_data/serial_sti_asc.h>
> +#include <dm/platform_data/sti_sdhci.h>

Why adds these header at here?

>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 9ed8da3..e1197b4 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -296,6 +296,13 @@ config MMC_SDHCI_TEGRA
>  	  platform with SD or MMC devices, say Y here.
>  
>  	  If unsure, say N.
> +	  
> +config MMC_SDHCI_STI
> +	bool "SDHCI support for STMicroelectronics SoC"
> +	depends on MMC_SDHCI
> +	help
> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
> +	  on STMicroelectronics STiH410 SoC.

Do the alphabet ordering

>  
>  config MMC_SUNXI
>  	bool "Allwinner sunxi SD/MMC Host Controller support"
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 4dca09c..78c0a73 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_MMC_SDHCI_KONA)		+= kona_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_MV)		+= mv_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_S5P)		+= s5p_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_SPEAR)		+= spear_sdhci.o
> +obj-$(CONFIG_MMC_SDHCI_STI) 		+= sti_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_TEGRA)		+= tegra_mmc.o
>  
>  obj-$(CONFIG_MMC_SUNXI)			+= sunxi_mmc.o
> diff --git a/drivers/mmc/sti_sdhci.c b/drivers/mmc/sti_sdhci.c
> new file mode 100644
> index 0000000..ca4013c
> --- /dev/null
> +++ b/drivers/mmc/sti_sdhci.c
> @@ -0,0 +1,137 @@
> +/*
> + *  Copyright (c) 2017
> + *  Patrice Chotard <patrice.chotard at st.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <mmc.h>
> +#include <sdhci.h>
> +#include <asm/arch/gpio.h>
> +#include <asm/arch/sdhci.h>
> +#include <asm/arch/sti.h>
> +#include <asm/arch/syscfg.h>
> +#include <dm/platform_data/sti_sdhci.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/**
> + * sti_mmc_core_config: configure the Arasan HC
> + * @ioaddr: base address

Where is ioaddr argument?

> + * Description: this function is to configure the Arasan MMC HC.
> + * This should be called when the system starts in case of, on the SoC,
> + * it is needed to configure the host controller.
> + * This happens on some SoCs, i.e. StiH410, where the MMC0 inside the flashSS
> + * needs to be configured as MMC 4.5 to have full capabilities.
> + * W/o these settings the SDHCI could configure and use the embedded controller
> + * with limited features.
> + */
> +static void sti_mmc_core_config(const u32 regbase, int mmc_instance)
> +{
> +	unsigned long sysconf;
> +
> +	/* reset MMC1 */
> +	if (mmc_instance) {
> +		sysconf = readl(STIH410_SYSCONF5_BASE + ST_MMC_CCONFIG_REG_5);
> +		SET_SYSCONF_BIT(sysconf, 1, SYSCONF_MMC1_ENABLE_BIT);
> +		writel(sysconf, STIH410_SYSCONF5_BASE + ST_MMC_CCONFIG_REG_5);
> +	}
> +
> +	writel(STI_FLASHSS_MMC_CORE_CONFIG_1, regbase + FLASHSS_MMC_CORE_CONFIG_1);
> +
> +	if  (mmc_instance) {
> +		writel(STI_FLASHSS_MMC_CORE_CONFIG2, regbase + FLASHSS_MMC_CORE_CONFIG_2);
> +		writel(STI_FLASHSS_MMC_CORE_CONFIG3, regbase + FLASHSS_MMC_CORE_CONFIG_3);
> +	} else {
> +		writel(STI_FLASHSS_SDCARD_CORE_CONFIG2, regbase + FLASHSS_MMC_CORE_CONFIG_2);
> +		writel(STI_FLASHSS_SDCARD_CORE_CONFIG3, regbase + FLASHSS_MMC_CORE_CONFIG_3);
> +	}
> +	writel(STI_FLASHSS_MMC_CORE_CONFIG4, regbase + FLASHSS_MMC_CORE_CONFIG_4);
> +}
> +
> +int sti_sdhci_probe(struct udevice *dev)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct sti_sdhci_plat *plat = dev_get_platdata(dev);
> +	struct sdhci_host *host = dev_get_priv(dev);
> +	int node = dev->of_offset;
> +	const void *blob = gd->fdt_blob;

Not need to use blob and node variables.

> +	int ret, count, mmc_instance;
> +
> +	/* identify current mmc instance, mmc1 has a reset, not mmc0 */

MMC1 is which card..? You have to add comment more corret.

> +	fdt_getprop(blob, node, "resets", &count);
> +	if (count < 0) {
> +		mmc_instance = 0;
> +		host->voltages = MMC_VDD_165_195;
> +	} else {
> +		mmc_instance = 1;
> +		host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
> +	}

if sdhci capabilities register is support VDD_330 and VDD_300 and VDD_180?
Your sdhci controller is broken CAPABILITES register?

what related with having "resets"? resets is for just checking mmc1?

> +
> +	sti_mmc_core_config((const u32) host->ioaddr, mmc_instance);
> +
> +	host->name = "sti_sdhci";

dev->name, not "sti_sdhci".

> +	host->quirks = SDHCI_QUIRK_BROKEN_VOLTAGE |
> +		       SDHCI_QUIRK_BROKEN_R1B |
> +		       SDHCI_QUIRK_WAIT_SEND_CMD |
> +		       SDHCI_QUIRK_32BIT_DMA_ADDR |
> +		       SDHCI_QUIRK_NO_HISPD_BIT;

Really need to set SDHCI_QUIRK_BROKEN_R1B?

> +
> +	host->host_caps = MMC_MODE_DDR_52MHz;
> +
> +	ret = sdhci_setup_cfg(&plat->cfg, host, 50000000, 400000);
> +
> +	host->mmc = &plat->mmc;

Why do you put this at here? and after this, checking "ret"?

> +	if (ret)
> +		return ret;
> +
> +	host->mmc->priv = host;
> +	host->mmc->dev = dev;
> +	upriv->mmc = host->mmc;
> +
> +	return sdhci_probe(dev);
> +}
> +
> +static int sti_sdhci_ofdata_to_platdata(struct udevice *dev)
> +{
> +	struct sdhci_host *host = dev_get_priv(dev);
> +
> +	host->name = strdup(dev->name);

duplicated host->name = "sti_sdhci" 

> +	host->ioaddr = (void *)dev_get_addr(dev);
> +
> +	host->bus_width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +					 "bus-width", 4);

default 4bit? i don't think so.

> +
> +	return 0;
> +}
> +
> +static int sti_sdhci_bind(struct udevice *dev)
> +{
> +	struct sti_sdhci_plat *plat = dev_get_platdata(dev);
> +	int ret;
> +
> +	ret = sdhci_bind(dev, &plat->mmc, &plat->cfg);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

just resturn sdhci_bind();

> +}
> +
> +static const struct udevice_id sti_sdhci_ids[] = {
> +	{ .compatible = "st,sdhci" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(sti_mmc) = {
> +	.name = "sti_sdhci",
> +	.id = UCLASS_MMC,
> +	.of_match = sti_sdhci_ids,
> +	.bind = sti_sdhci_bind,
> +	.ops = &sdhci_ops,
> +	.ofdata_to_platdata = sti_sdhci_ofdata_to_platdata,
> +	.probe = sti_sdhci_probe,
> +	.priv_auto_alloc_size = sizeof(struct sdhci_host),
> +	.platdata_auto_alloc_size = sizeof(struct sti_sdhci_plat),
> +};
> diff --git a/include/configs/stih410-b2260.h b/include/configs/stih410-b2260.h
> index b4b3d75..7d97847 100644
> --- a/include/configs/stih410-b2260.h
> +++ b/include/configs/stih410-b2260.h
> @@ -36,9 +36,13 @@
>  #define CONFIG_ENV_SIZE 0x4000
>  #define CONFIG_SYS_NO_FLASH
>  
> +#define CONFIG_GENERIC_MMC

Not need.

>  /* Extra Commands */
>  #define CONFIG_CMD_ASKENV
>  
> +#define CONFIG_DOS_PARTITION
> +#define CONFIG_SETUP_MEMORY_TAGS

what are these related with SDHCI driver?

> +
>  /* Size of malloc() pool */
>  #define CONFIG_SYS_MALLOC_LEN		0x1800000
>  #define CONFIG_SYS_GBL_DATA_SIZE	1024	/* Global data structures */
> diff --git a/include/dm/platform_data/sti_sdhci.h b/include/dm/platform_data/sti_sdhci.h
> new file mode 100644
> index 0000000..d8cc170
> --- /dev/null
> +++ b/include/dm/platform_data/sti_sdhci.h
> @@ -0,0 +1,17 @@
> +
> + * (C) Copyright 2017
> + * Patrice Chotard <patrice.chotard at st.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __STI_SDHCI_H
> +#define __STI_SDHCI_H
> +
> +struct sti_sdhci_plat {
> +	unsigned long *base;  /* address of registers in physical memory */

I didn't find this usage.

> +	struct mmc_config cfg;
> +	struct mmc mmc;
> +};

I want to put this structure into your sdhci driver.
There is no specific sti_sdhci platform data.

Best Regards,
Jaehoon Chung

> +
> +#endif /* __STI_SDHCI_H */
> 



More information about the U-Boot mailing list