Re: [PATCH] mmc: usdhi6rol0: Implement card_busy function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 12 Aug 2021 at 16:51, Mårten Lindahl <marten.lindahl@xxxxxxxx> wrote:
>
> When switching card voltage to UHS voltage the mmc framework tries to
> check that the card is driving pins CMD, and DAT[0-3] low before the
> switch is made. Drivers that does not implement the card_busy function
> will manage to do the switch anyway, but the framework will print a
> warning about not being able to verify the voltage signal.
>
> Implement card_busy function. Renesas host interface only exposes pins
> DAT[0] and DAT[3] for reading the busy state, which is why only these
> two pins are checked.
>
> Signed-off-by: Mårten Lindahl <marten.lindahl@xxxxxxxx>
> ---
>  drivers/mmc/host/usdhi6rol0.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/mmc/host/usdhi6rol0.c b/drivers/mmc/host/usdhi6rol0.c
> index b9b79b1089a0..e400a646e675 100644
> --- a/drivers/mmc/host/usdhi6rol0.c
> +++ b/drivers/mmc/host/usdhi6rol0.c
> @@ -77,6 +77,7 @@
>  #define USDHI6_SD_INFO1_WP             BIT(7)
>  #define USDHI6_SD_INFO1_D3_CARD_OUT    BIT(8)
>  #define USDHI6_SD_INFO1_D3_CARD_IN     BIT(9)
> +#define USDHI6_SD_INFO1_SDDAT3         BIT(10)
>
>  #define USDHI6_SD_INFO2_CMD_ERR                BIT(0)
>  #define USDHI6_SD_INFO2_CRC_ERR                BIT(1)
> @@ -1186,6 +1187,21 @@ static int usdhi6_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios)
>         return ret;
>  }
>
> +static int usdhi6_card_busy(struct mmc_host *mmc)
> +{
> +       struct usdhi6_host *host = mmc_priv(mmc);
> +       u32 info1 = usdhi6_read(host, USDHI6_SD_INFO1);
> +       u32 info2 = usdhi6_read(host, USDHI6_SD_INFO2);
> +
> +       /* Check if the SD bus is processing a command */

Hmm, this sounds a bit confusing to me. Don't you want to say
something like "the clock logic is enabled".

The point is, from the mmc core point of view, ->card_busy() is called
to poll for busy completion *after* a command has been processed.

> +       if (!(info2 & USDHI6_SD_INFO2_SCLKDIVEN))
> +               return 0;
> +
> +       /* Card is busy if it is pulling dat[0] & dat[3] low */
> +       return !(info2 & USDHI6_SD_INFO2_SDDAT0 ||
> +                info1 & USDHI6_SD_INFO1_SDDAT3);

In fact, it's sufficient to monitor any one of the DATA lines to check
for busy for the UHS-I switch, according to the SD spec.

However, since we are using ->card_busy() to monitor also other busy
signal scenarios, like for some commands with R1B responses that
assert only DAT0 to signal busy (at least according to specs), I think
it's better to monitor solely DAT0 here.

I realize that the comment for the ->card_busy() callback in
include/linux/mmc/host.h needs to be updated to clarify this as well.

> +}
> +
>  static const struct mmc_host_ops usdhi6_ops = {
>         .request        = usdhi6_request,
>         .set_ios        = usdhi6_set_ios,
> @@ -1193,6 +1209,7 @@ static const struct mmc_host_ops usdhi6_ops = {
>         .get_ro         = usdhi6_get_ro,
>         .enable_sdio_irq = usdhi6_enable_sdio_irq,
>         .start_signal_voltage_switch = usdhi6_sig_volt_switch,
> +       .card_busy = usdhi6_card_busy,
>  };
>
>  /*                     State machine handlers                          */
> --
> 2.20.1
>

Kind regards
Uffe




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux