[PATCH v2 0/9] mmc: mmci: add support for STM32 SD controller

[PATCH v2 0/9] mmc: mmci: add support for STM32 SD controller

[Andrea Merello]

The SD controller found in STM32 MCUs happens to be yet another variant
of the ARM PrimeCell PL18x SD host controller, for which the mmci driver
exists.

This series adds support for it to the mmci driver.

As other varians, this one need some specific quirks, that this series
address. Most notably this variant has not AMBA PrimeCell id registers,
so we can't probe it using the AMBA PrimeCell generic "compatible" and
mechanism; rather this series adds support to the mmci driver for
register itself also as platform driver, so that specific "copatible"
strings can also be used.

I tested this on my STM32F469-disco board, that is able to boot from an
SD card, and write/read to/from it.

RFT for other variants (to check I didn't broke anything) and with MMC
cards. In particular I tried to implement also support for open-collector
communication mode, that AFAICT is used only on MMC cards, but I couldn't
test it.

Changes since v1:
- fixed compile failure when CONFIG_ARM_AMBA is enabled
- fixed mmc CD pin claimed two times
- trivial whitespace and newline fixes

Andrea Merello (9):
  mmc: mmci: don't pretend IP variants with only one IRQ to have two
    mask regs
  mmc: mmci: add support for not-amba, but still compatible, variants
  mmc: mmci: don't pretend all variants to have MCI_STARBITERR flag
  mmc: mmci: add support for setting pad type via pinctrl
  mmc: mmci: add STM32 variant
  DT: stm32f429: add pin map for SDIO controller
  DT: stm32f429: add node for SDIO controller
  DT: stm32f469-disco: add node for SDIO controller
  Documentation: document mmci STM32 DT binding

 Documentation/devicetree/bindings/mmc/mmci.txt |   2 +-
 arch/arm/boot/dts/stm32f429.dtsi               |  47 ++-
 arch/arm/boot/dts/stm32f469-disco.dts          |  29 ++
 drivers/mmc/host/Kconfig                       |   8 +-
 drivers/mmc/host/mmci.c                        | 398 +++++++++++++++++++------
 drivers/mmc/host/mmci.h                        |   7 +-
 6 files changed, 396 insertions(+), 95 deletions(-)

--
2.7.4

[PATCH v2 1/9] mmc: mmci: don't pretend IP variants with only one IRQ to have two mask regs

[Andrea Merello]

Currently the driver supports both devices with one and two IRQ lines.

Two mask registers are used in order to select which events have to
actually generate an interrupt on each IRQ line.

It seems that in the single-IRQ case it's assumed that the IRQs lines
are simply OR-ed, while the two mask registers are still present. The
driver still programs the two mask registers separately.

However the STM32 variant has only one IRQ, and also has only one mask
register.

This patch prepares for STM32 variant support by making the driver using
only one mask register when only one IRQ is available.

Tested only on STM32 variant. RFT for variants other than STM32

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 drivers/mmc/host/mmci.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 01a8047..597fab8 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -404,9 +404,9 @@ static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
 		mask0 |= mask;
 
 		writel(mask0, base + MMCIMASK0);
+	} else {
+		writel(mask, base + MMCIMASK1);
 	}
-
-	writel(mask, base + MMCIMASK1);
 }
 
 static void mmci_stop_data(struct mmci_host *host)
@@ -1274,12 +1274,10 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 
 	do {
 		status = readl(host->base + MMCISTATUS);
+		status &= readl(host->base + MMCIMASK0);
 
 		if (host->singleirq) {
-			if (status & readl(host->base + MMCIMASK1))
-				mmci_pio_irq(irq, dev_id);
-
-			status &= ~MCI_IRQ1MASK;
+			mmci_pio_irq(irq, dev_id);
 		}
 
 		/*
@@ -1287,7 +1285,6 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		 * enabled) since the HW seems to be triggering the IRQ on both
 		 * edges while monitoring DAT0 for busy completion.
 		 */
-		status &= readl(host->base + MMCIMASK0);
 		writel(status, host->base + MMCICLEAR);
 
 		dev_dbg(mmc_dev(host->mmc), "irq0 (data+cmd) %08x\n", status);
@@ -1710,7 +1707,8 @@ static int mmci_probe(struct amba_device *dev,
 	spin_lock_init(&host->lock);
 
 	writel(0, host->base + MMCIMASK0);
-	writel(0, host->base + MMCIMASK1);
+	if (!host->singleirq)
+		writel(0, host->base + MMCIMASK1);
 	writel(0xfff, host->base + MMCICLEAR);
 
 	/*
@@ -1800,7 +1798,8 @@ static int mmci_remove(struct amba_device *dev)
 		mmc_remove_host(mmc);
 
 		writel(0, host->base + MMCIMASK0);
-		writel(0, host->base + MMCIMASK1);
+		if (!host->singleirq)
+			writel(0, host->base + MMCIMASK1);
 
 		writel(0, host->base + MMCICOMMAND);
 		writel(0, host->base + MMCIDATACTRL);
-- 
2.7.4

[PATCH v2 2/9] mmc: mmci: add support for not-amba, but still compatible, variants

[Andrea Merello]

The STM32F4xx family contain a SDIO IP that looks like a variant of
the PL180, however, inspite it's actually attached to a APB bus, it
cannot be handled by the AMBA bus code, because it lacks of the ID
registers that AMBA primecell IPs have.

This patch prepares for supporting the STM32 variant by letting the
driver register also as a platform driver, that can be matched from
the OF with specific "compatible" strings

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 drivers/mmc/host/Kconfig |   8 +-
 drivers/mmc/host/mmci.c  | 297 ++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 236 insertions(+), 69 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 2eb9701..f8f7f289 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -5,12 +5,12 @@
 comment "MMC/SD/SDIO Host Controller Drivers"
 
 config MMC_ARMMMCI
-	tristate "ARM AMBA Multimedia Card Interface support"
-	depends on ARM_AMBA
+	tristate "ARM AMBA Multimedia Card Interface and compatible support"
 	help
 	  This selects the ARM(R) AMBA(R) PrimeCell Multimedia Card
-	  Interface (PL180 and PL181) support.  If you have an ARM(R)
-	  platform with a Multimedia Card slot, say Y or M here.
+	  Interface (PL180, PL181 and compatible) support.
+	  If you have an ARM(R) platform with a Multimedia Card slot,
+	  say Y or M here.
 
 	  If unsure, say N.
 
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 597fab8..54c2fc3 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1,5 +1,6 @@
 /*
- *  linux/drivers/mmc/host/mmci.c - ARM PrimeCell MMCI PL180/1 driver
+ *  linux/drivers/mmc/host/mmci.c - ARM PrimeCell MMCI PL180/1 and
+ *  comaptible driver
  *
  *  Copyright (C) 2003 Deep Blue Solutions, Ltd, All Rights Reserved.
  *  Copyright (C) 2010 ST-Ericsson SA
@@ -37,6 +38,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/types.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/of_device.h>
 
 #include <asm/div64.h>
 #include <asm/io.h>
@@ -113,7 +116,7 @@ struct variant_data {
 	bool			reversed_irq_handling;
 };
 
-static struct variant_data variant_arm = {
+static __maybe_unused struct variant_data variant_arm = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.datalength_bits	= 16,
@@ -122,7 +125,7 @@ static struct variant_data variant_arm = {
 	.reversed_irq_handling	= true,
 };
 
-static struct variant_data variant_arm_extended_fifo = {
+static __maybe_unused struct variant_data variant_arm_extended_fifo = {
 	.fifosize		= 128 * 4,
 	.fifohalfsize		= 64 * 4,
 	.datalength_bits	= 16,
@@ -130,7 +133,7 @@ static struct variant_data variant_arm_extended_fifo = {
 	.f_max			= 100000000,
 };
 
-static struct variant_data variant_arm_extended_fifo_hwfc = {
+static __maybe_unused struct variant_data variant_arm_extended_fifo_hwfc = {
 	.fifosize		= 128 * 4,
 	.fifohalfsize		= 64 * 4,
 	.clkreg_enable		= MCI_ARM_HWFCEN,
@@ -139,7 +142,7 @@ static struct variant_data variant_arm_extended_fifo_hwfc = {
 	.f_max			= 100000000,
 };
 
-static struct variant_data variant_u300 = {
+static __maybe_unused struct variant_data variant_u300 = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg_enable		= MCI_ST_U300_HWFCEN,
@@ -154,7 +157,7 @@ static struct variant_data variant_u300 = {
 	.pwrreg_nopower		= true,
 };
 
-static struct variant_data variant_nomadik = {
+static __maybe_unused struct variant_data variant_nomadik = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
@@ -170,7 +173,7 @@ static struct variant_data variant_nomadik = {
 	.pwrreg_nopower		= true,
 };
 
-static struct variant_data variant_ux500 = {
+static __maybe_unused struct variant_data variant_ux500 = {
 	.fifosize		= 30 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
@@ -192,7 +195,7 @@ static struct variant_data variant_ux500 = {
 	.pwrreg_nopower		= true,
 };
 
-static struct variant_data variant_ux500v2 = {
+static __maybe_unused struct variant_data variant_ux500v2 = {
 	.fifosize		= 30 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
@@ -216,7 +219,7 @@ static struct variant_data variant_ux500v2 = {
 	.pwrreg_nopower		= true,
 };
 
-static struct variant_data variant_qcom = {
+static __maybe_unused struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
@@ -1528,31 +1531,32 @@ static int mmci_of_parse(struct device_node *np, struct mmc_host *mmc)
 	return 0;
 }
 
-static int mmci_probe(struct amba_device *dev,
-	const struct amba_id *id)
+static struct mmci_host *mmci_probe(struct device *dev,
+				    struct variant_data *variant,
+				    struct resource *res,
+				    unsigned int irq0, unsigned int irq1)
 {
-	struct mmci_platform_data *plat = dev->dev.platform_data;
-	struct device_node *np = dev->dev.of_node;
-	struct variant_data *variant = id->data;
-	struct mmci_host *host;
+	struct device_node *np = dev->of_node;
+	struct mmci_platform_data *plat = dev->platform_data;
 	struct mmc_host *mmc;
+	struct mmci_host *host;
 	int ret;
 
 	/* Must have platform data or Device Tree. */
 	if (!plat && !np) {
-		dev_err(&dev->dev, "No plat data or DT found\n");
-		return -EINVAL;
+		dev_err(dev, "No plat data or DT found\n");
+		return ERR_PTR(-EINVAL);
 	}
 
 	if (!plat) {
-		plat = devm_kzalloc(&dev->dev, sizeof(*plat), GFP_KERNEL);
+		plat = devm_kzalloc(dev, sizeof(*plat), GFP_KERNEL);
 		if (!plat)
-			return -ENOMEM;
+			return ERR_PTR(-ENOMEM);
 	}
 
-	mmc = mmc_alloc_host(sizeof(struct mmci_host), &dev->dev);
+	mmc = mmc_alloc_host(sizeof(struct mmci_host), dev);
 	if (!mmc)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	ret = mmci_of_parse(np, mmc);
 	if (ret)
@@ -1561,12 +1565,7 @@ static int mmci_probe(struct amba_device *dev,
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
 
-	host->hw_designer = amba_manf(dev);
-	host->hw_revision = amba_rev(dev);
-	dev_dbg(mmc_dev(mmc), "designer ID = 0x%02x\n", host->hw_designer);
-	dev_dbg(mmc_dev(mmc), "revision = 0x%01x\n", host->hw_revision);
-
-	host->clk = devm_clk_get(&dev->dev, NULL);
+	host->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
 		goto host_free;
@@ -1598,8 +1597,8 @@ static int mmci_probe(struct amba_device *dev,
 			host->mclk);
 	}
 
-	host->phybase = dev->res.start;
-	host->base = devm_ioremap_resource(&dev->dev, &dev->res);
+	host->phybase = res->start;
+	host->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(host->base)) {
 		ret = PTR_ERR(host->base);
 		goto clk_disable;
@@ -1742,49 +1741,41 @@ static int mmci_probe(struct amba_device *dev,
 		}
 	}
 
-	ret = devm_request_irq(&dev->dev, dev->irq[0], mmci_irq, IRQF_SHARED,
-			DRIVER_NAME " (cmd)", host);
+	ret = devm_request_irq(dev, irq0, mmci_irq, IRQF_SHARED,
+			       DRIVER_NAME " (cmd)", host);
 	if (ret)
 		goto clk_disable;
 
-	if (!dev->irq[1])
+	if (!irq1) {
 		host->singleirq = true;
-	else {
-		ret = devm_request_irq(&dev->dev, dev->irq[1], mmci_pio_irq,
-				IRQF_SHARED, DRIVER_NAME " (pio)", host);
+	} else {
+		ret = devm_request_irq(dev, irq1, mmci_pio_irq,
+				       IRQF_SHARED, DRIVER_NAME " (pio)", host);
 		if (ret)
 			goto clk_disable;
 	}
 
 	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
 
-	amba_set_drvdata(dev, mmc);
-
-	dev_info(&dev->dev, "%s: PL%03x manf %x rev%u at 0x%08llx irq %d,%d (pio)\n",
-		 mmc_hostname(mmc), amba_part(dev), amba_manf(dev),
-		 amba_rev(dev), (unsigned long long)dev->res.start,
-		 dev->irq[0], dev->irq[1]);
-
 	mmci_dma_setup(host);
 
-	pm_runtime_set_autosuspend_delay(&dev->dev, 50);
-	pm_runtime_use_autosuspend(&dev->dev);
+	pm_runtime_set_autosuspend_delay(dev, 50);
+	pm_runtime_use_autosuspend(dev);
 
 	mmc_add_host(mmc);
 
-	pm_runtime_put(&dev->dev);
-	return 0;
+	pm_runtime_put(dev);
+	return host;
 
  clk_disable:
 	clk_disable_unprepare(host->clk);
  host_free:
 	mmc_free_host(mmc);
-	return ret;
+	return ERR_PTR(ret);
 }
 
-static int mmci_remove(struct amba_device *dev)
+static int mmc_remove(struct mmc_host *mmc)
 {
-	struct mmc_host *mmc = amba_get_drvdata(dev);
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
@@ -1793,7 +1784,7 @@ static int mmci_remove(struct amba_device *dev)
 		 * Undo pm_runtime_put() in probe.  We use the _sync
 		 * version here so that we can access the primecell.
 		 */
-		pm_runtime_get_sync(&dev->dev);
+		pm_runtime_get_sync(mmc->parent);
 
 		mmc_remove_host(mmc);
 
@@ -1847,14 +1838,11 @@ static void mmci_restore(struct mmci_host *host)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
-static int mmci_runtime_suspend(struct device *dev)
+static int mmci_runtime_suspend(struct mmc_host *mmc)
 {
-	struct amba_device *adev = to_amba_device(dev);
-	struct mmc_host *mmc = amba_get_drvdata(adev);
-
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
-		pinctrl_pm_select_sleep_state(dev);
+		pinctrl_pm_select_sleep_state(mmc->parent);
 		mmci_save(host);
 		clk_disable_unprepare(host->clk);
 	}
@@ -1862,28 +1850,77 @@ static int mmci_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int mmci_runtime_resume(struct device *dev)
+static int mmci_runtime_resume(struct mmc_host *mmc)
 {
-	struct amba_device *adev = to_amba_device(dev);
-	struct mmc_host *mmc = amba_get_drvdata(adev);
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 		clk_prepare_enable(host->clk);
 		mmci_restore(host);
-		pinctrl_pm_select_default_state(dev);
+		pinctrl_pm_select_default_state(mmc->parent);
 	}
 
 	return 0;
 }
 #endif
 
-static const struct dev_pm_ops mmci_dev_pm_ops = {
+#ifdef CONFIG_ARM_AMBA
+static int mmci_remove_amba(struct amba_device *dev)
+{
+	struct mmc_host *mmc = amba_get_drvdata(dev);
+
+	return mmc_remove(mmc);
+}
+
+static int mmci_runtime_resume_amba(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
+
+	return mmci_runtime_resume(mmc);
+}
+
+static int mmci_runtime_suspend_amba(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct mmc_host *mmc = amba_get_drvdata(adev);
+
+	return mmci_runtime_suspend(mmc);
+}
+
+static const struct dev_pm_ops mmci_dev_pm_ops_amba = {
 	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
 				pm_runtime_force_resume)
-	SET_RUNTIME_PM_OPS(mmci_runtime_suspend, mmci_runtime_resume, NULL)
+	SET_RUNTIME_PM_OPS(mmci_runtime_suspend_amba, mmci_runtime_resume_amba,
+			   NULL)
 };
 
+static int mmci_probe_amba(struct amba_device *dev, const struct amba_id *id)
+{
+	struct mmci_host *host;
+	struct variant_data *variant = id->data;
+
+	host = mmci_probe(&dev->dev, variant, &dev->res,
+			  dev->irq[0], dev->irq[1]);
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	host->hw_designer = amba_manf(dev);
+	host->hw_revision = amba_rev(dev);
+	dev_dbg(mmc_dev(host->mmc), "designer ID = 0x%02x\n",
+		host->hw_designer);
+	dev_dbg(mmc_dev(host->mmc), "revision = 0x%01x\n", host->hw_revision);
+
+	amba_set_drvdata(dev, host->mmc);
+
+	dev_info(&dev->dev, "%s: PL%03x manf %x rev%u at 0x%08llx irq %d,%d (pio)\n",
+		 mmc_hostname(host->mmc), amba_part(dev), amba_manf(dev),
+		 amba_rev(dev), (unsigned long long)dev->res.start,
+		 dev->irq[0], dev->irq[1]);
+
+	return 0;
+}
+
 static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
@@ -1945,14 +1982,144 @@ MODULE_DEVICE_TABLE(amba, mmci_ids);
 static struct amba_driver mmci_driver = {
 	.drv		= {
 		.name	= DRIVER_NAME,
-		.pm	= &mmci_dev_pm_ops,
+		.pm	= &mmci_dev_pm_ops_amba,
 	},
-	.probe		= mmci_probe,
-	.remove		= mmci_remove,
+	.probe		= mmci_probe_amba,
+	.remove		= mmci_remove_amba,
 	.id_table	= mmci_ids,
 };
+#endif
+
+static const struct of_device_id mmci_pltfm_match[] = {
+	{},
+};
+
+static int mmci_probe_pltfm(struct platform_device *pdev)
+{
+	struct mmci_host *host;
+	struct variant_data *variant;
+	const struct of_device_id *match;
+	int irq0, irq1;
+	struct resource *res;
+
+	irq0 = platform_get_irq(pdev, 0);
+	if (irq0 < 0) {
+		dev_err(&pdev->dev, "Can't get IRQ");
+		return -EINVAL;
+	}
+
+	irq1 = platform_get_irq(pdev, 1);
+	/* optional. set to 0 to be compatible with original amba probe code */
+	if (irq1 < 0)
+		irq1 = 0;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Can't get MMIO range");
+		return -EINVAL;
+	}
+
+	match = of_match_device(mmci_pltfm_match, &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "Can't get variant data\n");
+		return -EINVAL;
+	}
+	variant = (struct variant_data *)match->data;
+
+	host = mmci_probe(&pdev->dev, variant, res,
+			  irq0, irq1);
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	platform_set_drvdata(pdev, host->mmc);
+	dev_info(&pdev->dev, "%s: PL81x compatible SDIO at 0x%08llx irq %d,%d (pio)\n",
+		 mmc_hostname(host->mmc),
+		 (unsigned long long)res->start, irq0, irq1);
+
+	return 0;
+}
+
+static int mmci_remove_pltfm(struct platform_device *pdev)
+{
+	struct mmc_host *mmc = platform_get_drvdata(pdev);
+
+	return mmc_remove(mmc);
+}
+
+static int mmci_runtime_resume_pltfm(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct mmc_host *mmc = platform_get_drvdata(pdev);
+
+	return mmci_runtime_resume(mmc);
+}
+
+static int mmci_runtime_suspend_pltfm(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct mmc_host *mmc = platform_get_drvdata(pdev);
+
+	return mmci_runtime_suspend(mmc);
+}
+
+MODULE_DEVICE_TABLE(of, mmci_pltfm_match);
+
+static const struct dev_pm_ops mmci_dev_pm_ops_pltfm = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(mmci_runtime_suspend_pltfm,
+			   mmci_runtime_resume_pltfm, NULL)
+};
+
+static struct platform_driver mmci_pltfm_driver = {
+	.probe		= mmci_probe_pltfm,
+	.remove		= mmci_remove_pltfm,
+	.driver		= {
+		.name		= DRIVER_NAME,
+		.of_match_table	= mmci_pltfm_match,
+		.pm		= &mmci_dev_pm_ops_pltfm,
+	},
+};
+
+static int __init mmci_init(void)
+{
+	int ret;
+	__maybe_unused int ret_amba;
+
+	ret = platform_driver_register(&mmci_pltfm_driver);
+
+#ifdef CONFIG_ARM_AMBA
+	ret_amba = amba_driver_register(&mmci_driver);
+
+	/* if both plaform and amba failed. Just fail. */
+	if (ret && ret_amba)
+		return ret;
+
+	/*
+	 * If only one of the two succeeded, don't fail here, because the
+	 * other one is probably usable, but spit a warning..
+	 */
+	if (ret_amba)
+		pr_warn("could not register MMCI amba driver");
+
+	if (ret)
+		pr_warn("could not register MMCI platform driver");
+
+	return 0;
+#endif
+	return ret;
+}
+
+static void __exit mmci_exit(void)
+{
+	platform_driver_unregister(&mmci_pltfm_driver);
+#ifdef CONFIG_ARM_AMBA
+	amba_driver_unregister(&mmci_driver);
+#endif
+}
 
-module_amba_driver(mmci_driver);
+module_init(mmci_init);
+module_exit(mmci_exit);
 
 module_param(fmax, uint, 0444);
 
-- 
2.7.4

[PATCH v2 3/9] mmc: mmci: don't pretend all variants to have MCI_STARBITERR flag

[Andrea Merello]

This patch prepares for supporting the STM32 variant, that has no such bit
in the status register.

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 drivers/mmc/host/mmci.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 54c2fc3..fbb5728 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -114,6 +114,7 @@ struct variant_data {
 	bool			qcom_fifo;
 	bool			qcom_dml;
 	bool			reversed_irq_handling;
+	bool			has_start_err;
 };
 
 static __maybe_unused struct variant_data variant_arm = {
@@ -123,6 +124,7 @@ static __maybe_unused struct variant_data variant_arm = {
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
 	.reversed_irq_handling	= true,
+	.has_start_err		= true,
 };
 
 static __maybe_unused struct variant_data variant_arm_extended_fifo = {
@@ -131,6 +133,7 @@ static __maybe_unused struct variant_data variant_arm_extended_fifo = {
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
+	.has_start_err		= true,
 };
 
 static __maybe_unused struct variant_data variant_arm_extended_fifo_hwfc = {
@@ -140,6 +143,7 @@ static __maybe_unused struct variant_data variant_arm_extended_fifo_hwfc = {
 	.datalength_bits	= 16,
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
+	.has_start_err		= true,
 };
 
 static __maybe_unused struct variant_data variant_u300 = {
@@ -155,6 +159,7 @@ static __maybe_unused struct variant_data variant_u300 = {
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
 	.pwrreg_nopower		= true,
+	.has_start_err		= true,
 };
 
 static __maybe_unused struct variant_data variant_nomadik = {
@@ -171,6 +176,7 @@ static __maybe_unused struct variant_data variant_nomadik = {
 	.signal_direction	= true,
 	.pwrreg_clkgate		= true,
 	.pwrreg_nopower		= true,
+	.has_start_err		= true,
 };
 
 static __maybe_unused struct variant_data variant_ux500 = {
@@ -193,6 +199,7 @@ static __maybe_unused struct variant_data variant_ux500 = {
 	.busy_detect_flag	= MCI_ST_CARDBUSY,
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
 	.pwrreg_nopower		= true,
+	.has_start_err		= true,
 };
 
 static __maybe_unused struct variant_data variant_ux500v2 = {
@@ -217,6 +224,7 @@ static __maybe_unused struct variant_data variant_ux500v2 = {
 	.busy_detect_flag	= MCI_ST_CARDBUSY,
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
 	.pwrreg_nopower		= true,
+	.has_start_err		= true,
 };
 
 static __maybe_unused struct variant_data variant_qcom = {
@@ -235,6 +243,7 @@ static __maybe_unused struct variant_data variant_qcom = {
 	.explicit_mclk_control	= true,
 	.qcom_fifo		= true,
 	.qcom_dml		= true,
+	.has_start_err		= true,
 };
 
 /* Busy detection for the ST Micro variant */
@@ -923,8 +932,9 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		return;
 
 	/* First check for errors */
-	if (status & (MCI_DATACRCFAIL|MCI_DATATIMEOUT|MCI_STARTBITERR|
-		      MCI_TXUNDERRUN|MCI_RXOVERRUN)) {
+	if (status & (MCI_DATACRCFAIL | MCI_DATATIMEOUT |
+			(host->variant->has_start_err ? MCI_STARTBITERR : 0) |
+			MCI_TXUNDERRUN | MCI_RXOVERRUN)) {
 		u32 remain, success;
 
 		/* Terminate the DMA transfer */
-- 
2.7.4

[PATCH v2 4/9] mmc: mmci: add support for setting pad type via pinctrl

[Andrea Merello]

The STM32 variant  hasn't the control bit to switch pads in opendrain mode.
In this case we can achive the same result by askint to the pinmux driver
to configure pins for us.

This patch make the mmci driver able to do this whenever needed.

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 drivers/mmc/host/mmci.c | 52 +++++++++++++++++++++++++++++++++++++++----------
 drivers/mmc/host/mmci.h |  7 +++++--
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index fbb5728..c581b81 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -115,6 +115,7 @@ struct variant_data {
 	bool			qcom_dml;
 	bool			reversed_irq_handling;
 	bool			has_start_err;
+	bool			has_pad_config;
 };
 
 static __maybe_unused struct variant_data variant_arm = {
@@ -125,6 +126,7 @@ static __maybe_unused struct variant_data variant_arm = {
 	.f_max			= 100000000,
 	.reversed_irq_handling	= true,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 static __maybe_unused struct variant_data variant_arm_extended_fifo = {
@@ -134,6 +136,7 @@ static __maybe_unused struct variant_data variant_arm_extended_fifo = {
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 static __maybe_unused struct variant_data variant_arm_extended_fifo_hwfc = {
@@ -144,6 +147,7 @@ static __maybe_unused struct variant_data variant_arm_extended_fifo_hwfc = {
 	.pwrreg_powerup		= MCI_PWR_UP,
 	.f_max			= 100000000,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 static __maybe_unused struct variant_data variant_u300 = {
@@ -160,6 +164,7 @@ static __maybe_unused struct variant_data variant_u300 = {
 	.pwrreg_clkgate		= true,
 	.pwrreg_nopower		= true,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 static __maybe_unused struct variant_data variant_nomadik = {
@@ -177,6 +182,7 @@ static __maybe_unused struct variant_data variant_nomadik = {
 	.pwrreg_clkgate		= true,
 	.pwrreg_nopower		= true,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 static __maybe_unused struct variant_data variant_ux500 = {
@@ -200,6 +206,7 @@ static __maybe_unused struct variant_data variant_ux500 = {
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
 	.pwrreg_nopower		= true,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 static __maybe_unused struct variant_data variant_ux500v2 = {
@@ -225,6 +232,7 @@ static __maybe_unused struct variant_data variant_ux500v2 = {
 	.busy_detect_mask	= MCI_ST_BUSYENDMASK,
 	.pwrreg_nopower		= true,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 static __maybe_unused struct variant_data variant_qcom = {
@@ -244,6 +252,7 @@ static __maybe_unused struct variant_data variant_qcom = {
 	.qcom_fifo		= true,
 	.qcom_dml		= true,
 	.has_start_err		= true,
+	.has_pad_config		= true,
 };
 
 /* Busy detection for the ST Micro variant */
@@ -1362,6 +1371,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	u32 pwr = 0;
 	unsigned long flags;
 	int ret;
+	struct pinctrl_state *pins;
+	bool is_opendrain;
 
 	if (host->plat->ios_handler &&
 		host->plat->ios_handler(mmc_dev(mmc), ios))
@@ -1420,16 +1431,31 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 				~MCI_ST_DATA2DIREN);
 	}
 
-	if (ios->bus_mode == MMC_BUSMODE_OPENDRAIN) {
-		if (host->hw_designer != AMBA_VENDOR_ST)
-			pwr |= MCI_ROD;
-		else {
-			/*
-			 * The ST Micro variant use the ROD bit for something
-			 * else and only has OD (Open Drain).
-			 */
-			pwr |= MCI_OD;
+	if (host->variant->has_pad_config) {
+		if (ios->bus_mode == MMC_BUSMODE_OPENDRAIN) {
+			if (host->hw_designer != AMBA_VENDOR_ST) {
+				pwr |= MCI_ROD;
+			} else {
+				/*
+				 * The ST Micro variant use the ROD bit for
+				 * something else and only has OD (Open Drain).
+				 */
+				pwr |= MCI_OD;
+			}
 		}
+	} else {
+		/*
+		 * If the variant cannot configure the pads by its own, then we
+		 * expect the pinctrl to be able to do that for us
+		 */
+		is_opendrain = (ios->bus_mode == MMC_BUSMODE_OPENDRAIN);
+		pins = pinctrl_lookup_state(host->pinctrl, is_opendrain ?
+					MMCI_PINCTRL_STATE_OPENDRAIN :
+					MMCI_PINCTRL_STATE_PUSHPULL);
+		if (IS_ERR(pins))
+			dev_warn(mmc_dev(mmc), "Cannot select pin drive type via pinctrl\n");
+		else
+			pinctrl_select_state(host->pinctrl, pins);
 	}
 
 	/*
@@ -1537,7 +1563,6 @@ static int mmci_of_parse(struct device_node *np, struct mmc_host *mmc)
 		mmc->caps |= MMC_CAP_MMC_HIGHSPEED;
 	if (of_get_property(np, "mmc-cap-sd-highspeed", NULL))
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
-
 	return 0;
 }
 
@@ -1575,6 +1600,13 @@ static struct mmci_host *mmci_probe(struct device *dev,
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
 
+	if (!variant->has_pad_config) {
+		host->pinctrl = devm_pinctrl_get(dev);
+		if (IS_ERR(host->pinctrl)) {
+			dev_err(dev, "failed to get pinctrl");
+			goto host_free;
+		}
+	}
 	host->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->clk)) {
 		ret = PTR_ERR(host->clk);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 56322c6..b5be66a 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -192,6 +192,10 @@
 
 #define NR_SG		128
 
+/* pinctrl configs */
+#define MMCI_PINCTRL_STATE_PUSHPULL "default"
+#define MMCI_PINCTRL_STATE_OPENDRAIN "opendrain"
+
 struct clk;
 struct variant_data;
 struct dma_chan;
@@ -226,7 +230,7 @@ struct mmci_host {
 	bool			vqmmc_enabled;
 	struct mmci_platform_data *plat;
 	struct variant_data	*variant;
-
+	struct pinctrl		*pinctrl;
 	u8			hw_designer;
 	u8			hw_revision:4;
 
@@ -251,4 +255,3 @@ struct mmci_host {
 #define dma_inprogress(host)	(0)
 #endif
 };
-
-- 
2.7.4

[PATCH v2 5/9] mmc: mmci: add STM32 variant

[Andrea Merello]

STM32 mcu has a SDIO controller that looks like an ARM pl810.
This patch adds the STM32 variant so that mmci driver supports it.

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 drivers/mmc/host/mmci.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index c581b81..88a350b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -235,6 +235,23 @@ static __maybe_unused struct variant_data variant_ux500v2 = {
 	.has_pad_config		= true,
 };
 
+static __maybe_unused struct variant_data variant_stm32 = {
+	.fifosize		= 32 * 4,
+	.fifohalfsize		= 8 * 4,
+	.clkreg			= MCI_CLK_ENABLE,
+	.clkreg_enable		= MCI_ST_UX500_HWFCEN,
+	.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
+	.clkreg_neg_edge_enable	= MCI_ST_UX500_NEG_EDGE,
+	.datalength_bits	= 24,
+	.datactrl_mask_sdio	= MCI_DPSM_ST_SDIOEN,
+	.st_sdio		= true,
+	.st_clkdiv		= true,
+	.pwrreg_powerup		= MCI_PWR_ON,
+	.f_max			= 48000000,
+	.pwrreg_clkgate		= true,
+	.pwrreg_nopower		= true,
+};
+
 static __maybe_unused struct variant_data variant_qcom = {
 	.fifosize		= 16 * 4,
 	.fifohalfsize		= 8 * 4,
@@ -2033,6 +2050,7 @@ static struct amba_driver mmci_driver = {
 #endif
 
 static const struct of_device_id mmci_pltfm_match[] = {
+	{ .compatible = "st,stm32f4xx-sdio", .data = &variant_stm32},
 	{},
 };
 
-- 
2.7.4

[PATCH v2 6/9] DT: stm32f429: add pin map for SDIO controller

[Andrea Merello]

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 arch/arm/boot/dts/stm32f429.dtsi | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index e4dae0e..de7b9c3 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -336,7 +336,38 @@
 				};
 			};
 
-			ethernet_mii: mii@0 {
+			sdio_pins: sdio_pins@0 {
+				pins {
+					pinmux = <STM32F429_PC8_FUNC_SDIO_D0>,
+						 <STM32F429_PC9_FUNC_SDIO_D1>,
+						 <STM32F429_PC10_FUNC_SDIO_D2>,
+						 <STM32F429_PC11_FUNC_SDIO_D3>,
+						 <STM32F429_PC12_FUNC_SDIO_CK>,
+						 <STM32F429_PD2_FUNC_SDIO_CMD>;
+					drive-push-pull;
+					slew-rate = <2>;
+				};
+			};
+
+			sdio_pins_od: sdio_pins_od@0 {
+				pins1 {
+					pinmux = <STM32F429_PC8_FUNC_SDIO_D0>,
+						 <STM32F429_PC9_FUNC_SDIO_D1>,
+						 <STM32F429_PC10_FUNC_SDIO_D2>,
+						 <STM32F429_PC11_FUNC_SDIO_D3>,
+						 <STM32F429_PC12_FUNC_SDIO_CK>;
+					drive-push-pull;
+					slew-rate = <2>;
+				};
+
+				pins2 {
+					pinmux = <STM32F429_PD2_FUNC_SDIO_CMD>;
+					drive-open-drain;
+					slew-rate = <2>;
+				};
+			};
+
+			ethernet0_mii: mii@0 {
 				pins {
 					pinmux = <STM32F429_PG13_FUNC_ETH_MII_TXD0_ETH_RMII_TXD0>,
 						 <STM32F429_PG14_FUNC_ETH_MII_TXD1_ETH_RMII_TXD1>,
-- 
2.7.4

[PATCH v2 7/9] DT: stm32f429: add node for SDIO controller

[Andrea Merello]

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 arch/arm/boot/dts/stm32f429.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index de7b9c3..c12a64e 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -388,6 +388,18 @@
 			};
 		};
 
+		sdio: sdio@40012c00 {
+			compatible = "st,stm32f4xx-sdio";
+			reg = <0x40012c00 0x400>;
+			clocks = <&rcc 0 171>;
+			interrupts = <49>;
+			status = "disabled";
+			pinctrl-0 = <&sdio_pins>;
+			pinctrl-1 = <&sdio_pins_od>;
+			pinctrl-names = "default", "opendrain";
+			max-frequency = <48000000>;
+		};
+
 		rcc: rcc@40023810 {
 			#reset-cells = <1>;
 			#clock-cells = <2>;
-- 
2.7.4

[PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Andrea Merello]

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
 arch/arm/boot/dts/stm32f469-disco.dts | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index c12a64e..5aba383 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -206,7 +206,7 @@
 			reg = <0x40007000 0x400>;
 		};
 
-		pin-controller {
+		pinctrl:pin-controller {
 			#address-cells = <1>;
 			#size-cells = <1>;
 			compatible = "st,stm32f429-pinctrl";
diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
index 8877c00..7b3458e 100644
--- a/arch/arm/boot/dts/stm32f469-disco.dts
+++ b/arch/arm/boot/dts/stm32f469-disco.dts
@@ -65,6 +65,13 @@
 		serial0 = &usart3;
 	};
 
+	mmc_vcard: mmc_vcard {
+		compatible = "regulator-fixed";
+		regulator-name = "mmc_vcard";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
 	soc {
 		dma-ranges = <0xc0000000 0x0 0x10000000>;
 	};
@@ -78,6 +85,28 @@
 	clock-frequency = <8000000>;
 };
 
+&pinctrl {
+	sdio-cd {
+		sdio_cd: sdio-cd {
+			pins {
+				pinmux = <STM32F429_PG2_FUNC_GPIO>;
+				bias-pull-up;
+			};
+		};
+	};
+};
+
+&sdio {
+	status = "okay";
+	vmmc-supply = <&mmc_vcard>;
+	cd-gpios = <&gpiog 2 0>;
+	cd-inverted;
+	pinctrl-names = "default", "opendrain";
+	pinctrl-0 = <&sdio_pins>;
+	pinctrl-1 = <&sdio_pins_od>;
+	bus-width = <4>;
+};
+
 &usart3 {
 	status = "okay";
 };
-- 
2.7.4

[PATCH v2 9/9] Documentation: document mmci STM32 DT binding

[Andrea Merello]

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 Documentation/devicetree/bindings/mmc/mmci.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmci.txt b/Documentation/devicetree/bindings/mmc/mmci.txt
index 03796cf..d6b5127 100644
--- a/Documentation/devicetree/bindings/mmc/mmci.txt
+++ b/Documentation/devicetree/bindings/mmc/mmci.txt
@@ -8,7 +8,7 @@ by mmc.txt and the properties used by the mmci driver. Using "st" as
 the prefix for a property, indicates support by the ST Micro variant.
 
 Required properties:
-- compatible             : contains "arm,pl18x", "arm,primecell".
+- compatible             : contains "arm,pl18x", "arm,primecell" or "st,stm32f4xx-sdio"
 - vmmc-supply            : phandle to the regulator device tree node, mentioned
                            as the VCC/VDD supply in the eMMC/SD specs.
 
-- 
2.7.4

Re: [PATCH v2 6/9] DT: stm32f429: add pin map for SDIO controller

[Alexandre Torgue]

Hi Andrea

On 01/10/2017 09:42 AM, Andrea Merello wrote:
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>

Can you please add a commit message.
Can you also change commit header like: "ARM: dts: stm32: add pin map 
for SDIO controller on stm32f429


> ---
>  arch/arm/boot/dts/stm32f429.dtsi | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index e4dae0e..de7b9c3 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -336,7 +336,38 @@
>  				};
>  			};
>
> -			ethernet_mii: mii@0 {
> +			sdio_pins: sdio_pins@0 {
> +				pins {
> +					pinmux = <STM32F429_PC8_FUNC_SDIO_D0>,
> +						 <STM32F429_PC9_FUNC_SDIO_D1>,
> +						 <STM32F429_PC10_FUNC_SDIO_D2>,
> +						 <STM32F429_PC11_FUNC_SDIO_D3>,
> +						 <STM32F429_PC12_FUNC_SDIO_CK>,
> +						 <STM32F429_PD2_FUNC_SDIO_CMD>;
> +					drive-push-pull;
> +					slew-rate = <2>;
> +				};
> +			};
> +
> +			sdio_pins_od: sdio_pins_od@0 {
> +				pins1 {
> +					pinmux = <STM32F429_PC8_FUNC_SDIO_D0>,
> +						 <STM32F429_PC9_FUNC_SDIO_D1>,
> +						 <STM32F429_PC10_FUNC_SDIO_D2>,
> +						 <STM32F429_PC11_FUNC_SDIO_D3>,
> +						 <STM32F429_PC12_FUNC_SDIO_CK>;
> +					drive-push-pull;
> +					slew-rate = <2>;
> +				};
> +
> +				pins2 {
> +					pinmux = <STM32F429_PD2_FUNC_SDIO_CMD>;
> +					drive-open-drain;
> +					slew-rate = <2>;
> +				};
> +			};
> +
> +			ethernet0_mii: mii@0 {
>  				pins {
>  					pinmux = <STM32F429_PG13_FUNC_ETH_MII_TXD0_ETH_RMII_TXD0>,
>  						 <STM32F429_PG14_FUNC_ETH_MII_TXD1_ETH_RMII_TXD1>,
>

Re: [PATCH v2 7/9] DT: stm32f429: add node for SDIO controller

[Alexandre Torgue]

Hi Andrea

On 01/10/2017 09:42 AM, Andrea Merello wrote:
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>

Can you please add a commit message.
Can you also change commit header like: "ARM: dts: stm32: add node for 
SDIO controller on stm32f429

> ---
>  arch/arm/boot/dts/stm32f429.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index de7b9c3..c12a64e 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -388,6 +388,18 @@
>  			};
>  		};
>
> +		sdio: sdio@40012c00 {
> +			compatible = "st,stm32f4xx-sdio";
> +			reg = <0x40012c00 0x400>;
> +			clocks = <&rcc 0 171>;
> +			interrupts = <49>;
> +			status = "disabled";
> +			pinctrl-0 = <&sdio_pins>;
> +			pinctrl-1 = <&sdio_pins_od>;
> +			pinctrl-names = "default", "opendrain";
> +			max-frequency = <48000000>;
> +		};
> +
>  		rcc: rcc@40023810 {
>  			#reset-cells = <1>;
>  			#clock-cells = <2>;
>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Alexandre Torgue]

Hi Andrea

On 01/10/2017 09:42 AM, Andrea Merello wrote:
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>

Can you please add a commit message.
Can you also change commit header like: "ARM: dts: stm32: enable SDIO 
controller on stm32f469 disco board


> ---
>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>  arch/arm/boot/dts/stm32f469-disco.dts | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index c12a64e..5aba383 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -206,7 +206,7 @@
>  			reg = <0x40007000 0x400>;
>  		};
>
> -		pin-controller {
> +		pinctrl:pin-controller {
>  			#address-cells = <1>;
>  			#size-cells = <1>;
>  			compatible = "st,stm32f429-pinctrl";



> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8877c00..7b3458e 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -65,6 +65,13 @@
>  		serial0 = &usart3;
>  	};
>
> +	mmc_vcard: mmc_vcard {
> +		compatible = "regulator-fixed";
> +		regulator-name = "mmc_vcard";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
>  	soc {
>  		dma-ranges = <0xc0000000 0x0 0x10000000>;
>  	};
> @@ -78,6 +85,28 @@
>  	clock-frequency = <8000000>;
>  };
>
> +&pinctrl {
> +	sdio-cd {
> +		sdio_cd: sdio-cd {
> +			pins {
> +				pinmux = <STM32F429_PG2_FUNC_GPIO>;
> +				bias-pull-up;
> +			};
> +		};
> +	};
> +};
> +

As you only have config for 469-disco, please add this configuration of 
pinmux directly in stm32f429.dtsi.


> +&sdio {
> +	status = "okay";
> +	vmmc-supply = <&mmc_vcard>;
> +	cd-gpios = <&gpiog 2 0>;
> +	cd-inverted;
> +	pinctrl-names = "default", "opendrain";
> +	pinctrl-0 = <&sdio_pins>;
> +	pinctrl-1 = <&sdio_pins_od>;
> +	bus-width = <4>;
> +};
> +
>  &usart3 {
>  	status = "okay";
>  };
>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Andrea Merello]

On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> Hi Andrea
>
> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>
>
> Can you please add a commit message.
> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
> controller on stm32f469 disco board

Sure.

>
>
>> ---
>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>  arch/arm/boot/dts/stm32f469-disco.dts | 29 +++++++++++++++++++++++++++++
>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>> b/arch/arm/boot/dts/stm32f429.dtsi
>> index c12a64e..5aba383 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -206,7 +206,7 @@
>>                         reg = <0x40007000 0x400>;
>>                 };
>>
>> -               pin-controller {
>> +               pinctrl:pin-controller {
>>                         #address-cells = <1>;
>>                         #size-cells = <1>;
>>                         compatible = "st,stm32f429-pinctrl";
>
>
>
>
>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>> b/arch/arm/boot/dts/stm32f469-disco.dts
>> index 8877c00..7b3458e 100644
>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>> @@ -65,6 +65,13 @@
>>                 serial0 = &usart3;
>>         };
>>
>> +       mmc_vcard: mmc_vcard {
>> +               compatible = "regulator-fixed";
>> +               regulator-name = "mmc_vcard";
>> +               regulator-min-microvolt = <3300000>;
>> +               regulator-max-microvolt = <3300000>;
>> +       };
>> +
>>         soc {
>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>         };
>> @@ -78,6 +85,28 @@
>>         clock-frequency = <8000000>;
>>  };
>>
>> +&pinctrl {
>> +       sdio-cd {
>> +               sdio_cd: sdio-cd {
>> +                       pins {
>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>> +                               bias-pull-up;
>> +                       };
>> +               };
>> +       };
>> +};
>> +
>
>
> As you only have config for 469-disco, please add this configuration of
> pinmux directly in stm32f429.dtsi.

I'm not sure I got your point here..

I would say that stm32f429.dtsi should contain the HW description of
the MCU, and should be board-agnostic, while stm32f469-disco should
contain the HW description of this specific board.

Even if we haven't this config for other boards yet, I would say this
is still a very board-specific stuff, unrelated to the stm MCU itself.
Please correct me if I'm wrong: AFAIK the CD could be routed to any
GPIO pin, and it is not dependant by any STM32 specific AFIO.

>
>> +&sdio {
>> +       status = "okay";
>> +       vmmc-supply = <&mmc_vcard>;
>> +       cd-gpios = <&gpiog 2 0>;
>> +       cd-inverted;
>> +       pinctrl-names = "default", "opendrain";
>> +       pinctrl-0 = <&sdio_pins>;
>> +       pinctrl-1 = <&sdio_pins_od>;
>> +       bus-width = <4>;
>> +};
>> +
>>  &usart3 {
>>         status = "okay";
>>  };
>>
>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Alexandre Torgue]

Hi Andrea

On 01/12/2017 08:25 AM, Andrea Merello wrote:
> On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
>> Hi Andrea
>>
>> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>>
>>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>>
>>
>> Can you please add a commit message.
>> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
>> controller on stm32f469 disco board
>
> Sure.
>
>>
>>
>>> ---
>>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>>  arch/arm/boot/dts/stm32f469-disco.dts | 29 +++++++++++++++++++++++++++++
>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>> index c12a64e..5aba383 100644
>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>> @@ -206,7 +206,7 @@
>>>                         reg = <0x40007000 0x400>;
>>>                 };
>>>
>>> -               pin-controller {
>>> +               pinctrl:pin-controller {
>>>                         #address-cells = <1>;
>>>                         #size-cells = <1>;
>>>                         compatible = "st,stm32f429-pinctrl";
>>
>>
>>
>>
>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>> index 8877c00..7b3458e 100644
>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>> @@ -65,6 +65,13 @@
>>>                 serial0 = &usart3;
>>>         };
>>>
>>> +       mmc_vcard: mmc_vcard {
>>> +               compatible = "regulator-fixed";
>>> +               regulator-name = "mmc_vcard";
>>> +               regulator-min-microvolt = <3300000>;
>>> +               regulator-max-microvolt = <3300000>;
>>> +       };
>>> +
>>>         soc {
>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>         };
>>> @@ -78,6 +85,28 @@
>>>         clock-frequency = <8000000>;
>>>  };
>>>
>>> +&pinctrl {
>>> +       sdio-cd {
>>> +               sdio_cd: sdio-cd {
>>> +                       pins {
>>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>>> +                               bias-pull-up;
>>> +                       };
>>> +               };
>>> +       };
>>> +};
>>> +
>>
>>
>> As you only have config for 469-disco, please add this configuration of
>> pinmux directly in stm32f429.dtsi.
>
> I'm not sure I got your point here..
>
> I would say that stm32f429.dtsi should contain the HW description of
> the MCU, and should be board-agnostic, while stm32f469-disco should
> contain the HW description of this specific board.
>
> Even if we haven't this config for other boards yet, I would say this
> is still a very board-specific stuff, unrelated to the stm MCU itself.
> Please correct me if I'm wrong: AFAIK the CD could be routed to any
> GPIO pin, and it is not dependant by any STM32 specific AFIO.

For pinctrl:
To my point of view pinmuxing defintion is SOC, because those muxing 
possibilities are offered by the SOC.
The pinmuxing choice is board-specific.

For GPIO:
yes it stays in board file (it is not a definition but a gpio choice)

I hope to be enough clear :$

>
>>
>>> +&sdio {
>>> +       status = "okay";
>>> +       vmmc-supply = <&mmc_vcard>;
>>> +       cd-gpios = <&gpiog 2 0>;
>>> +       cd-inverted;
>>> +       pinctrl-names = "default", "opendrain";
>>> +       pinctrl-0 = <&sdio_pins>;
>>> +       pinctrl-1 = <&sdio_pins_od>;
>>> +       bus-width = <4>;
>>> +};
>>> +
>>>  &usart3 {
>>>         status = "okay";
>>>  };
>>>
>>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Andrea Merello]

On Thu, Jan 12, 2017 at 9:34 AM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> Hi Andrea
>
>
> On 01/12/2017 08:25 AM, Andrea Merello wrote:
>>
>> On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
>> <alexandre.torgue@st.com> wrote:
>>>
>>> Hi Andrea
>>>
>>> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>>>
>>>>
>>>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>>>
>>>
>>>
>>> Can you please add a commit message.
>>> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
>>> controller on stm32f469 disco board
>>
>>
>> Sure.
>>
>>>
>>>
>>>> ---
>>>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>>>  arch/arm/boot/dts/stm32f469-disco.dts | 29
>>>> +++++++++++++++++++++++++++++
>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>> index c12a64e..5aba383 100644
>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>> @@ -206,7 +206,7 @@
>>>>                         reg = <0x40007000 0x400>;
>>>>                 };
>>>>
>>>> -               pin-controller {
>>>> +               pinctrl:pin-controller {
>>>>                         #address-cells = <1>;
>>>>                         #size-cells = <1>;
>>>>                         compatible = "st,stm32f429-pinctrl";
>>>
>>>
>>>
>>>
>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 8877c00..7b3458e 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -65,6 +65,13 @@
>>>>                 serial0 = &usart3;
>>>>         };
>>>>
>>>> +       mmc_vcard: mmc_vcard {
>>>> +               compatible = "regulator-fixed";
>>>> +               regulator-name = "mmc_vcard";
>>>> +               regulator-min-microvolt = <3300000>;
>>>> +               regulator-max-microvolt = <3300000>;
>>>> +       };
>>>> +
>>>>         soc {
>>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>         };
>>>> @@ -78,6 +85,28 @@
>>>>         clock-frequency = <8000000>;
>>>>  };
>>>>
>>>> +&pinctrl {
>>>> +       sdio-cd {
>>>> +               sdio_cd: sdio-cd {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>>>> +                               bias-pull-up;
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +};
>>>> +
>>>
>>>
>>>
>>> As you only have config for 469-disco, please add this configuration of
>>> pinmux directly in stm32f429.dtsi.
>>
>>
>> I'm not sure I got your point here..
>>
>> I would say that stm32f429.dtsi should contain the HW description of
>> the MCU, and should be board-agnostic, while stm32f469-disco should
>> contain the HW description of this specific board.
>>
>> Even if we haven't this config for other boards yet, I would say this
>> is still a very board-specific stuff, unrelated to the stm MCU itself.
>> Please correct me if I'm wrong: AFAIK the CD could be routed to any
>> GPIO pin, and it is not dependant by any STM32 specific AFIO.
>
>
> For pinctrl:
> To my point of view pinmuxing defintion is SOC, because those muxing
> possibilities are offered by the SOC.
> The pinmuxing choice is board-specific.
>
> For GPIO:
> yes it stays in board file (it is not a definition but a gpio choice)
>
> I hope to be enough clear :$

Yes, now it makes more sense to me. Thanks.
Going to perform all changes you requested and post a V3. Hopefully today.

>
>>
>>>
>>>> +&sdio {
>>>> +       status = "okay";
>>>> +       vmmc-supply = <&mmc_vcard>;
>>>> +       cd-gpios = <&gpiog 2 0>;
>>>> +       cd-inverted;
>>>> +       pinctrl-names = "default", "opendrain";
>>>> +       pinctrl-0 = <&sdio_pins>;
>>>> +       pinctrl-1 = <&sdio_pins_od>;
>>>> +       bus-width = <4>;
>>>> +};
>>>> +
>>>>  &usart3 {
>>>>         status = "okay";
>>>>  };
>>>>
>>>
>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Andrea Merello]

On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> Hi Andrea
>
> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>
>
> Can you please add a commit message.
> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
> controller on stm32f469 disco board
>
>
>
>> ---
>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>  arch/arm/boot/dts/stm32f469-disco.dts | 29 +++++++++++++++++++++++++++++
>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>> b/arch/arm/boot/dts/stm32f429.dtsi
>> index c12a64e..5aba383 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -206,7 +206,7 @@
>>                         reg = <0x40007000 0x400>;
>>                 };
>>
>> -               pin-controller {
>> +               pinctrl:pin-controller {
>>                         #address-cells = <1>;
>>                         #size-cells = <1>;
>>                         compatible = "st,stm32f429-pinctrl";
>
>
>
>
>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>> b/arch/arm/boot/dts/stm32f469-disco.dts
>> index 8877c00..7b3458e 100644
>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>> @@ -65,6 +65,13 @@
>>                 serial0 = &usart3;
>>         };
>>
>> +       mmc_vcard: mmc_vcard {
>> +               compatible = "regulator-fixed";
>> +               regulator-name = "mmc_vcard";
>> +               regulator-min-microvolt = <3300000>;
>> +               regulator-max-microvolt = <3300000>;
>> +       };
>> +
>>         soc {
>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>         };
>> @@ -78,6 +85,28 @@
>>         clock-frequency = <8000000>;
>>  };
>>
>> +&pinctrl {
>> +       sdio-cd {
>> +               sdio_cd: sdio-cd {
>> +                       pins {
>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>> +                               bias-pull-up;
>> +                       };
>> +               };
>> +       };
>> +};
>> +
>
>
> As you only have config for 469-disco, please add this configuration of
> pinmux directly in stm32f429.dtsi.

While in the process of moving this, I realized that there is
something wrong here..

Currently this pinmux config ended up to be not referenced by the SDIO
DT node at all. This happened in V2; it was intended to fix an
apparently double-claimed GPIO problem, but now I realized that I
misunderstood this thing, and it was a bad fix..

So, actually the SD works, but as soon as I reference the CD pinmux
config in the SDIO node, it fails with the following error:

[    0.980000] stm32f429-pinctrl soc:pin-controller: pin PG2 already
requested by 40012c00.sdio; cannot claim for GPIOG:98
[    0.990000] stm32f429-pinctrl soc:pin-controller: pin-98 (GPIOG:98)
status -22
[    1.000000] mmci-pl18x: probe of 40012c00.sdio failed with error -22

I've bisected this to happen since the 'strict' flag has been added to
the stm32 pinctrl driver.
commit c32c22eea0c47e13cffd6b5f7eedd7a6b6f2c18f

Right now It seems that few pinctrl drivers use the 'strict' flag, so
maybe it could be a bug in pinctrl-realated code.

I need to look more in detail about this, however as far as I could understand:

- when the mmc layer asks for the CD pin, due to the "cd-gpios"
property,  the owner name for this config request is not the one from
the OF node, but it is "conjured" - as written in comment - from the
GPIO number and name (GPIOG:98) in pinmux_request_gpio()

- the pinmux config done in behalf of the 'pinctrl-0' property has the
OF node name (40012c00.sdio) as owner.

This owner mismatch causes a check to fail in pin_request(), that
believes that two different owners try to use the same pin (while in
true both are in behalf of the same SDIO device).

I don't know when I'll have more time to look into this. If you want I
can temporary drop the CD pinmux config and post the V3 - that
nevertheless works - and we can add the CD pinmux cfg later, once it
has been fixed.

In someone have clues, or want to look into this, he/she would be
really welcome :)

>
>
>> +&sdio {
>> +       status = "okay";
>> +       vmmc-supply = <&mmc_vcard>;
>> +       cd-gpios = <&gpiog 2 0>;
>> +       cd-inverted;
>> +       pinctrl-names = "default", "opendrain";
>> +       pinctrl-0 = <&sdio_pins>;
>> +       pinctrl-1 = <&sdio_pins_od>;
>> +       bus-width = <4>;
>> +};
>> +
>>  &usart3 {
>>         status = "okay";
>>  };
>>
>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Alexandre Torgue]

Hi Andrea

On 01/13/2017 04:01 PM, Andrea Merello wrote:
> On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
>> Hi Andrea
>>
>> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>>
>>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>>
>>
>> Can you please add a commit message.
>> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
>> controller on stm32f469 disco board
>>
>>
>>
>>> ---
>>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>>  arch/arm/boot/dts/stm32f469-disco.dts | 29 +++++++++++++++++++++++++++++
>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>> index c12a64e..5aba383 100644
>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>> @@ -206,7 +206,7 @@
>>>                         reg = <0x40007000 0x400>;
>>>                 };
>>>
>>> -               pin-controller {
>>> +               pinctrl:pin-controller {
>>>                         #address-cells = <1>;
>>>                         #size-cells = <1>;
>>>                         compatible = "st,stm32f429-pinctrl";
>>
>>
>>
>>
>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>> index 8877c00..7b3458e 100644
>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>> @@ -65,6 +65,13 @@
>>>                 serial0 = &usart3;
>>>         };
>>>
>>> +       mmc_vcard: mmc_vcard {
>>> +               compatible = "regulator-fixed";
>>> +               regulator-name = "mmc_vcard";
>>> +               regulator-min-microvolt = <3300000>;
>>> +               regulator-max-microvolt = <3300000>;
>>> +       };
>>> +
>>>         soc {
>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>         };
>>> @@ -78,6 +85,28 @@
>>>         clock-frequency = <8000000>;
>>>  };
>>>
>>> +&pinctrl {
>>> +       sdio-cd {
>>> +               sdio_cd: sdio-cd {
>>> +                       pins {
>>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>>> +                               bias-pull-up;
>>> +                       };
>>> +               };
>>> +       };
>>> +};
>>> +
>>
>>
>> As you only have config for 469-disco, please add this configuration of
>> pinmux directly in stm32f429.dtsi.
>
> While in the process of moving this, I realized that there is
> something wrong here..
>
> Currently this pinmux config ended up to be not referenced by the SDIO
> DT node at all. This happened in V2; it was intended to fix an
> apparently double-claimed GPIO problem, but now I realized that I
> misunderstood this thing, and it was a bad fix..
>
> So, actually the SD works, but as soon as I reference the CD pinmux
> config in the SDIO node, it fails with the following error:
>
> [    0.980000] stm32f429-pinctrl soc:pin-controller: pin PG2 already
> requested by 40012c00.sdio; cannot claim for GPIOG:98
> [    0.990000] stm32f429-pinctrl soc:pin-controller: pin-98 (GPIOG:98)
> status -22
> [    1.000000] mmci-pl18x: probe of 40012c00.sdio failed with error -22
>
> I've bisected this to happen since the 'strict' flag has been added to
> the stm32 pinctrl driver.
> commit c32c22eea0c47e13cffd6b5f7eedd7a6b6f2c18f
>
> Right now It seems that few pinctrl drivers use the 'strict' flag, so
> maybe it could be a bug in pinctrl-realated code.

It is not a bug but added intentionally.

>
> I need to look more in detail about this, however as far as I could understand:
>
> - when the mmc layer asks for the CD pin, due to the "cd-gpios"
> property,  the owner name for this config request is not the one from
> the OF node, but it is "conjured" - as written in comment - from the
> GPIO number and name (GPIOG:98) in pinmux_request_gpio()
>
> - the pinmux config done in behalf of the 'pinctrl-0' property has the
> OF node name (40012c00.sdio) as owner.
>

I will look at deeper in the driver but first why you need to control 
this pin by both way (gpio lib through gpio definition and pinctrl 
framework through pinctrl definition) ?

In strict mode the error you get is "normal". You can't control the same 
pin in the same time by gpiolib and pinctrl.


> This owner mismatch causes a check to fail in pin_request(), that
> believes that two different owners try to use the same pin (while in
> true both are in behalf of the same SDIO device).
>
> I don't know when I'll have more time to look into this. If you want I
> can temporary drop the CD pinmux config and post the V3 - that
> nevertheless works - and we can add the CD pinmux cfg later, once it
> has been fixed.
>
> In someone have clues, or want to look into this, he/she would be
> really welcome :)

No problem, we can wait. Let's merge it when it will be fully functional.

Br
Alex

>
>>
>>
>>> +&sdio {
>>> +       status = "okay";
>>> +       vmmc-supply = <&mmc_vcard>;
>>> +       cd-gpios = <&gpiog 2 0>;
>>> +       cd-inverted;
>>> +       pinctrl-names = "default", "opendrain";
>>> +       pinctrl-0 = <&sdio_pins>;
>>> +       pinctrl-1 = <&sdio_pins_od>;
>>> +       bus-width = <4>;
>>> +};
>>> +
>>>  &usart3 {
>>>         status = "okay";
>>>  };
>>>
>>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Andrea Merello]

On Fri, Jan 13, 2017 at 4:52 PM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
> Hi Andrea
>
>
> On 01/13/2017 04:01 PM, Andrea Merello wrote:
>>
>> On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
>> <alexandre.torgue@st.com> wrote:
>>>
>>> Hi Andrea
>>>
>>> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>>>
>>>>
>>>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>>>
>>>
>>>
>>> Can you please add a commit message.
>>> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
>>> controller on stm32f469 disco board
>>>
>>>
>>>
>>>> ---
>>>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>>>  arch/arm/boot/dts/stm32f469-disco.dts | 29
>>>> +++++++++++++++++++++++++++++
>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>> index c12a64e..5aba383 100644
>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>> @@ -206,7 +206,7 @@
>>>>                         reg = <0x40007000 0x400>;
>>>>                 };
>>>>
>>>> -               pin-controller {
>>>> +               pinctrl:pin-controller {
>>>>                         #address-cells = <1>;
>>>>                         #size-cells = <1>;
>>>>                         compatible = "st,stm32f429-pinctrl";
>>>
>>>
>>>
>>>
>>>
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 8877c00..7b3458e 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -65,6 +65,13 @@
>>>>                 serial0 = &usart3;
>>>>         };
>>>>
>>>> +       mmc_vcard: mmc_vcard {
>>>> +               compatible = "regulator-fixed";
>>>> +               regulator-name = "mmc_vcard";
>>>> +               regulator-min-microvolt = <3300000>;
>>>> +               regulator-max-microvolt = <3300000>;
>>>> +       };
>>>> +
>>>>         soc {
>>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>         };
>>>> @@ -78,6 +85,28 @@
>>>>         clock-frequency = <8000000>;
>>>>  };
>>>>
>>>> +&pinctrl {
>>>> +       sdio-cd {
>>>> +               sdio_cd: sdio-cd {
>>>> +                       pins {
>>>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>>>> +                               bias-pull-up;
>>>> +                       };
>>>> +               };
>>>> +       };
>>>> +};
>>>> +
>>>
>>>
>>>
>>> As you only have config for 469-disco, please add this configuration of
>>> pinmux directly in stm32f429.dtsi.
>>
>>
>> While in the process of moving this, I realized that there is
>> something wrong here..
>>
>> Currently this pinmux config ended up to be not referenced by the SDIO
>> DT node at all. This happened in V2; it was intended to fix an
>> apparently double-claimed GPIO problem, but now I realized that I
>> misunderstood this thing, and it was a bad fix..
>>
>> So, actually the SD works, but as soon as I reference the CD pinmux
>> config in the SDIO node, it fails with the following error:
>>
>> [    0.980000] stm32f429-pinctrl soc:pin-controller: pin PG2 already
>> requested by 40012c00.sdio; cannot claim for GPIOG:98
>> [    0.990000] stm32f429-pinctrl soc:pin-controller: pin-98 (GPIOG:98)
>> status -22
>> [    1.000000] mmci-pl18x: probe of 40012c00.sdio failed with error -22
>>
>> I've bisected this to happen since the 'strict' flag has been added to
>> the stm32 pinctrl driver.
>> commit c32c22eea0c47e13cffd6b5f7eedd7a6b6f2c18f
>>
>> Right now It seems that few pinctrl drivers use the 'strict' flag, so
>> maybe it could be a bug in pinctrl-realated code.
>
>
> It is not a bug but added intentionally.
>
>>
>> I need to look more in detail about this, however as far as I could
>> understand:
>>
>> - when the mmc layer asks for the CD pin, due to the "cd-gpios"
>> property,  the owner name for this config request is not the one from
>> the OF node, but it is "conjured" - as written in comment - from the
>> GPIO number and name (GPIOG:98) in pinmux_request_gpio()
>>
>> - the pinmux config done in behalf of the 'pinctrl-0' property has the
>> OF node name (40012c00.sdio) as owner.
>>
>
> I will look at deeper in the driver but first why you need to control this
> pin by both way (gpio lib through gpio definition and pinctrl framework
> through pinctrl definition) ?

Hmm, sorry, maybe I'm missing something here.. Please correct me if I'm wrong :)

Don't I need to reference the pinmux configuration for CD in my
pinctrl configs in the SDIO DT node, in order to have it active? e.g.
pinctrl-0 = <&sdio_pins>, <&sdio_cd>;

Don't I need to provide "cs-gpios" to the MMC layer as well ? e.g.
cd-gpios = <&gpiog 2 0>;

If I do both, it fails (due to mmc_of_parse() failing). No matter what
I do in the driver, I believe.

> In strict mode the error you get is "normal". You can't control the same pin
> in the same time by gpiolib and pinctrl.
>
>
>> This owner mismatch causes a check to fail in pin_request(), that
>> believes that two different owners try to use the same pin (while in
>> true both are in behalf of the same SDIO device).
>>
>> I don't know when I'll have more time to look into this. If you want I
>> can temporary drop the CD pinmux config and post the V3 - that
>> nevertheless works - and we can add the CD pinmux cfg later, once it
>> has been fixed.
>>
>> In someone have clues, or want to look into this, he/she would be
>> really welcome :)
>
>
> No problem, we can wait. Let's merge it when it will be fully functional.
>
> Br
> Alex
>
>
>>
>>>
>>>
>>>> +&sdio {
>>>> +       status = "okay";
>>>> +       vmmc-supply = <&mmc_vcard>;
>>>> +       cd-gpios = <&gpiog 2 0>;
>>>> +       cd-inverted;
>>>> +       pinctrl-names = "default", "opendrain";
>>>> +       pinctrl-0 = <&sdio_pins>;
>>>> +       pinctrl-1 = <&sdio_pins_od>;
>>>> +       bus-width = <4>;
>>>> +};
>>>> +
>>>>  &usart3 {
>>>>         status = "okay";
>>>>  };
>>>>
>>>
>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Alexandre Torgue]

On 01/13/2017 05:12 PM, Andrea Merello wrote:
> On Fri, Jan 13, 2017 at 4:52 PM, Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
>> Hi Andrea
>>
>>
>> On 01/13/2017 04:01 PM, Andrea Merello wrote:
>>>
>>> On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
>>> <alexandre.torgue@st.com> wrote:
>>>>
>>>> Hi Andrea
>>>>
>>>> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>>>>
>>>>>
>>>>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>>>>
>>>>
>>>>
>>>> Can you please add a commit message.
>>>> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
>>>> controller on stm32f469 disco board
>>>>
>>>>
>>>>
>>>>> ---
>>>>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>>>>  arch/arm/boot/dts/stm32f469-disco.dts | 29
>>>>> +++++++++++++++++++++++++++++
>>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>>> index c12a64e..5aba383 100644
>>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>>> @@ -206,7 +206,7 @@
>>>>>                         reg = <0x40007000 0x400>;
>>>>>                 };
>>>>>
>>>>> -               pin-controller {
>>>>> +               pinctrl:pin-controller {
>>>>>                         #address-cells = <1>;
>>>>>                         #size-cells = <1>;
>>>>>                         compatible = "st,stm32f429-pinctrl";
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> index 8877c00..7b3458e 100644
>>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>> @@ -65,6 +65,13 @@
>>>>>                 serial0 = &usart3;
>>>>>         };
>>>>>
>>>>> +       mmc_vcard: mmc_vcard {
>>>>> +               compatible = "regulator-fixed";
>>>>> +               regulator-name = "mmc_vcard";
>>>>> +               regulator-min-microvolt = <3300000>;
>>>>> +               regulator-max-microvolt = <3300000>;
>>>>> +       };
>>>>> +
>>>>>         soc {
>>>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>>         };
>>>>> @@ -78,6 +85,28 @@
>>>>>         clock-frequency = <8000000>;
>>>>>  };
>>>>>
>>>>> +&pinctrl {
>>>>> +       sdio-cd {
>>>>> +               sdio_cd: sdio-cd {
>>>>> +                       pins {
>>>>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>>>>> +                               bias-pull-up;
>>>>> +                       };
>>>>> +               };
>>>>> +       };
>>>>> +};
>>>>> +
>>>>
>>>>
>>>>
>>>> As you only have config for 469-disco, please add this configuration of
>>>> pinmux directly in stm32f429.dtsi.
>>>
>>>
>>> While in the process of moving this, I realized that there is
>>> something wrong here..
>>>
>>> Currently this pinmux config ended up to be not referenced by the SDIO
>>> DT node at all. This happened in V2; it was intended to fix an
>>> apparently double-claimed GPIO problem, but now I realized that I
>>> misunderstood this thing, and it was a bad fix..
>>>
>>> So, actually the SD works, but as soon as I reference the CD pinmux
>>> config in the SDIO node, it fails with the following error:
>>>
>>> [    0.980000] stm32f429-pinctrl soc:pin-controller: pin PG2 already
>>> requested by 40012c00.sdio; cannot claim for GPIOG:98
>>> [    0.990000] stm32f429-pinctrl soc:pin-controller: pin-98 (GPIOG:98)
>>> status -22
>>> [    1.000000] mmci-pl18x: probe of 40012c00.sdio failed with error -22
>>>
>>> I've bisected this to happen since the 'strict' flag has been added to
>>> the stm32 pinctrl driver.
>>> commit c32c22eea0c47e13cffd6b5f7eedd7a6b6f2c18f
>>>
>>> Right now It seems that few pinctrl drivers use the 'strict' flag, so
>>> maybe it could be a bug in pinctrl-realated code.
>>
>>
>> It is not a bug but added intentionally.
>>
>>>
>>> I need to look more in detail about this, however as far as I could
>>> understand:
>>>
>>> - when the mmc layer asks for the CD pin, due to the "cd-gpios"
>>> property,  the owner name for this config request is not the one from
>>> the OF node, but it is "conjured" - as written in comment - from the
>>> GPIO number and name (GPIOG:98) in pinmux_request_gpio()
>>>
>>> - the pinmux config done in behalf of the 'pinctrl-0' property has the
>>> OF node name (40012c00.sdio) as owner.
>>>
>>
>> I will look at deeper in the driver but first why you need to control this
>> pin by both way (gpio lib through gpio definition and pinctrl framework
>> through pinctrl definition) ?
>
> Hmm, sorry, maybe I'm missing something here.. Please correct me if I'm wrong :)
>
> Don't I need to reference the pinmux configuration for CD in my
> pinctrl configs in the SDIO DT node, in order to have it active? e.g.
> pinctrl-0 = <&sdio_pins>, <&sdio_cd>;
>
> Don't I need to provide "cs-gpios" to the MMC layer as well ? e.g.
> cd-gpios = <&gpiog 2 0>;

Maybe I miss also something but it depends on "how" the driver uses this 
pin. If you want to control the GPIO (change direction or set gpio in 
input or read input ...) you have to use gpiolib and so declare 
"cd-gpios = <&gpiog 2 0>;". By doing that gpiolib will request the gpio 
to pinctrl for you. No need to declare a pinctrl entry for that.

>
> If I do both, it fails (due to mmc_of_parse() failing). No matter what
> I do in the driver, I believe.

It is normal that both failed; In strict mode you can't request several 
times the same pin (you request pinG2 one time by gpiolib and oneother 
time by pinctrl).

>
>> In strict mode the error you get is "normal". You can't control the same pin
>> in the same time by gpiolib and pinctrl.
>>
>>
>>> This owner mismatch causes a check to fail in pin_request(), that
>>> believes that two different owners try to use the same pin (while in
>>> true both are in behalf of the same SDIO device).
>>>
>>> I don't know when I'll have more time to look into this. If you want I
>>> can temporary drop the CD pinmux config and post the V3 - that
>>> nevertheless works - and we can add the CD pinmux cfg later, once it
>>> has been fixed.
>>>
>>> In someone have clues, or want to look into this, he/she would be
>>> really welcome :)
>>
>>
>> No problem, we can wait. Let's merge it when it will be fully functional.
>>
>> Br
>> Alex
>>
>>
>>>
>>>>
>>>>
>>>>> +&sdio {
>>>>> +       status = "okay";
>>>>> +       vmmc-supply = <&mmc_vcard>;
>>>>> +       cd-gpios = <&gpiog 2 0>;
>>>>> +       cd-inverted;
>>>>> +       pinctrl-names = "default", "opendrain";
>>>>> +       pinctrl-0 = <&sdio_pins>;
>>>>> +       pinctrl-1 = <&sdio_pins_od>;
>>>>> +       bus-width = <4>;
>>>>> +};
>>>>> +
>>>>>  &usart3 {
>>>>>         status = "okay";
>>>>>  };
>>>>>
>>>>
>>

Re: [PATCH v2 8/9] DT: stm32f469-disco: add node for SDIO controller

[Andrea Merello]

On Fri, Jan 13, 2017 at 5:28 PM, Alexandre Torgue
<alexandre.torgue@st.com> wrote:
>
>
> On 01/13/2017 05:12 PM, Andrea Merello wrote:
>>
>> On Fri, Jan 13, 2017 at 4:52 PM, Alexandre Torgue
>> <alexandre.torgue@st.com> wrote:
>>>
>>> Hi Andrea
>>>
>>>
>>> On 01/13/2017 04:01 PM, Andrea Merello wrote:
>>>>
>>>>
>>>> On Tue, Jan 10, 2017 at 10:18 AM, Alexandre Torgue
>>>> <alexandre.torgue@st.com> wrote:
>>>>>
>>>>>
>>>>> Hi Andrea
>>>>>
>>>>> On 01/10/2017 09:42 AM, Andrea Merello wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Can you please add a commit message.
>>>>> Can you also change commit header like: "ARM: dts: stm32: enable SDIO
>>>>> controller on stm32f469 disco board
>>>>>
>>>>>
>>>>>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/stm32f429.dtsi      |  2 +-
>>>>>>  arch/arm/boot/dts/stm32f469-disco.dts | 29
>>>>>> +++++++++++++++++++++++++++++
>>>>>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>>>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>>>>> index c12a64e..5aba383 100644
>>>>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>>>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>>>>> @@ -206,7 +206,7 @@
>>>>>>                         reg = <0x40007000 0x400>;
>>>>>>                 };
>>>>>>
>>>>>> -               pin-controller {
>>>>>> +               pinctrl:pin-controller {
>>>>>>                         #address-cells = <1>;
>>>>>>                         #size-cells = <1>;
>>>>>>                         compatible = "st,stm32f429-pinctrl";
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>> index 8877c00..7b3458e 100644
>>>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>>>> @@ -65,6 +65,13 @@
>>>>>>                 serial0 = &usart3;
>>>>>>         };
>>>>>>
>>>>>> +       mmc_vcard: mmc_vcard {
>>>>>> +               compatible = "regulator-fixed";
>>>>>> +               regulator-name = "mmc_vcard";
>>>>>> +               regulator-min-microvolt = <3300000>;
>>>>>> +               regulator-max-microvolt = <3300000>;
>>>>>> +       };
>>>>>> +
>>>>>>         soc {
>>>>>>                 dma-ranges = <0xc0000000 0x0 0x10000000>;
>>>>>>         };
>>>>>> @@ -78,6 +85,28 @@
>>>>>>         clock-frequency = <8000000>;
>>>>>>  };
>>>>>>
>>>>>> +&pinctrl {
>>>>>> +       sdio-cd {
>>>>>> +               sdio_cd: sdio-cd {
>>>>>> +                       pins {
>>>>>> +                               pinmux = <STM32F429_PG2_FUNC_GPIO>;
>>>>>> +                               bias-pull-up;
>>>>>> +                       };
>>>>>> +               };
>>>>>> +       };
>>>>>> +};
>>>>>> +
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> As you only have config for 469-disco, please add this configuration of
>>>>> pinmux directly in stm32f429.dtsi.
>>>>
>>>>
>>>>
>>>> While in the process of moving this, I realized that there is
>>>> something wrong here..
>>>>
>>>> Currently this pinmux config ended up to be not referenced by the SDIO
>>>> DT node at all. This happened in V2; it was intended to fix an
>>>> apparently double-claimed GPIO problem, but now I realized that I
>>>> misunderstood this thing, and it was a bad fix..
>>>>
>>>> So, actually the SD works, but as soon as I reference the CD pinmux
>>>> config in the SDIO node, it fails with the following error:
>>>>
>>>> [    0.980000] stm32f429-pinctrl soc:pin-controller: pin PG2 already
>>>> requested by 40012c00.sdio; cannot claim for GPIOG:98
>>>> [    0.990000] stm32f429-pinctrl soc:pin-controller: pin-98 (GPIOG:98)
>>>> status -22
>>>> [    1.000000] mmci-pl18x: probe of 40012c00.sdio failed with error -22
>>>>
>>>> I've bisected this to happen since the 'strict' flag has been added to
>>>> the stm32 pinctrl driver.
>>>> commit c32c22eea0c47e13cffd6b5f7eedd7a6b6f2c18f
>>>>
>>>> Right now It seems that few pinctrl drivers use the 'strict' flag, so
>>>> maybe it could be a bug in pinctrl-realated code.
>>>
>>>
>>>
>>> It is not a bug but added intentionally.
>>>
>>>>
>>>> I need to look more in detail about this, however as far as I could
>>>> understand:
>>>>
>>>> - when the mmc layer asks for the CD pin, due to the "cd-gpios"
>>>> property,  the owner name for this config request is not the one from
>>>> the OF node, but it is "conjured" - as written in comment - from the
>>>> GPIO number and name (GPIOG:98) in pinmux_request_gpio()
>>>>
>>>> - the pinmux config done in behalf of the 'pinctrl-0' property has the
>>>> OF node name (40012c00.sdio) as owner.
>>>>
>>>
>>> I will look at deeper in the driver but first why you need to control
>>> this
>>> pin by both way (gpio lib through gpio definition and pinctrl framework
>>> through pinctrl definition) ?
>>
>>
>> Hmm, sorry, maybe I'm missing something here.. Please correct me if I'm
>> wrong :)
>>
>> Don't I need to reference the pinmux configuration for CD in my
>> pinctrl configs in the SDIO DT node, in order to have it active? e.g.
>> pinctrl-0 = <&sdio_pins>, <&sdio_cd>;
>>
>> Don't I need to provide "cs-gpios" to the MMC layer as well ? e.g.
>> cd-gpios = <&gpiog 2 0>;
>
>
> Maybe I miss also something but it depends on "how" the driver uses this
> pin. If you want to control the GPIO (change direction or set gpio in input
> or read input ...) you have to use gpiolib and so declare "cd-gpios =
> <&gpiog 2 0>;". By doing that gpiolib will request the gpio to pinctrl for
> you. No need to declare a pinctrl entry for that.

Ah, ok. Thanks for clarifying :) It's my fault then: I've assumed that
for some reason both were needed, just because the DTS files I've
looked at seem to have both. (BTW apparently there are a lot of them
in arch/arm/dts.. are they all broken and working only because they
have not a "strict" pinctrl driver?)..

Anyway, it should be enough to drop completely the pinmux config for
CD. I'll post a new series next week.

>>
>> If I do both, it fails (due to mmc_of_parse() failing). No matter what
>> I do in the driver, I believe.
>
>
> It is normal that both failed; In strict mode you can't request several
> times the same pin (you request pinG2 one time by gpiolib and oneother time
> by pinctrl).
>
>
>>
>>> In strict mode the error you get is "normal". You can't control the same
>>> pin
>>> in the same time by gpiolib and pinctrl.
>>>
>>>
>>>> This owner mismatch causes a check to fail in pin_request(), that
>>>> believes that two different owners try to use the same pin (while in
>>>> true both are in behalf of the same SDIO device).
>>>>
>>>> I don't know when I'll have more time to look into this. If you want I
>>>> can temporary drop the CD pinmux config and post the V3 - that
>>>> nevertheless works - and we can add the CD pinmux cfg later, once it
>>>> has been fixed.
>>>>
>>>> In someone have clues, or want to look into this, he/she would be
>>>> really welcome :)
>>>
>>>
>>>
>>> No problem, we can wait. Let's merge it when it will be fully functional.
>>>
>>> Br
>>> Alex
>>>
>>>
>>>>
>>>>>
>>>>>
>>>>>> +&sdio {
>>>>>> +       status = "okay";
>>>>>> +       vmmc-supply = <&mmc_vcard>;
>>>>>> +       cd-gpios = <&gpiog 2 0>;
>>>>>> +       cd-inverted;
>>>>>> +       pinctrl-names = "default", "opendrain";
>>>>>> +       pinctrl-0 = <&sdio_pins>;
>>>>>> +       pinctrl-1 = <&sdio_pins_od>;
>>>>>> +       bus-width = <4>;
>>>>>> +};
>>>>>> +
>>>>>>  &usart3 {
>>>>>>         status = "okay";
>>>>>>  };
>>>>>>
>>>>>
>>>
>