On Fri, 3 May 2019 at 13:45, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > > On 3/05/19 12:59 PM, Ulf Hansson wrote: > > On Thu, 2 May 2019 at 09:53, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > >> > >> Some time ago, a fix was done for the sdhci-acpi driver, refer > >> commit 6e1c7d6103fe ("mmc: sdhci-acpi: Reduce Baytrail eMMC/SD/SDIO > >> hangs"). The same issue was not expected to affect the sdhci-pci driver, > >> but there have been reports to the contrary, so make the same hardware > >> setting change. > >> > >> This patch applies to v5.0+ but before that backports will be required. > >> > >> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> > >> Cc: stable@xxxxxxxxxxxxxxx > >> --- > >> drivers/mmc/host/Kconfig | 1 + > >> drivers/mmc/host/sdhci-pci-core.c | 89 +++++++++++++++++++++++++++++++ > >> 2 files changed, 90 insertions(+) > >> > >> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > >> index 9c01310a0d2e..d084a9d63623 100644 > >> --- a/drivers/mmc/host/Kconfig > >> +++ b/drivers/mmc/host/Kconfig > >> @@ -92,6 +92,7 @@ config MMC_SDHCI_PCI > >> tristate "SDHCI support on PCI bus" > >> depends on MMC_SDHCI && PCI > >> select MMC_CQHCI > >> + select IOSF_MBI if X86 > >> 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 a3d7a9db76c5..64e79a19d5ad 100644 > >> --- a/drivers/mmc/host/sdhci-pci-core.c > >> +++ b/drivers/mmc/host/sdhci-pci-core.c > >> @@ -31,6 +31,10 @@ > >> #include <linux/mmc/sdhci-pci-data.h> > >> #include <linux/acpi.h> > >> > >> +#ifdef CONFIG_X86 > >> +#include <asm/iosf_mbi.h> > >> +#endif > >> + > >> #include "cqhci.h" > >> > >> #include "sdhci.h" > >> @@ -451,6 +455,50 @@ static const struct sdhci_pci_fixes sdhci_intel_pch_sdio = { > >> .probe_slot = pch_hc_probe_slot, > >> }; > >> > >> +#ifdef CONFIG_X86 > >> + > >> +#define BYT_IOSF_SCCEP 0x63 > >> +#define BYT_IOSF_OCP_NETCTRL0 0x1078 > >> +#define BYT_IOSF_OCP_TIMEOUT_BASE GENMASK(10, 8) > >> + > >> +static void byt_ocp_setting(struct pci_dev *pdev) > >> +{ > >> + u32 val = 0; > >> + > >> + if (pdev->device != PCI_DEVICE_ID_INTEL_BYT_EMMC && > >> + pdev->device != PCI_DEVICE_ID_INTEL_BYT_SDIO && > >> + pdev->device != PCI_DEVICE_ID_INTEL_BYT_SD && > >> + pdev->device != PCI_DEVICE_ID_INTEL_BYT_EMMC2) > >> + return; > >> + > >> + if (iosf_mbi_read(BYT_IOSF_SCCEP, MBI_CR_READ, BYT_IOSF_OCP_NETCTRL0, > >> + &val)) { > >> + dev_err(&pdev->dev, "%s read error\n", __func__); > >> + return; > >> + } > >> + > >> + if (!(val & BYT_IOSF_OCP_TIMEOUT_BASE)) > >> + return; > >> + > >> + val &= ~BYT_IOSF_OCP_TIMEOUT_BASE; > >> + > >> + if (iosf_mbi_write(BYT_IOSF_SCCEP, MBI_CR_WRITE, BYT_IOSF_OCP_NETCTRL0, > >> + val)) { > >> + dev_err(&pdev->dev, "%s write error\n", __func__); > >> + return; > >> + } > >> + > >> + dev_dbg(&pdev->dev, "%s completed\n", __func__); > >> +} > >> + > >> +#else > >> + > >> +static inline void byt_ocp_setting(struct pci_dev *pdev) > >> +{ > >> +} > >> + > >> +#endif > >> + > >> enum { > >> INTEL_DSM_FNS = 0, > >> INTEL_DSM_V18_SWITCH = 3, > >> @@ -715,6 +763,8 @@ static void byt_probe_slot(struct sdhci_pci_slot *slot) > >> > >> byt_read_dsm(slot); > >> > >> + byt_ocp_setting(slot->chip->pdev); > >> + > >> ops->execute_tuning = intel_execute_tuning; > >> ops->start_signal_voltage_switch = intel_start_signal_voltage_switch; > >> > >> @@ -971,7 +1021,44 @@ static const struct sdhci_pci_fixes sdhci_intel_glk_emmc = { > >> .priv_size = sizeof(struct intel_host), > >> }; > >> > >> +#ifdef CONFIG_PM_SLEEP > >> + > >> +static int byt_resume(struct sdhci_pci_chip *chip) > >> +{ > >> + byt_ocp_setting(chip->pdev); > >> + > >> + return sdhci_pci_resume_host(chip); > >> +} > >> + > >> +#define BYT_SPM_OPS .resume = byt_resume, > >> + > >> +#else > >> + > >> +#define BYT_SPM_OPS > >> + > >> +#endif > >> + > >> +#ifdef CONFIG_PM > >> + > >> +static int byt_runtime_resume(struct sdhci_pci_chip *chip) > >> +{ > >> + byt_ocp_setting(chip->pdev); > >> + > >> + return sdhci_pci_runtime_resume_host(chip); > >> +} > >> + > >> +#define BYT_RPM_OPS .runtime_resume = byt_runtime_resume, > >> + > >> +#else > >> + > >> +#define BYT_RPM_OPS > >> + > >> +#endif > >> + > >> +#define BYT_PM_OPS BYT_SPM_OPS BYT_RPM_OPS > > > > This ifdef hackary is a bit too much for me. :-) > > > > Could you have the callbacks being assigned always and instead rely > > only on byt_ocp_setting() having different implementations, depending > > on if CONFIG_X86 is set or not? > > The callbacks themselves are under ifdef, so that wouldn't work. Wouldn't SET_SYSTEM_SLEEP_PM_OPS and SET_RUNTIME_PM_OPS help with that? > Could get rid of BYT_PM_OPS and put this instead: > > #ifdef CONFIG_PM_SLEEP > .resume = byt_resume, > #endif > #ifdef CONFIG_PM > .runtime_resume = byt_runtime_resume, > #endif That's better! [...] Kind regards Uffe