[PATCH v1 0/6] mmc: sdhci-pci: Add some CD GPIO related quirks

[PATCH v1 0/6] mmc: sdhci-pci: Add some CD GPIO related quirks

[Andy Shevchenko]

It appears that one of the supported platform magically worked with the
custom IRQ handler (any hints how?) while having two PCB designs with
an opposite CD sense level. Here is an attempt to fix it by quirking out
CD GPIO.

Patch 1 introduces two additional quirks (it's done this way due to
patch 3, see below). Patch 2 is an actual fix for the mentioned platform.
If backported need to be taken with patch 1 together. Patch 3 is (RFT)
clean up. The questionable part here is the locking scheme. Shouldn't
we do something similar in the generic IRQ handler of SDHCI? Or Broxton
case has something quite different in mind?

Patches 4-6 are dead-code removals. Patch 4 accompanying patch 2, patches
5-6 just similar to it, but (functionally) independent. Would like to hear
if it's okay to do.

Any comments, hints, advice are welcome!

Andy Shevchenko (6):
  mmc: sdhci: Introduce couple of quirks to ignore particular state of
    CD GPIO
  mmc: sdhci-pci: Read card detect from ACPI for Intel Merrifield
  mmc: sdhci: Replace bxt_get_cd() with
    SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH
  mmc: sdhci-pci: Remove dead code (struct sdhci_pci_data et al)
  mmc: sdhci-pci: Remove dead code (cd_gpio, cd_irq et al)
  mmc: sdhci-pci: Remove dead code (rst_n_gpio et al)

 drivers/mmc/host/Makefile          |   1 -
 drivers/mmc/host/sdhci-acpi.c      |  24 +----
 drivers/mmc/host/sdhci-pci-core.c  | 166 +++--------------------------
 drivers/mmc/host/sdhci-pci-data.c  |   6 --
 drivers/mmc/host/sdhci-pci.h       |   5 -
 drivers/mmc/host/sdhci.c           |  13 ++-
 drivers/mmc/host/sdhci.h           |   4 +
 include/linux/mmc/sdhci-pci-data.h |  18 ----
 8 files changed, 27 insertions(+), 210 deletions(-)
 delete mode 100644 drivers/mmc/host/sdhci-pci-data.c
 delete mode 100644 include/linux/mmc/sdhci-pci-data.h

-- 
2.33.0

[PATCH v1 1/6] mmc: sdhci: Introduce couple of quirks to ignore particular state of CD GPIO

[Andy Shevchenko]

Some platforms may provide contradictory info in some states of CD GPIO line,
and hence that state or states should be ignored. Introduce couple of quirks
for that.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mmc/host/sdhci.c | 13 ++++++++-----
 drivers/mmc/host/sdhci.h |  4 ++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 357b365bf0ec..a7960ee3ef4f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2395,7 +2395,7 @@ EXPORT_SYMBOL_GPL(sdhci_set_ios);
 static int sdhci_get_cd(struct mmc_host *mmc)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
-	int gpio_cd = mmc_gpio_get_cd(mmc);
+	int gpio_cd;
 
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		return 0;
@@ -2405,11 +2405,14 @@ static int sdhci_get_cd(struct mmc_host *mmc)
 		return 1;
 
 	/*
-	 * Try slot gpio detect, if defined it take precedence
-	 * over build in controller functionality
+	 * Try slot GPIO detect, if defined it take precedence
+	 * over build in controller functionality.
 	 */
-	if (gpio_cd >= 0)
-		return !!gpio_cd;
+	gpio_cd = mmc_gpio_get_cd(mmc);
+	if (gpio_cd == 0 && !(host->quirks2 & SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_LOW))
+		return 0;
+	if (gpio_cd > 0 && !(host->quirks2 & SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH))
+		return 1;
 
 	/* If polling, assume that the card is always present. */
 	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index e8d04e42a5af..fb7910d22b18 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -464,6 +464,10 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
 /* Controller has CRC in 136 bit Command Response */
 #define SDHCI_QUIRK2_RSP_136_HAS_CRC			(1<<16)
+/* Controller requires additional card detection test on GPIO low */
+#define SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_LOW		(1<<17)
+/* Controller requires additional card detection test on GPIO high */
+#define SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH		(1<<18)
 /*
  * Disable HW timeout if the requested timeout is more than the maximum
  * obtainable timeout.
-- 
2.33.0

[PATCH v1 2/6] mmc: sdhci-pci: Read card detect from ACPI for Intel Merrifield

[Andy Shevchenko]

Intel Merrifield platform had been converted to use ACPI enumeration.
However, the driver missed an update to retrieve card detect GPIO.
Fix it here.

Unfortunately we can't rely on CD GPIO state because there are two
different PCB designs in the wild that are using an opposite card
detection sense and there is no way to distinguish those platforms,
that's why ignore CD GPIO completely and use it only as an card plug
or unplug event.

Fixes: 4590d98f5a4f ("sfi: Remove framework for deprecated firmware")
BugLink: https://github.com/edison-fw/meta-intel-edison/issues/135
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index be19785227fe..70ab0a7a3de8 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1341,6 +1341,14 @@ static int intel_mrfld_mmc_probe_slot(struct sdhci_pci_slot *slot)
 					 MMC_CAP_1_8V_DDR;
 		break;
 	case INTEL_MRFLD_SD:
+		slot->cd_idx = 0;
+		/*
+		 * There are two PCB designs of SD card slot with the opposite
+		 * card detection sense. Quirk this out by ignoring GPIO state
+		 * completely.
+		 */
+		slot->host->quirks2 |= SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_LOW |
+				       SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH;
 		slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
 		break;
 	case INTEL_MRFLD_SDIO:
-- 
2.33.0

[PATCH v1 3/6] mmc: sdhci: Replace bxt_get_cd() with SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH

[Andy Shevchenko]

The bxt_get_cd() repeats the functionality of sdhci_get_cd() when
SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH is set. Thus, replace the
method with the setting of the quirk.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mmc/host/sdhci-acpi.c     | 24 +-----------------------
 drivers/mmc/host/sdhci-pci-core.c | 24 +-----------------------
 2 files changed, 2 insertions(+), 46 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 8fe65f172a61..126c3afaf21b 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -359,28 +359,6 @@ static inline bool sdhci_acpi_no_fixup_child_power(struct acpi_device *adev)
 
 #endif
 
-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;
-
-	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);
-
-	return ret;
-}
-
 static int intel_probe_slot(struct platform_device *pdev, struct acpi_device *adev)
 {
 	struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
@@ -393,7 +371,7 @@ static int intel_probe_slot(struct platform_device *pdev, struct acpi_device *ad
 		host->timeout_clk = 1000; /* 1000 kHz i.e. 1 MHz */
 
 	if (acpi_dev_hid_uid_match(adev, "80865ACA", NULL))
-		host->mmc_host_ops.get_cd = bxt_get_cd;
+		host->quirks2 |= SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH;
 
 	intel_dsm_init(intel_host, &pdev->dev, host->mmc);
 
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 70ab0a7a3de8..30caa0b325de 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -616,28 +616,6 @@ static int intel_select_drive_strength(struct mmc_card *card,
 	return intel_host->drv_strength;
 }
 
-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;
-
-	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);
-
-	return ret;
-}
-
 #define SDHCI_INTEL_PWR_TIMEOUT_CNT	20
 #define SDHCI_INTEL_PWR_TIMEOUT_UDELAY	100
 
@@ -1171,7 +1149,7 @@ static int byt_sd_probe_slot(struct sdhci_pci_slot *slot)
 	    slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_BXTM_SD ||
 	    slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_APL_SD ||
 	    slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_GLK_SD)
-		slot->host->mmc_host_ops.get_cd = bxt_get_cd;
+		slot->host->quirks2 |= SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH;
 
 	if (slot->chip->pdev->subsystem_vendor == PCI_VENDOR_ID_NI &&
 	    slot->chip->pdev->subsystem_device == PCI_SUBDEVICE_ID_NI_78E3)
-- 
2.33.0

[PATCH v1 4/6] mmc: sdhci-pci: Remove dead code (struct sdhci_pci_data et al)

[Andy Shevchenko]

The last user of this struct gone couple of releases ago. Besides that
there were not so many users of this API for 10+ years: one is
implied above Intel Merrifield (added 2016-08-31, removed 2021-02-11),
and another is Intel Sunrisepoint (added 2015-02-06, removed 2017-03-20).

Effectively this is a revert of the commit 52c506f0bc72 ("mmc: sdhci-pci:
add platform data").

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mmc/host/Makefile          |  1 -
 drivers/mmc/host/sdhci-pci-core.c  | 31 ++++--------------------------
 drivers/mmc/host/sdhci-pci-data.c  |  6 ------
 drivers/mmc/host/sdhci-pci.h       |  1 -
 include/linux/mmc/sdhci-pci-data.h | 18 -----------------
 5 files changed, 4 insertions(+), 53 deletions(-)
 delete mode 100644 drivers/mmc/host/sdhci-pci-data.c
 delete mode 100644 include/linux/mmc/sdhci-pci-data.h

diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 14004cc09aaa..ea36d379bd3c 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -14,7 +14,6 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
 sdhci-pci-y			+= sdhci-pci-core.o sdhci-pci-o2micro.o sdhci-pci-arasan.o \
 				   sdhci-pci-dwc-mshc.o sdhci-pci-gli.o
-obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI))	+= sdhci-pci-data.o
 obj-$(CONFIG_MMC_SDHCI_ACPI)	+= sdhci-acpi.o
 obj-$(CONFIG_MMC_SDHCI_PXAV3)	+= sdhci-pxav3.o
 obj-$(CONFIG_MMC_SDHCI_PXAV2)	+= sdhci-pxav2.o
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 30caa0b325de..8d01285e1b32 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -17,8 +17,6 @@
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
 #include <linux/device.h>
-#include <linux/mmc/host.h>
-#include <linux/mmc/mmc.h>
 #include <linux/scatterlist.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -26,11 +24,13 @@
 #include <linux/pm_runtime.h>
 #include <linux/pm_qos.h>
 #include <linux/debugfs.h>
-#include <linux/mmc/slot-gpio.h>
-#include <linux/mmc/sdhci-pci-data.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
 
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/slot-gpio.h>
+
 #ifdef CONFIG_X86
 #include <asm/iosf_mbi.h>
 #endif
@@ -2116,22 +2116,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 	slot->cd_gpio = -EINVAL;
 	slot->cd_idx = -1;
 
-	/* Retrieve platform data if there is any */
-	if (*sdhci_pci_get_data)
-		slot->data = sdhci_pci_get_data(pdev, slotno);
-
-	if (slot->data) {
-		if (slot->data->setup) {
-			ret = slot->data->setup(slot->data);
-			if (ret) {
-				dev_err(&pdev->dev, "platform setup failed\n");
-				goto free;
-			}
-		}
-		slot->rst_n_gpio = slot->data->rst_n_gpio;
-		slot->cd_gpio = slot->data->cd_gpio;
-	}
-
 	host->hw_name = "PCI";
 	host->ops = chip->fixes && chip->fixes->ops ?
 		    chip->fixes->ops :
@@ -2218,10 +2202,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 		chip->fixes->remove_slot(slot, 0);
 
 cleanup:
-	if (slot->data && slot->data->cleanup)
-		slot->data->cleanup(slot->data);
-
-free:
 	sdhci_free_host(host);
 
 	return ERR_PTR(ret);
@@ -2244,9 +2224,6 @@ static void sdhci_pci_remove_slot(struct sdhci_pci_slot *slot)
 	if (slot->chip->fixes && slot->chip->fixes->remove_slot)
 		slot->chip->fixes->remove_slot(slot, dead);
 
-	if (slot->data && slot->data->cleanup)
-		slot->data->cleanup(slot->data);
-
 	sdhci_free_host(slot->host);
 }
 
diff --git a/drivers/mmc/host/sdhci-pci-data.c b/drivers/mmc/host/sdhci-pci-data.c
deleted file mode 100644
index 18638fb363d8..000000000000
--- a/drivers/mmc/host/sdhci-pci-data.c
+++ /dev/null
@@ -1,6 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-#include <linux/module.h>
-#include <linux/mmc/sdhci-pci-data.h>
-
-struct sdhci_pci_data *(*sdhci_pci_get_data)(struct pci_dev *pdev, int slotno);
-EXPORT_SYMBOL_GPL(sdhci_pci_get_data);
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index 8f90c4163bb5..15b36cd47860 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -156,7 +156,6 @@ struct sdhci_pci_fixes {
 struct sdhci_pci_slot {
 	struct sdhci_pci_chip	*chip;
 	struct sdhci_host	*host;
-	struct sdhci_pci_data	*data;
 
 	int			rst_n_gpio;
 	int			cd_gpio;
diff --git a/include/linux/mmc/sdhci-pci-data.h b/include/linux/mmc/sdhci-pci-data.h
deleted file mode 100644
index 1d42872d22f3..000000000000
--- a/include/linux/mmc/sdhci-pci-data.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef LINUX_MMC_SDHCI_PCI_DATA_H
-#define LINUX_MMC_SDHCI_PCI_DATA_H
-
-struct pci_dev;
-
-struct sdhci_pci_data {
-	struct pci_dev	*pdev;
-	int		slotno;
-	int		rst_n_gpio; /* Set to -EINVAL if unused */
-	int		cd_gpio;    /* Set to -EINVAL if unused */
-	int		(*setup)(struct sdhci_pci_data *data);
-	void		(*cleanup)(struct sdhci_pci_data *data);
-};
-
-extern struct sdhci_pci_data *(*sdhci_pci_get_data)(struct pci_dev *pdev,
-				int slotno);
-#endif
-- 
2.33.0

[PATCH v1 5/6] mmc: sdhci-pci: Remove dead code (cd_gpio, cd_irq et al)

[Andy Shevchenko]

The last user of this struct gone couple of releases ago.
Remove the dead code for good and encourage people to use
MMC core functionality for that.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 76 +------------------------------
 drivers/mmc/host/sdhci-pci.h      |  2 -
 2 files changed, 1 insertion(+), 77 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 8d01285e1b32..a4279437bb5f 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -345,73 +345,6 @@ static int pch_hc_probe_slot(struct sdhci_pci_slot *slot)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-
-static irqreturn_t sdhci_pci_sd_cd(int irq, void *dev_id)
-{
-	struct sdhci_pci_slot *slot = dev_id;
-	struct sdhci_host *host = slot->host;
-
-	mmc_detect_change(host->mmc, msecs_to_jiffies(200));
-	return IRQ_HANDLED;
-}
-
-static void sdhci_pci_add_own_cd(struct sdhci_pci_slot *slot)
-{
-	int err, irq, gpio = slot->cd_gpio;
-
-	slot->cd_gpio = -EINVAL;
-	slot->cd_irq = -EINVAL;
-
-	if (!gpio_is_valid(gpio))
-		return;
-
-	err = devm_gpio_request(&slot->chip->pdev->dev, gpio, "sd_cd");
-	if (err < 0)
-		goto out;
-
-	err = gpio_direction_input(gpio);
-	if (err < 0)
-		goto out_free;
-
-	irq = gpio_to_irq(gpio);
-	if (irq < 0)
-		goto out_free;
-
-	err = request_irq(irq, sdhci_pci_sd_cd, IRQF_TRIGGER_RISING |
-			  IRQF_TRIGGER_FALLING, "sd_cd", slot);
-	if (err)
-		goto out_free;
-
-	slot->cd_gpio = gpio;
-	slot->cd_irq = irq;
-
-	return;
-
-out_free:
-	devm_gpio_free(&slot->chip->pdev->dev, gpio);
-out:
-	dev_warn(&slot->chip->pdev->dev, "failed to setup card detect wake up\n");
-}
-
-static void sdhci_pci_remove_own_cd(struct sdhci_pci_slot *slot)
-{
-	if (slot->cd_irq >= 0)
-		free_irq(slot->cd_irq, slot);
-}
-
-#else
-
-static inline void sdhci_pci_add_own_cd(struct sdhci_pci_slot *slot)
-{
-}
-
-static inline void sdhci_pci_remove_own_cd(struct sdhci_pci_slot *slot)
-{
-}
-
-#endif
-
 static int mfd_emmc_probe_slot(struct sdhci_pci_slot *slot)
 {
 	slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE;
@@ -2113,7 +2046,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 	slot->chip = chip;
 	slot->host = host;
 	slot->rst_n_gpio = -EINVAL;
-	slot->cd_gpio = -EINVAL;
 	slot->cd_idx = -1;
 
 	host->hw_name = "PCI";
@@ -2184,15 +2116,11 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 	if (ret)
 		goto remove;
 
-	sdhci_pci_add_own_cd(slot);
-
 	/*
 	 * Check if the chip needs a separate GPIO for card detect to wake up
 	 * from runtime suspend.  If it is not there, don't allow runtime PM.
-	 * Note sdhci_pci_add_own_cd() sets slot->cd_gpio to -EINVAL on failure.
 	 */
-	if (chip->fixes && chip->fixes->own_cd_for_runtime_pm &&
-	    !gpio_is_valid(slot->cd_gpio) && slot->cd_idx < 0)
+	if (chip->fixes && chip->fixes->own_cd_for_runtime_pm && slot->cd_idx < 0)
 		chip->allow_runtime_pm = false;
 
 	return slot;
@@ -2212,8 +2140,6 @@ static void sdhci_pci_remove_slot(struct sdhci_pci_slot *slot)
 	int dead;
 	u32 scratch;
 
-	sdhci_pci_remove_own_cd(slot);
-
 	dead = 0;
 	scratch = readl(slot->host->ioaddr + SDHCI_INT_STATUS);
 	if (scratch == (u32)-1)
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index 15b36cd47860..8bb3b9c78589 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -158,8 +158,6 @@ struct sdhci_pci_slot {
 	struct sdhci_host	*host;
 
 	int			rst_n_gpio;
-	int			cd_gpio;
-	int			cd_irq;
 
 	int			cd_idx;
 	bool			cd_override_level;
-- 
2.33.0

[PATCH v1 6/6] mmc: sdhci-pci: Remove dead code (rst_n_gpio et al)

[Andy Shevchenko]

There is no user of this member. Remove the dead code for good.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 27 ---------------------------
 drivers/mmc/host/sdhci-pci.h      |  2 --
 2 files changed, 29 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index a4279437bb5f..6c284e07675e 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1900,21 +1900,6 @@ int sdhci_pci_enable_dma(struct sdhci_host *host)
 	return 0;
 }
 
-static void sdhci_pci_gpio_hw_reset(struct sdhci_host *host)
-{
-	struct sdhci_pci_slot *slot = sdhci_priv(host);
-	int rst_n_gpio = slot->rst_n_gpio;
-
-	if (!gpio_is_valid(rst_n_gpio))
-		return;
-	gpio_set_value_cansleep(rst_n_gpio, 0);
-	/* For eMMC, minimum is 1us but give it 10us for good measure */
-	udelay(10);
-	gpio_set_value_cansleep(rst_n_gpio, 1);
-	/* For eMMC, minimum is 200us but give it 300us for good measure */
-	usleep_range(300, 1000);
-}
-
 static void sdhci_pci_hw_reset(struct sdhci_host *host)
 {
 	struct sdhci_pci_slot *slot = sdhci_priv(host);
@@ -2045,7 +2030,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 
 	slot->chip = chip;
 	slot->host = host;
-	slot->rst_n_gpio = -EINVAL;
 	slot->cd_idx = -1;
 
 	host->hw_name = "PCI";
@@ -2071,17 +2055,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
 			goto cleanup;
 	}
 
-	if (gpio_is_valid(slot->rst_n_gpio)) {
-		if (!devm_gpio_request(&pdev->dev, slot->rst_n_gpio, "eMMC_reset")) {
-			gpio_direction_output(slot->rst_n_gpio, 1);
-			slot->host->mmc->caps |= MMC_CAP_HW_RESET;
-			slot->hw_reset = sdhci_pci_gpio_hw_reset;
-		} else {
-			dev_warn(&pdev->dev, "failed to request rst_n_gpio\n");
-			slot->rst_n_gpio = -EINVAL;
-		}
-	}
-
 	host->mmc->pm_caps = MMC_PM_KEEP_POWER;
 	host->mmc->slotno = slotno;
 	host->mmc->caps2 |= MMC_CAP2_NO_PRESCAN_POWERUP;
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index 8bb3b9c78589..5e3193278ff9 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -157,8 +157,6 @@ struct sdhci_pci_slot {
 	struct sdhci_pci_chip	*chip;
 	struct sdhci_host	*host;
 
-	int			rst_n_gpio;
-
 	int			cd_idx;
 	bool			cd_override_level;
 
-- 
2.33.0

Re: [PATCH v1 0/6] mmc: sdhci-pci: Add some CD GPIO related quirks

[Andy Shevchenko]

On Tue, Oct 05, 2021 at 01:24:24PM +0300, Andy Shevchenko wrote:
> It appears that one of the supported platform magically worked with the
> custom IRQ handler (any hints how?) while having two PCB designs with
> an opposite CD sense level. Here is an attempt to fix it by quirking out
> CD GPIO.
> 
> Patch 1 introduces two additional quirks (it's done this way due to
> patch 3, see below). Patch 2 is an actual fix for the mentioned platform.
> If backported need to be taken with patch 1 together. Patch 3 is (RFT)
> clean up. The questionable part here is the locking scheme. Shouldn't
> we do something similar in the generic IRQ handler of SDHCI? Or Broxton
> case has something quite different in mind?
> 
> Patches 4-6 are dead-code removals. Patch 4 accompanying patch 2, patches
> 5-6 just similar to it, but (functionally) independent. Would like to hear
> if it's okay to do.
> 
> Any comments, hints, advice are welcome!

Adrian, Ulf, any comments on this? At least first two fix the regression and
would be nice to have them in sooner than later (I can split them separately
if required).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/6] mmc: sdhci-pci: Add some CD GPIO related quirks

[Adrian Hunter]

On 07/10/2021 19:42, Andy Shevchenko wrote:
> On Tue, Oct 05, 2021 at 01:24:24PM +0300, Andy Shevchenko wrote:
>> It appears that one of the supported platform magically worked with the
>> custom IRQ handler (any hints how?) while having two PCB designs with
>> an opposite CD sense level. Here is an attempt to fix it by quirking out
>> CD GPIO.
>>
>> Patch 1 introduces two additional quirks (it's done this way due to
>> patch 3, see below). Patch 2 is an actual fix for the mentioned platform.
>> If backported need to be taken with patch 1 together. Patch 3 is (RFT)
>> clean up. The questionable part here is the locking scheme. Shouldn't
>> we do something similar in the generic IRQ handler of SDHCI? Or Broxton
>> case has something quite different in mind?
>>
>> Patches 4-6 are dead-code removals. Patch 4 accompanying patch 2, patches
>> 5-6 just similar to it, but (functionally) independent. Would like to hear
>> if it's okay to do.
>>
>> Any comments, hints, advice are welcome!
> 
> Adrian, Ulf, any comments on this? At least first two fix the regression and
> would be nice to have them in sooner than later (I can split them separately
> if required).

I am not sure we need new quirks, given that we can just hook the callback
and do anything that way.  However I really haven't had time to look closely
yet, sorry.

Re: [PATCH v1 0/6] mmc: sdhci-pci: Add some CD GPIO related quirks

[Raul Rangel]

It looks like you were previously using the `cd-gpios` DT property to
determine this. It also sounds like you switched from DT to ACPI so
you lost the ability to use this field?

Can you not use something like the following:
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v5.10/drivers/mmc/host/sdhci-acpi.c;l=945

p.s.,
Sorry for resending. I sent it as rich text before.


On Thu, Oct 7, 2021 at 10:47 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Oct 05, 2021 at 01:24:24PM +0300, Andy Shevchenko wrote:
> > It appears that one of the supported platform magically worked with the
> > custom IRQ handler (any hints how?) while having two PCB designs with
> > an opposite CD sense level. Here is an attempt to fix it by quirking out
> > CD GPIO.
> >
> > Patch 1 introduces two additional quirks (it's done this way due to
> > patch 3, see below). Patch 2 is an actual fix for the mentioned platform.
> > If backported need to be taken with patch 1 together. Patch 3 is (RFT)
> > clean up. The questionable part here is the locking scheme. Shouldn't
> > we do something similar in the generic IRQ handler of SDHCI? Or Broxton
> > case has something quite different in mind?
> >
> > Patches 4-6 are dead-code removals. Patch 4 accompanying patch 2, patches
> > 5-6 just similar to it, but (functionally) independent. Would like to hear
> > if it's okay to do.
> >
> > Any comments, hints, advice are welcome!
>
> Adrian, Ulf, any comments on this? At least first two fix the regression and
> would be nice to have them in sooner than later (I can split them separately
> if required).
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v1 1/6] mmc: sdhci: Introduce couple of quirks to ignore particular state of CD GPIO

[Adrian Hunter]

On 05/10/2021 13:24, Andy Shevchenko wrote:
> Some platforms may provide contradictory info in some states of CD GPIO line,
> and hence that state or states should be ignored. Introduce couple of quirks
> for that.

OK, I have looked closer now :-)

Hooking ->get_cd() should work, and that is preferred to new quirks.
Please try that.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mmc/host/sdhci.c | 13 ++++++++-----
>  drivers/mmc/host/sdhci.h |  4 ++++
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 357b365bf0ec..a7960ee3ef4f 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2395,7 +2395,7 @@ EXPORT_SYMBOL_GPL(sdhci_set_ios);
>  static int sdhci_get_cd(struct mmc_host *mmc)
>  {
>  	struct sdhci_host *host = mmc_priv(mmc);
> -	int gpio_cd = mmc_gpio_get_cd(mmc);
> +	int gpio_cd;
>  
>  	if (host->flags & SDHCI_DEVICE_DEAD)
>  		return 0;
> @@ -2405,11 +2405,14 @@ static int sdhci_get_cd(struct mmc_host *mmc)
>  		return 1;
>  
>  	/*
> -	 * Try slot gpio detect, if defined it take precedence
> -	 * over build in controller functionality
> +	 * Try slot GPIO detect, if defined it take precedence
> +	 * over build in controller functionality.
>  	 */
> -	if (gpio_cd >= 0)
> -		return !!gpio_cd;
> +	gpio_cd = mmc_gpio_get_cd(mmc);
> +	if (gpio_cd == 0 && !(host->quirks2 & SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_LOW))
> +		return 0;
> +	if (gpio_cd > 0 && !(host->quirks2 & SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH))
> +		return 1;
>  
>  	/* If polling, assume that the card is always present. */
>  	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index e8d04e42a5af..fb7910d22b18 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -464,6 +464,10 @@ struct sdhci_host {
>  #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
>  /* Controller has CRC in 136 bit Command Response */
>  #define SDHCI_QUIRK2_RSP_136_HAS_CRC			(1<<16)
> +/* Controller requires additional card detection test on GPIO low */
> +#define SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_LOW		(1<<17)
> +/* Controller requires additional card detection test on GPIO high */
> +#define SDHCI_QUIRK_CARD_DETECTION_IF_GPIO_HIGH		(1<<18)
>  /*
>   * Disable HW timeout if the requested timeout is more than the maximum
>   * obtainable timeout.
> 

Re: [PATCH v1 4/6] mmc: sdhci-pci: Remove dead code (struct sdhci_pci_data et al)

[Adrian Hunter]

On 05/10/2021 13:24, Andy Shevchenko wrote:
> The last user of this struct gone couple of releases ago. Besides that
> there were not so many users of this API for 10+ years: one is
> implied above Intel Merrifield (added 2016-08-31, removed 2021-02-11),
> and another is Intel Sunrisepoint (added 2015-02-06, removed 2017-03-20).

Wouldn't hurt to identify the commits here if it is not too much trouble.

> 
> Effectively this is a revert of the commit 52c506f0bc72 ("mmc: sdhci-pci:
> add platform data").
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mmc/host/Makefile          |  1 -
>  drivers/mmc/host/sdhci-pci-core.c  | 31 ++++--------------------------
>  drivers/mmc/host/sdhci-pci-data.c  |  6 ------
>  drivers/mmc/host/sdhci-pci.h       |  1 -
>  include/linux/mmc/sdhci-pci-data.h | 18 -----------------
>  5 files changed, 4 insertions(+), 53 deletions(-)
>  delete mode 100644 drivers/mmc/host/sdhci-pci-data.c
>  delete mode 100644 include/linux/mmc/sdhci-pci-data.h
> 
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 14004cc09aaa..ea36d379bd3c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -14,7 +14,6 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
>  sdhci-pci-y			+= sdhci-pci-core.o sdhci-pci-o2micro.o sdhci-pci-arasan.o \
>  				   sdhci-pci-dwc-mshc.o sdhci-pci-gli.o
> -obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI))	+= sdhci-pci-data.o
>  obj-$(CONFIG_MMC_SDHCI_ACPI)	+= sdhci-acpi.o
>  obj-$(CONFIG_MMC_SDHCI_PXAV3)	+= sdhci-pxav3.o
>  obj-$(CONFIG_MMC_SDHCI_PXAV2)	+= sdhci-pxav2.o
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 30caa0b325de..8d01285e1b32 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -17,8 +17,6 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/slab.h>
>  #include <linux/device.h>
> -#include <linux/mmc/host.h>
> -#include <linux/mmc/mmc.h>
>  #include <linux/scatterlist.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> @@ -26,11 +24,13 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/pm_qos.h>
>  #include <linux/debugfs.h>
> -#include <linux/mmc/slot-gpio.h>
> -#include <linux/mmc/sdhci-pci-data.h>
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
>  
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/slot-gpio.h>
> +
>  #ifdef CONFIG_X86
>  #include <asm/iosf_mbi.h>
>  #endif
> @@ -2116,22 +2116,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>  	slot->cd_gpio = -EINVAL;
>  	slot->cd_idx = -1;
>  
> -	/* Retrieve platform data if there is any */
> -	if (*sdhci_pci_get_data)
> -		slot->data = sdhci_pci_get_data(pdev, slotno);
> -
> -	if (slot->data) {
> -		if (slot->data->setup) {
> -			ret = slot->data->setup(slot->data);
> -			if (ret) {
> -				dev_err(&pdev->dev, "platform setup failed\n");
> -				goto free;
> -			}
> -		}
> -		slot->rst_n_gpio = slot->data->rst_n_gpio;
> -		slot->cd_gpio = slot->data->cd_gpio;
> -	}
> -
>  	host->hw_name = "PCI";
>  	host->ops = chip->fixes && chip->fixes->ops ?
>  		    chip->fixes->ops :
> @@ -2218,10 +2202,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>  		chip->fixes->remove_slot(slot, 0);
>  
>  cleanup:
> -	if (slot->data && slot->data->cleanup)
> -		slot->data->cleanup(slot->data);
> -
> -free:
>  	sdhci_free_host(host);
>  
>  	return ERR_PTR(ret);
> @@ -2244,9 +2224,6 @@ static void sdhci_pci_remove_slot(struct sdhci_pci_slot *slot)
>  	if (slot->chip->fixes && slot->chip->fixes->remove_slot)
>  		slot->chip->fixes->remove_slot(slot, dead);
>  
> -	if (slot->data && slot->data->cleanup)
> -		slot->data->cleanup(slot->data);
> -
>  	sdhci_free_host(slot->host);
>  }
>  
> diff --git a/drivers/mmc/host/sdhci-pci-data.c b/drivers/mmc/host/sdhci-pci-data.c
> deleted file mode 100644
> index 18638fb363d8..000000000000
> --- a/drivers/mmc/host/sdhci-pci-data.c
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -#include <linux/module.h>
> -#include <linux/mmc/sdhci-pci-data.h>
> -
> -struct sdhci_pci_data *(*sdhci_pci_get_data)(struct pci_dev *pdev, int slotno);
> -EXPORT_SYMBOL_GPL(sdhci_pci_get_data);
> diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
> index 8f90c4163bb5..15b36cd47860 100644
> --- a/drivers/mmc/host/sdhci-pci.h
> +++ b/drivers/mmc/host/sdhci-pci.h
> @@ -156,7 +156,6 @@ struct sdhci_pci_fixes {
>  struct sdhci_pci_slot {
>  	struct sdhci_pci_chip	*chip;
>  	struct sdhci_host	*host;
> -	struct sdhci_pci_data	*data;
>  
>  	int			rst_n_gpio;
>  	int			cd_gpio;
> diff --git a/include/linux/mmc/sdhci-pci-data.h b/include/linux/mmc/sdhci-pci-data.h
> deleted file mode 100644
> index 1d42872d22f3..000000000000
> --- a/include/linux/mmc/sdhci-pci-data.h
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef LINUX_MMC_SDHCI_PCI_DATA_H
> -#define LINUX_MMC_SDHCI_PCI_DATA_H
> -
> -struct pci_dev;
> -
> -struct sdhci_pci_data {
> -	struct pci_dev	*pdev;
> -	int		slotno;
> -	int		rst_n_gpio; /* Set to -EINVAL if unused */
> -	int		cd_gpio;    /* Set to -EINVAL if unused */
> -	int		(*setup)(struct sdhci_pci_data *data);
> -	void		(*cleanup)(struct sdhci_pci_data *data);
> -};
> -
> -extern struct sdhci_pci_data *(*sdhci_pci_get_data)(struct pci_dev *pdev,
> -				int slotno);
> -#endif
> 

Re: [PATCH v1 5/6] mmc: sdhci-pci: Remove dead code (cd_gpio, cd_irq et al)

[Adrian Hunter]

On 05/10/2021 13:24, Andy Shevchenko wrote:
> The last user of this struct gone couple of releases ago.
> Remove the dead code for good and encourage people to use
> MMC core functionality for that.

So this is dependent on patch 4 i.e. that patch removed the
references to the code being removed in this patch.  Please
make a note of that in the commit message.


> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mmc/host/sdhci-pci-core.c | 76 +------------------------------
>  drivers/mmc/host/sdhci-pci.h      |  2 -
>  2 files changed, 1 insertion(+), 77 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 8d01285e1b32..a4279437bb5f 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -345,73 +345,6 @@ static int pch_hc_probe_slot(struct sdhci_pci_slot *slot)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM
> -
> -static irqreturn_t sdhci_pci_sd_cd(int irq, void *dev_id)
> -{
> -	struct sdhci_pci_slot *slot = dev_id;
> -	struct sdhci_host *host = slot->host;
> -
> -	mmc_detect_change(host->mmc, msecs_to_jiffies(200));
> -	return IRQ_HANDLED;
> -}
> -
> -static void sdhci_pci_add_own_cd(struct sdhci_pci_slot *slot)
> -{
> -	int err, irq, gpio = slot->cd_gpio;
> -
> -	slot->cd_gpio = -EINVAL;
> -	slot->cd_irq = -EINVAL;
> -
> -	if (!gpio_is_valid(gpio))
> -		return;
> -
> -	err = devm_gpio_request(&slot->chip->pdev->dev, gpio, "sd_cd");
> -	if (err < 0)
> -		goto out;
> -
> -	err = gpio_direction_input(gpio);
> -	if (err < 0)
> -		goto out_free;
> -
> -	irq = gpio_to_irq(gpio);
> -	if (irq < 0)
> -		goto out_free;
> -
> -	err = request_irq(irq, sdhci_pci_sd_cd, IRQF_TRIGGER_RISING |
> -			  IRQF_TRIGGER_FALLING, "sd_cd", slot);
> -	if (err)
> -		goto out_free;
> -
> -	slot->cd_gpio = gpio;
> -	slot->cd_irq = irq;
> -
> -	return;
> -
> -out_free:
> -	devm_gpio_free(&slot->chip->pdev->dev, gpio);
> -out:
> -	dev_warn(&slot->chip->pdev->dev, "failed to setup card detect wake up\n");
> -}
> -
> -static void sdhci_pci_remove_own_cd(struct sdhci_pci_slot *slot)
> -{
> -	if (slot->cd_irq >= 0)
> -		free_irq(slot->cd_irq, slot);
> -}
> -
> -#else
> -
> -static inline void sdhci_pci_add_own_cd(struct sdhci_pci_slot *slot)
> -{
> -}
> -
> -static inline void sdhci_pci_remove_own_cd(struct sdhci_pci_slot *slot)
> -{
> -}
> -
> -#endif
> -
>  static int mfd_emmc_probe_slot(struct sdhci_pci_slot *slot)
>  {
>  	slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE;
> @@ -2113,7 +2046,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>  	slot->chip = chip;
>  	slot->host = host;
>  	slot->rst_n_gpio = -EINVAL;
> -	slot->cd_gpio = -EINVAL;
>  	slot->cd_idx = -1;
>  
>  	host->hw_name = "PCI";
> @@ -2184,15 +2116,11 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>  	if (ret)
>  		goto remove;
>  
> -	sdhci_pci_add_own_cd(slot);
> -
>  	/*
>  	 * Check if the chip needs a separate GPIO for card detect to wake up
>  	 * from runtime suspend.  If it is not there, don't allow runtime PM.
> -	 * Note sdhci_pci_add_own_cd() sets slot->cd_gpio to -EINVAL on failure.
>  	 */
> -	if (chip->fixes && chip->fixes->own_cd_for_runtime_pm &&
> -	    !gpio_is_valid(slot->cd_gpio) && slot->cd_idx < 0)
> +	if (chip->fixes && chip->fixes->own_cd_for_runtime_pm && slot->cd_idx < 0)
>  		chip->allow_runtime_pm = false;
>  
>  	return slot;
> @@ -2212,8 +2140,6 @@ static void sdhci_pci_remove_slot(struct sdhci_pci_slot *slot)
>  	int dead;
>  	u32 scratch;
>  
> -	sdhci_pci_remove_own_cd(slot);
> -
>  	dead = 0;
>  	scratch = readl(slot->host->ioaddr + SDHCI_INT_STATUS);
>  	if (scratch == (u32)-1)
> diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
> index 15b36cd47860..8bb3b9c78589 100644
> --- a/drivers/mmc/host/sdhci-pci.h
> +++ b/drivers/mmc/host/sdhci-pci.h
> @@ -158,8 +158,6 @@ struct sdhci_pci_slot {
>  	struct sdhci_host	*host;
>  
>  	int			rst_n_gpio;
> -	int			cd_gpio;
> -	int			cd_irq;
>  
>  	int			cd_idx;
>  	bool			cd_override_level;
> 

Re: [PATCH v1 6/6] mmc: sdhci-pci: Remove dead code (rst_n_gpio et al)

[Adrian Hunter]

On 05/10/2021 13:24, Andy Shevchenko wrote:
> There is no user of this member. Remove the dead code for good.

So this is dependent on patch 5 i.e. that patch removed the
references to the code being removed in this patch.  Please
make a note of that in the commit message.


> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mmc/host/sdhci-pci-core.c | 27 ---------------------------
>  drivers/mmc/host/sdhci-pci.h      |  2 --
>  2 files changed, 29 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index a4279437bb5f..6c284e07675e 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -1900,21 +1900,6 @@ int sdhci_pci_enable_dma(struct sdhci_host *host)
>  	return 0;
>  }
>  
> -static void sdhci_pci_gpio_hw_reset(struct sdhci_host *host)
> -{
> -	struct sdhci_pci_slot *slot = sdhci_priv(host);
> -	int rst_n_gpio = slot->rst_n_gpio;
> -
> -	if (!gpio_is_valid(rst_n_gpio))
> -		return;
> -	gpio_set_value_cansleep(rst_n_gpio, 0);
> -	/* For eMMC, minimum is 1us but give it 10us for good measure */
> -	udelay(10);
> -	gpio_set_value_cansleep(rst_n_gpio, 1);
> -	/* For eMMC, minimum is 200us but give it 300us for good measure */
> -	usleep_range(300, 1000);
> -}
> -
>  static void sdhci_pci_hw_reset(struct sdhci_host *host)
>  {
>  	struct sdhci_pci_slot *slot = sdhci_priv(host);
> @@ -2045,7 +2030,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>  
>  	slot->chip = chip;
>  	slot->host = host;
> -	slot->rst_n_gpio = -EINVAL;
>  	slot->cd_idx = -1;
>  
>  	host->hw_name = "PCI";
> @@ -2071,17 +2055,6 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
>  			goto cleanup;
>  	}
>  
> -	if (gpio_is_valid(slot->rst_n_gpio)) {
> -		if (!devm_gpio_request(&pdev->dev, slot->rst_n_gpio, "eMMC_reset")) {
> -			gpio_direction_output(slot->rst_n_gpio, 1);
> -			slot->host->mmc->caps |= MMC_CAP_HW_RESET;
> -			slot->hw_reset = sdhci_pci_gpio_hw_reset;
> -		} else {
> -			dev_warn(&pdev->dev, "failed to request rst_n_gpio\n");
> -			slot->rst_n_gpio = -EINVAL;
> -		}
> -	}
> -
>  	host->mmc->pm_caps = MMC_PM_KEEP_POWER;
>  	host->mmc->slotno = slotno;
>  	host->mmc->caps2 |= MMC_CAP2_NO_PRESCAN_POWERUP;
> diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
> index 8bb3b9c78589..5e3193278ff9 100644
> --- a/drivers/mmc/host/sdhci-pci.h
> +++ b/drivers/mmc/host/sdhci-pci.h
> @@ -157,8 +157,6 @@ struct sdhci_pci_slot {
>  	struct sdhci_pci_chip	*chip;
>  	struct sdhci_host	*host;
>  
> -	int			rst_n_gpio;
> -
>  	int			cd_idx;
>  	bool			cd_override_level;
>  
> 

Re: [PATCH v1 0/6] mmc: sdhci-pci: Add some CD GPIO related quirks

[Andy Shevchenko]

On Thu, Oct 7, 2021 at 10:45 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 07/10/2021 19:42, Andy Shevchenko wrote:
> > On Tue, Oct 05, 2021 at 01:24:24PM +0300, Andy Shevchenko wrote:

...

> > Adrian, Ulf, any comments on this? At least first two fix the regression and
> > would be nice to have them in sooner than later (I can split them separately
> > if required).
>
> I am not sure we need new quirks, given that we can just hook the callback
> and do anything that way.  However I really haven't had time to look closely
> yet, sorry.

Adrian, thanks for review, I'll address your comments in v2.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/6] mmc: sdhci-pci: Add some CD GPIO related quirks

[Andy Shevchenko]

On Fri, Oct 8, 2021 at 1:29 AM Raul Rangel <rrangel@chromium.org> wrote:
>
> It looks like you were previously using the `cd-gpios` DT property to
> determine this.

No, it's not the case. The case is that we switched from hard coded
stuff (board files) to ACPI.

> It also sounds like you switched from DT to ACPI so
> you lost the ability to use this field?
>
> Can you not use something like the following:
> https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/v5.10/drivers/mmc/host/sdhci-acpi.c;l=945

No. It won't work. Anything like this won't work.

The problem here is on PCB level due to different uSD connectors ()
being use: one is Active High, the other is Active Low for CD signal.

...

> > > Any comments, hints, advice are welcome!

-- 
With Best Regards,
Andy Shevchenko