> On Wed, 14 Aug 2024 at 09:31, Avri Altman <avri.altman@xxxxxxx> wrote:
> >
> > ACMD41 was extended to support the host-card handshake during
> > initialization. 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 ACMD41 until
> > eventually, the host will timed out and give up.
> >
> > Tested-by: Ricky WU <ricky_wu@xxxxxxxxxxx>
> > Signed-off-by: Avri Altman <avri.altman@xxxxxxx>
> > ---
> > 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;
> > +
>
> This code doesn't belong in mmc_send_app_op_cond(), but rather in
> mmc_sd_get_cid(), which is where we add one various OCR bits before we call
> mmc_send_app_op_cond() with it.
>
> For example, if the response of the SD_SEND_IF_COND commands indicates
> an SD 2.0 compliant card, we tag on the SD_OCR_CCS bit. It looks like that
> needs to be extended to the SD_OCR_2T bit too.
OK.
>
> > 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;
>
> Please don't abuse the host->caps2 for this purpose.
>
> Instead let's keep using the card->state to keep track of what type of card this
> is. You may have a look at how the MMC_CARD_SDXC bit is being used and
> just follow that behaviour for the SDUC cards too.
>
> Moreover, rather than assigning card->state at this point, let's do that from
> mmc_decode_csd() instead, when we realize that the card supports the new
> CSD structure version 3.
Just to recap - so we are all on the same page:
Ricky suggested this in v1 as well.
And we had a discussion if we should use the state field to indicate the card type.
However, Ricky had some good point why it should be here:
"...
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 because according to the spec, SDUC identification is not mandated by its capacity, but rather by the rocr.
I will follow your suggestion to squash 1 & 2 and set ult capacity based on the rocr.
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
> >
>
> Kind regards
> Uffe
