[PATCH 0/6] mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14

[PATCH 0/6] mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14

[Adrian Hunter]

Hi

Here are some patches that fix broken card detect for ACPI HID
80860F14.  To do that the card detect GPIO API needed to be
extended to support the new GPIO descriptor API.

There are also 2 unrelated but self-explanatory patches
appended:
	mmc: sdhci-acpi: Add device id 80860F16
	mmc: sdhci: Allow for irq being shared.


Adrian Hunter (6):
      mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers
      mmc: slot-gpio: Split out CD IRQ request into a separate function
      mmc: slot-gpio: Add GPIO descriptor based CD GPIO API
      mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14 SD cards
      mmc: sdhci-acpi: Add device id 80860F16
      mmc: sdhci: Allow for irq being shared.


Regards
Adrian

[PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Adrian Hunter]

In preparation for adding a descriptor-based CD GPIO
API, switch from recording GPIO numbers to recording
GPIO descriptors.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 46596b71..86547a2 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -10,6 +10,7 @@
 
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/jiffies.h>
 #include <linux/mmc/host.h>
@@ -18,8 +19,10 @@
 #include <linux/slab.h>
 
 struct mmc_gpio {
-	int ro_gpio;
-	int cd_gpio;
+	struct gpio_desc *ro_gpio;
+	struct gpio_desc *cd_gpio;
+	bool override_ro_active_level;
+	bool override_cd_active_level;
 	char *ro_label;
 	char cd_label[0];
 };
@@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host)
 			ctx->ro_label = ctx->cd_label + len;
 			snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
 			snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent));
-			ctx->cd_gpio = -EINVAL;
-			ctx->ro_gpio = -EINVAL;
 			host->slot.handler_priv = ctx;
 		}
 	}
@@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host)
 {
 	struct mmc_gpio *ctx = host->slot.handler_priv;
 
-	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
+	if (!ctx || !ctx->ro_gpio)
 		return -ENOSYS;
 
-	return !gpio_get_value_cansleep(ctx->ro_gpio) ^
-		!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
+	if (ctx->override_ro_active_level)
+		return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^
+			!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
+
+	return gpiod_get_value_cansleep(ctx->ro_gpio);
 }
 EXPORT_SYMBOL(mmc_gpio_get_ro);
 
@@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host)
 {
 	struct mmc_gpio *ctx = host->slot.handler_priv;
 
-	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
+	if (!ctx || !ctx->cd_gpio)
 		return -ENOSYS;
 
-	return !gpio_get_value_cansleep(ctx->cd_gpio) ^
-		!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
+	if (ctx->override_cd_active_level)
+		return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^
+			!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
+
+	return gpiod_get_value_cansleep(ctx->cd_gpio);
 }
 EXPORT_SYMBOL(mmc_gpio_get_cd);
 
@@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
 	if (ret < 0)
 		return ret;
 
-	ctx->ro_gpio = gpio;
+	ctx->override_ro_active_level = true;
+	ctx->ro_gpio = gpio_to_desc(gpio);
 
 	return 0;
 }
@@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
 	if (irq < 0)
 		host->caps |= MMC_CAP_NEEDS_POLL;
 
-	ctx->cd_gpio = gpio;
+	ctx->override_cd_active_level = true;
+	ctx->cd_gpio = gpio_to_desc(gpio);
 
 	return 0;
 }
@@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host)
 	struct mmc_gpio *ctx = host->slot.handler_priv;
 	int gpio;
 
-	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
+	if (!ctx || !ctx->ro_gpio)
 		return;
 
-	gpio = ctx->ro_gpio;
-	ctx->ro_gpio = -EINVAL;
+	gpio = desc_to_gpio(ctx->ro_gpio);
+	ctx->ro_gpio = NULL;
 
 	devm_gpio_free(&host->class_dev, gpio);
 }
@@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host)
 	struct mmc_gpio *ctx = host->slot.handler_priv;
 	int gpio;
 
-	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
+	if (!ctx || !ctx->cd_gpio)
 		return;
 
 	if (host->slot.cd_irq >= 0) {
@@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host)
 		host->slot.cd_irq = -EINVAL;
 	}
 
-	gpio = ctx->cd_gpio;
-	ctx->cd_gpio = -EINVAL;
+	gpio = desc_to_gpio(ctx->cd_gpio);
+	ctx->cd_gpio = NULL;
 
 	devm_gpio_free(&host->class_dev, gpio);
 }
-- 
1.8.3.2

[PATCH 2/6] mmc: slot-gpio: Split out CD IRQ request into a separate function

[Adrian Hunter]

In preparation for adding a descriptor-based CD GPIO
API, split out CD IRQ request into a separate function.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/slot-gpio.c | 58 ++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 86547a2..47fa07e 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -139,6 +139,39 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
 }
 EXPORT_SYMBOL(mmc_gpio_request_ro);
 
+static void mmc_gpiod_request_cd_irq(struct mmc_host *host)
+{
+	struct mmc_gpio *ctx = host->slot.handler_priv;
+	int ret, irq;
+
+	if (host->slot.cd_irq >= 0 || !ctx || !ctx->cd_gpio)
+		return;
+
+	irq = gpiod_to_irq(ctx->cd_gpio);
+
+	/*
+	 * Even if gpiod_to_irq() returns a valid IRQ number, the platform might
+	 * still prefer to poll, e.g., because that IRQ number is already used
+	 * by another unit and cannot be shared.
+	 */
+	if (irq >= 0 && host->caps & MMC_CAP_NEEDS_POLL)
+		irq = -EINVAL;
+
+	if (irq >= 0) {
+		ret = devm_request_threaded_irq(&host->class_dev, irq,
+			NULL, mmc_gpio_cd_irqt,
+			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+			ctx->cd_label, host);
+		if (ret < 0)
+			irq = ret;
+	}
+
+	host->slot.cd_irq = irq;
+
+	if (irq < 0)
+		host->caps |= MMC_CAP_NEEDS_POLL;
+}
+
 /**
  * mmc_gpio_request_cd - request a gpio for card-detection
  * @host: mmc host
@@ -162,7 +195,6 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
 			unsigned int debounce)
 {
 	struct mmc_gpio *ctx;
-	int irq = gpio_to_irq(gpio);
 	int ret;
 
 	ret = mmc_gpio_alloc(host);
@@ -187,31 +219,11 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
 			return ret;
 	}
 
-	/*
-	 * Even if gpio_to_irq() returns a valid IRQ number, the platform might
-	 * still prefer to poll, e.g., because that IRQ number is already used
-	 * by another unit and cannot be shared.
-	 */
-	if (irq >= 0 && host->caps & MMC_CAP_NEEDS_POLL)
-		irq = -EINVAL;
-
-	if (irq >= 0) {
-		ret = devm_request_threaded_irq(&host->class_dev, irq,
-			NULL, mmc_gpio_cd_irqt,
-			IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-			ctx->cd_label, host);
-		if (ret < 0)
-			irq = ret;
-	}
-
-	host->slot.cd_irq = irq;
-
-	if (irq < 0)
-		host->caps |= MMC_CAP_NEEDS_POLL;
-
 	ctx->override_cd_active_level = true;
 	ctx->cd_gpio = gpio_to_desc(gpio);
 
+	mmc_gpiod_request_cd_irq(host);
+
 	return 0;
 }
 EXPORT_SYMBOL(mmc_gpio_request_cd);
-- 
1.8.3.2

[PATCH 3/6] mmc: slot-gpio: Add GPIO descriptor based CD GPIO API

[Adrian Hunter]

Add functions to request a CD GPIO using the GPIO
descriptor API.  Note that the new request function
is paired with mmc_gpiod_free_cd() not mmc_gpio_free_cd().
Note also that it must be called prior to mmc_add_host()
otherwise the caller must also call mmc_gpiod_request_cd_irq().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/core/core.c       |  4 +++
 drivers/mmc/core/slot-gpio.c  | 81 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/mmc/slot-gpio.h |  6 ++++
 3 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dc7a5fb..acbc3f2 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -34,6 +34,7 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/mmc.h>
 #include <linux/mmc/sd.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "core.h"
 #include "bus.h"
@@ -2471,6 +2472,7 @@ void mmc_start_host(struct mmc_host *host)
 		mmc_power_off(host);
 	else
 		mmc_power_up(host, host->ocr_avail);
+	mmc_gpiod_request_cd_irq(host);
 	_mmc_detect_change(host, 0, false);
 }
 
@@ -2482,6 +2484,8 @@ void mmc_stop_host(struct mmc_host *host)
 	host->removed = 1;
 	spin_unlock_irqrestore(&host->lock, flags);
 #endif
+	if (host->slot.cd_irq >= 0)
+		disable_irq(host->slot.cd_irq);
 
 	host->rescan_disable = 1;
 	cancel_delayed_work_sync(&host->detect);
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 47fa07e..f7650b8 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -139,7 +139,7 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
 }
 EXPORT_SYMBOL(mmc_gpio_request_ro);
 
-static void mmc_gpiod_request_cd_irq(struct mmc_host *host)
+void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 {
 	struct mmc_gpio *ctx = host->slot.handler_priv;
 	int ret, irq;
@@ -171,6 +171,7 @@ static void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 	if (irq < 0)
 		host->caps |= MMC_CAP_NEEDS_POLL;
 }
+EXPORT_SYMBOL(mmc_gpiod_request_cd_irq);
 
 /**
  * mmc_gpio_request_cd - request a gpio for card-detection
@@ -276,3 +277,81 @@ void mmc_gpio_free_cd(struct mmc_host *host)
 	devm_gpio_free(&host->class_dev, gpio);
 }
 EXPORT_SYMBOL(mmc_gpio_free_cd);
+
+/**
+ * mmc_gpiod_request_cd - request a gpio descriptor for card-detection
+ * @host: mmc host
+ * @con_id: function within the GPIO consumer
+ * @idx: index of the GPIO to obtain in the consumer
+ * @override_active_level: ignore %GPIO_ACTIVE_LOW flag
+ * @debounce: debounce time in microseconds
+ *
+ * Use this function in place of mmc_gpio_request_cd() to use the GPIO
+ * descriptor API.  Note that it is paired with mmc_gpiod_free_cd() not
+ * mmc_gpio_free_cd().  Note also that it must be called prior to mmc_add_host()
+ * otherwise the caller must also call mmc_gpiod_request_cd_irq().
+ *
+ * Returns zero on success, else an error.
+ */
+int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
+			 unsigned int idx, bool override_active_level,
+			 unsigned int debounce)
+{
+	struct mmc_gpio *ctx;
+	struct gpio_desc *desc;
+	int ret;
+
+	ret = mmc_gpio_alloc(host);
+	if (ret < 0)
+		return ret;
+
+	ctx = host->slot.handler_priv;
+
+	if (!con_id)
+		con_id = ctx->cd_label;
+
+	desc = devm_gpiod_get_index(host->parent, con_id, idx);
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	ret = gpiod_direction_input(desc);
+	if (ret < 0)
+		return ret;
+
+	if (debounce) {
+		ret = gpiod_set_debounce(desc, debounce);
+		if (ret < 0)
+			return ret;
+	}
+
+	ctx->override_cd_active_level = override_active_level;
+	ctx->cd_gpio = desc;
+
+	return 0;
+}
+EXPORT_SYMBOL(mmc_gpiod_request_cd);
+
+/**
+ * mmc_gpiod_free_cd - free the card-detection gpio descriptor
+ * @host: mmc host
+ *
+ * It's provided only for cases that client drivers need to manually free
+ * up the card-detection gpio requested by mmc_gpiod_request_cd().
+ */
+void mmc_gpiod_free_cd(struct mmc_host *host)
+{
+	struct mmc_gpio *ctx = host->slot.handler_priv;
+
+	if (!ctx || !ctx->cd_gpio)
+		return;
+
+	if (host->slot.cd_irq >= 0) {
+		devm_free_irq(&host->class_dev, host->slot.cd_irq, host);
+		host->slot.cd_irq = -EINVAL;
+	}
+
+	devm_gpiod_put(&host->class_dev, ctx->cd_gpio);
+
+	ctx->cd_gpio = NULL;
+}
+EXPORT_SYMBOL(mmc_gpiod_free_cd);
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index b0c73e4..d243338 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -22,4 +22,10 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
 			unsigned int debounce);
 void mmc_gpio_free_cd(struct mmc_host *host);
 
+int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
+			 unsigned int idx, bool override_active_level,
+			 unsigned int debounce);
+void mmc_gpiod_free_cd(struct mmc_host *host);
+void mmc_gpiod_request_cd_irq(struct mmc_host *host);
+
 #endif
-- 
1.8.3.2

[PATCH 4/6] mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14

[Adrian Hunter]

Some 80860F14 devices do not support card detect and must rely
completely on GPIO.  Presently the card detect GPIO is used
only to wake-up from runtime suspend.  Change to using
mmc_gpioid_request_cd() which will cause the SDHCI driver to
prefer the GPIO to the host controller's native card detect.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 77 +++++++++----------------------------------
 1 file changed, 15 insertions(+), 62 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 9ce17f6..98c7420 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -31,7 +31,6 @@
 #include <linux/bitops.h>
 #include <linux/types.h>
 #include <linux/err.h>
-#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/acpi.h>
 #include <linux/pm.h>
@@ -40,13 +39,15 @@
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/pm.h>
+#include <linux/mmc/slot-gpio.h>
 #include <linux/mmc/sdhci.h>
 
 #include "sdhci.h"
 
 enum {
-	SDHCI_ACPI_SD_CD	= BIT(0),
-	SDHCI_ACPI_RUNTIME_PM	= BIT(1),
+	SDHCI_ACPI_SD_CD		= BIT(0),
+	SDHCI_ACPI_RUNTIME_PM		= BIT(1),
+	SDHCI_ACPI_SD_CD_OVERRIDE_LEVEL	= BIT(2),
 };
 
 struct sdhci_acpi_chip {
@@ -128,7 +129,8 @@ static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sdio = {
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sd = {
-	.flags   = SDHCI_ACPI_SD_CD | SDHCI_ACPI_RUNTIME_PM,
+	.flags   = SDHCI_ACPI_SD_CD | SDHCI_ACPI_SD_CD_OVERRIDE_LEVEL |
+		   SDHCI_ACPI_RUNTIME_PM,
 	.quirks2 = SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON,
 };
 
@@ -192,59 +194,6 @@ static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(acpi_handle handle,
 	return slot;
 }
 
-#ifdef CONFIG_PM_RUNTIME
-
-static irqreturn_t sdhci_acpi_sd_cd(int irq, void *dev_id)
-{
-	mmc_detect_change(dev_id, msecs_to_jiffies(200));
-	return IRQ_HANDLED;
-}
-
-static int sdhci_acpi_add_own_cd(struct device *dev, struct mmc_host *mmc)
-{
-	struct gpio_desc *desc;
-	unsigned long flags;
-	int err, irq;
-
-	desc = devm_gpiod_get_index(dev, "sd_cd", 0);
-	if (IS_ERR(desc)) {
-		err = PTR_ERR(desc);
-		goto out;
-	}
-
-	err = gpiod_direction_input(desc);
-	if (err)
-		goto out_free;
-
-	irq = gpiod_to_irq(desc);
-	if (irq < 0) {
-		err = irq;
-		goto out_free;
-	}
-
-	flags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
-	err = devm_request_irq(dev, irq, sdhci_acpi_sd_cd, flags, "sd_cd", mmc);
-	if (err)
-		goto out_free;
-
-	return 0;
-
-out_free:
-	devm_gpiod_put(dev, desc);
-out:
-	dev_warn(dev, "failed to setup card detect wake up\n");
-	return err;
-}
-
-#else
-
-static int sdhci_acpi_add_own_cd(struct device *dev, struct mmc_host *mmc)
-{
-	return 0;
-}
-
-#endif
-
 static int sdhci_acpi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -332,15 +281,19 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 
 	host->mmc->caps2 |= MMC_CAP2_NO_PRESCAN_POWERUP;
 
-	err = sdhci_add_host(host);
-	if (err)
-		goto err_free;
-
 	if (sdhci_acpi_flag(c, SDHCI_ACPI_SD_CD)) {
-		if (sdhci_acpi_add_own_cd(dev, host->mmc))
+		bool v = sdhci_acpi_flag(c, SDHCI_ACPI_SD_CD_OVERRIDE_LEVEL);
+
+		if (mmc_gpiod_request_cd(host->mmc, NULL, 0, v, 0)) {
+			dev_warn(dev, "failed to setup card detect gpio\n");
 			c->use_runtime_pm = false;
+		}
 	}
 
+	err = sdhci_add_host(host);
+	if (err)
+		goto err_free;
+
 	if (c->use_runtime_pm) {
 		pm_runtime_set_active(dev);
 		pm_suspend_ignore_children(dev, 1);
-- 
1.8.3.2

[PATCH 5/6] mmc: sdhci-acpi: Add device id 80860F16

[Adrian Hunter]

Add ACPI HID 80860F16 as a host controller for
a SD card.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 98c7420..0d372f0 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -143,6 +143,7 @@ struct sdhci_acpi_uid_slot {
 static const struct sdhci_acpi_uid_slot sdhci_acpi_uids[] = {
 	{ "80860F14" , "1" , &sdhci_acpi_slot_int_emmc },
 	{ "80860F14" , "3" , &sdhci_acpi_slot_int_sd   },
+	{ "80860F16" , NULL, &sdhci_acpi_slot_int_sd   },
 	{ "INT33BB"  , "2" , &sdhci_acpi_slot_int_sdio },
 	{ "INT33C6"  , NULL, &sdhci_acpi_slot_int_sdio },
 	{ "INT3436"  , NULL, &sdhci_acpi_slot_int_sdio },
@@ -152,6 +153,7 @@ static const struct sdhci_acpi_uid_slot sdhci_acpi_uids[] = {
 
 static const struct acpi_device_id sdhci_acpi_ids[] = {
 	{ "80860F14" },
+	{ "80860F16" },
 	{ "INT33BB"  },
 	{ "INT33C6"  },
 	{ "INT3436"  },
-- 
1.8.3.2

[PATCH 6/6] mmc: sdhci: Allow for irq being shared.

[Adrian Hunter]

If the SDHCI irq is shared with another device then
the interrupt handler can get called while SDHCI
is runtime suspended.  That is harmless but the
warning message is no longer useful so remove it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7f95211..4b155d5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2434,8 +2434,6 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 
 	if (host->runtime_suspended) {
 		spin_unlock(&host->lock);
-		pr_warning("%s: got irq while runtime suspended\n",
-		       mmc_hostname(host->mmc));
 		return IRQ_HANDLED;
 	}
 
-- 
1.8.3.2

[PATCH V2 6/6] mmc: sdhci: Allow for irq being shared.

[Adrian Hunter]

If the SDHCI irq is shared with another device then
the interrupt handler can get called while SDHCI
is runtime suspended.  That is harmless but the
warning message is not useful so remove it.  Also
returning IRQ_NONE is more appropriate.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---

Changes in V2:
	Return IRQ_NONE instead of IRQ_HANDLED


 drivers/mmc/host/sdhci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7f95211..04a5e25 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2434,9 +2434,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 
 	if (host->runtime_suspended) {
 		spin_unlock(&host->lock);
-		pr_warning("%s: got irq while runtime suspended\n",
-		       mmc_hostname(host->mmc));
-		return IRQ_HANDLED;
+		return IRQ_NONE;
 	}
 
 	intmask = sdhci_readl(host, SDHCI_INT_STATUS);
-- 
1.8.3.2

Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Jaehoon Chung]

Dear Adrain.

I have tested with your patch-set related slot-gpio.
It's working fine. And looks good to me.

I have a question. what's override_cd/ro_active_level?
I didn't fully understand this value. Could you explain to me, plz?

Best Regards,
Jaehoon Chung

On 03/10/2014 10:02 PM, Adrian Hunter wrote:
> In preparation for adding a descriptor-based CD GPIO
> API, switch from recording GPIO numbers to recording
> GPIO descriptors.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
> index 46596b71..86547a2 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -10,6 +10,7 @@
>  
>  #include <linux/err.h>
>  #include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/interrupt.h>
>  #include <linux/jiffies.h>
>  #include <linux/mmc/host.h>
> @@ -18,8 +19,10 @@
>  #include <linux/slab.h>
>  
>  struct mmc_gpio {
> -	int ro_gpio;
> -	int cd_gpio;
> +	struct gpio_desc *ro_gpio;
> +	struct gpio_desc *cd_gpio;
> +	bool override_ro_active_level;
> +	bool override_cd_active_level;
>  	char *ro_label;
>  	char cd_label[0];
>  };
> @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host)
>  			ctx->ro_label = ctx->cd_label + len;
>  			snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
>  			snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent));
> -			ctx->cd_gpio = -EINVAL;
> -			ctx->ro_gpio = -EINVAL;
>  			host->slot.handler_priv = ctx;
>  		}
>  	}
> @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host)
>  {
>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>  
> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
> +	if (!ctx || !ctx->ro_gpio)
>  		return -ENOSYS;
>  
> -	return !gpio_get_value_cansleep(ctx->ro_gpio) ^
> -		!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
> +	if (ctx->override_ro_active_level)
> +		return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^
> +			!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
> +
> +	return gpiod_get_value_cansleep(ctx->ro_gpio);
>  }
>  EXPORT_SYMBOL(mmc_gpio_get_ro);
>  
> @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host)
>  {
>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>  
> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
> +	if (!ctx || !ctx->cd_gpio)
>  		return -ENOSYS;
>  
> -	return !gpio_get_value_cansleep(ctx->cd_gpio) ^
> -		!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
> +	if (ctx->override_cd_active_level)
> +		return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^
> +			!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
> +
> +	return gpiod_get_value_cansleep(ctx->cd_gpio);
>  }
>  EXPORT_SYMBOL(mmc_gpio_get_cd);
>  
> @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
>  	if (ret < 0)
>  		return ret;
>  
> -	ctx->ro_gpio = gpio;
> +	ctx->override_ro_active_level = true;
> +	ctx->ro_gpio = gpio_to_desc(gpio);
>  
>  	return 0;
>  }
> @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
>  	if (irq < 0)
>  		host->caps |= MMC_CAP_NEEDS_POLL;
>  
> -	ctx->cd_gpio = gpio;
> +	ctx->override_cd_active_level = true;
> +	ctx->cd_gpio = gpio_to_desc(gpio);
>  
>  	return 0;
>  }
> @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host)
>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>  	int gpio;
>  
> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
> +	if (!ctx || !ctx->ro_gpio)
>  		return;
>  
> -	gpio = ctx->ro_gpio;
> -	ctx->ro_gpio = -EINVAL;
> +	gpio = desc_to_gpio(ctx->ro_gpio);
> +	ctx->ro_gpio = NULL;
>  
>  	devm_gpio_free(&host->class_dev, gpio);
>  }
> @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>  	int gpio;
>  
> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
> +	if (!ctx || !ctx->cd_gpio)
>  		return;
>  
>  	if (host->slot.cd_irq >= 0) {
> @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>  		host->slot.cd_irq = -EINVAL;
>  	}
>  
> -	gpio = ctx->cd_gpio;
> -	ctx->cd_gpio = -EINVAL;
> +	gpio = desc_to_gpio(ctx->cd_gpio);
> +	ctx->cd_gpio = NULL;
>  
>  	devm_gpio_free(&host->class_dev, gpio);
>  }
> 

Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Adrian Hunter]

On 13.03.2014 15:00, Jaehoon Chung wrote:
> Dear Adrain.
> 
> I have tested with your patch-set related slot-gpio.
> It's working fine. And looks good to me.
> 
> I have a question. what's override_cd/ro_active_level?
> I didn't fully understand this value. Could you explain to me, plz?

The gpio descriptor records whether the gpio is active high or active low.
When using gpiod_get_value_cansleep() the value is inverted on that basis.
When using gpiod_get_raw_value_cansleep() the value is not inverted.

The legacy GPIO API did not record active low/high.  So then we use
gpiod_get_raw_value_cansleep().  The new API should be able to use
gpiod_get_value_cansleep().

In addition, for the device that I have the active high/low
information passed by ACPI is wrong anyway, so I also have to use the
override flag.


> 
> Best Regards,
> Jaehoon Chung
> 
> On 03/10/2014 10:02 PM, Adrian Hunter wrote:
>> In preparation for adding a descriptor-based CD GPIO
>> API, switch from recording GPIO numbers to recording
>> GPIO descriptors.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------
>>  1 file changed, 27 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
>> index 46596b71..86547a2 100644
>> --- a/drivers/mmc/core/slot-gpio.c
>> +++ b/drivers/mmc/core/slot-gpio.c
>> @@ -10,6 +10,7 @@
>>  
>>  #include <linux/err.h>
>>  #include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/jiffies.h>
>>  #include <linux/mmc/host.h>
>> @@ -18,8 +19,10 @@
>>  #include <linux/slab.h>
>>  
>>  struct mmc_gpio {
>> -	int ro_gpio;
>> -	int cd_gpio;
>> +	struct gpio_desc *ro_gpio;
>> +	struct gpio_desc *cd_gpio;
>> +	bool override_ro_active_level;
>> +	bool override_cd_active_level;
>>  	char *ro_label;
>>  	char cd_label[0];
>>  };
>> @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host)
>>  			ctx->ro_label = ctx->cd_label + len;
>>  			snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
>>  			snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent));
>> -			ctx->cd_gpio = -EINVAL;
>> -			ctx->ro_gpio = -EINVAL;
>>  			host->slot.handler_priv = ctx;
>>  		}
>>  	}
>> @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host)
>>  {
>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>  
>> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
>> +	if (!ctx || !ctx->ro_gpio)
>>  		return -ENOSYS;
>>  
>> -	return !gpio_get_value_cansleep(ctx->ro_gpio) ^
>> -		!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
>> +	if (ctx->override_ro_active_level)
>> +		return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^
>> +			!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
>> +
>> +	return gpiod_get_value_cansleep(ctx->ro_gpio);
>>  }
>>  EXPORT_SYMBOL(mmc_gpio_get_ro);
>>  
>> @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host)
>>  {
>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>  
>> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
>> +	if (!ctx || !ctx->cd_gpio)
>>  		return -ENOSYS;
>>  
>> -	return !gpio_get_value_cansleep(ctx->cd_gpio) ^
>> -		!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
>> +	if (ctx->override_cd_active_level)
>> +		return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^
>> +			!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
>> +
>> +	return gpiod_get_value_cansleep(ctx->cd_gpio);
>>  }
>>  EXPORT_SYMBOL(mmc_gpio_get_cd);
>>  
>> @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
>>  	if (ret < 0)
>>  		return ret;
>>  
>> -	ctx->ro_gpio = gpio;
>> +	ctx->override_ro_active_level = true;
>> +	ctx->ro_gpio = gpio_to_desc(gpio);
>>  
>>  	return 0;
>>  }
>> @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
>>  	if (irq < 0)
>>  		host->caps |= MMC_CAP_NEEDS_POLL;
>>  
>> -	ctx->cd_gpio = gpio;
>> +	ctx->override_cd_active_level = true;
>> +	ctx->cd_gpio = gpio_to_desc(gpio);
>>  
>>  	return 0;
>>  }
>> @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host)
>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>  	int gpio;
>>  
>> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
>> +	if (!ctx || !ctx->ro_gpio)
>>  		return;
>>  
>> -	gpio = ctx->ro_gpio;
>> -	ctx->ro_gpio = -EINVAL;
>> +	gpio = desc_to_gpio(ctx->ro_gpio);
>> +	ctx->ro_gpio = NULL;
>>  
>>  	devm_gpio_free(&host->class_dev, gpio);
>>  }
>> @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>  	int gpio;
>>  
>> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
>> +	if (!ctx || !ctx->cd_gpio)
>>  		return;
>>  
>>  	if (host->slot.cd_irq >= 0) {
>> @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>>  		host->slot.cd_irq = -EINVAL;
>>  	}
>>  
>> -	gpio = ctx->cd_gpio;
>> -	ctx->cd_gpio = -EINVAL;
>> +	gpio = desc_to_gpio(ctx->cd_gpio);
>> +	ctx->cd_gpio = NULL;
>>  
>>  	devm_gpio_free(&host->class_dev, gpio);
>>  }
>>
> 
> 
> 

Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Jaehoon Chung]

Hi, Adrian.

Thanks for kindly explanation.

Best Regards,
Jaehoon Chung

On 03/13/2014 10:33 PM, Adrian Hunter wrote:
> On 13.03.2014 15:00, Jaehoon Chung wrote:
>> Dear Adrain.
>>
>> I have tested with your patch-set related slot-gpio.
>> It's working fine. And looks good to me.
>>
>> I have a question. what's override_cd/ro_active_level?
>> I didn't fully understand this value. Could you explain to me, plz?
> 
> The gpio descriptor records whether the gpio is active high or active low.
> When using gpiod_get_value_cansleep() the value is inverted on that basis.
> When using gpiod_get_raw_value_cansleep() the value is not inverted.
> 
> The legacy GPIO API did not record active low/high.  So then we use
> gpiod_get_raw_value_cansleep().  The new API should be able to use
> gpiod_get_value_cansleep().
> 
> In addition, for the device that I have the active high/low
> information passed by ACPI is wrong anyway, so I also have to use the
> override flag.
> 
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>> On 03/10/2014 10:02 PM, Adrian Hunter wrote:
>>> In preparation for adding a descriptor-based CD GPIO
>>> API, switch from recording GPIO numbers to recording
>>> GPIO descriptors.
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> ---
>>>  drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------
>>>  1 file changed, 27 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
>>> index 46596b71..86547a2 100644
>>> --- a/drivers/mmc/core/slot-gpio.c
>>> +++ b/drivers/mmc/core/slot-gpio.c
>>> @@ -10,6 +10,7 @@
>>>  
>>>  #include <linux/err.h>
>>>  #include <linux/gpio.h>
>>> +#include <linux/gpio/consumer.h>
>>>  #include <linux/interrupt.h>
>>>  #include <linux/jiffies.h>
>>>  #include <linux/mmc/host.h>
>>> @@ -18,8 +19,10 @@
>>>  #include <linux/slab.h>
>>>  
>>>  struct mmc_gpio {
>>> -	int ro_gpio;
>>> -	int cd_gpio;
>>> +	struct gpio_desc *ro_gpio;
>>> +	struct gpio_desc *cd_gpio;
>>> +	bool override_ro_active_level;
>>> +	bool override_cd_active_level;
>>>  	char *ro_label;
>>>  	char cd_label[0];
>>>  };
>>> @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host)
>>>  			ctx->ro_label = ctx->cd_label + len;
>>>  			snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
>>>  			snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent));
>>> -			ctx->cd_gpio = -EINVAL;
>>> -			ctx->ro_gpio = -EINVAL;
>>>  			host->slot.handler_priv = ctx;
>>>  		}
>>>  	}
>>> @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host)
>>>  {
>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>  
>>> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
>>> +	if (!ctx || !ctx->ro_gpio)
>>>  		return -ENOSYS;
>>>  
>>> -	return !gpio_get_value_cansleep(ctx->ro_gpio) ^
>>> -		!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
>>> +	if (ctx->override_ro_active_level)
>>> +		return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^
>>> +			!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
>>> +
>>> +	return gpiod_get_value_cansleep(ctx->ro_gpio);
>>>  }
>>>  EXPORT_SYMBOL(mmc_gpio_get_ro);
>>>  
>>> @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host)
>>>  {
>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>  
>>> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
>>> +	if (!ctx || !ctx->cd_gpio)
>>>  		return -ENOSYS;
>>>  
>>> -	return !gpio_get_value_cansleep(ctx->cd_gpio) ^
>>> -		!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
>>> +	if (ctx->override_cd_active_level)
>>> +		return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^
>>> +			!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
>>> +
>>> +	return gpiod_get_value_cansleep(ctx->cd_gpio);
>>>  }
>>>  EXPORT_SYMBOL(mmc_gpio_get_cd);
>>>  
>>> @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
>>>  	if (ret < 0)
>>>  		return ret;
>>>  
>>> -	ctx->ro_gpio = gpio;
>>> +	ctx->override_ro_active_level = true;
>>> +	ctx->ro_gpio = gpio_to_desc(gpio);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
>>>  	if (irq < 0)
>>>  		host->caps |= MMC_CAP_NEEDS_POLL;
>>>  
>>> -	ctx->cd_gpio = gpio;
>>> +	ctx->override_cd_active_level = true;
>>> +	ctx->cd_gpio = gpio_to_desc(gpio);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host)
>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>  	int gpio;
>>>  
>>> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
>>> +	if (!ctx || !ctx->ro_gpio)
>>>  		return;
>>>  
>>> -	gpio = ctx->ro_gpio;
>>> -	ctx->ro_gpio = -EINVAL;
>>> +	gpio = desc_to_gpio(ctx->ro_gpio);
>>> +	ctx->ro_gpio = NULL;
>>>  
>>>  	devm_gpio_free(&host->class_dev, gpio);
>>>  }
>>> @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>  	int gpio;
>>>  
>>> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
>>> +	if (!ctx || !ctx->cd_gpio)
>>>  		return;
>>>  
>>>  	if (host->slot.cd_irq >= 0) {
>>> @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>>>  		host->slot.cd_irq = -EINVAL;
>>>  	}
>>>  
>>> -	gpio = ctx->cd_gpio;
>>> -	ctx->cd_gpio = -EINVAL;
>>> +	gpio = desc_to_gpio(ctx->cd_gpio);
>>> +	ctx->cd_gpio = NULL;
>>>  
>>>  	devm_gpio_free(&host->class_dev, gpio);
>>>  }
>>>
>>
>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Jaehoon Chung]

After applied "[PATCH 1/6]~[PATCH 3/6]", I have tested on exynos board.

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>

On 03/13/2014 10:39 PM, Jaehoon Chung wrote:
> Hi, Adrian.
> 
> Thanks for kindly explanation.
> 
> Best Regards,
> Jaehoon Chung
> 
> On 03/13/2014 10:33 PM, Adrian Hunter wrote:
>> On 13.03.2014 15:00, Jaehoon Chung wrote:
>>> Dear Adrain.
>>>
>>> I have tested with your patch-set related slot-gpio.
>>> It's working fine. And looks good to me.
>>>
>>> I have a question. what's override_cd/ro_active_level?
>>> I didn't fully understand this value. Could you explain to me, plz?
>>
>> The gpio descriptor records whether the gpio is active high or active low.
>> When using gpiod_get_value_cansleep() the value is inverted on that basis.
>> When using gpiod_get_raw_value_cansleep() the value is not inverted.
>>
>> The legacy GPIO API did not record active low/high.  So then we use
>> gpiod_get_raw_value_cansleep().  The new API should be able to use
>> gpiod_get_value_cansleep().
>>
>> In addition, for the device that I have the active high/low
>> information passed by ACPI is wrong anyway, so I also have to use the
>> override flag.
>>
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>> On 03/10/2014 10:02 PM, Adrian Hunter wrote:
>>>> In preparation for adding a descriptor-based CD GPIO
>>>> API, switch from recording GPIO numbers to recording
>>>> GPIO descriptors.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>  drivers/mmc/core/slot-gpio.c | 45 ++++++++++++++++++++++++++------------------
>>>>  1 file changed, 27 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
>>>> index 46596b71..86547a2 100644
>>>> --- a/drivers/mmc/core/slot-gpio.c
>>>> +++ b/drivers/mmc/core/slot-gpio.c
>>>> @@ -10,6 +10,7 @@
>>>>  
>>>>  #include <linux/err.h>
>>>>  #include <linux/gpio.h>
>>>> +#include <linux/gpio/consumer.h>
>>>>  #include <linux/interrupt.h>
>>>>  #include <linux/jiffies.h>
>>>>  #include <linux/mmc/host.h>
>>>> @@ -18,8 +19,10 @@
>>>>  #include <linux/slab.h>
>>>>  
>>>>  struct mmc_gpio {
>>>> -	int ro_gpio;
>>>> -	int cd_gpio;
>>>> +	struct gpio_desc *ro_gpio;
>>>> +	struct gpio_desc *cd_gpio;
>>>> +	bool override_ro_active_level;
>>>> +	bool override_cd_active_level;
>>>>  	char *ro_label;
>>>>  	char cd_label[0];
>>>>  };
>>>> @@ -57,8 +60,6 @@ static int mmc_gpio_alloc(struct mmc_host *host)
>>>>  			ctx->ro_label = ctx->cd_label + len;
>>>>  			snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent));
>>>>  			snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent));
>>>> -			ctx->cd_gpio = -EINVAL;
>>>> -			ctx->ro_gpio = -EINVAL;
>>>>  			host->slot.handler_priv = ctx;
>>>>  		}
>>>>  	}
>>>> @@ -72,11 +73,14 @@ int mmc_gpio_get_ro(struct mmc_host *host)
>>>>  {
>>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>>  
>>>> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
>>>> +	if (!ctx || !ctx->ro_gpio)
>>>>  		return -ENOSYS;
>>>>  
>>>> -	return !gpio_get_value_cansleep(ctx->ro_gpio) ^
>>>> -		!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
>>>> +	if (ctx->override_ro_active_level)
>>>> +		return !gpiod_get_raw_value_cansleep(ctx->ro_gpio) ^
>>>> +			!!(host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH);
>>>> +
>>>> +	return gpiod_get_value_cansleep(ctx->ro_gpio);
>>>>  }
>>>>  EXPORT_SYMBOL(mmc_gpio_get_ro);
>>>>  
>>>> @@ -84,11 +88,14 @@ int mmc_gpio_get_cd(struct mmc_host *host)
>>>>  {
>>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>>  
>>>> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
>>>> +	if (!ctx || !ctx->cd_gpio)
>>>>  		return -ENOSYS;
>>>>  
>>>> -	return !gpio_get_value_cansleep(ctx->cd_gpio) ^
>>>> -		!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
>>>> +	if (ctx->override_cd_active_level)
>>>> +		return !gpiod_get_raw_value_cansleep(ctx->cd_gpio) ^
>>>> +			!!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH);
>>>> +
>>>> +	return gpiod_get_value_cansleep(ctx->cd_gpio);
>>>>  }
>>>>  EXPORT_SYMBOL(mmc_gpio_get_cd);
>>>>  
>>>> @@ -125,7 +132,8 @@ int mmc_gpio_request_ro(struct mmc_host *host, unsigned int gpio)
>>>>  	if (ret < 0)
>>>>  		return ret;
>>>>  
>>>> -	ctx->ro_gpio = gpio;
>>>> +	ctx->override_ro_active_level = true;
>>>> +	ctx->ro_gpio = gpio_to_desc(gpio);
>>>>  
>>>>  	return 0;
>>>>  }
>>>> @@ -201,7 +209,8 @@ int mmc_gpio_request_cd(struct mmc_host *host, unsigned int gpio,
>>>>  	if (irq < 0)
>>>>  		host->caps |= MMC_CAP_NEEDS_POLL;
>>>>  
>>>> -	ctx->cd_gpio = gpio;
>>>> +	ctx->override_cd_active_level = true;
>>>> +	ctx->cd_gpio = gpio_to_desc(gpio);
>>>>  
>>>>  	return 0;
>>>>  }
>>>> @@ -219,11 +228,11 @@ void mmc_gpio_free_ro(struct mmc_host *host)
>>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>>  	int gpio;
>>>>  
>>>> -	if (!ctx || !gpio_is_valid(ctx->ro_gpio))
>>>> +	if (!ctx || !ctx->ro_gpio)
>>>>  		return;
>>>>  
>>>> -	gpio = ctx->ro_gpio;
>>>> -	ctx->ro_gpio = -EINVAL;
>>>> +	gpio = desc_to_gpio(ctx->ro_gpio);
>>>> +	ctx->ro_gpio = NULL;
>>>>  
>>>>  	devm_gpio_free(&host->class_dev, gpio);
>>>>  }
>>>> @@ -241,7 +250,7 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>>>>  	struct mmc_gpio *ctx = host->slot.handler_priv;
>>>>  	int gpio;
>>>>  
>>>> -	if (!ctx || !gpio_is_valid(ctx->cd_gpio))
>>>> +	if (!ctx || !ctx->cd_gpio)
>>>>  		return;
>>>>  
>>>>  	if (host->slot.cd_irq >= 0) {
>>>> @@ -249,8 +258,8 @@ void mmc_gpio_free_cd(struct mmc_host *host)
>>>>  		host->slot.cd_irq = -EINVAL;
>>>>  	}
>>>>  
>>>> -	gpio = ctx->cd_gpio;
>>>> -	ctx->cd_gpio = -EINVAL;
>>>> +	gpio = desc_to_gpio(ctx->cd_gpio);
>>>> +	ctx->cd_gpio = NULL;
>>>>  
>>>>  	devm_gpio_free(&host->class_dev, gpio);
>>>>  }
>>>>
>>>
>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Linus Walleij]

On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:

> In preparation for adding a descriptor-based CD GPIO
> API, switch from recording GPIO numbers to recording
> GPIO descriptors.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Certainly this makes things look better from a GPIO POV
so Acked-by.

However:

(...)
> +       ctx->ro_gpio = gpio_to_desc(gpio);
(...)
> +       ctx->cd_gpio = gpio_to_desc(gpio);

How much harder would it be to switch the platforms using this
MMC driver to pass GPIO descriptors using the gpiod_lookup_table
in affected platforms (or device tree or ACPI) and then use
devm_gpiod_get[_index]() in probe()?

This can certainly be step 2, but that is where we want to go with
the GPIO clean-up work in the long run.

Yours,
Linus Walleij

Re: [PATCH 3/6] mmc: slot-gpio: Add GPIO descriptor based CD GPIO API

[Linus Walleij]

On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:

> Add functions to request a CD GPIO using the GPIO
> descriptor API.  Note that the new request function
> is paired with mmc_gpiod_free_cd() not mmc_gpio_free_cd().
> Note also that it must be called prior to mmc_add_host()
> otherwise the caller must also call mmc_gpiod_request_cd_irq().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

This is very helpful.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Adrian Hunter]

On 14.03.2014 11:06, Linus Walleij wrote:
> On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> 
>> In preparation for adding a descriptor-based CD GPIO
>> API, switch from recording GPIO numbers to recording
>> GPIO descriptors.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Certainly this makes things look better from a GPIO POV
> so Acked-by.
> 
> However:
> 
> (...)
>> +       ctx->ro_gpio = gpio_to_desc(gpio);
> (...)
>> +       ctx->cd_gpio = gpio_to_desc(gpio);
> 
> How much harder would it be to switch the platforms using this
> MMC driver to pass GPIO descriptors using the gpiod_lookup_table
> in affected platforms (or device tree or ACPI) and then use
> devm_gpiod_get[_index]() in probe()?

There are 10 drivers plus users of mmc_of_config(), so too much work for me
right now.  Also parts that relate to device-tree are outside my sphere.

> This can certainly be step 2, but that is where we want to go with
> the GPIO clean-up work in the long run.

Perhaps Ulf or Guennadi would be interested.

Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers

[Adrian Hunter]

Resent with updated email address for Ulf.

On 14.03.2014 11:06, Linus Walleij wrote:
> On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> 
>> In preparation for adding a descriptor-based CD GPIO
>> API, switch from recording GPIO numbers to recording
>> GPIO descriptors.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Certainly this makes things look better from a GPIO POV
> so Acked-by.
> 
> However:
> 
> (...)
>> +       ctx->ro_gpio = gpio_to_desc(gpio);
> (...)
>> +       ctx->cd_gpio = gpio_to_desc(gpio);
> 
> How much harder would it be to switch the platforms using this
> MMC driver to pass GPIO descriptors using the gpiod_lookup_table
> in affected platforms (or device tree or ACPI) and then use
> devm_gpiod_get[_index]() in probe()?

There are 10 drivers plus users of mmc_of_config(), so too much work for me
right now.  Also parts that relate to device-tree are outside my sphere.

> This can certainly be step 2, but that is where we want to go with
> the GPIO clean-up work in the long run.

Perhaps Ulf or Guennadi would be interested.

Re: [PATCH 0/6] mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14

[Adrian Hunter]

On 10.03.2014 15:02, Adrian Hunter wrote:
> Hi
> 
> Here are some patches that fix broken card detect for ACPI HID
> 80860F14.  To do that the card detect GPIO API needed to be
> extended to support the new GPIO descriptor API.
> 
> There are also 2 unrelated but self-explanatory patches
> appended:
> 	mmc: sdhci-acpi: Add device id 80860F16
> 	mmc: sdhci: Allow for irq being shared.
> 
> 
> Adrian Hunter (6):
>       mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers
>       mmc: slot-gpio: Split out CD IRQ request into a separate function
>       mmc: slot-gpio: Add GPIO descriptor based CD GPIO API
>       mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14 SD cards
>       mmc: sdhci-acpi: Add device id 80860F16
>       mmc: sdhci: Allow for irq being shared.

Chris, can these be queued for 3.15?  Note I did a V2 of just "mmc: sdhci:
Allow for irq being shared"

Re: [PATCH 0/6] mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14

[Chris Ball]

Hi Adrian,

On Mon, Mar 17 2014, Adrian Hunter wrote:
>> Adrian Hunter (6):
>>       mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers
>>       mmc: slot-gpio: Split out CD IRQ request into a separate function
>>       mmc: slot-gpio: Add GPIO descriptor based CD GPIO API
>>       mmc: sdhci-acpi: Fix broken card detect for ACPI HID 80860F14 SD cards
>>       mmc: sdhci-acpi: Add device id 80860F16
>>       mmc: sdhci: Allow for irq being shared.
>
> Chris, can these be queued for 3.15?  Note I did a V2 of just "mmc: sdhci:
> Allow for irq being shared"

Thanks, merged all six patches (with v2 of 6/6) to mmc-next for 3.15.

- Chris.
-- 
Chris Ball   <chris@printf.net>   <http://printf.net/>