[PATCH v3 0/5] Armada 38x SDHCI driver improvements

[PATCH v3 0/5] Armada 38x SDHCI driver improvements

[Marcin Wojtas]

Hi,

Thank you for reviewing the patches. According to your remarks and some
new ideas I prepared third patchset. I modified my HW and now I could
check operation when using all three modes of detection (polling, gpio
and dat3) - it all seems working fine. Any remarks will be wellcome.

Best regards,
Marcin

Changes:
v1 -> v2
* enable SW polling as card detection
* in resume function change condition for mbus windows reconfiguration

v2 -> v3
* remove redundant print after fail of mbus_win_regs obtaining
* add big comment on possible card detection options in armada-388-gp.dts
* reconstruct dat3-cd support
	- use dedicated flag in sdhci_pxa structure instead of checking
	  property in DT
	- instead of global SDHCI quirk add wrapper for sdhci_set_clock
	  function in order to keep internal clock enabled
	- prevent using pm_runtime in case of using dat3-cd
* improve MMC_CARD bit modification - do not check Armada 38x compatible
  string in pxav3_init_card callback, but use pxa->mbus_win_regs as a flag

Marcin Wojtas (5):
  mmc: sdhci-pxav3: enable proper resuming on Armada 38x SoC
  mmc: sdhci-pxav3: enable usage of DAT3 pin as HW card detect
  ARM: mvebu: set SW polling as SDHCI card detection on A388-GP
  mmc: sdhci: add init_card callback to sdhci
  mmc: sdhci-pxav3: enable modifying MMC_CARD bit during card
    initialization

 .../devicetree/bindings/mmc/sdhci-pxa.txt          |   5 +
 arch/arm/boot/dts/armada-388-gp.dts                |  15 +-
 drivers/mmc/host/sdhci-pxav3.c                     | 157 ++++++++++++++++-----
 drivers/mmc/host/sdhci.c                           |   9 ++
 drivers/mmc/host/sdhci.h                           |   1 +
 5 files changed, 149 insertions(+), 38 deletions(-)

-- 
1.8.3.1

[PATCH v3 1/5] mmc: sdhci-pxav3: enable proper resuming on Armada 38x SoC

[Marcin Wojtas]

When resuming from suspend on Armada 38x SoC MBus windows have to be
re-configured and for that purpose mv_conf_mbus_windows function needed
rework. MBus windows register base address obtaining was moved to
armada_38x_quirks function in order to be kept in pxa global structure,
because it is used during a resume.

This commit fixes resuming from suspend by calling MBus windows
configuration routine and therefore enabling proper DMA operation.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/mmc/host/sdhci-pxav3.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index f5edf9d..54a253c0 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -63,6 +63,7 @@ struct sdhci_pxa {
 	struct clk *clk_io;
 	u8	power_mode;
 	void __iomem *sdio3_conf_reg;
+	void __iomem *mbus_win_regs;
 };
 
 /*
@@ -81,30 +82,16 @@ struct sdhci_pxa {
 #define SDIO3_CONF_CLK_INV	BIT(0)
 #define SDIO3_CONF_SD_FB_CLK	BIT(2)
 
-static int mv_conf_mbus_windows(struct platform_device *pdev,
+static int mv_conf_mbus_windows(struct device *dev, void __iomem *regs,
 				const struct mbus_dram_target_info *dram)
 {
 	int i;
-	void __iomem *regs;
-	struct resource *res;
 
 	if (!dram) {
-		dev_err(&pdev->dev, "no mbus dram info\n");
-		return -EINVAL;
-	}
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!res) {
-		dev_err(&pdev->dev, "cannot get mbus registers\n");
+		dev_err(dev, "no mbus dram info\n");
 		return -EINVAL;
 	}
 
-	regs = ioremap(res->start, resource_size(res));
-	if (!regs) {
-		dev_err(&pdev->dev, "cannot map mbus registers\n");
-		return -ENOMEM;
-	}
-
 	for (i = 0; i < SDHCI_MAX_WIN_NUM; i++) {
 		writel(0, regs + SDHCI_WINDOW_CTRL(i));
 		writel(0, regs + SDHCI_WINDOW_BASE(i));
@@ -122,8 +109,6 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
 		writel(cs->base, regs + SDHCI_WINDOW_BASE(i));
 	}
 
-	iounmap(regs);
-
 	return 0;
 }
 
@@ -135,6 +120,11 @@ static int armada_38x_quirks(struct platform_device *pdev,
 	struct sdhci_pxa *pxa = pltfm_host->priv;
 	struct resource *res;
 
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mbus");
+	pxa->mbus_win_regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pxa->mbus_win_regs))
+		return PTR_ERR(pxa->mbus_win_regs);
+
 	host->quirks &= ~SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
 	host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
@@ -403,7 +393,8 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 		ret = armada_38x_quirks(pdev, host);
 		if (ret < 0)
 			goto err_mbus_win;
-		ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
+		ret = mv_conf_mbus_windows(&pdev->dev, pxa->mbus_win_regs,
+					   mv_mbus_dram_info());
 		if (ret < 0)
 			goto err_mbus_win;
 	}
@@ -520,6 +511,12 @@ static int sdhci_pxav3_resume(struct device *dev)
 {
 	int ret;
 	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+
+	if (pxa->mbus_win_regs)
+		ret = mv_conf_mbus_windows(dev, pxa->mbus_win_regs,
+					   mv_mbus_dram_info());
 
 	pm_runtime_get_sync(dev);
 	ret = sdhci_resume_host(host);
-- 
1.8.3.1

[PATCH v3 2/5] mmc: sdhci-pxav3: enable usage of DAT3 pin as HW card detect

[Marcin Wojtas]

Marvell Armada 38x SDHCI controller enable using DAT3 pin as a hardware
card detection. According to the SD sdandard this signal can be used for
this purpose combined with a pull-down resistor, implying inverted (active
high) polarization of a card detect. MMC standard does not support this
feature and does not operate with such connectivity of DAT3.

When using DAT3-based detection Armada 38x SDIO IP expects its internal
clock to be always on, which had to be ensured by:
- Udating appropriate registers, each time controller is reset. On the
  occasion of adding new register @0x104, register @0x100 name is modified
  in order to the be aligned with Armada 38x documentation.
- Leaving the clock enabled despite power-down. For this purpose a wrapper
  for sdhci_set_clock function is added.
- Disabling pm_runtime - during suspend both io_clock and controller's
  bus power is switched off, hence it would prevent proper card detection.

In addition to the changes above this commit adds a new property to Armada
38x SDHCI node ('dat3-cd') with an according binding documentation update.
'sdhci_pxa' structure is also extended with dedicated flag for checking if
DAT3-based detection is in use.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 .../devicetree/bindings/mmc/sdhci-pxa.txt          |   5 ++
 drivers/mmc/host/sdhci-pxav3.c                     | 100 +++++++++++++++++----
 2 files changed, 87 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt b/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
index 3d1b449..ffd6b14 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
@@ -23,6 +23,11 @@ Required properties:
 
 Optional properties:
 - mrvl,clk-delay-cycles: Specify a number of cycles to delay for tuning.
+- dat3-cd: use DAT3 pin as hardware card detect - option available for
+  "marvell,armada-380-sdhci" only. The detection is supposed to work with
+  active high polarity, which implies usage of "cd-inverted" property.
+  Note that according to the specifications DAT3-based card detection can be
+  used with SD cards only. MMC standard doesn't support this feature.
 
 Example:
 
diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index 54a253c0..d813233 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -46,10 +46,14 @@
 #define SDCLK_DELAY_SHIFT	9
 #define SDCLK_DELAY_MASK	0x1f
 
-#define SD_CFG_FIFO_PARAM       0x100
+#define SD_EXTRA_PARAM_REG	0x100
 #define SDCFG_GEN_PAD_CLK_ON	(1<<6)
 #define SDCFG_GEN_PAD_CLK_CNT_MASK	0xFF
 #define SDCFG_GEN_PAD_CLK_CNT_SHIFT	24
+#define SD_FIFO_PARAM_REG	0x104
+#define SD_USE_DAT3		BIT(7)
+#define SD_OVRRD_CLK_OEN	BIT(11)
+#define SD_FORCE_CLK_ON		BIT(12)
 
 #define SD_SPI_MODE          0x108
 #define SD_CE_ATA_1          0x10C
@@ -64,6 +68,7 @@ struct sdhci_pxa {
 	u8	power_mode;
 	void __iomem *sdio3_conf_reg;
 	void __iomem *mbus_win_regs;
+	bool dat3_cd_enabled;
 };
 
 /*
@@ -160,13 +165,36 @@ static int armada_38x_quirks(struct platform_device *pdev,
 	}
 	host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
 
+	if (of_property_read_bool(np, "dat3-cd") &&
+	    !of_property_read_bool(np, "broken-cd"))
+		pxa->dat3_cd_enabled = true;
+
 	return 0;
 }
 
+static void pxav3_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+
+	sdhci_set_clock(host, clock);
+
+	/*
+	 * The interface clock enable is also used as control
+	 * for the A38x SDIO IP, so it can't be powered down
+	 * when using HW-based card detection.
+	 */
+	if (clock == 0 && pxa->dat3_cd_enabled)
+		sdhci_writew(host, SDHCI_CLOCK_INT_EN, SDHCI_CLOCK_CONTROL);
+}
+
 static void pxav3_reset(struct sdhci_host *host, u8 mask)
 {
 	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
 	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+	u32 reg_val;
 
 	sdhci_reset(host, mask);
 
@@ -184,6 +212,21 @@ static void pxav3_reset(struct sdhci_host *host, u8 mask)
 			tmp |= SDCLK_SEL;
 			writew(tmp, host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
 		}
+
+		if (pxa->dat3_cd_enabled) {
+			reg_val = sdhci_readl(host, SD_FIFO_PARAM_REG);
+			reg_val |= SD_USE_DAT3 | SD_OVRRD_CLK_OEN |
+				   SD_FORCE_CLK_ON;
+			sdhci_writel(host, reg_val, SD_FIFO_PARAM_REG);
+
+			/*
+			 * For HW detection based on DAT3 pin keep internal
+			 * clk switched on after controller reset.
+			 */
+			reg_val = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
+			reg_val |= SDHCI_CLOCK_INT_EN;
+			sdhci_writel(host, reg_val, SDHCI_CLOCK_CONTROL);
+		}
 	}
 }
 
@@ -211,9 +254,9 @@ static void pxav3_gen_init_74_clocks(struct sdhci_host *host, u8 power_mode)
 		writew(tmp, host->ioaddr + SD_CE_ATA_2);
 
 		/* start sending the 74 clocks */
-		tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
+		tmp = readw(host->ioaddr + SD_EXTRA_PARAM_REG);
 		tmp |= SDCFG_GEN_PAD_CLK_ON;
-		writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
+		writew(tmp, host->ioaddr + SD_EXTRA_PARAM_REG);
 
 		/* slowest speed is about 100KHz or 10usec per clock */
 		udelay(740);
@@ -298,7 +341,7 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
 }
 
 static const struct sdhci_ops pxav3_sdhci_ops = {
-	.set_clock = sdhci_set_clock,
+	.set_clock = pxav3_set_clock,
 	.platform_send_init_74_clocks = pxav3_gen_init_74_clocks,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
 	.set_bus_width = sdhci_set_bus_width,
@@ -407,6 +450,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 		sdhci_get_of_property(pdev);
 		pdata = pxav3_get_mmc_pdata(dev);
 		pdev->dev.platform_data = pdata;
+
 	} else if (pdata) {
 		/* on-chip device */
 		if (pdata->flags & PXA_FLAG_CARD_PERMANENT)
@@ -438,12 +482,15 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 		}
 	}
 
-	pm_runtime_get_noresume(&pdev->dev);
-	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_set_autosuspend_delay(&pdev->dev, PXAV3_RPM_DELAY_MS);
-	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
-	pm_suspend_ignore_children(&pdev->dev, 1);
+	if (!pxa->dat3_cd_enabled) {
+		pm_runtime_get_noresume(&pdev->dev);
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_set_autosuspend_delay(&pdev->dev,
+						 PXAV3_RPM_DELAY_MS);
+		pm_runtime_use_autosuspend(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+		pm_suspend_ignore_children(&pdev->dev, 1);
+	}
 
 	ret = sdhci_add_host(host);
 	if (ret) {
@@ -456,13 +503,16 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
 	if (host->mmc->pm_caps & MMC_PM_WAKE_SDIO_IRQ)
 		device_init_wakeup(&pdev->dev, 1);
 
-	pm_runtime_put_autosuspend(&pdev->dev);
+	if (!pxa->dat3_cd_enabled)
+		pm_runtime_put_autosuspend(&pdev->dev);
 
 	return 0;
 
 err_add_host:
-	pm_runtime_disable(&pdev->dev);
-	pm_runtime_put_noidle(&pdev->dev);
+	if (!pxa->dat3_cd_enabled) {
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+	}
 err_of_parse:
 err_cd_req:
 err_mbus_win:
@@ -479,9 +529,11 @@ static int sdhci_pxav3_remove(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_pxa *pxa = pltfm_host->priv;
 
-	pm_runtime_get_sync(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
-	pm_runtime_put_noidle(&pdev->dev);
+	if (!pxa->dat3_cd_enabled) {
+		pm_runtime_get_sync(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+	}
 
 	sdhci_remove_host(host, 1);
 
@@ -498,9 +550,16 @@ static int sdhci_pxav3_suspend(struct device *dev)
 {
 	int ret;
 	struct sdhci_host *host = dev_get_drvdata(dev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+
+	if (!pxa->dat3_cd_enabled)
+		pm_runtime_get_sync(dev);
 
-	pm_runtime_get_sync(dev);
 	ret = sdhci_suspend_host(host);
+	if (pxa->dat3_cd_enabled)
+		return ret;
+
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
@@ -518,8 +577,13 @@ static int sdhci_pxav3_resume(struct device *dev)
 		ret = mv_conf_mbus_windows(dev, pxa->mbus_win_regs,
 					   mv_mbus_dram_info());
 
-	pm_runtime_get_sync(dev);
+	if (!pxa->dat3_cd_enabled)
+		pm_runtime_get_sync(dev);
+
 	ret = sdhci_resume_host(host);
+	if (pxa->dat3_cd_enabled)
+		return ret;
+
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
-- 
1.8.3.1

[PATCH v3 3/5] ARM: mvebu: set SW polling as SDHCI card detection on A388-GP

[Marcin Wojtas]

The newest revisions of A388-GP (v1.5 and higher) support only
DAT3-based card detection. Revisions < v1.5 based on GPIO detection
via I2C expander, but this solution is supposed to be deprecated on
new boards. In order to satisfy all type of hardware this commit
changes card detection to use software polling mechanism. Also a
comment is added on possible card detection options in A388-GP
DT board file.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Acked-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/armada-388-gp.dts | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-388-gp.dts b/arch/arm/boot/dts/armada-388-gp.dts
index 391dea9..3deba13 100644
--- a/arch/arm/boot/dts/armada-388-gp.dts
+++ b/arch/arm/boot/dts/armada-388-gp.dts
@@ -213,8 +213,21 @@
 			sdhci at d8000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&sdhci_pins>;
-				cd-gpios = <&expander0 5 GPIO_ACTIVE_LOW>;
 				no-1-8-v;
+				/*
+				 * A388-GP board v1.5 and higher replace
+				 * hitherto card detection method based on GPIO
+				 * with the one using DAT3 pin. As they are
+				 * incompatible, software-based polling is
+				 * enabled with 'broken-cd' property. For boards
+				 * older than v1.5 it can be replaced with:
+				 * 'cd-gpios = <&expander0 5 GPIO_ACTIVE_LOW>;',
+				 * whereas for the newer ones following can be
+				 * used instead:
+				 * 'dat3-cd;'
+				 * 'cd-inverted;'
+				 */
+				broken-cd;
 				wp-inverted;
 				bus-width = <8>;
 				status = "okay";
-- 
1.8.3.1

[PATCH v3 4/5] mmc: sdhci: add init_card callback to sdhci

[Marcin Wojtas]

Some sdhci hosts may require handling quirks during card initialization at
the time when its type is already known. Hence a new callback (init_card)
is added in sdhci_ops.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/mmc/host/sdhci.c | 9 +++++++++
 drivers/mmc/host/sdhci.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fbc7efd..0739749 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2199,6 +2199,14 @@ static void sdhci_card_event(struct mmc_host *mmc)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
+static void sdhci_init_card(struct mmc_host *mmc, struct mmc_card *card)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (host->ops->init_card)
+		host->ops->init_card(host, card);
+}
+
 static const struct mmc_host_ops sdhci_ops = {
 	.request	= sdhci_request,
 	.post_req	= sdhci_post_req,
@@ -2214,6 +2222,7 @@ static const struct mmc_host_ops sdhci_ops = {
 	.select_drive_strength		= sdhci_select_drive_strength,
 	.card_event			= sdhci_card_event,
 	.card_busy	= sdhci_card_busy,
+	.init_card	= sdhci_init_card,
 };
 
 /*****************************************************************************\
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 9d4aa31..12f4115 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -549,6 +549,7 @@ struct sdhci_ops {
 					 struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
 					 int card_drv, int *drv_type);
+	void	(*init_card)(struct sdhci_host *host, struct mmc_card *card);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
-- 
1.8.3.1

[PATCH v3 5/5] mmc: sdhci-pxav3: enable modifying MMC_CARD bit during card initialization

[Marcin Wojtas]

On Marvell Armada 38x SoC's the MMC_CARD bit in SD_CE_ATA_1 register must
be set to 0x1 when a MMC card is supposed to work in DDR mode, or when
commands CMD11, CMD14 and CMD20 are used.

This commit enables the above for all MMC cards by modifying the host
registers during card initialization. It is done by using init_card()
callback with pxa->mbus_win_regs as a flag, which notifies if Armada 38x
controller is in use.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/mmc/host/sdhci-pxav3.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
index d813233..8742afd 100644
--- a/drivers/mmc/host/sdhci-pxav3.c
+++ b/drivers/mmc/host/sdhci-pxav3.c
@@ -57,6 +57,7 @@
 
 #define SD_SPI_MODE          0x108
 #define SD_CE_ATA_1          0x10C
+#define SDCE_MMC_CARD		BIT(28)
 
 #define SD_CE_ATA_2          0x10E
 #define SDCE_MISC_INT		(1<<2)
@@ -230,6 +231,26 @@ static void pxav3_reset(struct sdhci_host *host, u8 mask)
 	}
 }
 
+static void pxav3_init_card(struct sdhci_host *host, struct mmc_card *card)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+	u32 reg_val;
+
+	/*
+	 * Armada 38x SDHCI controller requires update of
+	 * MMC_CARD bit depending on inserted card type.
+	 */
+	if (pxa->mbus_win_regs) {
+		reg_val = sdhci_readl(host, SD_CE_ATA_1);
+		if (mmc_card_mmc(card))
+			reg_val |= SDCE_MMC_CARD;
+		else
+			reg_val &= ~SDCE_MMC_CARD;
+		sdhci_writel(host, reg_val, SD_CE_ATA_1);
+	}
+}
+
 #define MAX_WAIT_COUNT 5
 static void pxav3_gen_init_74_clocks(struct sdhci_host *host, u8 power_mode)
 {
@@ -347,6 +368,7 @@ static const struct sdhci_ops pxav3_sdhci_ops = {
 	.set_bus_width = sdhci_set_bus_width,
 	.reset = pxav3_reset,
 	.set_uhs_signaling = pxav3_set_uhs_signaling,
+	.init_card = pxav3_init_card,
 };
 
 static struct sdhci_pltfm_data sdhci_pxav3_pdata = {
-- 
1.8.3.1

[PATCH v3 3/5] ARM: mvebu: set SW polling as SDHCI card detection on A388-GP

[Gregory CLEMENT]

Hi Marcin,
 
 On jeu., oct. 15 2015, Marcin Wojtas <mw@semihalf.com> wrote:

> The newest revisions of A388-GP (v1.5 and higher) support only
> DAT3-based card detection. Revisions < v1.5 based on GPIO detection
> via I2C expander, but this solution is supposed to be deprecated on
> new boards. In order to satisfy all type of hardware this commit
> changes card detection to use software polling mechanism. Also a
> comment is added on possible card detection options in A388-GP
> DT board file.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Acked-by: Andrew Lunn <andrew@lunn.ch>

Applied on mvebu/dt

Thanks,

Gregory

> ---
>  arch/arm/boot/dts/armada-388-gp.dts | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/armada-388-gp.dts b/arch/arm/boot/dts/armada-388-gp.dts
> index 391dea9..3deba13 100644
> --- a/arch/arm/boot/dts/armada-388-gp.dts
> +++ b/arch/arm/boot/dts/armada-388-gp.dts
> @@ -213,8 +213,21 @@
>  			sdhci at d8000 {
>  				pinctrl-names = "default";
>  				pinctrl-0 = <&sdhci_pins>;
> -				cd-gpios = <&expander0 5 GPIO_ACTIVE_LOW>;
>  				no-1-8-v;
> +				/*
> +				 * A388-GP board v1.5 and higher replace
> +				 * hitherto card detection method based on GPIO
> +				 * with the one using DAT3 pin. As they are
> +				 * incompatible, software-based polling is
> +				 * enabled with 'broken-cd' property. For boards
> +				 * older than v1.5 it can be replaced with:
> +				 * 'cd-gpios = <&expander0 5 GPIO_ACTIVE_LOW>;',
> +				 * whereas for the newer ones following can be
> +				 * used instead:
> +				 * 'dat3-cd;'
> +				 * 'cd-inverted;'
> +				 */
> +				broken-cd;
>  				wp-inverted;
>  				bus-width = <8>;
>  				status = "okay";
> -- 
> 1.8.3.1
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH v3 0/5] Armada 38x SDHCI driver improvements

[Marcin Wojtas]

Hi Ulf,

Do you have any remarks or comments to the series?

Best regards,
Marcin

2015-10-15 18:25 GMT+02:00 Marcin Wojtas <mw@semihalf.com>:
> Hi,
>
> Thank you for reviewing the patches. According to your remarks and some
> new ideas I prepared third patchset. I modified my HW and now I could
> check operation when using all three modes of detection (polling, gpio
> and dat3) - it all seems working fine. Any remarks will be wellcome.
>
> Best regards,
> Marcin
>
> Changes:
> v1 -> v2
> * enable SW polling as card detection
> * in resume function change condition for mbus windows reconfiguration
>
> v2 -> v3
> * remove redundant print after fail of mbus_win_regs obtaining
> * add big comment on possible card detection options in armada-388-gp.dts
> * reconstruct dat3-cd support
>         - use dedicated flag in sdhci_pxa structure instead of checking
>           property in DT
>         - instead of global SDHCI quirk add wrapper for sdhci_set_clock
>           function in order to keep internal clock enabled
>         - prevent using pm_runtime in case of using dat3-cd
> * improve MMC_CARD bit modification - do not check Armada 38x compatible
>   string in pxav3_init_card callback, but use pxa->mbus_win_regs as a flag
>
> Marcin Wojtas (5):
>   mmc: sdhci-pxav3: enable proper resuming on Armada 38x SoC
>   mmc: sdhci-pxav3: enable usage of DAT3 pin as HW card detect
>   ARM: mvebu: set SW polling as SDHCI card detection on A388-GP
>   mmc: sdhci: add init_card callback to sdhci
>   mmc: sdhci-pxav3: enable modifying MMC_CARD bit during card
>     initialization
>
>  .../devicetree/bindings/mmc/sdhci-pxa.txt          |   5 +
>  arch/arm/boot/dts/armada-388-gp.dts                |  15 +-
>  drivers/mmc/host/sdhci-pxav3.c                     | 157 ++++++++++++++++-----
>  drivers/mmc/host/sdhci.c                           |   9 ++
>  drivers/mmc/host/sdhci.h                           |   1 +
>  5 files changed, 149 insertions(+), 38 deletions(-)
>
> --
> 1.8.3.1
>

[PATCH v3 1/5] mmc: sdhci-pxav3: enable proper resuming on Armada 38x SoC

[Ulf Hansson]

On 15 October 2015 at 18:25, Marcin Wojtas <mw@semihalf.com> wrote:
> When resuming from suspend on Armada 38x SoC MBus windows have to be
> re-configured and for that purpose mv_conf_mbus_windows function needed
> rework. MBus windows register base address obtaining was moved to
> armada_38x_quirks function in order to be kept in pxa global structure,
> because it is used during a resume.
>
> This commit fixes resuming from suspend by calling MBus windows
> configuration routine and therefore enabling proper DMA operation.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  drivers/mmc/host/sdhci-pxav3.c | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index f5edf9d..54a253c0 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -63,6 +63,7 @@ struct sdhci_pxa {
>         struct clk *clk_io;
>         u8      power_mode;
>         void __iomem *sdio3_conf_reg;
> +       void __iomem *mbus_win_regs;
>  };
>
>  /*
> @@ -81,30 +82,16 @@ struct sdhci_pxa {
>  #define SDIO3_CONF_CLK_INV     BIT(0)
>  #define SDIO3_CONF_SD_FB_CLK   BIT(2)
>
> -static int mv_conf_mbus_windows(struct platform_device *pdev,
> +static int mv_conf_mbus_windows(struct device *dev, void __iomem *regs,
>                                 const struct mbus_dram_target_info *dram)
>  {
>         int i;
> -       void __iomem *regs;
> -       struct resource *res;
>
>         if (!dram) {
> -               dev_err(&pdev->dev, "no mbus dram info\n");
> -               return -EINVAL;
> -       }
> -
> -       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -       if (!res) {
> -               dev_err(&pdev->dev, "cannot get mbus registers\n");
> +               dev_err(dev, "no mbus dram info\n");
>                 return -EINVAL;
>         }
>
> -       regs = ioremap(res->start, resource_size(res));
> -       if (!regs) {
> -               dev_err(&pdev->dev, "cannot map mbus registers\n");
> -               return -ENOMEM;
> -       }
> -
>         for (i = 0; i < SDHCI_MAX_WIN_NUM; i++) {
>                 writel(0, regs + SDHCI_WINDOW_CTRL(i));
>                 writel(0, regs + SDHCI_WINDOW_BASE(i));
> @@ -122,8 +109,6 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
>                 writel(cs->base, regs + SDHCI_WINDOW_BASE(i));
>         }
>
> -       iounmap(regs);
> -
>         return 0;
>  }
>
> @@ -135,6 +120,11 @@ static int armada_38x_quirks(struct platform_device *pdev,
>         struct sdhci_pxa *pxa = pltfm_host->priv;
>         struct resource *res;
>
> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mbus");

You change from using platform_get_resource(pdev, IORESOURCE_MEM, 1)
to above name based lookup.

I guess it's intentional, but there's no information about it in the
change-log, perhaps add it?

> +       pxa->mbus_win_regs = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(pxa->mbus_win_regs))
> +               return PTR_ERR(pxa->mbus_win_regs);
> +
>         host->quirks &= ~SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN;
>         host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
>         res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> @@ -403,7 +393,8 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>                 ret = armada_38x_quirks(pdev, host);
>                 if (ret < 0)
>                         goto err_mbus_win;
> -               ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
> +               ret = mv_conf_mbus_windows(&pdev->dev, pxa->mbus_win_regs,
> +                                          mv_mbus_dram_info());
>                 if (ret < 0)
>                         goto err_mbus_win;
>         }
> @@ -520,6 +511,12 @@ static int sdhci_pxav3_resume(struct device *dev)
>  {
>         int ret;
>         struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +
> +       if (pxa->mbus_win_regs)
> +               ret = mv_conf_mbus_windows(dev, pxa->mbus_win_regs,
> +                                          mv_mbus_dram_info());
>
>         pm_runtime_get_sync(dev);
>         ret = sdhci_resume_host(host);
> --
> 1.8.3.1
>

Kind regards
Uffe

[PATCH v3 2/5] mmc: sdhci-pxav3: enable usage of DAT3 pin as HW card detect

[Ulf Hansson]

On 15 October 2015 at 18:25, Marcin Wojtas <mw@semihalf.com> wrote:
> Marvell Armada 38x SDHCI controller enable using DAT3 pin as a hardware
> card detection. According to the SD sdandard this signal can be used for
> this purpose combined with a pull-down resistor, implying inverted (active
> high) polarization of a card detect. MMC standard does not support this
> feature and does not operate with such connectivity of DAT3.
>
> When using DAT3-based detection Armada 38x SDIO IP expects its internal
> clock to be always on, which had to be ensured by:
> - Udating appropriate registers, each time controller is reset. On the

Isn't the reset sequence going to enable the clock again? I though
reset was used to recover from failures.

To me, it seems odd that you need to deal with this, but of course
there are lots of odd things with sdhci. :-)

>   occasion of adding new register @0x104, register @0x100 name is modified
>   in order to the be aligned with Armada 38x documentation.
> - Leaving the clock enabled despite power-down. For this purpose a wrapper
>   for sdhci_set_clock function is added.
> - Disabling pm_runtime - during suspend both io_clock and controller's
>   bus power is switched off, hence it would prevent proper card detection.
>
> In addition to the changes above this commit adds a new property to Armada
> 38x SDHCI node ('dat3-cd') with an according binding documentation update.
> 'sdhci_pxa' structure is also extended with dedicated flag for checking if
> DAT3-based detection is in use.

There is an easier way. Just check the new DT binding and if it set,
execute pm_runtime_forbid() during ->probe(). No more runtime PM
changes should be needed.

Actually, I wonder if we should make this a new mmc core feature. We
could via mmc_add_host() check if this DT property is set and then
invoke the pm_runtime_forbid(). What do you think?

One question, for clarity, you don't expect card detect IRQs to be
treated as wakeups while being system PM suspended, right?

>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  .../devicetree/bindings/mmc/sdhci-pxa.txt          |   5 ++
>  drivers/mmc/host/sdhci-pxav3.c                     | 100 +++++++++++++++++----
>  2 files changed, 87 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt b/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> index 3d1b449..ffd6b14 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> @@ -23,6 +23,11 @@ Required properties:
>
>  Optional properties:
>  - mrvl,clk-delay-cycles: Specify a number of cycles to delay for tuning.
> +- dat3-cd: use DAT3 pin as hardware card detect - option available for
> +  "marvell,armada-380-sdhci" only. The detection is supposed to work with
> +  active high polarity, which implies usage of "cd-inverted" property.
> +  Note that according to the specifications DAT3-based card detection can be
> +  used with SD cards only. MMC standard doesn't support this feature.
>
>  Example:
>
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index 54a253c0..d813233 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -46,10 +46,14 @@
>  #define SDCLK_DELAY_SHIFT      9
>  #define SDCLK_DELAY_MASK       0x1f
>
> -#define SD_CFG_FIFO_PARAM       0x100
> +#define SD_EXTRA_PARAM_REG     0x100
>  #define SDCFG_GEN_PAD_CLK_ON   (1<<6)
>  #define SDCFG_GEN_PAD_CLK_CNT_MASK     0xFF
>  #define SDCFG_GEN_PAD_CLK_CNT_SHIFT    24
> +#define SD_FIFO_PARAM_REG      0x104
> +#define SD_USE_DAT3            BIT(7)
> +#define SD_OVRRD_CLK_OEN       BIT(11)
> +#define SD_FORCE_CLK_ON                BIT(12)
>
>  #define SD_SPI_MODE          0x108
>  #define SD_CE_ATA_1          0x10C
> @@ -64,6 +68,7 @@ struct sdhci_pxa {
>         u8      power_mode;
>         void __iomem *sdio3_conf_reg;
>         void __iomem *mbus_win_regs;
> +       bool dat3_cd_enabled;
>  };
>
>  /*
> @@ -160,13 +165,36 @@ static int armada_38x_quirks(struct platform_device *pdev,
>         }
>         host->caps1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_USE_SDR50_TUNING);
>
> +       if (of_property_read_bool(np, "dat3-cd") &&
> +           !of_property_read_bool(np, "broken-cd"))

Please, don't involve "broken-cd" as that has its own rules and purpose.

> +               pxa->dat3_cd_enabled = true;
> +
>         return 0;
>  }
>
> +static void pxav3_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +
> +       sdhci_set_clock(host, clock);
> +
> +       /*
> +        * The interface clock enable is also used as control
> +        * for the A38x SDIO IP, so it can't be powered down
> +        * when using HW-based card detection.
> +        */
> +       if (clock == 0 && pxa->dat3_cd_enabled)
> +               sdhci_writew(host, SDHCI_CLOCK_INT_EN, SDHCI_CLOCK_CONTROL);
> +}
> +
>  static void pxav3_reset(struct sdhci_host *host, u8 mask)
>  {
>         struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
>         struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +       u32 reg_val;
>
>         sdhci_reset(host, mask);
>
> @@ -184,6 +212,21 @@ static void pxav3_reset(struct sdhci_host *host, u8 mask)
>                         tmp |= SDCLK_SEL;
>                         writew(tmp, host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
>                 }
> +
> +               if (pxa->dat3_cd_enabled) {
> +                       reg_val = sdhci_readl(host, SD_FIFO_PARAM_REG);
> +                       reg_val |= SD_USE_DAT3 | SD_OVRRD_CLK_OEN |
> +                                  SD_FORCE_CLK_ON;
> +                       sdhci_writel(host, reg_val, SD_FIFO_PARAM_REG);
> +
> +                       /*
> +                        * For HW detection based on DAT3 pin keep internal
> +                        * clk switched on after controller reset.
> +                        */
> +                       reg_val = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> +                       reg_val |= SDHCI_CLOCK_INT_EN;
> +                       sdhci_writel(host, reg_val, SDHCI_CLOCK_CONTROL);
> +               }
>         }
>  }
>
> @@ -211,9 +254,9 @@ static void pxav3_gen_init_74_clocks(struct sdhci_host *host, u8 power_mode)
>                 writew(tmp, host->ioaddr + SD_CE_ATA_2);
>
>                 /* start sending the 74 clocks */
> -               tmp = readw(host->ioaddr + SD_CFG_FIFO_PARAM);
> +               tmp = readw(host->ioaddr + SD_EXTRA_PARAM_REG);
>                 tmp |= SDCFG_GEN_PAD_CLK_ON;
> -               writew(tmp, host->ioaddr + SD_CFG_FIFO_PARAM);
> +               writew(tmp, host->ioaddr + SD_EXTRA_PARAM_REG);
>
>                 /* slowest speed is about 100KHz or 10usec per clock */
>                 udelay(740);
> @@ -298,7 +341,7 @@ static void pxav3_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
>  }
>
>  static const struct sdhci_ops pxav3_sdhci_ops = {
> -       .set_clock = sdhci_set_clock,
> +       .set_clock = pxav3_set_clock,
>         .platform_send_init_74_clocks = pxav3_gen_init_74_clocks,
>         .get_max_clock = sdhci_pltfm_clk_get_max_clock,
>         .set_bus_width = sdhci_set_bus_width,
> @@ -407,6 +450,7 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>                 sdhci_get_of_property(pdev);
>                 pdata = pxav3_get_mmc_pdata(dev);
>                 pdev->dev.platform_data = pdata;
> +
>         } else if (pdata) {
>                 /* on-chip device */
>                 if (pdata->flags & PXA_FLAG_CARD_PERMANENT)
> @@ -438,12 +482,15 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>                 }
>         }
>
> -       pm_runtime_get_noresume(&pdev->dev);
> -       pm_runtime_set_active(&pdev->dev);
> -       pm_runtime_set_autosuspend_delay(&pdev->dev, PXAV3_RPM_DELAY_MS);
> -       pm_runtime_use_autosuspend(&pdev->dev);
> -       pm_runtime_enable(&pdev->dev);
> -       pm_suspend_ignore_children(&pdev->dev, 1);
> +       if (!pxa->dat3_cd_enabled) {
> +               pm_runtime_get_noresume(&pdev->dev);
> +               pm_runtime_set_active(&pdev->dev);
> +               pm_runtime_set_autosuspend_delay(&pdev->dev,
> +                                                PXAV3_RPM_DELAY_MS);
> +               pm_runtime_use_autosuspend(&pdev->dev);
> +               pm_runtime_enable(&pdev->dev);
> +               pm_suspend_ignore_children(&pdev->dev, 1);
> +       }
>
>         ret = sdhci_add_host(host);
>         if (ret) {
> @@ -456,13 +503,16 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
>         if (host->mmc->pm_caps & MMC_PM_WAKE_SDIO_IRQ)
>                 device_init_wakeup(&pdev->dev, 1);
>
> -       pm_runtime_put_autosuspend(&pdev->dev);
> +       if (!pxa->dat3_cd_enabled)
> +               pm_runtime_put_autosuspend(&pdev->dev);
>
>         return 0;
>
>  err_add_host:
> -       pm_runtime_disable(&pdev->dev);
> -       pm_runtime_put_noidle(&pdev->dev);
> +       if (!pxa->dat3_cd_enabled) {
> +               pm_runtime_disable(&pdev->dev);
> +               pm_runtime_put_noidle(&pdev->dev);
> +       }
>  err_of_parse:
>  err_cd_req:
>  err_mbus_win:
> @@ -479,9 +529,11 @@ static int sdhci_pxav3_remove(struct platform_device *pdev)
>         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>         struct sdhci_pxa *pxa = pltfm_host->priv;
>
> -       pm_runtime_get_sync(&pdev->dev);
> -       pm_runtime_disable(&pdev->dev);
> -       pm_runtime_put_noidle(&pdev->dev);
> +       if (!pxa->dat3_cd_enabled) {
> +               pm_runtime_get_sync(&pdev->dev);
> +               pm_runtime_disable(&pdev->dev);
> +               pm_runtime_put_noidle(&pdev->dev);
> +       }
>
>         sdhci_remove_host(host, 1);
>
> @@ -498,9 +550,16 @@ static int sdhci_pxav3_suspend(struct device *dev)
>  {
>         int ret;
>         struct sdhci_host *host = dev_get_drvdata(dev);
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_pxa *pxa = pltfm_host->priv;
> +
> +       if (!pxa->dat3_cd_enabled)
> +               pm_runtime_get_sync(dev);
>
> -       pm_runtime_get_sync(dev);
>         ret = sdhci_suspend_host(host);
> +       if (pxa->dat3_cd_enabled)
> +               return ret;
> +
>         pm_runtime_mark_last_busy(dev);
>         pm_runtime_put_autosuspend(dev);
>
> @@ -518,8 +577,13 @@ static int sdhci_pxav3_resume(struct device *dev)
>                 ret = mv_conf_mbus_windows(dev, pxa->mbus_win_regs,
>                                            mv_mbus_dram_info());
>
> -       pm_runtime_get_sync(dev);
> +       if (!pxa->dat3_cd_enabled)
> +               pm_runtime_get_sync(dev);
> +
>         ret = sdhci_resume_host(host);
> +       if (pxa->dat3_cd_enabled)
> +               return ret;
> +
>         pm_runtime_mark_last_busy(dev);
>         pm_runtime_put_autosuspend(dev);
>
> --
> 1.8.3.1
>

Kind regards
Uffe

[PATCH v3 1/5] mmc: sdhci-pxav3: enable proper resuming on Armada 38x SoC

[Marcin Wojtas]

Hi Ulf

2015-10-22 15:29 GMT+02:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 15 October 2015 at 18:25, Marcin Wojtas <mw@semihalf.com> wrote:
>> When resuming from suspend on Armada 38x SoC MBus windows have to be
>> re-configured and for that purpose mv_conf_mbus_windows function needed
>> rework. MBus windows register base address obtaining was moved to
>> armada_38x_quirks function in order to be kept in pxa global structure,
>> because it is used during a resume.
>>
>> This commit fixes resuming from suspend by calling MBus windows
>> configuration routine and therefore enabling proper DMA operation.
>>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  drivers/mmc/host/sdhci-pxav3.c | 35 ++++++++++++++++-------------------
>>  1 file changed, 16 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
>> index f5edf9d..54a253c0 100644
>> --- a/drivers/mmc/host/sdhci-pxav3.c
>> +++ b/drivers/mmc/host/sdhci-pxav3.c
>> @@ -63,6 +63,7 @@ struct sdhci_pxa {
>>         struct clk *clk_io;
>>         u8      power_mode;
>>         void __iomem *sdio3_conf_reg;
>> +       void __iomem *mbus_win_regs;
>>  };
>>
>>  /*
>> @@ -81,30 +82,16 @@ struct sdhci_pxa {
>>  #define SDIO3_CONF_CLK_INV     BIT(0)
>>  #define SDIO3_CONF_SD_FB_CLK   BIT(2)
>>
>> -static int mv_conf_mbus_windows(struct platform_device *pdev,
>> +static int mv_conf_mbus_windows(struct device *dev, void __iomem *regs,
>>                                 const struct mbus_dram_target_info *dram)
>>  {
>>         int i;
>> -       void __iomem *regs;
>> -       struct resource *res;
>>
>>         if (!dram) {
>> -               dev_err(&pdev->dev, "no mbus dram info\n");
>> -               return -EINVAL;
>> -       }
>> -
>> -       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> -       if (!res) {
>> -               dev_err(&pdev->dev, "cannot get mbus registers\n");
>> +               dev_err(dev, "no mbus dram info\n");
>>                 return -EINVAL;
>>         }
>>
>> -       regs = ioremap(res->start, resource_size(res));
>> -       if (!regs) {
>> -               dev_err(&pdev->dev, "cannot map mbus registers\n");
>> -               return -ENOMEM;
>> -       }
>> -
>>         for (i = 0; i < SDHCI_MAX_WIN_NUM; i++) {
>>                 writel(0, regs + SDHCI_WINDOW_CTRL(i));
>>                 writel(0, regs + SDHCI_WINDOW_BASE(i));
>> @@ -122,8 +109,6 @@ static int mv_conf_mbus_windows(struct platform_device *pdev,
>>                 writel(cs->base, regs + SDHCI_WINDOW_BASE(i));
>>         }
>>
>> -       iounmap(regs);
>> -
>>         return 0;
>>  }
>>
>> @@ -135,6 +120,11 @@ static int armada_38x_quirks(struct platform_device *pdev,
>>         struct sdhci_pxa *pxa = pltfm_host->priv;
>>         struct resource *res;
>>
>> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mbus");
>
> You change from using platform_get_resource(pdev, IORESOURCE_MEM, 1)
> to above name based lookup.
>
> I guess it's intentional, but there's no information about it in the
> change-log, perhaps add it?
>

Sure, good point.

Best regards,
Marcin

[PATCH v3 2/5] mmc: sdhci-pxav3: enable usage of DAT3 pin as HW card detect

[Marcin Wojtas]

Hi Ulf,

2015-10-22 15:29 GMT+02:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 15 October 2015 at 18:25, Marcin Wojtas <mw@semihalf.com> wrote:
>> Marvell Armada 38x SDHCI controller enable using DAT3 pin as a hardware
>> card detection. According to the SD sdandard this signal can be used for
>> this purpose combined with a pull-down resistor, implying inverted (active
>> high) polarization of a card detect. MMC standard does not support this
>> feature and does not operate with such connectivity of DAT3.
>>
>> When using DAT3-based detection Armada 38x SDIO IP expects its internal
>> clock to be always on, which had to be ensured by:
>> - Udating appropriate registers, each time controller is reset. On the
>
> Isn't the reset sequence going to enable the clock again? I though
> reset was used to recover from failures.
>
> To me, it seems odd that you need to deal with this, but of course
> there are lots of odd things with sdhci. :-)
>

Setting clock enable in sdhci_control register is one thing, but this
IP requires also setting other register, which is set to default after
reset and does not get updated automatically.

>>   occasion of adding new register @0x104, register @0x100 name is modified
>>   in order to the be aligned with Armada 38x documentation.
>> - Leaving the clock enabled despite power-down. For this purpose a wrapper
>>   for sdhci_set_clock function is added.
>> - Disabling pm_runtime - during suspend both io_clock and controller's
>>   bus power is switched off, hence it would prevent proper card detection.
>>
>> In addition to the changes above this commit adds a new property to Armada
>> 38x SDHCI node ('dat3-cd') with an according binding documentation update.
>> 'sdhci_pxa' structure is also extended with dedicated flag for checking if
>> DAT3-based detection is in use.
>
> There is an easier way. Just check the new DT binding and if it set,
> execute pm_runtime_forbid() during ->probe(). No more runtime PM
> changes should be needed.
>

I will check it.

> Actually, I wonder if we should make this a new mmc core feature. We
> could via mmc_add_host() check if this DT property is set and then
> invoke the pm_runtime_forbid(). What do you think?
>

I thought about it. dat3 detect is defined by SDIO standard
(https://www.sdcard.org/developers/overview/sdio/sdio_spec/Simplified_SDIO_Card_Spec.pdf,
section 4.6), so it can be enabled as a global feature. Each IP
however would have to handle its own quirks related to it in the
drivers. I don't know if clock_enable should be always set only for
sdhci-pxav3 or for all hosts willing to use dat3 detection -
intuitively it should be a common feature, but I cannot confirm it...

On the other hand I will play with MMC_PM_KEEP_POWER capability, maybe
with this flag mmc subsystem is already able to support dat3 cd and
what has to be done is rather to satisfy sdhci-pxav3 quirks. Anyway, I
cannot find any explicit reference to it.

> One question, for clarity, you don't expect card detect IRQs to be
> treated as wakeups while being system PM suspended, right?

I'll check if enabling MMC_PM_KEEP_POWER is sufficient.

> Please, don't involve "broken-cd" as that has its own rules and purpose.

Indeed, this check is redundant.

Best regards,
Marcin