Re: [PATCH V13 06/10] mmc: sdhci-pci: Add CQHCI support for Intel GLK

[Ulf Hansson]

On 3 November 2017 at 14:20, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> Add CQHCI initialization and implement CQHCI operations for Intel GLK.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>

This looks good to me!

Kind regards
Uffe

> ---
>  drivers/mmc/host/Kconfig          |   1 +
>  drivers/mmc/host/sdhci-pci-core.c | 155 +++++++++++++++++++++++++++++++++++++-
>  2 files changed, 155 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 3092b7085cb5..2b02a9788bb6 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -81,6 +81,7 @@ config MMC_SDHCI_BIG_ENDIAN_32BIT_BYTE_SWAPPER
>  config MMC_SDHCI_PCI
>         tristate "SDHCI support on PCI bus"
>         depends on MMC_SDHCI && PCI
> +       select MMC_CQHCI
>         help
>           This selects the PCI Secure Digital Host Controller Interface.
>           Most controllers found today are PCI devices.
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 3e4f04fd5175..110c634cfb43 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -30,6 +30,8 @@
>  #include <linux/mmc/sdhci-pci-data.h>
>  #include <linux/acpi.h>
>
> +#include "cqhci.h"
> +
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
>
> @@ -116,6 +118,28 @@ int sdhci_pci_resume_host(struct sdhci_pci_chip *chip)
>
>         return 0;
>  }
> +
> +static int sdhci_cqhci_suspend(struct sdhci_pci_chip *chip)
> +{
> +       int ret;
> +
> +       ret = cqhci_suspend(chip->slots[0]->host->mmc);
> +       if (ret)
> +               return ret;
> +
> +       return sdhci_pci_suspend_host(chip);
> +}
> +
> +static int sdhci_cqhci_resume(struct sdhci_pci_chip *chip)
> +{
> +       int ret;
> +
> +       ret = sdhci_pci_resume_host(chip);
> +       if (ret)
> +               return ret;
> +
> +       return cqhci_resume(chip->slots[0]->host->mmc);
> +}
>  #endif
>
>  #ifdef CONFIG_PM
> @@ -166,8 +190,48 @@ static int sdhci_pci_runtime_resume_host(struct sdhci_pci_chip *chip)
>
>         return 0;
>  }
> +
> +static int sdhci_cqhci_runtime_suspend(struct sdhci_pci_chip *chip)
> +{
> +       int ret;
> +
> +       ret = cqhci_suspend(chip->slots[0]->host->mmc);
> +       if (ret)
> +               return ret;
> +
> +       return sdhci_pci_runtime_suspend_host(chip);
> +}
> +
> +static int sdhci_cqhci_runtime_resume(struct sdhci_pci_chip *chip)
> +{
> +       int ret;
> +
> +       ret = sdhci_pci_runtime_resume_host(chip);
> +       if (ret)
> +               return ret;
> +
> +       return cqhci_resume(chip->slots[0]->host->mmc);
> +}
>  #endif
>
> +static u32 sdhci_cqhci_irq(struct sdhci_host *host, u32 intmask)
> +{
> +       int cmd_error = 0;
> +       int data_error = 0;
> +
> +       if (!sdhci_cqe_irq(host, intmask, &cmd_error, &data_error))
> +               return intmask;
> +
> +       cqhci_irq(host->mmc, intmask, cmd_error, data_error);
> +
> +       return 0;
> +}
> +
> +static void sdhci_pci_dumpregs(struct mmc_host *mmc)
> +{
> +       sdhci_dumpregs(mmc_priv(mmc));
> +}
> +
>  /*****************************************************************************\
>   *                                                                           *
>   * Hardware specific quirk handling                                          *
> @@ -583,6 +647,18 @@ static void sdhci_intel_voltage_switch(struct sdhci_host *host)
>         .voltage_switch         = sdhci_intel_voltage_switch,
>  };
>
> +static const struct sdhci_ops sdhci_intel_glk_ops = {
> +       .set_clock              = sdhci_set_clock,
> +       .set_power              = sdhci_intel_set_power,
> +       .enable_dma             = sdhci_pci_enable_dma,
> +       .set_bus_width          = sdhci_set_bus_width,
> +       .reset                  = sdhci_reset,
> +       .set_uhs_signaling      = sdhci_set_uhs_signaling,
> +       .hw_reset               = sdhci_pci_hw_reset,
> +       .voltage_switch         = sdhci_intel_voltage_switch,
> +       .irq                    = sdhci_cqhci_irq,
> +};
> +
>  static void byt_read_dsm(struct sdhci_pci_slot *slot)
>  {
>         struct intel_host *intel_host = sdhci_pci_priv(slot);
> @@ -612,12 +688,80 @@ static int glk_emmc_probe_slot(struct sdhci_pci_slot *slot)
>  {
>         int ret = byt_emmc_probe_slot(slot);
>
> +       slot->host->mmc->caps2 |= MMC_CAP2_CQE;
> +
>         if (slot->chip->pdev->device != PCI_DEVICE_ID_INTEL_GLK_EMMC) {
>                 slot->host->mmc->caps2 |= MMC_CAP2_HS400_ES,
>                 slot->host->mmc_host_ops.hs400_enhanced_strobe =
>                                                 intel_hs400_enhanced_strobe;
> +               slot->host->mmc->caps2 |= MMC_CAP2_CQE_DCMD;
> +       }
> +
> +       return ret;
> +}
> +
> +static void glk_cqe_enable(struct mmc_host *mmc)
> +{
> +       struct sdhci_host *host = mmc_priv(mmc);
> +       u32 reg;
> +
> +       /*
> +        * CQE gets stuck if it sees Buffer Read Enable bit set, which can be
> +        * the case after tuning, so ensure the buffer is drained.
> +        */
> +       reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
> +       while (reg & SDHCI_DATA_AVAILABLE) {
> +               sdhci_readl(host, SDHCI_BUFFER);
> +               reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
> +       }
> +
> +       sdhci_cqe_enable(mmc);
> +}
> +
> +static const struct cqhci_host_ops glk_cqhci_ops = {
> +       .enable         = glk_cqe_enable,
> +       .disable        = sdhci_cqe_disable,
> +       .dumpregs       = sdhci_pci_dumpregs,
> +};
> +
> +static int glk_emmc_add_host(struct sdhci_pci_slot *slot)
> +{
> +       struct device *dev = &slot->chip->pdev->dev;
> +       struct sdhci_host *host = slot->host;
> +       struct cqhci_host *cq_host;
> +       bool dma64;
> +       int ret;
> +
> +       ret = sdhci_setup_host(host);
> +       if (ret)
> +               return ret;
> +
> +       cq_host = devm_kzalloc(dev, sizeof(*cq_host), GFP_KERNEL);
> +       if (!cq_host) {
> +               ret = -ENOMEM;
> +               goto cleanup;
>         }
>
> +       cq_host->mmio = host->ioaddr + 0x200;
> +       cq_host->quirks |= CQHCI_QUIRK_SHORT_TXFR_DESC_SZ;
> +       cq_host->ops = &glk_cqhci_ops;
> +
> +       dma64 = host->flags & SDHCI_USE_64_BIT_DMA;
> +       if (dma64)
> +               cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
> +
> +       ret = cqhci_init(cq_host, host->mmc, dma64);
> +       if (ret)
> +               goto cleanup;
> +
> +       ret = __sdhci_add_host(host);
> +       if (ret)
> +               goto cleanup;
> +
> +       return 0;
> +
> +cleanup:
> +       sdhci_cleanup_host(host);
>         return ret;
>  }
>
> @@ -699,11 +843,20 @@ static int byt_sd_probe_slot(struct sdhci_pci_slot *slot)
>  static const struct sdhci_pci_fixes sdhci_intel_glk_emmc = {
>         .allow_runtime_pm       = true,
>         .probe_slot             = glk_emmc_probe_slot,
> +       .add_host               = glk_emmc_add_host,
> +#ifdef CONFIG_PM_SLEEP
> +       .suspend                = sdhci_cqhci_suspend,
> +       .resume                 = sdhci_cqhci_resume,
> +#endif
> +#ifdef CONFIG_PM
> +       .runtime_suspend        = sdhci_cqhci_runtime_suspend,
> +       .runtime_resume         = sdhci_cqhci_runtime_resume,
> +#endif
>         .quirks                 = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>         .quirks2                = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
>                                   SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 |
>                                   SDHCI_QUIRK2_STOP_WITH_TC,
> -       .ops                    = &sdhci_intel_byt_ops,
> +       .ops                    = &sdhci_intel_glk_ops,
>         .priv_size              = sizeof(struct intel_host),
>  };
>
> --
> 1.9.1
>