[U-Boot] [PATCH v3 06/11] mmc: update the Faraday FTSDC010 driver to fix performance issue

Andy Fleming
2013-05-03T22:35:11Z

On Fri, Apr 26, 2013 at 3:02 AM, Kuo-Jung Su <dantesu@gmail.com> wrote:

> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>
> Faraday FTSDC010 is a MMC/SD host controller.
> Although there is already a driver in current u-boot release,
> which is modified from eSHDC and contributed by Andes Tech.
>
> Its performance is too terrible on Faraday A36x SoC platforms,
> so I turn to implement this new version of driver which is
> 10+ times faster than the old one.
>
> It's carefully designed to be compatible to Andes chips,
> so it should be safe to replace it.
>
> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> CC: Andy Fleming <afleming@gmail.com>
>
>

> +
> + if (chip->acmd) {
> + cmd |= FTSDC010_CMD_APP_CMD;
> + chip->acmd = 0;
> + }
>

[...]

> + if (ret) {
> + debug("ftsdc010: cmd timeout (op code=%d)\n",
> + mmc_cmd->cmdidx);
> + } else if (mmc_cmd->cmdidx == MMC_CMD_APP_CMD) {
> + chip->acmd = 1;
> + }
>

Interesting. I've not seen any other devices that needed to have the sub
command of the APP command flagged as such. I can't think of a better way
to do this right now, so it's fine. Perhaps if there were other examples,
we could implement a separate callback for APP cmds? Or we could modify the
higher-level code to set a flag for the subcommands? I feel like
maintaining this kind of state shouldn't be necessary, but possibly this
hardware is strange.

> +static void ftsdc010_set_ios(struct mmc *mmc)
> +{
> + struct ftsdc010_chip *chip = mmc->priv;
> + struct ftsdc010_mmc __iomem *regs = chip->regs;
> +
> + ftsdc010_clkset(chip, mmc->clock);
> +
> + if (mmc->clock > 25000000)
> + setbits_le32(&regs->ccr, FTSDC010_CCR_CLK_HISPD);
> + else
> + clrbits_le32(&regs->ccr, FTSDC010_CCR_CLK_HISPD);
>

Why not put this part in ftsdc010_clkset, and set ccr all at once?

> +int ftsdc010_mmc_init(int devid)
> +{
>

[...]

> +
> + sprintf(mmc->name, "ftsdc010");
> + mmc->send_cmd = ftsdc010_request;
> + mmc->set_ios = ftsdc010_set_ios;
> + mmc->init = ftsdc010_init;
> +
> + switch ((readl(&regs->bwr) >> 3) & 3) {
>

Could we get named constants for those mask/shift numbers?

> + case 1:
> + mmc->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz
> + | MMC_MODE_4BIT;
> + break;
> + case 2:
>

Andy