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.
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
>
>> +
>> static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
>> + BYT_PM_OPS
>> .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
>> SDHCI_QUIRK_NO_LED,
>> .quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
>> @@ -983,6 +1070,7 @@ static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
>> };
>>
>> static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
>> + BYT_PM_OPS
>> .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
>> SDHCI_QUIRK_NO_LED,
>> .quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
>> @@ -994,6 +1082,7 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
>> };
>>
>> static const struct sdhci_pci_fixes sdhci_intel_byt_sd = {
>> + BYT_PM_OPS
>> .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
>> SDHCI_QUIRK_NO_LED,
>> .quirks2 = SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON |
>> --
>> 2.17.1
>>
>
> Kind regards
> Uffe
>
