Re: [RFC PATCH v2 05/13] mmc: add nexell driver

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.

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?

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.

• }
• debug("\n");

+}

+static void nx_dw_mmc_clksel(struct dwmci_host *host) +{

• u32 val;

+#ifdef CONFIG_BOOST_MMC

What is BOOST_MMC?

• 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?

• 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 ???

• 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();

• 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.

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),

+};