From f8870ae6e2d6be75b1accc2db981169fdfbea7ab Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Wed, 14 Feb 2018 15:57:43 +0200 Subject: [PATCH 1/6] mmc: sdhci-pci: Fix S0i3 for Intel BYT-based controllers Tuning can leave the IP in an active state (Buffer Read Enable bit set) which prevents the entry to low power states (i.e. S0i3). Data reset will clear it. Generally tuning is followed by a data transfer which will anyway sort out the state, so it is rare that S0i3 is actually prevented. Signed-off-by: Adrian Hunter Cc: stable@vger.kernel.org Signed-off-by: Ulf Hansson --- drivers/mmc/host/sdhci-pci-core.c | 35 +++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c index 6d1a983e6227..82c4f05f91d8 100644 --- a/drivers/mmc/host/sdhci-pci-core.c +++ b/drivers/mmc/host/sdhci-pci-core.c @@ -654,9 +654,36 @@ static void byt_read_dsm(struct sdhci_pci_slot *slot) slot->chip->rpm_retune = intel_host->d3_retune; } +static int intel_execute_tuning(struct mmc_host *mmc, u32 opcode) +{ + int err = sdhci_execute_tuning(mmc, opcode); + struct sdhci_host *host = mmc_priv(mmc); + + if (err) + return err; + + /* + * Tuning can leave the IP in an active state (Buffer Read Enable bit + * set) which prevents the entry to low power states (i.e. S0i3). Data + * reset will clear it. + */ + sdhci_reset(host, SDHCI_RESET_DATA); + + return 0; +} + +static void byt_probe_slot(struct sdhci_pci_slot *slot) +{ + struct mmc_host_ops *ops = &slot->host->mmc_host_ops; + + byt_read_dsm(slot); + + ops->execute_tuning = intel_execute_tuning; +} + static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot) { - byt_read_dsm(slot); + byt_probe_slot(slot); slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE | MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR | MMC_CAP_CMD_DURING_TFR | @@ -779,7 +806,7 @@ static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot) { int err; - byt_read_dsm(slot); + byt_probe_slot(slot); err = ni_set_max_freq(slot); if (err) @@ -792,7 +819,7 @@ static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot) static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot) { - byt_read_dsm(slot); + byt_probe_slot(slot); slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE | MMC_CAP_WAIT_WHILE_BUSY; return 0; @@ -800,7 +827,7 @@ static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot) static int byt_sd_probe_slot(struct sdhci_pci_slot *slot) { - byt_read_dsm(slot); + byt_probe_slot(slot); slot->host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_AGGRESSIVE_PM | MMC_CAP_CD_WAKE; slot->cd_idx = 0; From 325501d9360eb42c7c51e6daa0d733844c1e790b Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 23 Feb 2018 13:44:19 +0100 Subject: [PATCH 2/6] mmc: dw_mmc-k3: Fix out-of-bounds access through DT alias The hs_timing_cfg[] array is indexed using a value derived from the "mshcN" alias in DT, which may lead to an out-of-bounds access. Fix this by adding a range check. Fixes: 361c7fe9b02eee7e ("mmc: dw_mmc-k3: add sd support for hi3660") Signed-off-by: Geert Uytterhoeven Reviewed-by: Shawn Lin Cc: Signed-off-by: Ulf Hansson --- drivers/mmc/host/dw_mmc-k3.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c index 73fd75c3c824..75ae5803b0db 100644 --- a/drivers/mmc/host/dw_mmc-k3.c +++ b/drivers/mmc/host/dw_mmc-k3.c @@ -135,6 +135,9 @@ static int dw_mci_hi6220_parse_dt(struct dw_mci *host) if (priv->ctrl_id < 0) priv->ctrl_id = 0; + if (priv->ctrl_id >= TIMING_MODE) + return -EINVAL; + host->priv = priv; return 0; } From a4faa4929ed3be15e2d500d2405f992f6dedc8eb Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Sat, 24 Feb 2018 14:17:22 +0800 Subject: [PATCH 3/6] mmc: dw_mmc: Factor out dw_mci_init_slot_caps Factor out dw_mci_init_slot_caps to consolidate parsing all differents types of capabilities from host contrllers. No functional change intended. Signed-off-by: Shawn Lin Fixes: 800d78bfccb3 ("mmc: dw_mmc: add support for implementation specific callbacks") Cc: Signed-off-by: Ulf Hansson --- drivers/mmc/host/dw_mmc.c | 73 +++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 0aa39975f33b..4033cf96c7d7 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -2778,12 +2778,50 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } +static int dw_mci_init_slot_caps(struct dw_mci_slot *slot) +{ + struct dw_mci *host = slot->host; + const struct dw_mci_drv_data *drv_data = host->drv_data; + struct mmc_host *mmc = slot->mmc; + int ctrl_id; + + if (host->pdata->caps) + mmc->caps = host->pdata->caps; + + /* + * Support MMC_CAP_ERASE by default. + * It needs to use trim/discard/erase commands. + */ + mmc->caps |= MMC_CAP_ERASE; + + if (host->pdata->pm_caps) + mmc->pm_caps = host->pdata->pm_caps; + + if (host->dev->of_node) { + ctrl_id = of_alias_get_id(host->dev->of_node, "mshc"); + if (ctrl_id < 0) + ctrl_id = 0; + } else { + ctrl_id = to_platform_device(host->dev)->id; + } + if (drv_data && drv_data->caps) + mmc->caps |= drv_data->caps[ctrl_id]; + + if (host->pdata->caps2) + mmc->caps2 = host->pdata->caps2; + + /* Process SDIO IRQs through the sdio_irq_work. */ + if (mmc->caps & MMC_CAP_SDIO_IRQ) + mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD; + + return 0; +} + static int dw_mci_init_slot(struct dw_mci *host) { struct mmc_host *mmc; struct dw_mci_slot *slot; - const struct dw_mci_drv_data *drv_data = host->drv_data; - int ctrl_id, ret; + int ret; u32 freq[2]; mmc = mmc_alloc_host(sizeof(struct dw_mci_slot), host->dev); @@ -2817,38 +2855,13 @@ static int dw_mci_init_slot(struct dw_mci *host) if (!mmc->ocr_avail) mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34; - if (host->pdata->caps) - mmc->caps = host->pdata->caps; - - /* - * Support MMC_CAP_ERASE by default. - * It needs to use trim/discard/erase commands. - */ - mmc->caps |= MMC_CAP_ERASE; - - if (host->pdata->pm_caps) - mmc->pm_caps = host->pdata->pm_caps; - - if (host->dev->of_node) { - ctrl_id = of_alias_get_id(host->dev->of_node, "mshc"); - if (ctrl_id < 0) - ctrl_id = 0; - } else { - ctrl_id = to_platform_device(host->dev)->id; - } - if (drv_data && drv_data->caps) - mmc->caps |= drv_data->caps[ctrl_id]; - - if (host->pdata->caps2) - mmc->caps2 = host->pdata->caps2; - ret = mmc_of_parse(mmc); if (ret) goto err_host_allocated; - /* Process SDIO IRQs through the sdio_irq_work. */ - if (mmc->caps & MMC_CAP_SDIO_IRQ) - mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD; + ret = dw_mci_init_slot_caps(slot); + if (ret) + goto err_host_allocated; /* Useful defaults if platform data is unset. */ if (host->use_dma == TRANS_MODE_IDMAC) { From 0d84b9e5631d923744767dc6608672df906dd092 Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Sat, 24 Feb 2018 14:17:23 +0800 Subject: [PATCH 4/6] mmc: dw_mmc: Fix out-of-bounds access for slot's caps Add num_caps field for dw_mci_drv_data to validate the controller id from DT alias and non-DT ways. Reported-by: Geert Uytterhoeven Signed-off-by: Shawn Lin Fixes: 800d78bfccb3 ("mmc: dw_mmc: add support for implementation specific callbacks") Cc: Signed-off-by: Ulf Hansson --- drivers/mmc/host/dw_mmc-exynos.c | 1 + drivers/mmc/host/dw_mmc-k3.c | 1 + drivers/mmc/host/dw_mmc-rockchip.c | 1 + drivers/mmc/host/dw_mmc-zx.c | 1 + drivers/mmc/host/dw_mmc.c | 9 ++++++++- drivers/mmc/host/dw_mmc.h | 2 ++ 6 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c index 35026795be28..fa41d9422d57 100644 --- a/drivers/mmc/host/dw_mmc-exynos.c +++ b/drivers/mmc/host/dw_mmc-exynos.c @@ -487,6 +487,7 @@ static unsigned long exynos_dwmmc_caps[4] = { static const struct dw_mci_drv_data exynos_drv_data = { .caps = exynos_dwmmc_caps, + .num_caps = ARRAY_SIZE(exynos_dwmmc_caps), .init = dw_mci_exynos_priv_init, .set_ios = dw_mci_exynos_set_ios, .parse_dt = dw_mci_exynos_parse_dt, diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c index 75ae5803b0db..89cdb3d533bb 100644 --- a/drivers/mmc/host/dw_mmc-k3.c +++ b/drivers/mmc/host/dw_mmc-k3.c @@ -210,6 +210,7 @@ static int dw_mci_hi6220_execute_tuning(struct dw_mci_slot *slot, u32 opcode) static const struct dw_mci_drv_data hi6220_data = { .caps = dw_mci_hi6220_caps, + .num_caps = ARRAY_SIZE(dw_mci_hi6220_caps), .switch_voltage = dw_mci_hi6220_switch_voltage, .set_ios = dw_mci_hi6220_set_ios, .parse_dt = dw_mci_hi6220_parse_dt, diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c index a3f1c2b30145..339295212935 100644 --- a/drivers/mmc/host/dw_mmc-rockchip.c +++ b/drivers/mmc/host/dw_mmc-rockchip.c @@ -319,6 +319,7 @@ static const struct dw_mci_drv_data rk2928_drv_data = { static const struct dw_mci_drv_data rk3288_drv_data = { .caps = dw_mci_rk3288_dwmmc_caps, + .num_caps = ARRAY_SIZE(dw_mci_rk3288_dwmmc_caps), .set_ios = dw_mci_rk3288_set_ios, .execute_tuning = dw_mci_rk3288_execute_tuning, .parse_dt = dw_mci_rk3288_parse_dt, diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c index d38e94ae2b85..c06b5393312f 100644 --- a/drivers/mmc/host/dw_mmc-zx.c +++ b/drivers/mmc/host/dw_mmc-zx.c @@ -195,6 +195,7 @@ static unsigned long zx_dwmmc_caps[3] = { static const struct dw_mci_drv_data zx_drv_data = { .caps = zx_dwmmc_caps, + .num_caps = ARRAY_SIZE(zx_dwmmc_caps), .execute_tuning = dw_mci_zx_execute_tuning, .prepare_hs400_tuning = dw_mci_zx_prepare_hs400_tuning, .parse_dt = dw_mci_zx_parse_dt, diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 4033cf96c7d7..a850f8d7d4b5 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -2804,8 +2804,15 @@ static int dw_mci_init_slot_caps(struct dw_mci_slot *slot) } else { ctrl_id = to_platform_device(host->dev)->id; } - if (drv_data && drv_data->caps) + + if (drv_data && drv_data->caps) { + if (ctrl_id >= drv_data->num_caps) { + dev_err(host->dev, "invalid controller id %d\n", + ctrl_id); + return -EINVAL; + } mmc->caps |= drv_data->caps[ctrl_id]; + } if (host->pdata->caps2) mmc->caps2 = host->pdata->caps2; diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h index e3124f06a47e..1424bd490dd1 100644 --- a/drivers/mmc/host/dw_mmc.h +++ b/drivers/mmc/host/dw_mmc.h @@ -543,6 +543,7 @@ struct dw_mci_slot { /** * dw_mci driver data - dw-mshc implementation specific driver data. * @caps: mmc subsystem specified capabilities of the controller(s). + * @num_caps: number of capabilities specified by @caps. * @init: early implementation specific initialization. * @set_ios: handle bus specific extensions. * @parse_dt: parse implementation specific device tree properties. @@ -554,6 +555,7 @@ struct dw_mci_slot { */ struct dw_mci_drv_data { unsigned long *caps; + u32 num_caps; int (*init)(struct dw_mci *host); void (*set_ios)(struct dw_mci *host, struct mmc_ios *ios); int (*parse_dt)(struct dw_mci *host); From 5b43df8b4c1a7f0c3fbf793c9566068e6b1e570c Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Fri, 23 Feb 2018 16:47:25 +0800 Subject: [PATCH 5/6] mmc: dw_mmc: Avoid accessing registers in runtime suspended state cat /sys/kernel/debug/mmc0/regs will hang up the system since it's in runtime suspended state, so the genpd and biu_clk is off. This patch fixes this problem by calling pm_runtime_get_sync to wake it up before reading the registers. Fixes: e9ed8835e990 ("mmc: dw_mmc: add runtime PM callback") Cc: Signed-off-by: Shawn Lin Reviewed-by: Jaehoon Chung Signed-off-by: Ulf Hansson --- drivers/mmc/host/dw_mmc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index a850f8d7d4b5..d9b4acefed31 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -165,6 +165,8 @@ static int dw_mci_regs_show(struct seq_file *s, void *v) { struct dw_mci *host = s->private; + pm_runtime_get_sync(host->dev); + seq_printf(s, "STATUS:\t0x%08x\n", mci_readl(host, STATUS)); seq_printf(s, "RINTSTS:\t0x%08x\n", mci_readl(host, RINTSTS)); seq_printf(s, "CMD:\t0x%08x\n", mci_readl(host, CMD)); @@ -172,6 +174,8 @@ static int dw_mci_regs_show(struct seq_file *s, void *v) seq_printf(s, "INTMASK:\t0x%08x\n", mci_readl(host, INTMASK)); seq_printf(s, "CLKENA:\t0x%08x\n", mci_readl(host, CLKENA)); + pm_runtime_put_autosuspend(host->dev); + return 0; } From 3a574919f0cc15a46ec14c3e5e08300908991915 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Tue, 27 Feb 2018 11:49:09 +0100 Subject: [PATCH 6/6] mmc: core: Avoid hanging to claim host for mmc via some nested calls As the block layer, since the conversion to blkmq, claims the host using a context, a following nested call to mmc_claim_host(), which isn't using a context, may hang. Calling mmc_interrupt_hpi() and mmc_read_bkops_status() via the mmc block layer, may suffer from this problem, as these functions are calling mmc_claim|release_host(). Let's fix the problem by removing the calls to mmc_claim|release_host() from the above mentioned functions and instead make the callers responsible of claiming/releasing the host. As a matter of fact, the existing callers already deals with it. Fixes: 81196976ed94 ("mmc: block: Add blk-mq support") Reported-by: Dmitry Osipenko Suggested-by: Adrian Hunter Tested-by: Dmitry Osipenko Signed-off-by: Ulf Hansson Acked-by: Adrian Hunter Reviewed-by: Shawn Lin --- drivers/mmc/core/mmc_ops.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index 908e4db03535..42d6aa89a48a 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -848,7 +848,6 @@ int mmc_interrupt_hpi(struct mmc_card *card) return 1; } - mmc_claim_host(card->host); err = mmc_send_status(card, &status); if (err) { pr_err("%s: Get card status fail\n", mmc_hostname(card->host)); @@ -890,7 +889,6 @@ int mmc_interrupt_hpi(struct mmc_card *card) } while (!err); out: - mmc_release_host(card->host); return err; } @@ -932,9 +930,7 @@ static int mmc_read_bkops_status(struct mmc_card *card) int err; u8 *ext_csd; - mmc_claim_host(card->host); err = mmc_get_ext_csd(card, &ext_csd); - mmc_release_host(card->host); if (err) return err;