Hi,
On 4/10/20 2:33 AM, Stefan B. wrote:
Hi,
see below my answers to your questions.
Regards Stefan Bosch
Hi,
thanks a lot for your reply. As you already guessed I have ported the outdated U-Boot from FriendlyARM, see: https://protect2.fireeye.com/url?k=00de144b-5d1421fc-00df9f04-0cc47a3003e8-6...
The original MMC-driver has been nexell_dw_mmc.c, so I renamed it to nexell_dw_mmc_dm.c after changing to DM.
I will have a closer look at your suggestions and give you feedback ASAP.
I don't know that you had received reviews about other patches. If you want to apply new chip, then i think you need to implement drivers based on DM.
Regards Stefan Bosch
Am 02.04.20 um 13:03 schrieb Jaehoon Chung:
Hi,
On 3/28/20 6:43 PM, Stefan Bosch wrote:
Changes in relation to FriendlyARM's U-Boot nanopi2-v2016.01:
- mmc: nexell_dw_mmc.c changed to nexell_dw_mmc_dm.c (switched to DM).
It doesn't need to add postfix as _dm.
Ok, I have renamed it to nexell_dw_mmc.c.
Signed-off-by: Stefan Bosch stefan_b@posteo.net
Changes in v2:
- commit "i2c: mmc: add nexell driver (gpio, i2c, mmc, pwm)" splitted
into separate commits for gpio, i2c, mmc, pwm.
drivers/mmc/Kconfig | 6 + drivers/mmc/Makefile | 1 + drivers/mmc/nexell_dw_mmc_dm.c | 350 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 357 insertions(+) create mode 100644 drivers/mmc/nexell_dw_mmc_dm.c
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index 2f0eedc..bb8e7c0 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -253,6 +253,12 @@ config MMC_DW_SNPS This selects support for Synopsys DesignWare Memory Card Interface driver extensions used in various Synopsys ARC devboards. +config NEXELL_DWMMC + bool "Nexell SD/MMC controller support" + depends on ARCH_NEXELL + depends on MMC_DW + default y
Not depends on DM_MMC?
You are right, I have inserted "depends on DM_MMC". I missed this when changing to DM.
config MMC_MESON_GX bool "Meson GX EMMC controller support" depends on DM_MMC && BLK && ARCH_MESON diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index 9c1f8e5..a7b5a7b 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -43,6 +43,7 @@ obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o obj-$(CONFIG_SH_SDHI) += sh_sdhi.o obj-$(CONFIG_STM32_SDMMC2) += stm32_sdmmc2.o obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o +obj-$(CONFIG_NEXELL_DWMMC) += nexell_dw_mmc_dm.o # SDHCI obj-$(CONFIG_MMC_SDHCI) += sdhci.o diff --git a/drivers/mmc/nexell_dw_mmc_dm.c b/drivers/mmc/nexell_dw_mmc_dm.c new file mode 100644 index 0000000..b06b60d --- /dev/null +++ b/drivers/mmc/nexell_dw_mmc_dm.c @@ -0,0 +1,350 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- (C) Copyright 2016 Nexell
- Youngbok, Park park@nexell.co.kr
- (C) Copyright 2019 Stefan Bosch stefan_b@posteo.net
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <dt-structs.h> +#include <dwmmc.h> +#include <syscon.h> +#include <asm/gpio.h> +#include <asm/arch/nx_gpio.h> +#include <asm/arch/reset.h>
+#define DWMCI_CLKSEL 0x09C +#define DWMCI_SHIFT_0 0x0 +#define DWMCI_SHIFT_1 0x1 +#define DWMCI_SHIFT_2 0x2 +#define DWMCI_SHIFT_3 0x3 +#define DWMCI_SET_SAMPLE_CLK(x) (x) +#define DWMCI_SET_DRV_CLK(x) ((x) << 16) +#define DWMCI_SET_DIV_RATIO(x) ((x) << 24) +#define DWMCI_CLKCTRL 0x114 +#define NX_MMC_CLK_DELAY(x, y, a, b) ((((x) & 0xFF) << 0) |\ + (((y) & 0x03) << 16) |\ + (((a) & 0xFF) << 8) |\ + (((b) & 0x03) << 24))
+struct nexell_mmc_plat { + struct mmc_config cfg; + struct mmc mmc; +};
+struct nexell_dwmmc_priv { + struct clk *clk; + struct dwmci_host host; + int fifo_size; + bool fifo_mode; + int frequency; + u32 min_freq; + u32 max_freq; + int d_delay; + int d_shift; + int s_delay; + int s_shift;
+};
+struct clk *clk_get(const char *id);
+static void set_pin_stat(int index, int bit, int value) +{ +#if !defined(CONFIG_SPL_BUILD) + nx_gpio_set_pad_function(index, bit, value); +#else +#if defined(CONFIG_ARCH_S5P4418) || \ + defined(CONFIG_ARCH_S5P6818)
+ unsigned long base[5] = { + PHY_BASEADDR_GPIOA, PHY_BASEADDR_GPIOB, + PHY_BASEADDR_GPIOC, PHY_BASEADDR_GPIOD, + PHY_BASEADDR_GPIOE, + };
I don't understand why gpio pin is set in mmc driver? If nexell soc will change the gpio map and function value, does it needs to add other gpio control?
+ dw_mmc_set_pin(base[index], bit, value); +#endif +#endif +}
+static void nx_dw_mmc_set_pin(struct dwmci_host *host) +{ + debug(" %s(): dev_index == %d", __func__, host->dev_index);
+ switch (host->dev_index) { + case 0: + set_pin_stat(0, 29, 1); + set_pin_stat(0, 31, 1); + set_pin_stat(1, 1, 1); + set_pin_stat(1, 3, 1); + set_pin_stat(1, 5, 1); + set_pin_stat(1, 7, 1); + break; + case 1: + set_pin_stat(3, 22, 1); + set_pin_stat(3, 23, 1); + set_pin_stat(3, 24, 1); + set_pin_stat(3, 25, 1); + set_pin_stat(3, 26, 1); + set_pin_stat(3, 27, 1); + break; + case 2: + set_pin_stat(2, 18, 2); + set_pin_stat(2, 19, 2); + set_pin_stat(2, 20, 2); + set_pin_stat(2, 21, 2); + set_pin_stat(2, 22, 2); + set_pin_stat(2, 23, 2); + if (host->buswidth == 8) { + set_pin_stat(4, 21, 2); + set_pin_stat(4, 22, 2); + set_pin_stat(4, 23, 2); + set_pin_stat(4, 24, 2); + } + break; + default: + debug(" is invalid!");
Is invalid..then will not work fine?
i don't check your patch fully. Your patch doesn't control gpio with dt? Well, i don't agree this concept. it need to get opinion how to think about this.
Nexell has not implemented an pinctrl-driver, so the GPIOs are switched to MMC-function here. I have removed the above "default", now I check for a valid dev_index in nexell_dwmmc_ofdata_to_platdata().
Then it needs to implement pinctrl driver.
+ } + debug("\n"); +}
+static void nx_dw_mmc_clksel(struct dwmci_host *host) +{ + u32 val;
+#ifdef CONFIG_BOOST_MMC
What is BOOST_MMC?
This influences what value is written to register offset 0x9C of the MMC-controller. Unfortunatelly, I can not give you more info because this register is indicated as "Reserved" in the S5P4418-datasheet I have access to (Revision 0.10 from October 2014).
I didn't find where BOOST_MMC config is defined. And it can be used with device-tree property. As i know it's relevant to clk phase and timing.
+ val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) | + DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(1); +#else + val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) | + DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(3); +#endif
+ dwmci_writel(host, DWMCI_CLKSEL, val); +}
+static void nx_dw_mmc_reset(int ch) +{ + int rst_id = RESET_ID_SDMMC0 + ch;
RESET_ID_SDMMC0?
This is defined in arch/arm/include/asm/arch/nexell.h.
+ nx_rstcon_setrst(rst_id, 0); + nx_rstcon_setrst(rst_id, 1); +}
+static void nx_dw_mmc_clk_delay(struct udevice *dev) +{ + unsigned int delay; + struct nexell_dwmmc_priv *priv = dev_get_priv(dev); + struct dwmci_host *host = &priv->host;
+ delay = NX_MMC_CLK_DELAY(priv->d_delay, + priv->d_shift, priv->s_delay, priv->s_shift);
+ writel(delay, (host->ioaddr + DWMCI_CLKCTRL)); + debug("%s(): Values set: d_delay==%d, d_shift==%d, s_delay==%d, " + "s_shift==%d\n", __func__, priv->d_delay, priv->d_shift, + priv->s_delay, priv->s_shift); +}
+static unsigned int nx_dw_mmc_get_clk(struct dwmci_host *host, uint freq) +{ + struct clk *clk; + struct udevice *dev = host->priv; + struct nexell_dwmmc_priv *priv = dev_get_priv(dev);
+ int index = host->dev_index; + char name[50] = { 0, };
+ clk = priv->clk; + if (!clk) { + sprintf(name, "%s.%d", DEV_NAME_SDHC, index);
DEV_NAME_SDHC ???
This is also defined in arch/arm/include/asm/arch/nexell.h: "nx-sdhc"
+ clk = clk_get((const char *)name); + if (!clk) + return 0; + priv->clk = clk; + }
+ return clk_get_rate(clk) / 2; +}
+static unsigned long nx_dw_mmc_set_clk(struct dwmci_host *host, + unsigned int rate) +{ + struct clk *clk; + char name[50] = { 0, }; + struct udevice *dev = host->priv; + struct nexell_dwmmc_priv *priv = dev_get_priv(dev);
+ int index = host->dev_index;
+ clk = priv->clk; + if (!clk) { + sprintf(name, "%s.%d", DEV_NAME_SDHC, index); + clk = clk_get((const char *)name); + if (!clk) + return 0; + priv->clk = clk; + }
+ clk_disable(clk); + rate = clk_set_rate(clk, rate); + clk_enable(clk);
+ return rate; +}
+static int nexell_dwmmc_ofdata_to_platdata(struct udevice *dev) +{ + /* if (dev): *priv = dev->priv, else: *priv = NULL */ + struct nexell_dwmmc_priv *priv = dev_get_priv(dev); + struct dwmci_host *host = &priv->host; + int val = -1;
+ debug("%s()\n", __func__);
+ host->name = dev->name; + host->ioaddr = dev_read_addr_ptr(dev);
+ val = dev_read_u32_default(dev, "nexell,bus-width", -1); + if (val < 0) { + debug(" 'nexell,bus-width' missing/invalid!\n"); + return -EINVAL; + } + host->buswidth = val; + host->get_mmc_clk = nx_dw_mmc_get_clk; + host->clksel = nx_dw_mmc_clksel; + host->priv = dev;
+ val = dev_read_u32_default(dev, "index", -1); + if (val < 0) { + debug(" 'index' missing/invalid!\n"); + return -EINVAL; + } + host->dev_index = val;
+ val = dev_read_u32_default(dev, "fifo-size", 0x20); + if (val <= 0) { + debug(" 'fifo-size' missing/invalid!\n"); + return -EINVAL; + } + priv->fifo_size = val;
+ priv->fifo_mode = dev_read_bool(dev, "fifo-mode");
+ val = dev_read_u32_default(dev, "frequency", -1); + if (val < 0) { + debug(" 'frequency' missing/invalid!\n"); + return -EINVAL; + } + priv->frequency = val;
+ val = dev_read_u32_default(dev, "max-frequency", -1); + if (val < 0) { + debug(" 'max-frequency' missing/invalid!\n"); + return -EINVAL; + } + priv->max_freq = val; + priv->min_freq = 400000; /* 400 kHz */
+ val = dev_read_u32_default(dev, "nexell,drive_dly", -1); + if (val < 0) { + debug(" 'nexell,drive_dly' missing/invalid!\n"); + return -EINVAL; + } + priv->d_delay = val;
+ val = dev_read_u32_default(dev, "nexell,drive_shift", -1); + if (val < 0) { + debug(" 'nexell,drive_shift' missing/invalid!\n"); + return -EINVAL; + } + priv->d_shift = val;
+ val = dev_read_u32_default(dev, "nexell,sample_dly", -1); + if (val < 0) { + debug(" 'nexell,sample_dly' missing/invalid!\n"); + return -EINVAL; + } + priv->s_delay = val;
+ val = dev_read_u32_default(dev, "nexell,sample_shift", -1); + if (val < 0) { + debug(" 'nexell,sample_shift' missing/invalid!\n"); + return -EINVAL; + } + priv->s_shift = val;
+ debug(" index==%d, name==%s, ioaddr==0x%08x, buswidth==%d, " + "fifo_size==%d, fifo_mode==%d, frequency==%d\n", + host->dev_index, host->name, (u32)host->ioaddr, + host->buswidth, priv->fifo_size, priv->fifo_mode, + priv->frequency); + debug(" min_freq==%d, max_freq==%d, delay: " + "0x%02x:0x%02x:0x%02x:0x%02x\n", + priv->min_freq, priv->max_freq, priv->d_delay, + priv->d_shift, priv->s_delay, priv->s_shift);
entirely not readable. not need to assign again with val.
priv->s_deley = dev_read_u32_default();
I have reworked nexell_dwmmc_ofdata_to_platdata(), i.e. valid default values are used now (where possible) and the appropriate if-blocks have been removed.
+ return 0; +}
+static int nexell_dwmmc_probe(struct udevice *dev) +{ + struct nexell_mmc_plat *plat = dev_get_platdata(dev); + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); + struct nexell_dwmmc_priv *priv = dev_get_priv(dev); + struct dwmci_host *host = &priv->host; + struct udevice *pwr_dev __maybe_unused;
+ debug("%s():\n", __func__);
Don't add unnecessary debug code. it's meaningless even if it's enabled.
Ok, I have removed this debug code.
Well, i didn't check other patches..but i think that your patches seems to based on older u-boot version.
Best Regards, Jaehoon Chung
+ host->fifoth_val = MSIZE(0x2) | + RX_WMARK(priv->fifo_size / 2 - 1) | + TX_WMARK(priv->fifo_size / 2);
+ host->fifo_mode = priv->fifo_mode;
+ dwmci_setup_cfg(&plat->cfg, host, priv->max_freq, priv->min_freq); + host->mmc = &plat->mmc; + host->mmc->priv = &priv->host; + host->mmc->dev = dev; + upriv->mmc = host->mmc;
+ nx_dw_mmc_set_pin(host);
+ debug(" nx_dw_mmc_set_clk(host, frequency * 4 == %d)\n", + priv->frequency * 4); + nx_dw_mmc_set_clk(host, priv->frequency * 4);
+ nx_dw_mmc_reset(host->dev_index); + nx_dw_mmc_clk_delay(dev);
+ return dwmci_probe(dev); +}
+static int nexell_dwmmc_bind(struct udevice *dev) +{ + struct nexell_mmc_plat *plat = dev_get_platdata(dev);
+ return dwmci_bind(dev, &plat->mmc, &plat->cfg); +}
+static const struct udevice_id nexell_dwmmc_ids[] = { + { .compatible = "nexell,nexell-dwmmc" }, + { } +};
+U_BOOT_DRIVER(nexell_dwmmc_drv) = { + .name = "nexell_dwmmc", + .id = UCLASS_MMC, + .of_match = nexell_dwmmc_ids, + .ofdata_to_platdata = nexell_dwmmc_ofdata_to_platdata, + .ops = &dm_dwmci_ops, + .bind = nexell_dwmmc_bind, + .probe = nexell_dwmmc_probe, + .priv_auto_alloc_size = sizeof(struct nexell_dwmmc_priv), + .platdata_auto_alloc_size = sizeof(struct nexell_mmc_plat), +};