[PATCH V2 0/4] mmc: sdhci: Fix card detect race for Intel BXT/APL

[PATCH V2 0/4] mmc: sdhci: Fix card detect race for Intel BXT/APL

[Adrian Hunter]

Hi

Here is V2 of patches to fix a problem on Intel BXT/APL.
The PCI and ACPI Ids for BXT/APL were added in v4.4 so the
patches are cc stable v4.4+.

Changes in V2:

Override the mmc host ->get_cd() operation instead of adding a
new sdhci host operation.


Adrian Hunter (4):
      mmc: sdhci: Allow override of mmc host operations
      mmc: sdhci: Allow override of get_cd() called from sdhci_request()
      mmc: sdhci-pci: Fix card detect race for Intel BXT/APL
      mmc: sdhci-acpi: Fix card detect race for Intel BXT/APL

 drivers/mmc/host/sdhci-acpi.c     | 30 ++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-pci-core.c | 31 +++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c          |  5 +++--
 drivers/mmc/host/sdhci.h          |  1 +
 4 files changed, 65 insertions(+), 2 deletions(-)


Regards
Adrian

[PATCH V2 1/4] mmc: sdhci: Allow override of mmc host operations

[Adrian Hunter]

In the past, fixes for specific hardware devices were implemented
in sdhci using quirks.  That approach is no longer accepted because
the growing number of quirks was starting to make the code difficult
to understand and maintain.

One alternative to quirks, is to allow drivers to override the default
mmc host operations.  This patch makes it easy to do that, and it is
needed for a subsequent bug fix, for which separate patches are
provided.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.4+
---
 drivers/mmc/host/sdhci.c | 3 ++-
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d622435d1bcc..9bfa66df963a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2849,6 +2849,8 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
 
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
+	host->mmc_host_ops = sdhci_ops;
+	mmc->ops = &host->mmc_host_ops;
 
 	return host;
 }
@@ -3037,7 +3039,6 @@ int sdhci_add_host(struct sdhci_host *host)
 	/*
 	 * Set host parameters.
 	 */
-	mmc->ops = &sdhci_ops;
 	max_clk = host->max_clk;
 
 	if (host->ops->get_min_clock)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7654ae5d2b4e..0115e9907bf8 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -430,6 +430,7 @@ struct sdhci_host {
 
 	/* Internal data */
 	struct mmc_host *mmc;	/* MMC structure */
+	struct mmc_host_ops mmc_host_ops;	/* MMC host ops */
 	u64 dma_mask;		/* custom DMA mask */
 
 #if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE)
-- 
1.9.1

[PATCH V2 2/4] mmc: sdhci: Allow override of get_cd() called from sdhci_request()

[Adrian Hunter]

Drivers may need to provide their own get_cd() mmc host op, but
currently the internals of the current op (sdhci_get_cd()) are
provided by sdhci_do_get_cd() which is also called from
sdhci_request().

To allow override of the get_cd functionality, change sdhci_request()
to call ->get_cd() instead of sdhci_do_get_cd().

Note, in the future the call to ->get_cd() will likely be removed
from sdhci_request() since most drivers don't need actually it.
However this change is being done now to facilitate a subsequent
bug fix.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.4+
---
 drivers/mmc/host/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9bfa66df963a..add9fdfd1d8f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1360,7 +1360,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	sdhci_runtime_pm_get(host);
 
 	/* Firstly check card presence */
-	present = sdhci_do_get_cd(host);
+	present = mmc->ops->get_cd(mmc);
 
 	spin_lock_irqsave(&host->lock, flags);
 
-- 
1.9.1

[PATCH V2 3/4] mmc: sdhci-pci: Fix card detect race for Intel BXT/APL

[Adrian Hunter]

Intel BXT/APL use a card detect GPIO however the host controller
will not enable bus power unless it's card detect also reflects
the presence of a card.  Unfortunately those 2 things race which
can result in commands not starting, after which the controller
does nothing and there is a 10 second wait for the driver's
10-second timer to timeout.

That is fixed by having the driver look also at the present state
register to determine if the card is present.  Consequently, provide
a 'get_cd' mmc host operation for BXT/APL that does that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.4+
---
 drivers/mmc/host/sdhci-pci-core.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index cc851b065d0a..df3b8eced8c4 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -330,6 +330,33 @@ static void spt_read_drive_strength(struct sdhci_host *host)
 	sdhci_pci_spt_drive_strength = 0x10 | ((val >> 12) & 0xf);
 }
 
+static int bxt_get_cd(struct mmc_host *mmc)
+{
+	int gpio_cd = mmc_gpio_get_cd(mmc);
+	struct sdhci_host *host = mmc_priv(mmc);
+	unsigned long flags;
+	int ret = 0;
+
+	if (!gpio_cd)
+		return 0;
+
+	pm_runtime_get_sync(mmc->parent);
+
+	spin_lock_irqsave(&host->lock, flags);
+
+	if (host->flags & SDHCI_DEVICE_DEAD)
+		goto out;
+
+	ret = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT);
+out:
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	pm_runtime_mark_last_busy(mmc->parent);
+	pm_runtime_put_autosuspend(mmc->parent);
+
+	return ret;
+}
+
 static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
 {
 	slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE |
@@ -362,6 +389,10 @@ static int byt_sd_probe_slot(struct sdhci_pci_slot *slot)
 	slot->cd_con_id = NULL;
 	slot->cd_idx = 0;
 	slot->cd_override_level = true;
+	if (slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_BXT_SD ||
+	    slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_APL_SD)
+		slot->host->mmc_host_ops.get_cd = bxt_get_cd;
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH V2 4/4] mmc: sdhci-acpi: Fix card detect race for Intel BXT/APL

[Adrian Hunter]

Intel BXT/APL use a card detect GPIO however the host controller
will not enable bus power unless it's card detect also reflects
the presence of a card.  Unfortunately those 2 things race which
can result in commands not starting, after which the controller
does nothing and there is a 10 second wait for the driver's
10-second timer to timeout.

That is fixed by having the driver look also at the present state
register to determine if the card is present.  Consequently, provide
a 'get_cd' mmc host operation for BXT/APL that does that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: stable@vger.kernel.org # v4.4+
---
 drivers/mmc/host/sdhci-acpi.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 3d27f2de0054..195ff0853dc8 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -146,6 +146,33 @@ static const struct sdhci_acpi_chip sdhci_acpi_chip_int = {
 	.ops = &sdhci_acpi_ops_int,
 };
 
+static int bxt_get_cd(struct mmc_host *mmc)
+{
+	int gpio_cd = mmc_gpio_get_cd(mmc);
+	struct sdhci_host *host = mmc_priv(mmc);
+	unsigned long flags;
+	int ret = 0;
+
+	if (!gpio_cd)
+		return 0;
+
+	pm_runtime_get_sync(mmc->parent);
+
+	spin_lock_irqsave(&host->lock, flags);
+
+	if (host->flags & SDHCI_DEVICE_DEAD)
+		goto out;
+
+	ret = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT);
+out:
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	pm_runtime_mark_last_busy(mmc->parent);
+	pm_runtime_put_autosuspend(mmc->parent);
+
+	return ret;
+}
+
 static int sdhci_acpi_emmc_probe_slot(struct platform_device *pdev,
 				      const char *hid, const char *uid)
 {
@@ -196,6 +223,9 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
 
 	/* Platform specific code during sd probe slot goes here */
 
+	if (hid && !strcmp(hid, "80865ACA"))
+		host->mmc_host_ops.get_cd = bxt_get_cd;
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH V2 0/4] mmc: sdhci: Fix card detect race for Intel BXT/APL

[Ulf Hansson]

On 9 February 2016 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here is V2 of patches to fix a problem on Intel BXT/APL.
> The PCI and ACPI Ids for BXT/APL were added in v4.4 so the
> patches are cc stable v4.4+.
>
> Changes in V2:
>
> Override the mmc host ->get_cd() operation instead of adding a
> new sdhci host operation.
>
>
> Adrian Hunter (4):
>       mmc: sdhci: Allow override of mmc host operations
>       mmc: sdhci: Allow override of get_cd() called from sdhci_request()
>       mmc: sdhci-pci: Fix card detect race for Intel BXT/APL
>       mmc: sdhci-acpi: Fix card detect race for Intel BXT/APL
>
>  drivers/mmc/host/sdhci-acpi.c     | 30 ++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci-pci-core.c | 31 +++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci.c          |  5 +++--
>  drivers/mmc/host/sdhci.h          |  1 +
>  4 files changed, 65 insertions(+), 2 deletions(-)
>
>
> Regards
> Adrian

This seems like step in good direction for sdhci, as well as it solves
a regression.

Do you want me to pick it up for fixes?

Kind regards
Uffe

Re: [PATCH V2 0/4] mmc: sdhci: Fix card detect race for Intel BXT/APL

[Adrian Hunter]

On 11/02/16 12:00, Ulf Hansson wrote:
> On 9 February 2016 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> Hi
>>
>> Here is V2 of patches to fix a problem on Intel BXT/APL.
>> The PCI and ACPI Ids for BXT/APL were added in v4.4 so the
>> patches are cc stable v4.4+.
>>
>> Changes in V2:
>>
>> Override the mmc host ->get_cd() operation instead of adding a
>> new sdhci host operation.
>>
>>
>> Adrian Hunter (4):
>>       mmc: sdhci: Allow override of mmc host operations
>>       mmc: sdhci: Allow override of get_cd() called from sdhci_request()
>>       mmc: sdhci-pci: Fix card detect race for Intel BXT/APL
>>       mmc: sdhci-acpi: Fix card detect race for Intel BXT/APL
>>
>>  drivers/mmc/host/sdhci-acpi.c     | 30 ++++++++++++++++++++++++++++++
>>  drivers/mmc/host/sdhci-pci-core.c | 31 +++++++++++++++++++++++++++++++
>>  drivers/mmc/host/sdhci.c          |  5 +++--
>>  drivers/mmc/host/sdhci.h          |  1 +
>>  4 files changed, 65 insertions(+), 2 deletions(-)
>>
>>
>> Regards
>> Adrian
> 
> This seems like step in good direction for sdhci, as well as it solves
> a regression.
> 
> Do you want me to pick it up for fixes?

That would be good, thanks!

Re: [PATCH V2 0/4] mmc: sdhci: Fix card detect race for Intel BXT/APL

[Ulf Hansson]

On 11 February 2016 at 11:32, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 11/02/16 12:00, Ulf Hansson wrote:
>> On 9 February 2016 at 15:12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> Hi
>>>
>>> Here is V2 of patches to fix a problem on Intel BXT/APL.
>>> The PCI and ACPI Ids for BXT/APL were added in v4.4 so the
>>> patches are cc stable v4.4+.
>>>
>>> Changes in V2:
>>>
>>> Override the mmc host ->get_cd() operation instead of adding a
>>> new sdhci host operation.
>>>
>>>
>>> Adrian Hunter (4):
>>>       mmc: sdhci: Allow override of mmc host operations
>>>       mmc: sdhci: Allow override of get_cd() called from sdhci_request()
>>>       mmc: sdhci-pci: Fix card detect race for Intel BXT/APL
>>>       mmc: sdhci-acpi: Fix card detect race for Intel BXT/APL
>>>
>>>  drivers/mmc/host/sdhci-acpi.c     | 30 ++++++++++++++++++++++++++++++
>>>  drivers/mmc/host/sdhci-pci-core.c | 31 +++++++++++++++++++++++++++++++
>>>  drivers/mmc/host/sdhci.c          |  5 +++--
>>>  drivers/mmc/host/sdhci.h          |  1 +
>>>  4 files changed, 65 insertions(+), 2 deletions(-)
>>>
>>>
>>> Regards
>>> Adrian
>>
>> This seems like step in good direction for sdhci, as well as it solves
>> a regression.
>>
>> Do you want me to pick it up for fixes?
>
> That would be good, thanks!
>

Thanks, applied for fixes!

Kind regards
Uffe