mmc: introduce multi-block read gap tuning
Subsystems (3)
v2 cover mmc: introduce multi-block read gap tuning
This patchset implements the multi-block read gap tuning for the SDHCI cadence driver. To do so, a generic function (mmc_read_blocks) to read blocks from the MMC is added to mmc_ops in the first patch.
The second patch is the implementation proper in the driver which try to do a multi-block read and increase the gap delay until it succeeds. The read gap is done by the controller when its internal fifo is full when reading multiple blocks. That is why we cannot use mmc_get_ext_csd to do the tuning since it only read one block.
The implementation of the mmc_read_blocks also brings a helper to check for CMD23 support by MMC card, similar to mmc_host_can_cmd23 for host, so the last two patches uses it instead of local implementation in mmc_test.c and block.c.
v1 -> v2:
Split the code between the core and the driver by adding a generic function to read blocks from the MMC.
Benoît Monin (4):
mmc: core: add mmc_read_blocks to mmc_ops
mmc: sdhci-cadence: implement multi-block read gap tuning
mmc: mmc_test: use mmc_card_can_cmd23
mmc: block: use mmc_card_can_cmd23
drivers/mmc/core/block.c | 12 ++---- drivers/mmc/core/card.h | 10 +++++ drivers/mmc/core/mmc_ops.c | 69 ++++++++++++++++++++++++++++++ drivers/mmc/core/mmc_test.c | 9 +--- drivers/mmc/host/sdhci-cadence.c | 72 +++++++++++++++++++++++++++++++- include/linux/mmc/host.h | 3 ++ 6 files changed, 157 insertions(+), 18 deletions(-)
PATCH v2 1/4 mmc: core: add mmc_read_blocks to mmc_ops
Add a generic function to read some blocks of data from the MMC, to be used by drivers as part of their tuning.
--- drivers/mmc/core/card.h | 10 ++++++ drivers/mmc/core/mmc_ops.c | 69 ++++++++++++++++++++++++++++++++++++++ include/linux/mmc/host.h | 3 ++ 3 files changed, 82 insertions(+)
drivers/mmc/core/card.h +10 -0
@@ -11,6 +11,7 @@#define _MMC_CORE_CARD_H#include <linux/mmc/card.h> +#include <linux/mmc/mmc.h> #define mmc_card_name (c) ((c)->cid.prod_name)#define mmc_card_id (c) (dev_name(&(c)->dev))
@@ -300,4 +301,13 @@ static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c) return c->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING; } +static inline bool mmc_card_can_cmd23 (struct mmc_card *card) +{ +return ((mmc_card_mmc (card) && + card->csd.mmca_vsn >=CSD_SPEC_VER_3 ) || + (mmc_card_sd (card) && !mmc_card_ult_capacity (card) && + card->scr.cmds &SD_SCR_CMD23_SUPPORT )) && + !(card->quirks &MMC_QUIRK_BLK_NO_CMD23 ); +} +#endif
drivers/mmc/core/mmc_ops.c +69 -0
72 lines not on this page · raw message
include/linux/mmc/host.h +3 -0
@@ -743,5 +743,8 @@ int mmc_send_status(struct mmc_card *card, u32 *status);int mmc_send_tuning (struct mmc_host *host,u32 opcode,int *cmd_error);int mmc_send_abort_tuning (struct mmc_host *host,u32 opcode);int mmc_get_ext_csd (struct mmc_card *card,u8 **new_ext_csd); +int mmc_read_blocks (struct mmc_card *card,struct mmc_host *host, +unsigned int blksz,unsigned int blocks, +unsigned int blk_addr,void *buf,unsigned int len);#endif /* LINUX_MMC_HOST_H */
Re: [PATCH v2 1/4] mmc: core: add mmc_read_blocks to mmc_ops
Quoted 24 lines · Benoît Monin
Add a generic function to read some blocks of data from the MMC, to be used by drivers as part of their tuning.
Signed-off-by Benoît Monin <[email protected]>--- drivers/mmc/core/card.h | 10 ++++++ drivers/mmc/core/mmc_ops.c | 69 ++++++++++++++++++++++++++++++++++++++ include/linux/mmc/host.h | 3 ++ 3 files changed, 82 insertions(+)drivers/mmc/core/card.h +1
-0@@ -11,6 +11,7 @@#define _MMC_CORE_CARD_H#include <linux/mmc/card.h> +#include <linux/mmc/mmc.h> #define mmc_card_name (c) ((c)->cid.prod_name)#define mmc_card_id (c) (dev_name(&(c)->dev))
drivers/mmc/core/card.h static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c) · line 310
Quoted 10 lines
drivers/mmc/core/card.h +7
-0@@ -300,4 +301,13 @@ static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c)return c->quirks &MMC_QUIRK_NO_UHS_DDR50_TUNING ; } +static inline bool mmc_card_can_cmd23 (struct mmc_card *card) +{ +return ((mmc_card_mmc (card) && + card->csd.mmca_vsn >=CSD_SPEC_VER_3 ) || + (mmc_card_sd (card) && !mmc_card_ult_capacity (card) && + card->scr.cmds &SD_SCR_CMD23_SUPPORT )) && + !(card->quirks &MMC_QUIRK_BLK_NO_CMD23 );
First, please make the above part a separate patch. It makes sense to add a helper for this, as you show in patch3 and patch4. I also recommend that these patches should also be re-ordered so they come first in the series.
Second, I don't think we should mix mmc_card_can* functions with the card-quirks. Better to have two separate helpers, especially since CMD23 is used for other things too, like RPMB and reliable writes, for example. Thus I suggest we add:
mmc_card_can_cmd23() - which looks at what the card supports, similar to above without MMC_QUIRK_BLK_NO_CMD23. Put the definition in drivers/mmc/core/core.h and export the symbols, similar to what we do for mmc_card_can_erase() and friends.
mmc_card_broken_blk_cmd23() - which should only check MMC_QUIRK_BLK_NO_CMD23. This belongs in drivers/mmc/core/card.h.
Quoted 85 lines
drivers/mmc/core/card.h +2
-0+} +#endif drivers/mmc/core/mmc_ops.c +69
-0@@ -1077,3 +1077,72 @@ int mmc_sanitize(struct mmc_card *card, unsigned int timeout_ms)return err; }EXPORT_SYMBOL_GPL (mmc_sanitize); + +/** +* mmc_read_blocks() - read data blocks from the mmc +* @card: mmc card to read from, can be NULL +* @host: mmc host doing the read +* @blksz: data block size +* @blocks: number of blocks to read +* @blk_addr: first block address +* @buf: output buffer +* @len: size of the buffer +* +* Read one or more blocks of data from the mmc. This is a low-level helper for +* tuning operation. If card is NULL, it is assumed that CMD23 can be used for +* multi-block read. +* +* Return: 0 in case of success, otherwise -EIO +*/ +int mmc_read_blocks (struct mmc_card *card,struct mmc_host *host, +unsigned int blksz,unsigned int blocks, +unsigned int blk_addr,void *buf,unsigned int len) +{ +struct mmc_request mrq = {}; +struct mmc_command sbc = {}; +struct mmc_command cmd = {}; +struct mmc_command stop = {}; +struct mmc_data data = {}; +struct scatterlist sg; + +if (blocks >1 ) { +if (mmc_host_can_cmd23 (host) && + (!card ||mmc_card_can_cmd23 (card))) { + mrq.sbc = &sbc; + sbc.opcode =MMC_SET_BLOCK_COUNT ; + sbc.arg = blocks; + sbc.flags =MMC_RSP_R1 |MMC_CMD_AC ; + } + cmd.opcode =MMC_READ_MULTIPLE_BLOCK ; + mrq.stop = &stop; + stop.opcode =MMC_STOP_TRANSMISSION ; + stop.flags =MMC_RSP_SPI_R1 |MMC_RSP_R1 |MMC_CMD_AC ; + }else { + cmd.opcode =MMC_READ_SINGLE_BLOCK ; + } + + mrq.cmd = &cmd; + cmd.flags =MMC_RSP_SPI_R1 |MMC_RSP_R1 |MMC_CMD_ADTC ; + + mrq.data = &data; + data.flags =MMC_DATA_READ ; + data.blksz = blksz; + data.blocks = blocks; + data.blk_addr = blk_addr; + data.sg = &sg; + data.sg_len =1 ; +if (card) +mmc_set_data_timeout (&data, card); +else + data.timeout_ns =1000000000 ; + +sg_init_one (&sg, buf, len); + +mmc_wait_for_req (host, &mrq); + +if (sbc.error || cmd.error || data.error) +return -EIO ; + +return 0 ; +} +EXPORT_SYMBOL_GPL (mmc_read_blocks);include/linux/mmc/host.h +0
-0
include/linux/mmc/host.h int mmc_send_status(struct mmc_card *card, u32 *status); · line 748
Quoted 6 lines
include/linux/mmc/host.h +3
-0@@ -743,5 +743,8 @@ int mmc_send_status(struct mmc_card *card, u32 *status);int mmc_send_tuning (struct mmc_host *host,u32 opcode,int *cmd_error);int mmc_send_abort_tuning (struct mmc_host *host,u32 opcode);int mmc_get_ext_csd (struct mmc_card *card,u8 **new_ext_csd); +int mmc_read_blocks (struct mmc_card *card,struct mmc_host *host, +unsigned int blksz,unsigned int blocks, +unsigned int blk_addr,void *buf,unsigned int len);
I really think we must avoid exporting such a generic function. This becomes visible outside the mmc subsystem and I am worried that it will be abused.
Can we perhaps make it harder to integrate with the tuning support on the core, somehow? I haven't thought much about it, but maybe you can propose something along those lines - otherwise I will try to think of another way to do it.
include/linux/mmc/host.h int mmc_send_status(struct mmc_card *card, u32 *status); · line 750
Quoted 2 lines
include/linux/mmc/host.h +0
-0#endif /* LINUX_MMC_HOST_H */
Kind regards
Uffe
Re: [PATCH v2 1/4] mmc: core: add mmc_read_blocks to mmc_ops
Hi Ulf,
Thanks for the review.
Quoted 46 lines · Ulf Hansson
Quoted 24 lines · Benoît Monin
Add a generic function to read some blocks of data from the MMC, to be used by drivers as part of their tuning.
Signed-off-by Benoît Monin <[email protected]>--- drivers/mmc/core/card.h | 10 ++++++ drivers/mmc/core/mmc_ops.c | 69 ++++++++++++++++++++++++++++++++++++++ include/linux/mmc/host.h | 3 ++ 3 files changed, 82 insertions(+)drivers/mmc/core/card.h +1
-0@@ -11,6 +11,7 @@#define _MMC_CORE_CARD_H#include <linux/mmc/card.h> +#include <linux/mmc/mmc.h> #define mmc_card_name (c) ((c)->cid.prod_name)#define mmc_card_id (c) (dev_name(&(c)->dev))drivers/mmc/core/card.h static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c) · line 310
Quoted 10 lines
drivers/mmc/core/card.h +7
-0@@ -300,4 +301,13 @@ static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c)return c->quirks &MMC_QUIRK_NO_UHS_DDR50_TUNING ; } +static inline bool mmc_card_can_cmd23 (struct mmc_card *card) +{ +return ((mmc_card_mmc (card) && + card->csd.mmca_vsn >=CSD_SPEC_VER_3 ) || + (mmc_card_sd (card) && !mmc_card_ult_capacity (card) && + card->scr.cmds &SD_SCR_CMD23_SUPPORT )) && + !(card->quirks &MMC_QUIRK_BLK_NO_CMD23 );First, please make the above part a separate patch. It makes sense to add a helper for this, as you show in patch3 and patch4. I also recommend that these patches should also be re-ordered so they come first in the series.
Second, I don't think we should mix mmc_card_can* functions with the card-quirks. Better to have two separate helpers, especially since CMD23 is used for other things too, like RPMB and reliable writes, for example. Thus I suggest we add:
Quoted 8 lines
mmc_card_can_cmd23()- which looks at what the card supports, similar to above withoutMMC_QUIRK_BLK_NO_CMD23. Put the definition indrivers/mmc/core/core.hand export the symbols, similar to what we do formmc_card_can_erase()and friends.
mmc_card_broken_blk_cmd23()- which should only checkMMC_QUIRK_BLK_NO_CMD23. This belongs indrivers/mmc/core/card.h.
Ok, I will do that.
Quoted 96 lines
Quoted 85 lines
drivers/mmc/core/card.h +2
-0+} +#endif drivers/mmc/core/mmc_ops.c +69
-0@@ -1077,3 +1077,72 @@ int mmc_sanitize(struct mmc_card *card, unsigned int timeout_ms)return err; }EXPORT_SYMBOL_GPL (mmc_sanitize); + +/** +* mmc_read_blocks() - read data blocks from the mmc +* @card: mmc card to read from, can be NULL +* @host: mmc host doing the read +* @blksz: data block size +* @blocks: number of blocks to read +* @blk_addr: first block address +* @buf: output buffer +* @len: size of the buffer +* +* Read one or more blocks of data from the mmc. This is a low-level helper for +* tuning operation. If card is NULL, it is assumed that CMD23 can be used for +* multi-block read. +* +* Return: 0 in case of success, otherwise -EIO +*/ +int mmc_read_blocks (struct mmc_card *card,struct mmc_host *host, +unsigned int blksz,unsigned int blocks, +unsigned int blk_addr,void *buf,unsigned int len) +{ +struct mmc_request mrq = {}; +struct mmc_command sbc = {}; +struct mmc_command cmd = {}; +struct mmc_command stop = {}; +struct mmc_data data = {}; +struct scatterlist sg; + +if (blocks >1 ) { +if (mmc_host_can_cmd23 (host) && + (!card ||mmc_card_can_cmd23 (card))) { + mrq.sbc = &sbc; + sbc.opcode =MMC_SET_BLOCK_COUNT ; + sbc.arg = blocks; + sbc.flags =MMC_RSP_R1 |MMC_CMD_AC ; + } + cmd.opcode =MMC_READ_MULTIPLE_BLOCK ; + mrq.stop = &stop; + stop.opcode =MMC_STOP_TRANSMISSION ; + stop.flags =MMC_RSP_SPI_R1 |MMC_RSP_R1 |MMC_CMD_AC ; + }else { + cmd.opcode =MMC_READ_SINGLE_BLOCK ; + } + + mrq.cmd = &cmd; + cmd.flags =MMC_RSP_SPI_R1 |MMC_RSP_R1 |MMC_CMD_ADTC ; + + mrq.data = &data; + data.flags =MMC_DATA_READ ; + data.blksz = blksz; + data.blocks = blocks; + data.blk_addr = blk_addr; + data.sg = &sg; + data.sg_len =1 ; +if (card) +mmc_set_data_timeout (&data, card); +else + data.timeout_ns =1000000000 ; + +sg_init_one (&sg, buf, len); + +mmc_wait_for_req (host, &mrq); + +if (sbc.error || cmd.error || data.error) +return -EIO ; + +return 0 ; +} +EXPORT_SYMBOL_GPL (mmc_read_blocks);include/linux/mmc/host.h +0
-0include/linux/mmc/host.h int mmc_send_status(struct mmc_card *card, u32 *status); · line 748
Quoted 6 lines
include/linux/mmc/host.h +3
-0@@ -743,5 +743,8 @@ int mmc_send_status(struct mmc_card *card, u32 *status);int mmc_send_tuning (struct mmc_host *host,u32 opcode,int *cmd_error);int mmc_send_abort_tuning (struct mmc_host *host,u32 opcode);int mmc_get_ext_csd (struct mmc_card *card,u8 **new_ext_csd); +int mmc_read_blocks (struct mmc_card *card,struct mmc_host *host, +unsigned int blksz,unsigned int blocks, +unsigned int blk_addr,void *buf,unsigned int len);I really think we must avoid exporting such a generic function. This becomes visible outside the mmc subsystem and I am worried that it will be abused.
Quoted 5 lines
Can we perhaps make it harder to integrate with the tuning support on the core, somehow? I haven't thought much about it, but maybe you can propose something along those lines - otherwise I will try to think of another way to do it.
I agree that the function might be too generic now. Here are some of the ideas I have to make less appealing for abuse:
* Rename it to mention tuning (mmc_tuning_read?)
* Drop some parameters:
* blk_addr: Reading from 0 should be all that is needed for tuning
* other?
* Move its declaration to a header private to drivers/mmc (where?)
Let me know what you think.
Quoted 6 lines
include/linux/mmc/host.h int mmc_send_status(struct mmc_card *card, u32 *status); · line 750
Quoted 2 lines
include/linux/mmc/host.h +0
-0#endif /* LINUX_MMC_HOST_H */ Kind regards
Uffe
Best regards,
Signature · 3 lines
Benoît Monin, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 1/4] mmc: core: add mmc_read_blocks to mmc_ops
Quoted 101 lines
Quoted 96 lines
Quoted 85 lines
drivers/mmc/core/card.h +2
-0+} +#endif drivers/mmc/core/mmc_ops.c +69
-0@@ -1077,3 +1077,72 @@ int mmc_sanitize(struct mmc_card *card, unsigned int timeout_ms)return err; }EXPORT_SYMBOL_GPL (mmc_sanitize); + +/** +* mmc_read_blocks() - read data blocks from the mmc +* @card: mmc card to read from, can be NULL +* @host: mmc host doing the read +* @blksz: data block size +* @blocks: number of blocks to read +* @blk_addr: first block address +* @buf: output buffer +* @len: size of the buffer +* +* Read one or more blocks of data from the mmc. This is a low-level helper for +* tuning operation. If card is NULL, it is assumed that CMD23 can be used for +* multi-block read. +* +* Return: 0 in case of success, otherwise -EIO +*/ +int mmc_read_blocks (struct mmc_card *card,struct mmc_host *host, +unsigned int blksz,unsigned int blocks, +unsigned int blk_addr,void *buf,unsigned int len) +{ +struct mmc_request mrq = {}; +struct mmc_command sbc = {}; +struct mmc_command cmd = {}; +struct mmc_command stop = {}; +struct mmc_data data = {}; +struct scatterlist sg; + +if (blocks >1 ) { +if (mmc_host_can_cmd23 (host) && + (!card ||mmc_card_can_cmd23 (card))) { + mrq.sbc = &sbc; + sbc.opcode =MMC_SET_BLOCK_COUNT ; + sbc.arg = blocks; + sbc.flags =MMC_RSP_R1 |MMC_CMD_AC ; + } + cmd.opcode =MMC_READ_MULTIPLE_BLOCK ; + mrq.stop = &stop; + stop.opcode =MMC_STOP_TRANSMISSION ; + stop.flags =MMC_RSP_SPI_R1 |MMC_RSP_R1 |MMC_CMD_AC ; + }else { + cmd.opcode =MMC_READ_SINGLE_BLOCK ; + } + + mrq.cmd = &cmd; + cmd.flags =MMC_RSP_SPI_R1 |MMC_RSP_R1 |MMC_CMD_ADTC ; + + mrq.data = &data; + data.flags =MMC_DATA_READ ; + data.blksz = blksz; + data.blocks = blocks; + data.blk_addr = blk_addr; + data.sg = &sg; + data.sg_len =1 ; +if (card) +mmc_set_data_timeout (&data, card); +else + data.timeout_ns =1000000000 ; + +sg_init_one (&sg, buf, len); + +mmc_wait_for_req (host, &mrq); + +if (sbc.error || cmd.error || data.error) +return -EIO ; + +return 0 ; +} +EXPORT_SYMBOL_GPL (mmc_read_blocks);include/linux/mmc/host.h +0
-0include/linux/mmc/host.h int mmc_send_status(struct mmc_card *card, u32 *status); · line 748
Quoted 6 lines
include/linux/mmc/host.h +3
-0@@ -743,5 +743,8 @@ int mmc_send_status(struct mmc_card *card, u32 *status);int mmc_send_tuning (struct mmc_host *host,u32 opcode,int *cmd_error);int mmc_send_abort_tuning (struct mmc_host *host,u32 opcode);int mmc_get_ext_csd (struct mmc_card *card,u8 **new_ext_csd); +int mmc_read_blocks (struct mmc_card *card,struct mmc_host *host, +unsigned int blksz,unsigned int blocks, +unsigned int blk_addr,void *buf,unsigned int len);I really think we must avoid exporting such a generic function. This becomes visible outside the mmc subsystem and I am worried that it will be abused.
Quoted 5 lines
Can we perhaps make it harder to integrate with the tuning support on the core, somehow? I haven't thought much about it, but maybe you can propose something along those lines - otherwise I will try to think of another way to do it.
Quoted 4 lines
I agree that the function might be too generic now. Here are some of the ideas I have to make less appealing for abuse:
* Rename it to mention tuning (
mmc_tuning_read?)
Yes, something like that or possibly "mmc_read_tuning".
Quoted 3 lines
* Drop some parameters:
*blk_addr: Reading from 0 should be all that is needed for tuning
* other?
Yes, I like that. If we can make it useful only for the tuning case, that would be great.
I think we should drop struct mmc_card* too. The ->execute_tuning() host ops takes a struct mmc_host* and during initialization when the callback is invoked from the core, host->card has not yet been set. In other words, there are no "card" available for the host to use.
Moreover, do we really need to pass along the data that we have read? Can't we just alloc the buffer, read the data and free the buffer. The important part is to provide an error code to the caller, letting it know whether we succeeded in reading the data or whether it failed, right?
Quoted 1 line
* Move its declaration to a header private to
drivers/mmc(where?)
Unfortunately not, we currently don't have any other suitable place, but next to mmc_get_ext_csd().
Quoted 3 lines
Let me know what you think.
Kind regards
Uffe
PATCH v2 2/4 mmc: sdhci-cadence: implement multi-block read gap tuning
In reply to Benoît MoninThe controller suspends the clock between blocks when reading from the MMC as part of its flow-control, called read block gap. At higher clock speed and with IO delay between the controller and the MMC, this clock pause can happen too late, during the read of the next block and trigger a read error.
To prevent this, the delay can be programmed for each mode via the pair of registers HRS37/38. This delay is obtained during tuning, by trying a multi-block read and increasing the delay until the read succeeds.
For now, the tuning is only done in HS200, as the read error has only been observed at that speed.
--- drivers/mmc/host/sdhci-cadence.c | 72 +++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-)
drivers/mmc/host/sdhci-cadence.c +71 -1
@@ -36,6 +36,24 @@#define SDHCI_CDNS_HRS06_MODE_MMC_HS400 0x5#define SDHCI_CDNS_HRS06_MODE_MMC_HS400ES 0x6 +/* Read block gap */ +#define SDHCI_CDNS_HRS37 0x94/* interface mode select */ +#define SDHCI_CDNS_HRS37_MODE_DS 0x0 +#define SDHCI_CDNS_HRS37_MODE_HS 0x1 +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR12 0x8 +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR25 0x9 +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR50 0xa +#define SDHCI_CDNS_HRS37_MODE_UDS_SDR104 0xb +#define SDHCI_CDNS_HRS37_MODE_UDS_DDR50 0xc +#define SDHCI_CDNS_HRS37_MODE_MMC_LEGACY 0x20 +#define SDHCI_CDNS_HRS37_MODE_MMC_SDR 0x21 +#define SDHCI_CDNS_HRS37_MODE_MMC_DDR 0x22 +#define SDHCI_CDNS_HRS37_MODE_MMC_HS200 0x23 +#define SDHCI_CDNS_HRS37_MODE_MMC_HS400 0x24 +#define SDHCI_CDNS_HRS37_MODE_MMC_HS400ES 0x25 +#define SDHCI_CDNS_HRS38 0x98/* Read block gap coefficient */ +#define SDHCI_CDNS_HRS38_BLKGAP_MAX 0xf +/* SRS - Slot Register Set (SDHCI-compatible) */ #define SDHCI_CDNS_SRS_BASE 0x200
@@ -251,6 +269,52 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)return 0 ; } +/** +* sdhci_cdns_tune_blkgap() - tune multi-block read gap +* @mmc: MMC host +* +* Tune delay used in multi block read. To do so, +* try sending multi-block read command with incremented gap, unless +* it succeeds. +* +* Return: error code +*/ +static int sdhci_cdns_tune_blkgap (struct mmc_host *mmc) +{ +struct sdhci_host *host =mmc_priv (mmc); +struct sdhci_pltfm_host *pltfm_host =sdhci_priv (host); +struct sdhci_cdns_priv *priv =sdhci_pltfm_priv (pltfm_host); +void __iomem *hrs37_reg = priv->hrs_addr +SDHCI_CDNS_HRS37 ; +void __iomem *hrs38_reg = priv->hrs_addr +SDHCI_CDNS_HRS38 ; +int ret; +u32 gap; +u32 hrs37_mode; +void *buf; + +switch (host->timing) { +case MMC_TIMING_MMC_HS200 : + hrs37_mode =SDHCI_CDNS_HRS37_MODE_MMC_HS200 ; +break ; +default : +return 0 ;/* no tuning in this mode */ + } + +writel (hrs37_mode, hrs37_reg); + + buf =kmalloc (512 *32 ,GFP_KERNEL ); +for (gap =0 ; gap <=SDHCI_CDNS_HRS38_BLKGAP_MAX ; gap++) { +writel (gap, hrs38_reg); + ret =mmc_read_blocks (NULL , mmc,512 ,32 ,0 , buf,512 *32 ); +if (ret ==0 ) +break ; + } +kfree (buf); + +dev_dbg (mmc_dev (mmc),"read block gap tune %s, gap %d\n" , + ret ==0 ?"OK" :"failed" , gap); +return ret; +} +/* * In SD mode, software must not use the hardware tuning and instead perform * an almost identical procedure to eMMC.
@@ -261,6 +325,7 @@ static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)int max_streak =0 ;int end_of_streak =0 ;int i; +int ret;/* * Do not execute tuning for UHS_SDR50 or UHS_DDR50.
@@ -288,7 +353,12 @@ static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)return -EIO ; } -return sdhci_cdns_set_tune_val (host, end_of_streak - max_streak /2 ); + ret =sdhci_cdns_set_tune_val (host, end_of_streak - max_streak /2 ); + +if (!ret) + ret =sdhci_cdns_tune_blkgap (host->mmc); + +return ret; } static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
PATCH v2 3/4 mmc: mmc_test: use mmc_card_can_cmd23
In reply to Benoît MoninUse the helper instead of using a local implementation.
--- drivers/mmc/core/mmc_test.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
drivers/mmc/core/mmc_test.c +1 -8
@@ -180,20 +180,13 @@ static int mmc_test_set_blksize(struct mmc_test_card *test, unsigned size)return mmc_set_blocklen (test->card, size); } -static bool mmc_test_card_cmd23 (struct mmc_card *card) -{ -return mmc_card_mmc (card) || - (mmc_card_sd (card) && card->scr.cmds &SD_SCR_CMD23_SUPPORT ); -} -static void mmc_test_prepare_sbc (struct mmc_test_card *test,struct mmc_request *mrq,unsigned int blocks) {struct mmc_card *card = test->card;if (!mrq->sbc || !mmc_host_can_cmd23 (card->host) || - !mmc_test_card_cmd23 (card) || !mmc_op_multi (mrq->cmd->opcode) || - (card->quirks &MMC_QUIRK_BLK_NO_CMD23 )) { + !mmc_card_can_cmd23 (card) || !mmc_op_multi (mrq->cmd->opcode)) { mrq->sbc =NULL ;return ; }
PATCH v2 4/4 mmc: block: use mmc_card_can_cmd23
In reply to Benoît MoninUse the helper to test for CMD23 card support. Remove the check on the NO_CMD23 quirk as it is already done in the helper.
--- drivers/mmc/core/block.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
drivers/mmc/core/block.c +3 -9
@@ -1768,8 +1768,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,* these, while retaining features like reliable writes. */ if ((md->flags &MMC_BLK_CMD23 ) &&mmc_op_multi (brq->cmd.opcode) && - (do_rel_wr || !(card->quirks &MMC_QUIRK_BLK_NO_CMD23 ) || - do_data_tag)) { + (do_rel_wr || do_data_tag)) { brq->sbc.opcode =MMC_SET_BLOCK_COUNT ; brq->sbc.arg = brq->data.blocks | (do_rel_wr ? (1 << 31) : 0) |
@@ -2618,13 +2617,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,*/ md->read_only =mmc_blk_readonly (card); -if (mmc_host_can_cmd23 (card->host)) { -if ((mmc_card_mmc (card) && - card->csd.mmca_vsn >=CSD_SPEC_VER_3 ) || - (mmc_card_sd (card) && !mmc_card_ult_capacity (card) && - card->scr.cmds &SD_SCR_CMD23_SUPPORT )) - md->flags |=MMC_BLK_CMD23 ; - } +if (mmc_host_can_cmd23 (card->host) &&mmc_card_can_cmd23 (card)) + md->flags |=MMC_BLK_CMD23 ; if (md->flags & MMC_BLK_CMD23 && ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||