On 31/07/18 10:04, Chunyan Zhang wrote: > Hi Adrian, > > On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: >> On 23/07/18 13:08, Chunyan Zhang wrote: >>> As SD Host Controller Specification v4.10 documents: >>> Host Controller Version 4.10 defines this "Auto CMD Auto Select" mode. >>> Selection of Auto CMD depends on setting of CMD23 Enable in the Host >>> Control 2 register which indicates whether card supports CMD23. If CMD23 >>> Enable =1, Auto CMD23 is used and if CMD23 Enable =0, Auto CMD12 is >>> used. In case of Version 4.10 or later, use of Auto CMD Auto Select is >>> recommended rather than use of Auto CMD12 Enable or Auto CMD23 >>> Enable. >>> >>> This patch add this new mode support. >>> >>> Signed-off-by: Chunyan Zhang <zhang.chunyan@xxxxxxxxxx> >>> --- >>> drivers/mmc/host/sdhci.c | 61 +++++++++++++++++++++++++++++++++++++++--------- >>> drivers/mmc/host/sdhci.h | 2 ++ >>> 2 files changed, 52 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index 5acea3d..5c60590 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -311,6 +311,23 @@ static void sdhci_config_dma(struct sdhci_host *host) >>> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); >>> } >>> >>> +static void sdhci_enable_cmd23(struct sdhci_host *host) >>> +{ >>> + u16 ctrl2; >>> + >>> + /* >>> + * This is used along with "Auto CMD Auto Select" feature, >>> + * which is introduced from v4.10, if card supports CMD23, >>> + * Auto CMD23 should be used instead of Auto CMD12. >>> + */ >>> + if (host->version >= SDHCI_SPEC_410 && >>> + (host->mmc->caps & MMC_CAP_CMD23)) { >> >> That is the host capability. It needs to be the card capability. > > Could you please elaborate the issue? > > I thought we're setting for host here. Do you mean we need to see the > card capability? Yes. SDHCI_TRNS_AUTO_SEL selects auto-CMD23 or auto-CMD12 based on this setting, so this must reflect the card's capability. > >> >>> + ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >>> + ctrl2 |= SDHCI_CMD23_ENABLE; >>> + sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2); >>> + } >>> +} >>> + >>> static void sdhci_init(struct sdhci_host *host, int soft) >>> { >>> struct mmc_host *mmc = host->mmc; >>> @@ -329,6 +346,8 @@ static void sdhci_init(struct sdhci_host *host, int soft) >>> host->clock = 0; >>> mmc->ops->set_ios(mmc, &mmc->ios); >>> } >>> + >>> + sdhci_enable_cmd23(host); >>> } >>> >>> static void sdhci_reinit(struct sdhci_host *host) >>> @@ -1093,6 +1112,36 @@ static inline bool sdhci_auto_cmd12(struct sdhci_host *host, >>> !mrq->cap_cmd_during_tfr; >>> } >>> >>> +static inline void sdhci_auto_cmd_select(struct sdhci_host *host, >>> + struct mmc_command *cmd, >>> + u16 *mode) >>> +{ >>> + bool use_cmd12 = sdhci_auto_cmd12(host, cmd->mrq) && >>> + (cmd->opcode != SD_IO_RW_EXTENDED); >>> + bool use_cmd23 = cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23); >>> + >>> + /* >>> + * In case of Version 4.10 or later, use of 'Auto CMD Auto >>> + * Select' is recommended rather than use of 'Auto CMD12 >>> + * Enable' or 'Auto CMD23 Enable'. >>> + */ >>> + if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) { >>> + *mode |= SDHCI_TRNS_AUTO_SEL; >> >> As noted in patch 4, the CMD23 argument is not just the block count for eMMC. > > Probably I haven't got your point... > >>From what I understand, auto_sel mode doesn't need argument2. Doesn't > this work for eMMC? CMD23 always needs an argument > > The test platform I used was just eMMC only, haven't found problem. Because the bits that are missing from the CMD23 argument (reliable write, context id, etc) will not make the command fail. > > Thanks, > Chunyan > >> >>> + return; >>> + } >>> + >>> + /* >>> + * If we are sending CMD23, CMD12 never gets sent >>> + * on successful completion (so no Auto-CMD12). >>> + */ >>> + if (use_cmd12) { >>> + *mode |= SDHCI_TRNS_AUTO_CMD12; >>> + } else if (use_cmd23) { >>> + *mode |= SDHCI_TRNS_AUTO_CMD23; >>> + sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2); >>> + } >>> +} >>> + >>> static void sdhci_set_transfer_mode(struct sdhci_host *host, >>> struct mmc_command *cmd) >>> { >>> @@ -1119,17 +1168,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host, >>> >>> if (mmc_op_multi(cmd->opcode) || data->blocks > 1) { >>> mode = SDHCI_TRNS_BLK_CNT_EN | SDHCI_TRNS_MULTI; >>> - /* >>> - * If we are sending CMD23, CMD12 never gets sent >>> - * on successful completion (so no Auto-CMD12). >>> - */ >>> - if (sdhci_auto_cmd12(host, cmd->mrq) && >>> - (cmd->opcode != SD_IO_RW_EXTENDED)) >>> - mode |= SDHCI_TRNS_AUTO_CMD12; >>> - else if (cmd->mrq->sbc && (host->flags & SDHCI_AUTO_CMD23)) { >>> - mode |= SDHCI_TRNS_AUTO_CMD23; >>> - sdhci_writel(host, cmd->mrq->sbc->arg, SDHCI_ARGUMENT2); >>> - } >>> + sdhci_auto_cmd_select(host, cmd, &mode); >>> } >>> >>> if (data->flags & MMC_DATA_READ) >>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >>> index 81aae07..a8f4ec2 100644 >>> --- a/drivers/mmc/host/sdhci.h >>> +++ b/drivers/mmc/host/sdhci.h >>> @@ -42,6 +42,7 @@ >>> #define SDHCI_TRNS_BLK_CNT_EN 0x02 >>> #define SDHCI_TRNS_AUTO_CMD12 0x04 >>> #define SDHCI_TRNS_AUTO_CMD23 0x08 >>> +#define SDHCI_TRNS_AUTO_SEL 0x0C >>> #define SDHCI_TRNS_READ 0x10 >>> #define SDHCI_TRNS_MULTI 0x20 >>> >>> @@ -185,6 +186,7 @@ >>> #define SDHCI_CTRL_DRV_TYPE_D 0x0030 >>> #define SDHCI_CTRL_EXEC_TUNING 0x0040 >>> #define SDHCI_CTRL_TUNED_CLK 0x0080 >>> +#define SDHCI_CMD23_ENABLE 0x0800 >>> #define SDHCI_CTRL_V4_MODE 0x1000 >>> #define SDHCI_CTRL_64BIT_ADDR 0x2000 >>> #define SDHCI_CTRL_PRESET_VAL_ENABLE 0x8000 >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html