> -----Original Message----- > From: Nath, Arindam [mailto:Arindam.Nath@xxxxxxx] > Sent: Tuesday, March 15, 2011 5:05 PM > To: Subhash Jadavani; cjb@xxxxxxxxxx > Cc: zhangfei.gao@xxxxxxxxx; prakity@xxxxxxxxxxx; linux- > mmc@xxxxxxxxxxxxxxx; Su, Henry; Lu, Aaron; anath.amd@xxxxxxxxx > Subject: RE: [PATCH v2 01/12] mmc: sdhci: add support for auto CMD23 > > Hi Subhash, > > > > -----Original Message----- > > From: Subhash Jadavani [mailto:subhashj@xxxxxxxxxxxxxx] > > Sent: Tuesday, March 15, 2011 4:54 PM > > To: Nath, Arindam; cjb@xxxxxxxxxx > > Cc: zhangfei.gao@xxxxxxxxx; prakity@xxxxxxxxxxx; linux- > > mmc@xxxxxxxxxxxxxxx; Su, Henry; Lu, Aaron; anath.amd@xxxxxxxxx > > Subject: RE: [PATCH v2 01/12] mmc: sdhci: add support for auto CMD23 > > > > Arindam, > > > > Capability to send CMD23 automatically is something specific to your > > controller. > > Auto CMD23 is different from ACMD23. Auto CMD23 is a feature added into > Host Controller Spec v3.00, and the code follows the procedure to check > the 4 preconditions required to enable Host to send CMD23 > automatically. I had similar discussion on the community with Arnd few > weeks back, and we agreed that this should be part of controller > specific code. I am not talking about ACMD23 which is SET_WR_BLK_ERASE_COUNT. I am talking about newly added CMD23 (SET_BLOCK_COUNT) command in SD3.0. Shouldn't the logic to send this command be part of generic mmc layers (block/core) rather than at host controller driver level. It's good that your controller have the hw capability to send the CMD23 automatically but what about for controllers which doesn't have this type of capability available? Then it has to rely on the block/core layer to send the CMD23 before CMD19/CMD25. I think this should be part of block/core layer functionality in order to be SD3.01 spec compliant. > > Thanks, > Arindam > > > CMD23 (SET_BLOCK_COUNT) is a new command added in SD3.0 spec and > > mandatory > > for SDR104 mode. Why are you not adding this command in core/block > > layer > > before sending multi byte read command to host controller driver? > > > > Regards, > > Subhash > > > > > -----Original Message----- > > > From: linux-mmc-owner@xxxxxxxxxxxxxxx [mailto:linux-mmc- > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Arindam Nath > > > Sent: Friday, March 04, 2011 5:03 PM > > > To: cjb@xxxxxxxxxx > > > Cc: zhangfei.gao@xxxxxxxxx; prakity@xxxxxxxxxxx; > > > subhashj@xxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; > henry.su@xxxxxxx; > > > aaron.lu@xxxxxxx; anath.amd@xxxxxxxxx; Arindam Nath > > > Subject: [PATCH v2 01/12] mmc: sdhci: add support for auto CMD23 > > > > > > Host Controller v3.00 and later support Auto CMD23 in the Transfer > > > Mode register. Since Auto CMD23 can be used with or without DMA, > > > and if used with DMA, it should _only_ be ADMA, we check against > > > SDHCI_USE_SDMA not being set. This flag is reset when > SDHCI_USE_ADMA > > > is set. > > > > > > A new definition for SDHCI_ARGUMENT2 register has been added in > v3.00 > > > spec, which is the same as SDHCI_DMA_ADDRESS. We program the block > > > count for CMD23 in SDHCI_ARGUMENT2, so we don't need CMD12 to stop > > > multiple block transfers. But during error recovery procedure, we > > will > > > need to send Abort command, so we use a variable saved_abort_cmd to > > > save the stop command to be used later. > > > > > > Two bits are added to SCR register as per the Physical Layer Spec > > > v3.01, > > > which specify whether the card supports CMD20 and/or CMD23. We use > > this > > > as one of the conditions to decide whether to enable Auto CMD23 or > > not. > > > > > > Signed-off-by: Arindam Nath <arindam.nath@xxxxxxx> > > > --- > > > drivers/mmc/core/sd.c | 6 ++++ > > > drivers/mmc/host/sdhci.c | 69 > > > +++++++++++++++++++++++++++++++++++++++++--- > > > drivers/mmc/host/sdhci.h | 2 + > > > include/linux/mmc/card.h | 4 ++ > > > include/linux/mmc/sd.h | 2 +- > > > include/linux/mmc/sdhci.h | 2 + > > > 6 files changed, 79 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > > > index d18c32b..b3f8a3c 100644 > > > --- a/drivers/mmc/core/sd.c > > > +++ b/drivers/mmc/core/sd.c > > > @@ -188,6 +188,12 @@ static int mmc_decode_scr(struct mmc_card > *card) > > > > > > scr->sda_vsn = UNSTUFF_BITS(resp, 56, 4); > > > scr->bus_widths = UNSTUFF_BITS(resp, 48, 4); > > > + if (scr->sda_vsn == SCR_SPEC_VER_2) { > > > + /* Check if Physical Layer Spec v3.0 is supported*/ > > > + scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1); > > > + if (scr->sda_spec3) > > > + scr->cmd_support = UNSTUFF_BITS(resp, 32, 2); > > > + } > > > > > > if (UNSTUFF_BITS(resp, 55, 1)) > > > card->erased_byte = 0xFF; > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > > index 9e15f41..8914a25 100644 > > > --- a/drivers/mmc/host/sdhci.c > > > +++ b/drivers/mmc/host/sdhci.c > > > @@ -25,6 +25,7 @@ > > > > > > #include <linux/mmc/mmc.h> > > > #include <linux/mmc/host.h> > > > +#include <linux/mmc/card.h> > > > > > > #include "sdhci.h" > > > > > > @@ -812,6 +813,30 @@ static void sdhci_prepare_data(struct > sdhci_host > > > *host, struct mmc_data *data) > > > sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT); > > > } > > > > > > +/* > > > + * Does the Host Controller support Auto CMD23? > > > + * > > > + * There are four preconditions for Auto CMD23 to be supported: > > > + * 1. Host Controller v3.00 or later > > > + * 2. Card supports CMD23 > > > + * 3. If DMA is used, it should be ADMA > > > + * 4. Only when CMD18 or CMD25 is issued > > > + */ > > > +static int sdhci_host_auto_cmd23_supported(struct sdhci_host > *host, > > > + struct mmc_request *mrq) > > > +{ > > > + struct mmc_card *card = host->mmc->card; > > > + > > > + if ((host->version >= SDHCI_SPEC_300) && > > > + card && (card->scr.cmd_support & SD_SCR_CMD23_SUPPORT) && > > > + !(host->flags & SDHCI_USE_SDMA) && > > > + ((mrq->cmd->opcode == MMC_READ_MULTIPLE_BLOCK) || > > > + (mrq->cmd->opcode == MMC_WRITE_MULTIPLE_BLOCK))) > > > + return 1; > > > + > > > + return 0; > > > +} > > > + > > > static void sdhci_set_transfer_mode(struct sdhci_host *host, > > > struct mmc_data *data) > > > { > > > @@ -825,10 +850,21 @@ static void sdhci_set_transfer_mode(struct > > > sdhci_host *host, > > > mode = SDHCI_TRNS_BLK_CNT_EN; > > > if (data->blocks > 1) { > > > if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) > > > - mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12; > > > - else > > > - mode |= SDHCI_TRNS_MULTI; > > > + mode |= SDHCI_TRNS_ACMD12; > > > + else if (sdhci_host_auto_cmd23_supported(host, host->mrq)) > > > { > > > + /* > > > + * Host Controller v3.00 can automatically send > > CMD23 > > > + * before CMD18 or CMD25. For that, we need to > > enable > > > + * Auto CMD23 in the Transfer Mode register and > > > + * program the block count in Argument 2 register. > > > + */ > > > + mode |= SDHCI_TRNS_AUTO_CMD23; > > > + sdhci_writel(host, data->blocks, SDHCI_ARGUMENT2); > > > + } > > > + > > > + mode |= SDHCI_TRNS_MULTI; > > > } > > > + > > > if (data->flags & MMC_DATA_READ) > > > mode |= SDHCI_TRNS_READ; > > > if (host->flags & SDHCI_REQ_USE_DMA) > > > @@ -1131,6 +1167,23 @@ static void sdhci_request(struct mmc_host > > *mmc, > > > struct mmc_request *mrq) > > > #ifndef SDHCI_USE_LEDS_CLASS > > > sdhci_activate_led(host); > > > #endif > > > + /* > > > + * Since the block count for CMD23 has already been specified in > > > the > > > + * Argument 2 register, we don't need CMD12 to stop multiple > > > block > > > + * transfers. > > > + */ > > > + if (sdhci_host_auto_cmd23_supported(host, mrq)) { > > > + if (mrq->stop) { > > > + /* > > > + * Save the stop command here to be used during > > > + * error recovery > > > + */ > > > + host->saved_abort_cmd = mrq->data->stop; > > > + mrq->data->stop = NULL; > > > + mrq->stop = NULL; > > > + } > > > + } > > > + > > > if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) { > > > if (mrq->stop) { > > > mrq->data->stop = NULL; > > > @@ -1396,6 +1449,7 @@ static void sdhci_timeout_timer(unsigned long > > > data) > > > > > > if (host->data) { > > > host->data->error = -ETIMEDOUT; > > > + host->data->stop = host->saved_abort_cmd; > > > sdhci_finish_data(host); > > > } else { > > > if (host->cmd) > > > @@ -1534,9 +1588,10 @@ static void sdhci_data_irq(struct sdhci_host > > > *host, u32 intmask) > > > host->data->error = -EIO; > > > } > > > > > > - if (host->data->error) > > > + if (host->data->error) { > > > + host->data->stop = host->saved_abort_cmd; > > > sdhci_finish_data(host); > > > - else { > > > + } else { > > > if (intmask & (SDHCI_INT_DATA_AVAIL | > > > SDHCI_INT_SPACE_AVAIL)) > > > sdhci_transfer_pio(host); > > > > > > @@ -1792,6 +1847,10 @@ int sdhci_add_host(struct sdhci_host *host) > > > host->flags &= ~SDHCI_USE_ADMA; > > > } > > > > > > + /* If the host can perform ADMA operation, we reset SDMA flag */ > > > + if (host->flags & SDHCI_USE_ADMA) > > > + host->flags &= ~SDHCI_USE_SDMA; > > > + > > > if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA)) { > > > if (host->ops->enable_dma) { > > > if (host->ops->enable_dma(host)) { > > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > > > index 6e0969e..223762c 100644 > > > --- a/drivers/mmc/host/sdhci.h > > > +++ b/drivers/mmc/host/sdhci.h > > > @@ -25,6 +25,7 @@ > > > */ > > > > > > #define SDHCI_DMA_ADDRESS 0x00 > > > +#define SDHCI_ARGUMENT2 SDHCI_DMA_ADDRESS > > > > > > #define SDHCI_BLOCK_SIZE 0x04 > > > #define SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | > (blksz > > & > > > 0xFFF)) > > > @@ -37,6 +38,7 @@ > > > #define SDHCI_TRNS_DMA 0x01 > > > #define SDHCI_TRNS_BLK_CNT_EN 0x02 > > > #define SDHCI_TRNS_ACMD12 0x04 > > > +#define SDHCI_TRNS_AUTO_CMD23 0x08 > > > #define SDHCI_TRNS_READ 0x10 > > > #define SDHCI_TRNS_MULTI 0x20 > > > > > > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > > > index 8ce0827..22b0335 100644 > > > --- a/include/linux/mmc/card.h > > > +++ b/include/linux/mmc/card.h > > > @@ -58,9 +58,13 @@ struct mmc_ext_csd { > > > > > > struct sd_scr { > > > unsigned char sda_vsn; > > > + unsigned char sda_spec3; > > > unsigned char bus_widths; > > > #define SD_SCR_BUS_WIDTH_1 (1<<0) > > > #define SD_SCR_BUS_WIDTH_4 (1<<2) > > > + unsigned char cmd_support; > > > +#define SD_SCR_CMD20_SUPPORT (1<<0) > > > +#define SD_SCR_CMD23_SUPPORT (1<<1) > > > }; > > > > > > struct sd_ssr { > > > diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h > > > index 3fd85e0..178363b 100644 > > > --- a/include/linux/mmc/sd.h > > > +++ b/include/linux/mmc/sd.h > > > @@ -59,7 +59,7 @@ > > > > > > #define SCR_SPEC_VER_0 0 /* Implements system > > specification > > > 1.0 - 1.01 */ > > > #define SCR_SPEC_VER_1 1 /* Implements system > > specification > > > 1.10 */ > > > -#define SCR_SPEC_VER_2 2 /* Implements system > > specification > > > 2.00 */ > > > +#define SCR_SPEC_VER_2 2 /* Implements system > > specification > > > 2.00 - 3.0x */ > > > > > > /* > > > * SD bus widths > > > diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h > > > index 83bd9f7..282d158 100644 > > > --- a/include/linux/mmc/sdhci.h > > > +++ b/include/linux/mmc/sdhci.h > > > @@ -145,6 +145,8 @@ struct sdhci_host { > > > unsigned int ocr_avail_sd; > > > unsigned int ocr_avail_mmc; > > > > > > + struct mmc_command *saved_abort_cmd; /* Abort command saved > > > for data error recovery */ > > > + > > > unsigned long private[0] ____cacheline_aligned; > > > }; > > > #endif /* __SDHCI_H */ > > > -- > > > 1.7.1 > > > > > > -- > > > 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 > > > -- 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
