Re: [PATCH V4 07/11] mmc: mmc: Enable CQE's

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

 



On 21 July 2017 at 11:49, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> Enable or disable CQE when a card is added or removed respectively.

As a standalone change, this is hard to understand.

Perhaps if you squash this with some the patche calling ->cqe_off()
and the one actually setting ext_csd.cmdq_en, I can get a better
picture.

>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>  drivers/mmc/core/bus.c |  7 +++++++
>  drivers/mmc/core/mmc.c | 13 +++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index 301246513a37..a4b49e25fe96 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -369,10 +369,17 @@ int mmc_add_card(struct mmc_card *card)
>   */
>  void mmc_remove_card(struct mmc_card *card)
>  {
> +       struct mmc_host *host = card->host;
> +
>  #ifdef CONFIG_DEBUG_FS
>         mmc_remove_card_debugfs(card);
>  #endif
>
> +       if (host->cqe_enabled) {
> +               host->cqe_ops->cqe_disable(host);
> +               host->cqe_enabled = false;
> +       }
> +

This doesn't feel like the correct place to disable cqe.

Primarily because I don't think you enable cqe in mmc_add_card(), so
this isn't consistent.

>         if (mmc_card_present(card)) {
>                 if (mmc_host_is_spi(card->host)) {
>                         pr_info("%s: SPI card removed\n",
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 2ff0caf92bc8..92c6167d64e0 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1804,6 +1804,19 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>          */
>         card->reenable_cmdq = card->ext_csd.cmdq_en;
>
> +       if (card->ext_csd.cmdq_en && (host->caps2 & MMC_CAP2_CQE) &&
> +           !host->cqe_enabled) {

I assume card->ext_csd.cmdq_en can't be set, unless the MMC_CAP2_CQE
bit also is set. So there should be no reason to check it here again?

Also, can ever host->cqe_enabled be true in this path? If so, isn't
that wrong by itself?

> +               err = host->cqe_ops->cqe_enable(host, card);
> +               if (err) {
> +                       pr_err("%s: Failed to enable CQE, error %d\n",
> +                               mmc_hostname(host), err);
> +               } else {
> +                       host->cqe_enabled = true;
> +                       pr_info("%s: Command Queue Engine enabled\n",
> +                               mmc_hostname(host));
> +               }
> +       }
> +
>         /*
>          * The mandatory minimum values are defined for packed command.
>          * read: 5, write: 3
> --
> 1.9.1
>

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

  Powered by Linux