All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rajat Jain <rajatja@google.com>
To: adrian.hunter@intel.com, Ulf Hansson <ulf.hansson@linaro.org>,
	linux-mmc@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Rajat Jain <rajatxjain@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Dmitry Torokhov <dtor@google.com>,
	linux-gpio@vger.kernel.org, linux-acpi@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Rajat Jain <rajatja@google.com>
Subject: [PATCH v2] mmc: sdhci-pci: Try "cd" for card-detect lookup before using NULL
Date: Mon, 29 Oct 2018 15:17:01 -0700	[thread overview]
Message-ID: <20181029221701.51376-1-rajatja@google.com> (raw)
In-Reply-To: <CAA93t1oaZ5zkv+BsGnYZg6pqKv0DzXi3j8oZHUyTyffUWHtHug@mail.gmail.com>

Problem:

The card detect IRQ does not work with modern BIOS (that want
to use _DSD to provide the card detect GPIO to the driver).

Details:

The mmc core provides the mmc_gpiod_request_cd() API to let host drivers
request the gpio descriptor for the "card detect" pin.
This pin is specified in the ACPI for the SDHC device:

 * Either as a resource using _CRS. This is a method used by legacy BIOS.
   (The driver needs to tell which resource index).

 * Or as a named property ("cd-gpios"/"cd-gpio") in _DSD (which internally
   points to an entry in _CRS). This way, the driver can lookup using a
   string. This is what modern BIOS prefer to use.

This API finally results in a call to the following code:

struct gpio_desc *acpi_find_gpio(..., const char *con_id,...)
{
...
   /* Lookup gpio (using "<con_id>-gpio") in the _DSD */
...
   if (!acpi_can_fallback_to_crs(adev, con_id))
          return ERR_PTR(-ENOENT);
...
   /* Falling back to _CRS is allowed, Lookup gpio in the _CRS */
...
}

Note that this means that if the ACPI has _DSD properties, the kernel
will never use _CRS for the lookup (Because acpi_can_fallback_to_crs()
will always be false for any device hat has _DSD entries).

The SDHCI driver is thus currently broken on a modern BIOS, even if
BIOS provides both _CRS (for index based lookup) and _DSD entries (for
string based lookup). Ironically, none of these will be used for the
lookup currently because:

* Since the con_id is NULL, acpi_find_gpio() does not find a matching
  entry in DSDT. (The _DSDT entry has the property name = "cd-gpios")

* Because ACPI contains DSDT entries, thus acpi_can_fallback_to_crs()
  returns false (because device properties have been populated from
  _DSD), thus the _CRS is never used for the lookup.

Fix:

Try "cd" for lookup in the _DSD before falling back to using NULL so
as to try looking up in the _CRS.

I've tested this patch successfully with both Legacy BIOS (that
provide only _CRS method) as well as modern BIOS (that provide both
_CRS and _DSD). Also the use of "cd" appears to be fairly consistent
across other users of this API (other MMC host controller drivers).

Link: https://lkml.org/lkml/2018/9/25/1113
Signed-off-by: Rajat Jain <rajatja@google.com>
---
v2: Fix the commit log to take care of Andy's comments.

 drivers/mmc/host/sdhci-pci-core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 7bfd366d970d..e53333c695b3 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1762,8 +1762,13 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 		device_init_wakeup(&pdev->dev, true);
 
 	if (slot->cd_idx >= 0) {
-		ret = mmc_gpiod_request_cd(host->mmc, NULL, slot->cd_idx,
+		ret = mmc_gpiod_request_cd(host->mmc, "cd", slot->cd_idx,
 					   slot->cd_override_level, 0, NULL);
+		if (ret && ret != -EPROBE_DEFER)
+			ret = mmc_gpiod_request_cd(host->mmc, NULL,
+						   slot->cd_idx,
+						   slot->cd_override_level,
+						   0, NULL);
 		if (ret == -EPROBE_DEFER)
 			goto remove;
 
-- 
2.19.1.568.g152ad8e336-goog

  reply	other threads:[~2018-10-29 22:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-25 20:54 sdhci driver card-detect is broken because gpiolib can't fallback to _CRS? Rajat Jain
2018-09-25 20:54 ` Rajat Jain
2018-09-26  7:47 ` Mika Westerberg
2018-09-26  8:42   ` Andy Shevchenko
2018-09-26 19:25     ` Rajat Jain
2018-09-27  7:26       ` Andy Shevchenko
2018-09-27 17:56         ` Rajat Jain
2018-09-28  8:42           ` Linus Walleij
2018-09-28 12:34             ` Rajat Jain
2018-09-28 13:13               ` Linus Walleij
2018-10-18 21:51                 ` [PATCH] mmc: sdhci-pci: Try "cd" for card-detect lookup before using NULL Rajat Jain
2018-10-19  9:13                   ` Andy Shevchenko
2018-10-22 23:34                     ` Rajat Jain
2018-10-24 10:02                       ` Andy Shevchenko
2018-10-24 18:03                         ` Dmitry Torokhov
2018-10-29 15:23                           ` Andy Shevchenko
2018-10-29 17:22                             ` Rajat Jain
2018-10-29 17:43                               ` Andy Shevchenko
2018-10-29 19:43                                 ` Rajat Jain
2018-10-29 22:17                                   ` Rajat Jain [this message]
2018-10-30  7:53                                     ` [PATCH v2] " Adrian Hunter
2018-11-12 11:05                                     ` Ulf Hansson
2018-11-12 11:25                                       ` Andy Shevchenko
2018-11-13  1:26                                         ` Rajat Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181029221701.51376-1-rajatja@google.com \
    --to=rajatja@google.com \
    --cc=adrian.hunter@intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=dtor@google.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rajatxjain@gmail.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.