On 31/07/18 11:36, Chunyan Zhang wrote: > On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: >> 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. > > Got it, but how to know if the card supports CMD23? At the moment the only way of knowing is if a request is received with mrq->sbc > >> >>> >>>> >>>>> + 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 > > But setting argument for CMD23 will cover the block count value we set > before, that will lead mounting emmc to fail. Does it fail because it contains cmd->mrq->sbc->arg or does it fail because it gets written twice? > >> >>> >>> 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