On 10/01/20 10:56 am, Veerabhadrarao Badiganti wrote: > > On 1/2/2020 5:00 PM, Veerabhadrarao Badiganti wrote: >> >> On 12/20/2019 7:29 PM, Adrian Hunter wrote: >>> On 17/12/19 2:37 pm, Veerabhadrarao Badiganti wrote: >>>> From: Ritesh Harjani<riteshh@xxxxxxxxxxxxxx> >>>> >>>> This adds CQHCI support for sdhci-msm platforms. >>>> >>>> Signed-off-by: Ritesh Harjani<riteshh@xxxxxxxxxxxxxx> >>>> Signed-off-by: Veerabhadrarao Badiganti<vbadigan@xxxxxxxxxxxxxx> >>>> >>>> --- >>>> This patch is based on RFC patch >>>> https://lkml.org/lkml/2017/8/30/313 >>>> >>>> Changes since RFC: >>>> - Updated settings so that TDLBA won't get reset when >>>> CQE is enabled. >>>> - Removed new compatible string and moved to supports-cqe >>>> dt flag to identify CQE support. >>>> - Incorporated review comments. >>>> >>>> Tested on: qcs404, sc7180 >>>> --- >>>> drivers/mmc/host/sdhci-msm.c | 115 >>>> ++++++++++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 114 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c >>>> index 3d0bb5e..a4e3507 100644 >>>> --- a/drivers/mmc/host/sdhci-msm.c >>>> +++ b/drivers/mmc/host/sdhci-msm.c >>>> @@ -15,6 +15,7 @@ >>>> #include <linux/regulator/consumer.h> >>>> #include "sdhci-pltfm.h" >>>> +#include "cqhci.h" >>>> #define CORE_MCI_VERSION 0x50 >>>> #define CORE_VERSION_MAJOR_SHIFT 28 >>>> @@ -122,6 +123,10 @@ >>>> #define msm_host_writel(msm_host, val, host, offset) \ >>>> msm_host->var_ops->msm_writel_relaxed(val, host, offset) >>>> +/* CQHCI vendor specific registers */ >>>> +#define CQHCI_VENDOR_CFG1 0xA00 >>>> +#define DISABLE_RST_ON_CMDQ_EN (0x3 << 13) >>>> + >>>> struct sdhci_msm_offset { >>>> u32 core_hc_mode; >>>> u32 core_mci_data_cnt; >>>> @@ -1567,6 +1572,109 @@ static void sdhci_msm_set_clock(struct >>>> sdhci_host *host, unsigned int clock) >>>> __sdhci_msm_set_clock(host, clock); >>>> } >>>> +/*****************************************************************************\ >>>> >>>> + * * >>>> + * MSM Command Queue Engine >>>> (CQE) * >>>> + * * >>>> +\*****************************************************************************/ >>>> >>>> + >>>> +static u32 sdhci_msm_cqe_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; >>>> +} >>>> + >>>> +void sdhci_msm_cqe_disable(struct mmc_host *mmc, bool recovery) >>>> +{ >>>> + struct sdhci_host *host = mmc_priv(mmc); >>>> + unsigned long flags; >>>> + u32 ctrl; >>>> + >>>> + /* >>>> + * When CQE is halted, the legacy SDHCI path operates only >>>> + * on 128bit descriptors in 64bit mode. >>>> + */ >>>> + if (host->flags & SDHCI_USE_64_BIT_DMA) >>>> + host->desc_sz = 16; >>> The adma_table_sz depends on desc_sz, so it cannot be changed here. >>> If you do something like below, then you can set desc_sz before calling >>> sdhci_setup_host() >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index f4540f9892ce..f1d3b70ff769 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -3825,9 +3825,10 @@ int sdhci_setup_host(struct sdhci_host *host) >>> void *buf; >>> if (host->flags & SDHCI_USE_64_BIT_DMA) { >>> + if (!host->desc_sz) >>> + host->desc_sz = SDHCI_ADMA2_64_DESC_SZ(host); >>> host->adma_table_sz = host->adma_table_cnt * >>> - SDHCI_ADMA2_64_DESC_SZ(host); >>> - host->desc_sz = SDHCI_ADMA2_64_DESC_SZ(host); >>> + host->desc_sz; >>> } else { >>> host->adma_table_sz = host->adma_table_cnt * >>> SDHCI_ADMA2_32_DESC_SZ; >> >> Thanks Adrian for the suggestion. I will add this change. >> >> But even with this change, still i will have to override 'host->desc_sz' >> variable since qcom sdhci controller expects/operates-on >> >> 12-byte descriptor as long was CQE is not enabled. When CQE is enabled, it >> operates only on 16-bype descriptors (even when CQE is halted). >> >> If i fix "host->desc_sz" to 16 then all the data transfer commands during >> card initialization (till CQE is enabled) would fail. >> >> I may have to update as below: >> >> host->desc_sz = 16; >> >> sdhci_add_host() ; >> >> host->desc_sz = 12; >> >> And then cqhci_host_ops->enable() -> host->desc_sz = 16; >> >> Please let me know if this is fine or if you have any other suggestion to >> support this limitation of qcom controller w.r.t ADMA descriptors with CQE. >> > Hi Adrian, > > Do you have any suggestions on the way to support both the descriptor sizes? How about we have 2 variables: alloc_desc_sz and desc_sz Then, in sdhci_setup_host(): host->alloc_desc_sz = SDHCI_ADMA2_64/32_DESC_SZ(host); host->desc_sz = host->alloc_desc_sz; Then desc_sz can be changed (in between requests) so long as desc_sz < alloc_desc_sz. > >>>> + >>>> + spin_lock_irqsave(&host->lock, flags); >>>> + >>>> + /* >>>> + * During CQE operation command complete bit gets latched. >>>> + * So s/w should clear command complete interrupt status when CQE is >>>> + * halted. Otherwise unexpected SDCHI legacy interrupt gets >>>> + * triggered when CQE is halted. >>>> + */ >>>> + ctrl = sdhci_readl(host, SDHCI_INT_ENABLE); >>>> + ctrl |= SDHCI_INT_RESPONSE; >>>> + sdhci_writel(host, ctrl, SDHCI_INT_ENABLE); >>>> + sdhci_writel(host, SDHCI_INT_RESPONSE, SDHCI_INT_STATUS); >>>> + >>>> + spin_unlock_irqrestore(&host->lock, flags); >>>> + >>>> + sdhci_cqe_disable(mmc, recovery); >>>> +} >>>> + >>>> +static const struct cqhci_host_ops sdhci_msm_cqhci_ops = { >>>> + .enable = sdhci_cqe_enable, >>>> + .disable = sdhci_msm_cqe_disable, >>>> +}; >>>> + >>>> +static int sdhci_msm_cqe_add_host(struct sdhci_host *host, >>>> + struct platform_device *pdev) >>>> +{ >>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host); >>>> + struct cqhci_host *cq_host; >>>> + bool dma64; >>>> + int ret; >>>> + >>>> + ret = sdhci_setup_host(host); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + cq_host = cqhci_pltfm_init(pdev); >>>> + if (IS_ERR(cq_host)) { >>>> + ret = PTR_ERR(cq_host); >>>> + dev_err(&pdev->dev, "cqhci-pltfm init: failed: %d\n", ret); >>>> + goto cleanup; >>>> + } >>>> + >>>> + msm_host->mmc->caps2 |= MMC_CAP2_CQE | MMC_CAP2_CQE_DCMD; >>>> + cq_host->ops = &sdhci_msm_cqhci_ops; >>>> + >>>> + dma64 = host->flags & SDHCI_USE_64_BIT_DMA; >>>> + >>>> + ret = cqhci_init(cq_host, host->mmc, dma64); >>>> + if (ret) { >>>> + dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n", >>>> + mmc_hostname(host->mmc), ret); >>>> + goto cleanup; >>>> + } >>>> + >>>> + /* Disable cqe reset due to cqe enable signal */ >>>> + cqhci_writel(cq_host, cqhci_readl(cq_host, CQHCI_VENDOR_CFG1) | >>>> + DISABLE_RST_ON_CMDQ_EN, CQHCI_VENDOR_CFG1); >>>> + >>>> + ret = __sdhci_add_host(host); >>>> + if (ret) >>>> + goto cleanup; >>>> + >>>> + dev_info(&pdev->dev, "%s: CQE init: success\n", >>>> + mmc_hostname(host->mmc)); >>>> + return ret; >>>> + >>>> +cleanup: >>>> + sdhci_cleanup_host(host); >>>> + return ret; >>>> +} >>>> + >>>> /* >>>> * Platform specific register write functions. This is so that, if any >>>> * register write needs to be followed up by platform specific actions, >>>> @@ -1731,6 +1839,7 @@ static void sdhci_msm_set_regulator_caps(struct >>>> sdhci_msm_host *msm_host) >>>> .set_uhs_signaling = sdhci_msm_set_uhs_signaling, >>>> .write_w = sdhci_msm_writew, >>>> .write_b = sdhci_msm_writeb, >>>> + .irq = sdhci_msm_cqe_irq, >>>> }; >>>> static const struct sdhci_pltfm_data sdhci_msm_pdata = { >>>> @@ -1754,6 +1863,7 @@ static int sdhci_msm_probe(struct platform_device >>>> *pdev) >>>> u8 core_major; >>>> const struct sdhci_msm_offset *msm_offset; >>>> const struct sdhci_msm_variant_info *var_info; >>>> + struct device_node *node = pdev->dev.of_node; >>>> host = sdhci_pltfm_init(pdev, &sdhci_msm_pdata, sizeof(*msm_host)); >>>> if (IS_ERR(host)) >>>> @@ -1952,7 +2062,10 @@ static int sdhci_msm_probe(struct platform_device >>>> *pdev) >>>> pm_runtime_use_autosuspend(&pdev->dev); >>>> host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning; >>>> - ret = sdhci_add_host(host); >>>> + if (of_property_read_bool(node, "supports-cqe")) >>>> + ret = sdhci_msm_cqe_add_host(host, pdev); >>>> + else >>>> + ret = sdhci_add_host(host); >>>> if (ret) >>>> goto pm_runtime_disable; >>>> sdhci_msm_set_regulator_caps(msm_host); >>>> > Thanks > > Veera
