Hi, On 01/21/2017 01:05 AM, patrice.chot...@st.com wrote: > From: Patrice Chotard <patrice.chot...@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.chot...@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.chot...@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.chot...@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.chot...@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 */ > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot