[U-Boot] [PATCH 1/3] nds32: mmc: Support ftsdc010 DM.

rick at andestech.com rick at andestech.com
Thu Jun 1 06:37:29 UTC 2017


Hi Jaehoon

Thank you for reviewing!

I will modify as your advise .

But only the ftsdc010_wait,
If I don't modify as that, SD card read will become very slow even like system hang when execute fatload .
I trace and find it was slow down in /lib/time.c .
Maybe our target platform is only 60Mhz and highlight this problem.
But if timer is not in dm flow. It will not have this problem.

That is why I touch this function.

Rick

-----Original Message-----
From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
Sent: Wednesday, May 31, 2017 2:50 PM
To: Open Source Project uboot; u-boot at lists.denx.de; wd at denx.de; dzu at denx.de
Cc: Rick Jian-Zhi Chen(陳建志)
Subject: Re: [PATCH 1/3] nds32: mmc: Support ftsdc010 DM.

On 05/31/2017 10:36 AM, Andes wrote:
> From: rick <rick at andestech.com>
>
> Support Andestech ftsdc010 SD/MMC device tree flow on AG101P/AE3XX
> platforms.
>
> Signed-off-by: rick <rick at andestech.com>
> ---
>  drivers/mmc/Kconfig        |   12 ++++
>  drivers/mmc/Makefile       |    1 +
>  drivers/mmc/ftsdc010_mci.c |  140 ++++++++++++++++++++++++++++++++++----------
>  drivers/mmc/ftsdc010_mci.h |   54 +++++++++++++++++
>  drivers/mmc/nds32_mmc.c    |  139 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 316 insertions(+), 30 deletions(-)  create mode
> 100644 drivers/mmc/ftsdc010_mci.h  create mode 100644
> drivers/mmc/nds32_mmc.c
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index
> 0dd4443..ca1376a 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -377,6 +377,18 @@ config GENERIC_ATMEL_MCI
>         the SD Memory Card Specification V2.0, the SDIO V2.0 specification
>         and CE-ATA V1.1.
>
> +config MMC_NDS32
> +     bool "Andestech SD/MMC controller support"
> +     depends on DM_MMC && OF_CONTROL
> +     help
> +       This enables support for the Andestech SD/MMM controller, which is
> +       based on Faraday IP.

If my understanding is correct, nds32_mmc has the dependency to ftsdc010_mci file.

> +
> +config FTSDC010
> +     bool "Ftsdc010 SD/MMC controller Support"
> +     help
> +       This SD/MMC controller is present in Andestech SoCs which is based on Faraday IP.
> +
>  endif
>
>  config TEGRA124_MMC_DISABLE_EXT_LOOPBACK diff --git
> a/drivers/mmc/Makefile b/drivers/mmc/Makefile index a078649..08a552a
> 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_S3C_SDI) += s3c_sdi.o
>  obj-$(CONFIG_MMC_SANDBOX)            += sandbox_mmc.o
>  obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>  obj-$(CONFIG_SH_SDHI) += sh_sdhi.o
> +obj-$(CONFIG_MMC_NDS32) += nds32_mmc.o
>
>  # SDHCI
>  obj-$(CONFIG_MMC_SDHCI)                      += sdhci.o
> diff --git a/drivers/mmc/ftsdc010_mci.c b/drivers/mmc/ftsdc010_mci.c
> index 652a718..ec0bc6b 100644
> --- a/drivers/mmc/ftsdc010_mci.c
> +++ b/drivers/mmc/ftsdc010_mci.c
> @@ -12,24 +12,15 @@
>  #include <part.h>
>  #include <mmc.h>
>
> -#include <asm/io.h>
> +#include <linux/io.h>
>  #include <linux/errno.h>
>  #include <asm/byteorder.h>
>  #include <faraday/ftsdc010.h>
> +#include "ftsdc010_mci.h"
>
>  #define CFG_CMD_TIMEOUT (CONFIG_SYS_HZ >> 4) /* 250 ms */  #define
> CFG_RST_TIMEOUT CONFIG_SYS_HZ /* 1 sec reset timeout */
>
> -struct ftsdc010_chip {
> -     void __iomem *regs;
> -     uint32_t wprot;   /* write protected (locked) */
> -     uint32_t rate;    /* actual SD clock in Hz */
> -     uint32_t sclk;    /* FTSDC010 source clock in Hz */
> -     uint32_t fifo;    /* fifo depth in bytes */
> -     uint32_t acmd;
> -     struct mmc_config cfg;  /* mmc configuration */
> -};
> -
>  static inline int ftsdc010_send_cmd(struct mmc *mmc, struct mmc_cmd
> *mmc_cmd)  {
>       struct ftsdc010_chip *chip = mmc->priv; @@ -127,9 +118,8 @@ static
> void ftsdc010_clkset(struct mmc *mmc, uint32_t rate)  static int
> ftsdc010_wait(struct ftsdc010_mmc __iomem *regs, uint32_t mask)  {
>       int ret = -ETIMEDOUT;
> -     uint32_t st, ts;
> -
> -     for (ts = get_timer(0); get_timer(ts) < CFG_CMD_TIMEOUT; ) {
> +     uint32_t st, timeout = 10000000;
> +     while (timeout--) {

Why did you touch this code?

>               st = readl(&regs->status);
>               if (!(st & mask))
>                       continue;
> @@ -147,10 +137,16 @@ static int ftsdc010_wait(struct ftsdc010_mmc
> __iomem *regs, uint32_t mask)
>  /*
>   * u-boot mmc api
>   */
> -
> +#ifdef CONFIG_DM_MMC_OPS
> +static int ftsdc010_request(struct udevice *dev, struct mmc_cmd *cmd,
> +     struct mmc_data *data)
> +{
> +     struct mmc *mmc = mmc_get_mmc_dev(dev); #else
>  static int ftsdc010_request(struct mmc *mmc, struct mmc_cmd *cmd,
>       struct mmc_data *data)
>  {
> +#endif
>       int ret = -EOPNOTSUPP;
>       uint32_t len = 0;
>       struct ftsdc010_chip *chip = mmc->priv; @@ -251,8 +247,14 @@ static
> int ftsdc010_request(struct mmc *mmc, struct mmc_cmd *cmd,
>       return ret;
>  }
>
> +#ifdef CONFIG_DM_MMC_OPS
> +static int ftsdc010_set_ios(struct udevice *dev) {
> +     struct mmc *mmc = mmc_get_mmc_dev(dev); #else
>  static int ftsdc010_set_ios(struct mmc *mmc)  {
> +#endif
>       struct ftsdc010_chip *chip = mmc->priv;
>       struct ftsdc010_mmc __iomem *regs = chip->regs;
>
> @@ -274,20 +276,46 @@ static int ftsdc010_set_ios(struct mmc *mmc)
>       return 0;
>  }
>
> -static int ftsdc010_init(struct mmc *mmc)
> +#ifdef CONFIG_DM_MMC_OPS
> +static int ftsdc010_get_cd(struct udevice *dev)
>  {
> +     struct mmc *mmc = mmc_get_mmc_dev(dev); #else static int
> +ftsdc010_getcd(struct mmc *mmc)

Use "ftsdc010_get_cd"

> +{
> +#endif
>       struct ftsdc010_chip *chip = mmc->priv;
>       struct ftsdc010_mmc __iomem *regs = chip->regs;
> -     uint32_t ts;
> -
>       if (readl(&regs->status) & FTSDC010_STATUS_CARD_DETECT)
> -             return -ENOMEDIUM;
> +             return 0;
> +
> +     return 1;

Can use the just "return !(readl() & FTDC010_...);"

> +}
>
> +#ifdef CONFIG_DM_MMC_OPS
> +static int ftsdc010_get_wp(struct udevice *dev) {
> +     struct mmc *mmc = mmc_get_mmc_dev(dev); #else static int
> +ftsdc010_getwp(struct mmc *mmc)

ftsdc010_get_wp()

> +{
> +#endif
> +     struct ftsdc010_chip *chip = mmc->priv;
> +     struct ftsdc010_mmc __iomem *regs = chip->regs;
>       if (readl(&regs->status) & FTSDC010_STATUS_WRITE_PROT) {
>               printf("ftsdc010: write protected\n");
>               chip->wprot = 1;
>       }
>
> +     return 0;
> +}
> +
> +static int ftsdc010_init(struct mmc *mmc) {
> +     struct ftsdc010_chip *chip = mmc->priv;
> +     struct ftsdc010_mmc __iomem *regs = chip->regs;
> +     uint32_t ts;
> +
>       chip->fifo = (readl(&regs->feature) & 0xff) << 2;
>
>       /* 1. chip reset */
> @@ -311,11 +339,70 @@ static int ftsdc010_init(struct mmc *mmc)
>       return 0;
>  }
>
> +#ifdef CONFIG_DM_MMC_OPS
> +int ftsdc010_probe(struct udevice *dev) {
> +     struct mmc *mmc = mmc_get_mmc_dev(dev);
> +     return ftsdc010_init(mmc);
> +}
> +
> +const struct dm_mmc_ops dm_ftsdc010_ops = {
> +     .send_cmd       = ftsdc010_request,
> +     .set_ios        = ftsdc010_set_ios,
> +     .get_cd         = ftsdc010_get_cd,
> +     .get_wp         = ftsdc010_get_wp,
> +};
> +
> +#else
>  static const struct mmc_ops ftsdc010_ops = {
>       .send_cmd       = ftsdc010_request,
>       .set_ios        = ftsdc010_set_ios,
> +     .getcd          = ftsdc010_getcd,
> +     .getwp          = ftsdc010_getwp,
>       .init           = ftsdc010_init,
>  };
> +#endif
> +
> +void ftsdc_setup_cfg(struct mmc_config *cfg, const char *name, int buswidth,
> +                  uint caps, u32 max_clk, u32 min_clk) {
> +     cfg->name = name;
> +     cfg->f_min = min_clk;
> +     cfg->f_max = max_clk;
> +     cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
> +     cfg->host_caps = caps;
> +     if (buswidth == 8) {
> +             cfg->host_caps |= MMC_MODE_8BIT;
> +             cfg->host_caps &= ~MMC_MODE_4BIT;
> +     } else {
> +             cfg->host_caps |= MMC_MODE_4BIT;
> +             cfg->host_caps &= ~MMC_MODE_8BIT;
> +     }
> +     cfg->host_caps |= MMC_MODE_HS | MMC_MODE_HS_52MHz;
> +     cfg->part_type = PART_TYPE_DOS;
> +     cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT; }
> +
> +void set_bus_width(struct ftsdc010_mmc __iomem *regs, struct
> +mmc_config *cfg) {
> +     switch (readl(&regs->bwr) & FTSDC010_BWR_CAPS_MASK) {
> +     case FTSDC010_BWR_CAPS_4BIT:
> +             cfg->host_caps |= MMC_MODE_4BIT;
> +             break;
> +     case FTSDC010_BWR_CAPS_8BIT:
> +             cfg->host_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
> +             break;
> +     default:
> +             break;
> +     }
> +}
> +
> +#ifdef CONFIG_BLK
> +int ftsdc010_bind(struct udevice *dev, struct mmc *mmc, struct
> +mmc_config *cfg) {
> +     return mmc_bind(dev, mmc, cfg);
> +}
> +#else
>
>  int ftsdc010_mmc_init(int devid)
>  {
> @@ -345,19 +432,11 @@ int ftsdc010_mmc_init(int devid)  #endif
>
>       chip->cfg.name = "ftsdc010";
> +#ifndef CONFIG_DM_MMC_OPS
>       chip->cfg.ops = &ftsdc010_ops;
> +#endif
>       chip->cfg.host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz;
> -     switch (readl(&regs->bwr) & FTSDC010_BWR_CAPS_MASK) {
> -     case FTSDC010_BWR_CAPS_4BIT:
> -             chip->cfg.host_caps |= MMC_MODE_4BIT;
> -             break;
> -     case FTSDC010_BWR_CAPS_8BIT:
> -             chip->cfg.host_caps |= MMC_MODE_4BIT | MMC_MODE_8BIT;
> -             break;
> -     default:
> -             break;
> -     }
> -
> +     set_bus_width(regs , &chip->cfg);
>       chip->cfg.voltages  = MMC_VDD_32_33 | MMC_VDD_33_34;
>       chip->cfg.f_max     = chip->sclk / 2;
>       chip->cfg.f_min     = chip->sclk / 0x100;
> @@ -373,3 +452,4 @@ int ftsdc010_mmc_init(int devid)
>
>       return 0;
>  }

The above codes can be split to fdtsc010 patch..

> +#endif
> diff --git a/drivers/mmc/ftsdc010_mci.h b/drivers/mmc/ftsdc010_mci.h
> new file mode 100644 index 0000000..f74015e
> --- /dev/null
> +++ b/drivers/mmc/ftsdc010_mci.h
> @@ -0,0 +1,54 @@
> +/*
> + * Faraday FTSDC010 Secure Digital Memory Card Host Controller
> + *
> + * Copyright (C) 2011 Andes Technology Corporation
> + * Macpaul Lin, Andes Technology Corporation <macpaul at andestech.com>
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +#include <mmc.h>
> +
> +#ifndef __FTSDC010_MCI_H
> +#define __FTSDC010_MCI_H
> +
> +struct ftsdc010_chip {
> +     void __iomem *regs;
> +     uint32_t wprot;   /* write protected (locked) */
> +     uint32_t rate;    /* actual SD clock in Hz */
> +     uint32_t sclk;    /* FTSDC010 source clock in Hz */
> +     uint32_t fifo;    /* fifo depth in bytes */
> +     uint32_t acmd;
> +     struct mmc_config cfg;  /* mmc configuration */
> +     const char *name;
> +     void *ioaddr;
> +     unsigned int quirks;

there is no quirks usage.

> +     unsigned int caps;
> +     unsigned int version;
> +     unsigned int clock;
> +     unsigned int bus_hz;
> +     unsigned int div;
> +     int dev_index;
> +     int dev_id;
> +     int buswidth;
> +     u32 fifoth_val;
> +     struct mmc *mmc;
> +     void *priv;
> +     bool fifo_mode;
> +};
> +
> +
> +#ifdef CONFIG_DM_MMC_OPS
> +/* Export the operations to drivers */ int ftsdc010_probe(struct
> +udevice *dev); extern const struct dm_mmc_ops dm_ftsdc010_ops; #endif
> +void ftsdc_setup_cfg(struct mmc_config *cfg, const char *name, int buswidth,
> +                  uint caps, u32 max_clk, u32 min_clk); void
> +set_bus_width(struct ftsdc010_mmc __iomem *regs, struct mmc_config
> +*cfg);
> +
> +#ifdef CONFIG_BLK
> +int ftsdc010_bind(struct udevice *dev, struct mmc *mmc, struct
> +mmc_config *cfg); #endif
> +
> +
> +#endif /* __FTSDC010_MCI_H */
> diff --git a/drivers/mmc/nds32_mmc.c b/drivers/mmc/nds32_mmc.c new
> file mode 100644 index 0000000..ec25fe9
> --- /dev/null
> +++ b/drivers/mmc/nds32_mmc.c
> @@ -0,0 +1,139 @@
> +/*
> + * Andestech ATFSDC010 SD/MMC driver
> + *
> + * (C) Copyright 2016
> + * Rick Chen, NDS32 Software Engineering, rick at andestech.com

Not 2017?

> +
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <dt-structs.h>
> +#include <faraday/ftsdc010.h>
> +#include <errno.h>
> +#include <mapmem.h>
> +#include <pwrseq.h>
> +#include <syscon.h>
> +#include <mmc.h>
> +#include <linux/err.h>

Orderring.

> +#include "ftsdc010_mci.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +struct nds_mmc {
> +     fdt32_t         bus_width;
> +     bool            cap_mmc_highspeed;
> +     bool            cap_sd_highspeed;
> +     fdt32_t         card_detect_delay;
> +     fdt32_t         clock_freq_min_max[2];
> +     struct phandle_2_cell   clocks[4];
> +     bool            disable_wp;
> +     fdt32_t         fifo_depth;
> +     fdt32_t         interrupts[3];
> +     fdt32_t         num_slots;
> +     fdt32_t         reg[2];
> +     fdt32_t         vmmc_supply;

Remove the unused members.

> +};
> +#endif
> +
> +struct nds_mmc_plat {
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +     struct nds_mmc dtplat;
> +#endif
> +     struct mmc_config cfg;
> +     struct mmc mmc;
> +};
> +
> +struct ftsdc_priv {
> +     struct clk clk;
> +     struct ftsdc010_chip chip;
> +     int fifo_depth;
> +     bool fifo_mode;
> +     u32 minmax[2];
> +};
> +
> +static int nds32_mmc_ofdata_to_platdata(struct udevice *dev) { #if
> +!CONFIG_IS_ENABLED(OF_PLATDATA)
> +     struct ftsdc_priv *priv = dev_get_priv(dev);
> +     struct ftsdc010_chip *chip = &priv->chip;
> +     chip->name = dev->name;
> +     chip->ioaddr = (void *)dev_get_addr(dev);
> +     chip->buswidth = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +                                     "bus-width", 4);
> +     chip->priv = dev;
> +     /* use non-removeable as sdcard and emmc as judgement */
> +     if (fdtdec_get_bool(gd->fdt_blob, dev->of_offset, "non-removable"))
> +             chip->dev_index = 0;
> +     else
> +             chip->dev_index = 1;

Not acceptable this..non-removable can't judge whether eMMC or not.
Use the mmc alias..

> +
> +     priv->fifo_depth = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
> +                                 "fifo-depth", 0);
> +     priv->fifo_mode = fdtdec_get_bool(gd->fdt_blob, dev->of_offset,
> +                                       "fifo-mode");
> +     if (fdtdec_get_int_array(gd->fdt_blob, dev->of_offset,
> +                              "clock-freq-min-max", priv->minmax, 2))
> +             return -EINVAL;

Are you sure "return -EINVAL"?
Is there the default value?

> +#endif
> +     chip->sclk = priv->minmax[1];
> +     chip->regs = chip->ioaddr;
> +     return 0;
> +}
> +
> +static int nds32_mmc_probe(struct udevice *dev) {
> +     struct nds_mmc_plat *plat = dev_get_platdata(dev);
> +     struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +     struct ftsdc_priv *priv = dev_get_priv(dev);
> +     struct ftsdc010_chip *chip = &priv->chip;
> +     struct udevice *pwr_dev __maybe_unused; #if
> +CONFIG_IS_ENABLED(OF_PLATDATA)
> +     int ret;
> +     struct nds_mmc *dtplat = &plat->dtplat;
> +     chip->name = dev->name;
> +     chip->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
> +     chip->buswidth = dtplat->bus_width;
> +     chip->priv = dev;
> +     chip->dev_index = 0;

At above,, you are setting to dev_index with "non-removable"..

> +     priv->fifo_depth = dtplat->fifo_depth;
> +     priv->fifo_mode = 0;

Remove the duplicated and unnecessary settings.

> +     memcpy(priv->minmax, dtplat->clock_freq_min_max, sizeof(priv->minmax));
> +     ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
> +     if (ret < 0)
> +             return ret;
> +#endif
> +     ftsdc_setup_cfg(&plat->cfg, dev->name, chip->buswidth, chip->caps,
> +                     priv->minmax[1] , priv->minmax[0]);
> +     chip->mmc = &plat->mmc;
> +     chip->mmc->priv = &priv->chip;
> +     chip->mmc->dev = dev;
> +     upriv->mmc = chip->mmc;
> +     return ftsdc010_probe(dev);
> +}
> +
> +static int nds32_mmc_bind(struct udevice *dev) {
> +     struct nds_mmc_plat *plat = dev_get_platdata(dev);
> +     return ftsdc010_bind(dev, &plat->mmc, &plat->cfg); }
> +
> +static const struct udevice_id nds32_mmc_ids[] = {
> +     { .compatible = "andestech,atsdc010" },
> +     { }
> +};
> +
> +U_BOOT_DRIVER(nds32_mmc_drv) = {
> +     .name           = "nds32_mmc",
> +     .id             = UCLASS_MMC,
> +     .of_match       = nds32_mmc_ids,
> +     .ofdata_to_platdata = nds32_mmc_ofdata_to_platdata,
> +     .ops            = &dm_ftsdc010_ops,
> +     .bind           = nds32_mmc_bind,
> +     .probe          = nds32_mmc_probe,
> +     .priv_auto_alloc_size = sizeof(struct ftsdc_priv),
> +     .platdata_auto_alloc_size = sizeof(struct nds_mmc_plat), };
>

CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.


More information about the U-Boot mailing list