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 <stefa...@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 <p...@nexell.co.kr>
> + *
> + * (C) Copyright 2019 Stefan Bosch <stefa...@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),
> +};
> 

Reply via email to