| Message ID | 20240807060309.2403023-1-avri.altman@wdc.com |
|---|---|
| Headers | show |
| Series | Add SDUC Support | expand |
Hi Avri, I tested this SDUC patch for Realtek card readers and it works > Ultra Capacity SD cards (SDUC) was already introduced in SD7.0. Those cards > support capacity larger than 2TB and up to including 128TB. Thus, the address > range of the card expands beyond the 32-bit command argument. To that end, > a new command - CMD22 is defined, to carry the extra 6-bit upper part of the > 38-bit block address that enable access to 128TB memory space. > > SDUC capacity is agnostic to the interface mode: UHS-I and UHS-II – Same as > SDXC. > > The spec defines several extensions/modifications to the current SDXC cards, > which we address in patches 1 - 10. Otherwise requirements are out-of-scope > of this change. Specifically, CMDQ (CMD44+CMD45), and Extension for > Video Speed Class (CMD20). > > First publication of SDUC was in [1]. This series was developed and tested > separately from [1] and does not borrow from it. > > [1] https://lwn.net/Articles/982566/ > > --- > Changes in v2: > - Attend kernel test robot warnings > > --- > > Avri Altman (10): > mmc: sd: SDUC Support Recognition > mmc: sd: Add SD CSD version 3.0 > mmc: sd: Add Extension memory addressing > mmc: core: Add open-ended Ext memory addressing > mmc: host: Always use manual-cmd23 in SDUC > mmc: core: Add close-ended Ext memory addressing > mmc: host: Add close-ended Ext memory addressing > mmc: core: Allow mmc erase to carry large addresses > mmc: core: Add Ext memory addressing for erase > mmc: core: Adjust ACMD22 to SDUC > > drivers/mmc/core/block.c | 45 +++++++++++++++++++++++++++++------- > drivers/mmc/core/bus.c | 4 +++- > drivers/mmc/core/card.h | 3 +++ > drivers/mmc/core/core.c | 48 +++++++++++++++++++++++++++++++-------- > drivers/mmc/core/core.h | 2 +- > drivers/mmc/core/queue.h | 1 + > drivers/mmc/core/sd.c | 16 +++++++++---- > drivers/mmc/core/sd_ops.c | 34 +++++++++++++++++++++++---- > drivers/mmc/core/sd_ops.h | 1 + drivers/mmc/host/sdhci.c | 38 > ++++++++++++++++++++++++++----- include/linux/mmc/card.h | 2 +- > include/linux/mmc/core.h | 1 + include/linux/mmc/host.h | 6 +++++ > include/linux/mmc/sd.h | 4 ++++ > 14 files changed, 169 insertions(+), 36 deletions(-) > > -- > 2.25.1
> > Hi Avri, > > I tested this SDUC patch for Realtek card readers and it works Ricky thanks. Will add your "Tested-by" tag to my next spin. Would appreciate any further constructive reviews - helping to move this series forward. Thanks a lot, Avri
> > ACMD21 was extended to support the host-card handshake during initialization. Is ACMD41? > The card expects that the HCS & HO2T bits to be set in the command > argument, and sets the applicable bits in the R3 returned response. On the > contrary, if a SDUC card is inserted to a non-supporting host, it will never > respond to this ACMD21 until eventually, the host will timed out and give up. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/mmc/core/sd_ops.c | 19 +++++++++++++++---- > include/linux/mmc/host.h | 6 ++++++ > include/linux/mmc/sd.h | 1 + > 3 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c index > 8b9b34286ef3..7f6963dac873 100644 > --- a/drivers/mmc/core/sd_ops.c > +++ b/drivers/mmc/core/sd_ops.c > @@ -168,12 +168,16 @@ int mmc_send_app_op_cond(struct mmc_host *host, > u32 ocr, u32 *rocr) > .cmd = &cmd > }; > int err; > + u32 sduc_arg = SD_OCR_CCS | SD_OCR_2T; > > cmd.opcode = SD_APP_OP_COND; > + cmd.arg = ocr; > + > if (mmc_host_is_spi(host)) > - cmd.arg = ocr & (1 << 30); /* SPI only defines one bit */ > + cmd.arg &= (1 << 30); /* SPI only defines one bit */ > else > - cmd.arg = ocr; > + cmd.arg |= sduc_arg; > + > cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR; > > err = __mmc_poll_for_busy(host, SD_APP_OP_COND_PERIOD_US, > @@ -182,8 +186,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, > u32 ocr, u32 *rocr) > if (err) > return err; > > - if (rocr && !mmc_host_is_spi(host)) > - *rocr = cmd.resp[0]; > + if (!mmc_host_is_spi(host)) { > + if (rocr) > + *rocr = cmd.resp[0]; > + > + if ((cmd.resp[0] & sduc_arg) == sduc_arg) > + host->caps2 |= MMC_CAP2_SD_SDUC; > + else > + host->caps2 &= ~MMC_CAP2_SD_SDUC; I think host->caps2 is for host to claim caps, here can just call mmc_card_set_ult_capacity? Don't need to wait csd, because SDXC and SDHC need to identify by capacity, but SDUC can be identified here And all your mmc_card_is_sduc() I think change to mmc_card_ult_capacity() to know the card type > + } > > return 0; > } > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index > 88c6a76042ee..a9c36a3e1a10 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -427,6 +427,7 @@ struct mmc_host { > #define MMC_CAP2_CRYPTO 0 > #endif > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC > that has GPT entry at a non-standard location */ > +#define MMC_CAP2_SD_SDUC (1 << 29) /* SD over 2TB */ > > int fixed_drv_type; /* fixed driver type for > non-removable media */ > > @@ -638,6 +639,11 @@ static inline int mmc_card_uhs(struct mmc_card > *card) > card->host->ios.timing <= MMC_TIMING_UHS_DDR50; } > > +static inline int mmc_card_is_sduc(struct mmc_host *host) { > + return host->caps2 & MMC_CAP2_SD_SDUC; } > + > void mmc_retune_timer_stop(struct mmc_host *host); > > static inline void mmc_retune_needed(struct mmc_host *host) diff --git > a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h index > 6727576a8755..865cc0ca8543 100644 > --- a/include/linux/mmc/sd.h > +++ b/include/linux/mmc/sd.h > @@ -36,6 +36,7 @@ > /* OCR bit definitions */ > #define SD_OCR_S18R (1 << 24) /* 1.8V switching request > */ > #define SD_ROCR_S18A SD_OCR_S18R /* 1.8V switching > accepted by card */ > +#define SD_OCR_2T (1 << 27) /* HO2T/CO2T - SDUC > support */ > #define SD_OCR_XPC (1 << 28) /* SDXC power control */ > #define SD_OCR_CCS (1 << 30) /* Card Capacity Status */ > > -- > 2.25.1
> SDUC memory addressing spans beyond 2TB and up to 128TB. Therefore, 38 > bits are required to access the entire memory space of all sectors. > Those extra 6 bits are to be carried by CMD22 prior of sending > read/write/erase commands: CMD17, CMD18, CMD24, CMD25, CMD32, and > CMD33. > > CMD22 will carry the higher order 6 bits, and must precedes any of the above > commands even if it targets sector < 2TB. > > No error related to address or length is indicated in CMD22 but rather in the > read/write command itself. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/mmc/core/sd_ops.c | 15 +++++++++++++++ > drivers/mmc/core/sd_ops.h | 1 + > include/linux/mmc/sd.h | 3 +++ > 3 files changed, 19 insertions(+) > > diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c index > 7f6963dac873..8b69129d7b61 100644 > --- a/drivers/mmc/core/sd_ops.c > +++ b/drivers/mmc/core/sd_ops.c > @@ -199,6 +199,21 @@ int mmc_send_app_op_cond(struct mmc_host *host, > u32 ocr, u32 *rocr) > return 0; > } > > +int mmc_send_ext_addr(struct mmc_host *host, sector_t addr) { > + struct mmc_command cmd = { > + .opcode = SD_ADDR_EXT, > + .arg = (u32)((addr >> 32) & 0x3F), > + .flags = MMC_RSP_R1 | MMC_CMD_AC, > + }; > + > + if (!mmc_card_is_sduc(host)) > + return 0; > + I think here can be removed, all mmc_send_ext_addr caller are have if-statement to check the card is SDUC > + return mmc_wait_for_cmd(host, &cmd, 0); } > +EXPORT_SYMBOL_GPL(mmc_send_ext_addr); > + > static int __mmc_send_if_cond(struct mmc_host *host, u32 ocr, u8 pcie_bits, > u32 *resp) { diff --git > a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h index > 7667fc223b74..462efd43acfa 100644 > --- a/drivers/mmc/core/sd_ops.h > +++ b/drivers/mmc/core/sd_ops.h > @@ -21,6 +21,7 @@ int mmc_send_relative_addr(struct mmc_host *host, > unsigned int *rca); int mmc_app_send_scr(struct mmc_card *card); int > mmc_app_sd_status(struct mmc_card *card, void *ssr); int > mmc_app_cmd(struct mmc_host *host, struct mmc_card *card); > +int mmc_send_ext_addr(struct mmc_host *host, sector_t addr); > > #endif > > diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h index > 865cc0ca8543..af5fc70e09a2 100644 > --- a/include/linux/mmc/sd.h > +++ b/include/linux/mmc/sd.h > @@ -15,6 +15,9 @@ > #define SD_SEND_IF_COND 8 /* bcr [11:0] See below R7 > */ > #define SD_SWITCH_VOLTAGE 11 /* ac > R1 */ > > +/* Class 2 */ > +#define SD_ADDR_EXT 22 /* ac [5:0] > R1 */ > + > /* class 10 */ > #define SD_SWITCH 6 /* adtc [31:0] See below R1 > */ > > -- > 2.25.1
> > ACMD21 was extended to support the host-card handshake during initialization. > > Is ACMD41? Yes. Thank you. > > > The card expects that the HCS & HO2T bits to be set in the command > > argument, and sets the applicable bits in the R3 returned response. > > On the contrary, if a SDUC card is inserted to a non-supporting host, > > it will never respond to this ACMD21 until eventually, the host will timed out > and give up. > > > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > > --- > > drivers/mmc/core/sd_ops.c | 19 +++++++++++++++---- > > include/linux/mmc/host.h | 6 ++++++ > > include/linux/mmc/sd.h | 1 + > > 3 files changed, 22 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c > > index > > 8b9b34286ef3..7f6963dac873 100644 > > --- a/drivers/mmc/core/sd_ops.c > > +++ b/drivers/mmc/core/sd_ops.c > > @@ -168,12 +168,16 @@ int mmc_send_app_op_cond(struct mmc_host > *host, > > u32 ocr, u32 *rocr) > > .cmd = &cmd > > }; > > int err; > > + u32 sduc_arg = SD_OCR_CCS | SD_OCR_2T; > > > > cmd.opcode = SD_APP_OP_COND; > > + cmd.arg = ocr; > > + > > if (mmc_host_is_spi(host)) > > - cmd.arg = ocr & (1 << 30); /* SPI only defines one bit */ > > + cmd.arg &= (1 << 30); /* SPI only defines one bit */ > > else > > - cmd.arg = ocr; > > + cmd.arg |= sduc_arg; > > + > > cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR; > > > > err = __mmc_poll_for_busy(host, SD_APP_OP_COND_PERIOD_US, @@ > > -182,8 +186,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, > > u32 ocr, u32 *rocr) > > if (err) > > return err; > > > > - if (rocr && !mmc_host_is_spi(host)) > > - *rocr = cmd.resp[0]; > > + if (!mmc_host_is_spi(host)) { > > + if (rocr) > > + *rocr = cmd.resp[0]; > > + > > + if ((cmd.resp[0] & sduc_arg) == sduc_arg) > > + host->caps2 |= MMC_CAP2_SD_SDUC; > > + else > > + host->caps2 &= ~MMC_CAP2_SD_SDUC; > > I think host->caps2 is for host to claim caps, here can just call > mmc_card_set_ult_capacity? > Don't need to wait csd, because SDXC and SDHC need to identify by capacity, but > SDUC can be identified here And all your mmc_card_is_sduc() I think change to > mmc_card_ult_capacity() to know the card type This is an interesting idea - and yes, we can do that. Also the line of reasoning that you provide makes a lot of sense. However, SDUC is not a card state, so I am not sure I should use it. I added it to comply with SDXC, but using the card state to imply of a card type doesn't seems right. Thanks, Avri > > > + } > > > > return 0; > > } > > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index > > 88c6a76042ee..a9c36a3e1a10 100644 > > --- a/include/linux/mmc/host.h > > +++ b/include/linux/mmc/host.h > > @@ -427,6 +427,7 @@ struct mmc_host { > > #define MMC_CAP2_CRYPTO 0 > > #endif > > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC > > that has GPT entry at a non-standard location */ > > +#define MMC_CAP2_SD_SDUC (1 << 29) /* SD over 2TB */ > > > > int fixed_drv_type; /* fixed driver type for > > non-removable media */ > > > > @@ -638,6 +639,11 @@ static inline int mmc_card_uhs(struct mmc_card > > *card) > > card->host->ios.timing <= MMC_TIMING_UHS_DDR50; } > > > > +static inline int mmc_card_is_sduc(struct mmc_host *host) { > > + return host->caps2 & MMC_CAP2_SD_SDUC; } > > + > > void mmc_retune_timer_stop(struct mmc_host *host); > > > > static inline void mmc_retune_needed(struct mmc_host *host) diff > > --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h index > > 6727576a8755..865cc0ca8543 100644 > > --- a/include/linux/mmc/sd.h > > +++ b/include/linux/mmc/sd.h > > @@ -36,6 +36,7 @@ > > /* OCR bit definitions */ > > #define SD_OCR_S18R (1 << 24) /* 1.8V switching request > > */ > > #define SD_ROCR_S18A SD_OCR_S18R /* 1.8V switching > > accepted by card */ > > +#define SD_OCR_2T (1 << 27) /* HO2T/CO2T - SDUC > > support */ > > #define SD_OCR_XPC (1 << 28) /* SDXC power control */ > > #define SD_OCR_CCS (1 << 30) /* Card Capacity Status */ > > > > -- > > 2.25.1
> > SDUC memory addressing spans beyond 2TB and up to 128TB. Therefore, > > 38 bits are required to access the entire memory space of all sectors. > > Those extra 6 bits are to be carried by CMD22 prior of sending > > read/write/erase commands: CMD17, CMD18, CMD24, CMD25, CMD32, and > > CMD33. > > > > CMD22 will carry the higher order 6 bits, and must precedes any of the > > above commands even if it targets sector < 2TB. > > > > No error related to address or length is indicated in CMD22 but rather > > in the read/write command itself. > > > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > > --- > > drivers/mmc/core/sd_ops.c | 15 +++++++++++++++ > > drivers/mmc/core/sd_ops.h | 1 + > > include/linux/mmc/sd.h | 3 +++ > > 3 files changed, 19 insertions(+) > > > > diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c > > index > > 7f6963dac873..8b69129d7b61 100644 > > --- a/drivers/mmc/core/sd_ops.c > > +++ b/drivers/mmc/core/sd_ops.c > > @@ -199,6 +199,21 @@ int mmc_send_app_op_cond(struct mmc_host *host, > > u32 ocr, u32 *rocr) > > return 0; > > } > > > > +int mmc_send_ext_addr(struct mmc_host *host, sector_t addr) { > > + struct mmc_command cmd = { > > + .opcode = SD_ADDR_EXT, > > + .arg = (u32)((addr >> 32) & 0x3F), > > + .flags = MMC_RSP_R1 | MMC_CMD_AC, > > + }; > > + > > + if (!mmc_card_is_sduc(host)) > > + return 0; > > + > I think here can be removed, all mmc_send_ext_addr caller are have if-statement > to check the card is SDUC Theoretically yes. But since it is exported (kernel test robot warning), we should protect from loadable modules callers. Also, IMO it serves as some sort of documentation and improves readability. Thanks, Avri > > > + return mmc_wait_for_cmd(host, &cmd, 0); } > > +EXPORT_SYMBOL_GPL(mmc_send_ext_addr); > > + > > static int __mmc_send_if_cond(struct mmc_host *host, u32 ocr, u8 pcie_bits, > > u32 *resp) { diff --git > > a/drivers/mmc/core/sd_ops.h b/drivers/mmc/core/sd_ops.h index > > 7667fc223b74..462efd43acfa 100644 > > --- a/drivers/mmc/core/sd_ops.h > > +++ b/drivers/mmc/core/sd_ops.h > > @@ -21,6 +21,7 @@ int mmc_send_relative_addr(struct mmc_host *host, > > unsigned int *rca); int mmc_app_send_scr(struct mmc_card *card); int > > mmc_app_sd_status(struct mmc_card *card, void *ssr); int > > mmc_app_cmd(struct mmc_host *host, struct mmc_card *card); > > +int mmc_send_ext_addr(struct mmc_host *host, sector_t addr); > > > > #endif > > > > diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h index > > 865cc0ca8543..af5fc70e09a2 100644 > > --- a/include/linux/mmc/sd.h > > +++ b/include/linux/mmc/sd.h > > @@ -15,6 +15,9 @@ > > #define SD_SEND_IF_COND 8 /* bcr [11:0] See below R7 > > */ > > #define SD_SWITCH_VOLTAGE 11 /* ac > > R1 */ > > > > +/* Class 2 */ > > +#define SD_ADDR_EXT 22 /* ac [5:0] > > R1 */ > > + > > /* class 10 */ > > #define SD_SWITCH 6 /* adtc [31:0] See below R1 > > */ > > > > -- > > 2.25.1
> > > -182,8 +186,15 @@ int mmc_send_app_op_cond(struct mmc_host *host, > > > u32 ocr, u32 *rocr) > > > if (err) > > > return err; > > > > > > - if (rocr && !mmc_host_is_spi(host)) > > > - *rocr = cmd.resp[0]; > > > + if (!mmc_host_is_spi(host)) { > > > + if (rocr) > > > + *rocr = cmd.resp[0]; > > > + > > > + if ((cmd.resp[0] & sduc_arg) == sduc_arg) > > > + host->caps2 |= MMC_CAP2_SD_SDUC; > > > + else > > > + host->caps2 &= ~MMC_CAP2_SD_SDUC; > > > > I think host->caps2 is for host to claim caps, here can just call > > mmc_card_set_ult_capacity? > > Don't need to wait csd, because SDXC and SDHC need to identify by > > capacity, but SDUC can be identified here And all your > > mmc_card_is_sduc() I think change to > > mmc_card_ult_capacity() to know the card type > This is an interesting idea - and yes, we can do that. > Also the line of reasoning that you provide makes a lot of sense. > However, SDUC is not a card state, so I am not sure I should use it. > I added it to comply with SDXC, but using the card state to imply of a card type > doesn't seems right. How about extending the use of card->type for that? e.g., adding the various SD sub-types etc. But this needs to be done in a separate patch, probably after this series get accepted. Thanks, Avri > > Thanks, > Avri > > > > > > + } > > > > > > return 0; > > > } > > > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > > > index > > > 88c6a76042ee..a9c36a3e1a10 100644 > > > --- a/include/linux/mmc/host.h > > > +++ b/include/linux/mmc/host.h > > > @@ -427,6 +427,7 @@ struct mmc_host { > > > #define MMC_CAP2_CRYPTO 0 > > > #endif > > > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC > > > that has GPT entry at a non-standard location */ > > > +#define MMC_CAP2_SD_SDUC (1 << 29) /* SD over 2TB */ > > > > > > int fixed_drv_type; /* fixed driver type for > > > non-removable media */ > > > > > > @@ -638,6 +639,11 @@ static inline int mmc_card_uhs(struct mmc_card > > > *card) > > > card->host->ios.timing <= MMC_TIMING_UHS_DDR50; } > > > > > > +static inline int mmc_card_is_sduc(struct mmc_host *host) { > > > + return host->caps2 & MMC_CAP2_SD_SDUC; } > > > + > > > void mmc_retune_timer_stop(struct mmc_host *host); > > > > > > static inline void mmc_retune_needed(struct mmc_host *host) diff > > > --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h index > > > 6727576a8755..865cc0ca8543 100644 > > > --- a/include/linux/mmc/sd.h > > > +++ b/include/linux/mmc/sd.h > > > @@ -36,6 +36,7 @@ > > > /* OCR bit definitions */ > > > #define SD_OCR_S18R (1 << 24) /* 1.8V switching request > > > */ > > > #define SD_ROCR_S18A SD_OCR_S18R /* 1.8V switching > > > accepted by card */ > > > +#define SD_OCR_2T (1 << 27) /* HO2T/CO2T - SDUC > > > support */ > > > #define SD_OCR_XPC (1 << 28) /* SDXC power control */ > > > #define SD_OCR_CCS (1 << 30) /* Card Capacity Status */ > > > > > > -- > > > 2.25.1