On Thu, 22 Aug 2024 at 15:17, Avri Altman <Avri.Altman@xxxxxxx> wrote:
>
> > 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.
In principle you are right. The rocr indicates whether it's an SDSC,
SDHC, SDXC or an SDUC card.
On the other hand we are checking the CSD structure version for
SDSC/SDHC/SDXC - so I would rather be consistent with that way, as it
seems to work fine.
According to the spec the CSD structure version 3 is dedicated for
SDUC cards, right?
[...]
Kind regards
Uffe
