| Message ID | 1532340508-8749-1-git-send-email-zhang.chunyan@linaro.org |
|---|---|
| Headers | show |
| Series | mmc: add support for sdhci 4.0 | expand |
On 23/07/18 13:08, Chunyan Zhang wrote: > ADMA2 64-bit addressing support is divided into V3 mode and V4 mode. > So there are two kinds of descriptors for ADMA2 64-bit addressing > i.e. 96-bit Descriptor for V3 mode, and 128-bit Descriptor for V4 > mode. 128-bit Descriptor is aligned to 8-byte. > > For V4 mode, ADMA2 64-bit addressing is enabled via Host Control 2 > register. > > Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> > --- > drivers/mmc/host/sdhci.c | 90 ++++++++++++++++++++++++++++++++++-------------- > drivers/mmc/host/sdhci.h | 15 ++++++-- > 2 files changed, 78 insertions(+), 27 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 9cb17c0..ce71afa 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -271,6 +271,46 @@ static void sdhci_set_default_irqs(struct sdhci_host *host) > sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); > } > > +static void sdhci_config_dma(struct sdhci_host *host) > +{ > + u8 ctrl; > + u16 ctrl2; > + > + if (host->version < SDHCI_SPEC_200) > + return; > + > + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); > + > + /* > + * Always adjust the DMA selection as some controllers > + * (e.g. JMicron) can't do PIO properly when the selection > + * is ADMA. > + */ > + ctrl &= ~SDHCI_CTRL_DMA_MASK; > + if ((host->flags & SDHCI_REQ_USE_DMA) && if (!(host->flags & SDHCI_REQ_USE_DMA)) goto out; > + (host->flags & SDHCI_USE_ADMA)) > + ctrl |= SDHCI_CTRL_ADMA32; /* Note if DMA Select is zero then SDMA is selected */ if (host->flags & SDHCI_USE_ADMA) ctrl |= SDHCI_CTRL_ADMA32; > + > + if (host->flags & SDHCI_USE_64_BIT_DMA) { > + /* > + * If v4 mode, all supported DMA can be 64-bit addressing if > + * controller supports 64-bit system address, otherwise only > + * ADMA can support 64-bit addressing. > + */ > + if (host->v4_mode) { > + ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + ctrl2 |= SDHCI_CTRL_64BIT_ADDR; > + sdhci_writew(host, ctrl2, SDHCI_HOST_CONTROL2); > + } else { > + if ((host->flags & SDHCI_REQ_USE_DMA) && The 'else' and 'if' should be together i.e. } else if (host->flags & SDHCI_USE_ADMA) { /* * Don't need to undo SDHCI_CTRL_ADMA32 in order to set * SDHCI_CTRL_ADMA64. */ ctrl |= SDHCI_CTRL_ADMA64; } > + (host->flags & SDHCI_USE_ADMA)) > + ctrl |= SDHCI_CTRL_ADMA64; > + } > + } > + out: > + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); > +} > + > static void sdhci_init(struct sdhci_host *host, int soft) > { > struct mmc_host *mmc = host->mmc; > @@ -916,7 +956,6 @@ static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd) > > static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd) > { > - u8 ctrl; > struct mmc_data *data = cmd->data; > > host->data_timeout = 0; > @@ -1012,25 +1051,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd) > } > } > > - /* > - * Always adjust the DMA selection as some controllers > - * (e.g. JMicron) can't do PIO properly when the selection > - * is ADMA. > - */ > - if (host->version >= SDHCI_SPEC_200) { > - ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); > - ctrl &= ~SDHCI_CTRL_DMA_MASK; > - if ((host->flags & SDHCI_REQ_USE_DMA) && > - (host->flags & SDHCI_USE_ADMA)) { > - if (host->flags & SDHCI_USE_64_BIT_DMA) > - ctrl |= SDHCI_CTRL_ADMA64; > - else > - ctrl |= SDHCI_CTRL_ADMA32; > - } else { > - ctrl |= SDHCI_CTRL_SDMA; > - } > - sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); > - } > + sdhci_config_dma(host); > > if (!(host->flags & SDHCI_REQ_USE_DMA)) { > int flags; > @@ -3503,6 +3524,19 @@ static int sdhci_allocate_bounce_buffer(struct sdhci_host *host) > return 0; > } > > +static inline bool sdhci_can_64bit_dma(struct sdhci_host *host) > +{ > + /* > + * According to SD Host Controller spec v4.10, bit[27] added from > + * version 4.10 in Capabilities Register is used as 64-bit System > + * Address support for V4 mode. > + */ > + if (host->version >= SDHCI_SPEC_410 && host->v4_mode) > + return host->caps & SDHCI_CAN_64BIT_V4; > + > + return host->caps & SDHCI_CAN_64BIT; > +} > + > int sdhci_setup_host(struct sdhci_host *host) > { > struct mmc_host *mmc; > @@ -3539,7 +3573,7 @@ int sdhci_setup_host(struct sdhci_host *host) > > override_timeout_clk = host->timeout_clk; > > - if (host->version > SDHCI_SPEC_300) { > + if (host->version > SDHCI_SPEC_420) { Please make this and the addition of the SDHCI_SPEC_4xx defines a separate patch. > pr_err("%s: Unknown controller version (%d). You may experience problems.\n", > mmc_hostname(mmc), host->version); > } > @@ -3574,7 +3608,7 @@ int sdhci_setup_host(struct sdhci_host *host) > * SDHCI_QUIRK2_BROKEN_64_BIT_DMA must be left to the drivers to > * implement. > */ > - if (host->caps & SDHCI_CAN_64BIT) > + if (sdhci_can_64bit_dma(host)) > host->flags |= SDHCI_USE_64_BIT_DMA; > > if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) { > @@ -3608,8 +3642,8 @@ int sdhci_setup_host(struct sdhci_host *host) > */ > if (host->flags & SDHCI_USE_64_BIT_DMA) { > host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) * > - SDHCI_ADMA2_64_DESC_SZ; > - host->desc_sz = SDHCI_ADMA2_64_DESC_SZ; > + SDHCI_ADMA2_64_DESC_SZ(host); > + host->desc_sz = SDHCI_ADMA2_64_DESC_SZ(host); > } else { > host->adma_table_sz = (SDHCI_MAX_SEGS * 2 + 1) * > SDHCI_ADMA2_32_DESC_SZ; > @@ -3617,7 +3651,13 @@ int sdhci_setup_host(struct sdhci_host *host) > } > > host->align_buffer_sz = SDHCI_MAX_SEGS * SDHCI_ADMA2_ALIGN; > - buf = dma_alloc_coherent(mmc_dev(mmc), host->align_buffer_sz + > + /* > + * Host Controller Version 4.00 or later can support 128-bit > + * and 96-bit Descriptor for 64-bit addressing mode. 128-bit > + * Descriptor is for v4 mode, and high 32-bit of it is reserved > + * according to the specification v4.10. > + */ But the point is zalloc() lets us skip writing the reserved bits. How about: /* * Use zalloc to zero the reserved high 32-bits of 128-bit * descriptors so that they never need to be written. */ > + buf = dma_zalloc_coherent(mmc_dev(mmc), host->align_buffer_sz + > host->adma_table_sz, &dma, GFP_KERNEL); > if (!buf) { > pr_warn("%s: Unable to allocate ADMA buffers - falling back to standard DMA\n", > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 519d939..23318ff 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -185,6 +185,7 @@ > #define SDHCI_CTRL_EXEC_TUNING 0x0040 > #define SDHCI_CTRL_TUNED_CLK 0x0080 > #define SDHCI_CTRL_V4_MODE 0x1000 > +#define SDHCI_CTRL_64BIT_ADDR 0x2000 > #define SDHCI_CTRL_PRESET_VAL_ENABLE 0x8000 > > #define SDHCI_CAPABILITIES 0x40 > @@ -205,6 +206,7 @@ > #define SDHCI_CAN_VDD_330 0x01000000 > #define SDHCI_CAN_VDD_300 0x02000000 > #define SDHCI_CAN_VDD_180 0x04000000 > +#define SDHCI_CAN_64BIT_V4 0x08000000 > #define SDHCI_CAN_64BIT 0x10000000 > > #define SDHCI_SUPPORT_SDR50 0x00000001 > @@ -271,6 +273,9 @@ > #define SDHCI_SPEC_100 0 > #define SDHCI_SPEC_200 1 > #define SDHCI_SPEC_300 2 > +#define SDHCI_SPEC_400 3 > +#define SDHCI_SPEC_410 4 > +#define SDHCI_SPEC_420 5 > > /* > * End of controller registers. > @@ -306,8 +311,14 @@ struct sdhci_adma2_32_desc { > */ > #define SDHCI_ADMA2_DESC_ALIGN 8 > > -/* ADMA2 64-bit DMA descriptor size */ > -#define SDHCI_ADMA2_64_DESC_SZ 12 > +/* > + * ADMA2 64-bit DMA descriptor size > + * According to SD Host Controller spec v4.10, there are two kinds of > + * descriptors for 64-bit addressing mode: 96-bit Descriptor and 128-bit > + * Descriptor, if Host Version 4 Enable is set in the Host Control 2 > + * register, 128-bit Descriptor will be selected. > + */ > +#define SDHCI_ADMA2_64_DESC_SZ(host) ((host)->v4_mode ? 16 : 12) > > /* > * ADMA2 64-bit descriptor. Note 12-byte descriptor can't always be 8-byte > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Adrian, On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >> --- >> 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? > >> + 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? The test platform I used was just eMMC only, haven't found problem. 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31/07/18 10:04, Chunyan Zhang wrote: > Hi Adrian, > > On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>> --- >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 31/07/18 10:04, Chunyan Zhang wrote: >> Hi Adrian, >> >> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>>> --- >>>> 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? > >> >>> >>>> + 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. > >> >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31/07/18 11:36, Chunyan Zhang wrote: > On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote: >> On 31/07/18 10:04, Chunyan Zhang wrote: >>> Hi Adrian, >>> >>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>>>> --- >>>>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 31/07/18 11:36, Chunyan Zhang wrote: >> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> On 31/07/18 10:04, Chunyan Zhang wrote: >>>> Hi Adrian, >>>> >>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>>>>> --- >>>>>> 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? Seems that the argument is too big compared with block count, hardware think it is a super block count. More details of the error information: https://www.irccloud.com/pastebin/uYlVEUsP/ > >> >>> >>>> >>>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 31/07/18 11:36, Chunyan Zhang wrote: >> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> On 31/07/18 10:04, Chunyan Zhang wrote: >>>> Hi Adrian, >>>> >>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>>>>> --- >>>>>> 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 Ok. I will move this setting to sdhci_auto_cmd_select() for use_cmd23 is true. > >> >>> >>>> >>>>> >>>>>> + 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31/07/18 12:20, Chunyan Zhang wrote: > On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote: >> On 31/07/18 11:36, Chunyan Zhang wrote: >>> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>> On 31/07/18 10:04, Chunyan Zhang wrote: >>>>> Hi Adrian, >>>>> >>>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>>>>>> --- >>>>>>> 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? > > Seems that the argument is too big compared with block count, hardware > think it is a super block count. > > More details of the error information: > https://www.irccloud.com/pastebin/uYlVEUsP/ Does it work with auto-CMD23 instead of auto-CMD-auto-select? Does it work if the 16-bit block count register is used? Obviously, if we can't pass the CMD23 argument correctly then we can't use auto-CMD23. > >> >>> >>>> >>>>> >>>>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31 July 2018 at 17:36, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 31/07/18 12:20, Chunyan Zhang wrote: >> On 31 July 2018 at 16:56, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> On 31/07/18 11:36, Chunyan Zhang wrote: >>>> On 31 July 2018 at 16:05, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>>> On 31/07/18 10:04, Chunyan Zhang wrote: >>>>>> Hi Adrian, >>>>>> >>>>>> On 30 July 2018 at 21:06, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>>>>>>> --- >>>>>>>> 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? >> >> Seems that the argument is too big compared with block count, hardware >> think it is a super block count. >> >> More details of the error information: >> https://www.irccloud.com/pastebin/uYlVEUsP/ > > Does it work with auto-CMD23 instead of auto-CMD-auto-select? No, so long as SDHCI_32BIT_BLK_CNT was set with cmd->mrq->sbc->arg which is a big value, it would report error "I/O error while writing superblock" when mounting emmc. > Does it work if the 16-bit block count register is used? 16-bit block count register is Read Only on my board :-( > > Obviously, if we can't pass the CMD23 argument correctly then we can't use > auto-CMD23. On my board, if argument2 was limited to the maximum of 65535, then everything works, otherwise I would see an error "EXT4-fs (mmcblk0p28): I/O error while writing superblock". Haven't found the root cause. I will continue to look at the problem, any comments would be appreciated. Thanks, Chunyan > >> >>> >>>> >>>>> >>>>>> >>>>>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html