Skip to contentSkip to conversation

MMC: use mmc_of_parse in sdhci-esdhc-imx

PATCH New Sascha Hauer 4 participants 14 messages May 23, 2014 – Jun 21, 2014

Subsystems (5)
Device tree bindings Arm i.MX and Layerscape MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver +1 more subsystems
lore
Share & tools
lore

Sascha Hauer

author maintainer
#0 lore raw

PATCH 1/1 MMC: use mmc_of_parse in sdhci-esdhc-imx

The following series changes the sdhci-esdhc-imx to use mmc_of_parse to profit from the generic devicetree parser. Unfortunately this is not straight forward because the driver parses all devicetree data into platform_data instead of directly into the mmc host, so we have to refactor the parsing and move platform_data parsing out of the way for the devicetree path first.

The last patch fixes the card/write protection for the babbage board and can be taken independently of the rest by Shawn.

Sascha

----------------------------------------------------------------
Sascha Hauer (5):
mmc: sdhci-esdhc-imx: add f_max field to private data
mmc: sdhci-esdhc-imx: introduce function for parsing platform_data
mmc: sdhci-esdhc-imx: straighten SDHCI_QUIRK_BROKEN_CARD_DETECTION flag
mmc: sdhci-esdhc-imx: use mmc_of_parse
ARM: dts: imx51-babbage: Fix esdhc setup

 arch/arm/boot/dts/imx51-babbage.dts |  14 +--
 drivers/mmc/host/sdhci-esdhc-imx.c  | 205 +++++++++++++++++++-----------------
 2 files changed, 118 insertions(+), 101 deletions(-)
author maintainer
#1 lore raw

PATCH 1/5 mmc: sdhci-esdhc-imx: add f_max field to private data

The driver filled in the clk and clock fields in struct sdhci_pltfm_host, but they are unused in the sdhci-pltfm code and only ever used it for internal use. This adds a field f_max to private data and uses it instead.

Signed-off-by Sascha Hauer <[email protected]>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +9 -9
index b841bb7..d530820 100644
@@ -152,6 +152,7 @@ struct pltfm_imx_data {
 	struct pinctrl_state *pins_200mhz;
 	const struct esdhc_soc_data *socdata;
 	struct esdhc_platform_data boarddata;
+	unsigned long f_max;
 	struct clk *clk_ipg;
 	struct clk *clk_ahb;
 	struct clk *clk_per;

@@ -574,19 +575,16 @@ static unsigned int esdhc_pltfm_get_max_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
-	if (boarddata->f_max && (boarddata->f_max < pltfm_host->clock))
-		return boarddata->f_max;
-	else
-		return pltfm_host->clock;
+	return imx_data->f_max;
 }
 
 static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
 
-	return pltfm_host->clock / 256 / 16;
+	return imx_data->f_max / 256 / 16;
 }
 
 static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,

@@ -594,7 +592,7 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	unsigned int host_clock = pltfm_host->clock;
+	unsigned int host_clock = clk_get_rate(imx_data->clk_per);
 	int pre_div = 2;
 	int div = 1;
 	u32 temp, val;

@@ -994,8 +992,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		goto free_sdhci;
 	}
 
-	pltfm_host->clk = imx_data->clk_per;
-	pltfm_host->clock = clk_get_rate(pltfm_host->clk);
 	clk_prepare_enable(imx_data->clk_per);
 	clk_prepare_enable(imx_data->clk_ipg);
 	clk_prepare_enable(imx_data->clk_ahb);

@@ -1051,6 +1047,10 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 					host->mmc->parent->platform_data);
 	}
 
+	imx_data->f_max = clk_get_rate(imx_data->clk_per);
+	if (boarddata->f_max && boarddata->f_max < imx_data->f_max)
+		imx_data->f_max = boarddata->f_max;
+
 	/* write_protect */
 	if (boarddata->wp_type == ESDHC_WP_GPIO) {
 		err = mmc_gpio_request_ro(host->mmc, boarddata->wp_gpio);
Signature · 1 line
2.0.0.rc0
#2 lore raw

PATCH 1/5 mmc: sdhci-esdhc-imx: add f_max field to private data

Quoted 46 lines · Sascha Hauer

The driver filled in the clk and clock fields in struct sdhci_pltfm_host, but they are unused in the sdhci-pltfm code and only ever used it for internal use. This adds a field f_max to private data and uses it instead.

Signed-off-by Sascha Hauer <[email protected]>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +4 -6
index b841bb7..d530820 100644
@@ -152,6 +152,7 @@ struct pltfm_imx_data {
 	struct pinctrl_state *pins_200mhz;
 	const struct esdhc_soc_data *socdata;
 	struct esdhc_platform_data boarddata;
+	unsigned long f_max;
 	struct clk *clk_ipg;
 	struct clk *clk_ahb;
 	struct clk *clk_per;

@@ -574,19 +575,16 @@ static unsigned int esdhc_pltfm_get_max_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
-	if (boarddata->f_max && (boarddata->f_max < pltfm_host->clock))
-		return boarddata->f_max;
-	else
-		return pltfm_host->clock;
+	return imx_data->f_max;
 }
 
 static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
 
-	return pltfm_host->clock / 256 / 16;
+	return imx_data->f_max / 256 / 16;
 }
 
 static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,

drivers/mmc/host/sdhci-esdhc-imx.c static inline void esdhc_pltfm_set_clock(struct sdhci_host *host, · line 595

Quoted 5 lines
drivers/mmc/host/sdhci-esdhc-imx.c +1 -1
@@ -594,7 +592,7 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	unsigned int host_clock = pltfm_host->clock;
+	unsigned int host_clock = clk_get_rate(imx_data->clk_per);

Will this bring the issue that commit a974862faee1 (mmc: sdhci-esdhc-imx:
fix access hardirq-unsafe lock in atomic context) fixed back to us?

Shawn

Quoted 26 lines
drivers/mmc/host/sdhci-esdhc-imx.c +4 -2
 	int pre_div = 2;
 	int div = 1;
 	u32 temp, val;

@@ -994,8 +992,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		goto free_sdhci;
 	}
 
-	pltfm_host->clk = imx_data->clk_per;
-	pltfm_host->clock = clk_get_rate(pltfm_host->clk);
 	clk_prepare_enable(imx_data->clk_per);
 	clk_prepare_enable(imx_data->clk_ipg);
 	clk_prepare_enable(imx_data->clk_ahb);

@@ -1051,6 +1047,10 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 					host->mmc->parent->platform_data);
 	}
 
+	imx_data->f_max = clk_get_rate(imx_data->clk_per);
+	if (boarddata->f_max && boarddata->f_max < imx_data->f_max)
+		imx_data->f_max = boarddata->f_max;
+
 	/* write_protect */
 	if (boarddata->wp_type == ESDHC_WP_GPIO) {
 		err = mmc_gpio_request_ro(host->mmc, boarddata->wp_gpio);
Signature · 1 line
2.0.0.rc0
author maintainer
#3 lore raw

PATCH 1/5 mmc: sdhci-esdhc-imx: add f_max field to private data

Quoted 9 lines · Shawn Guo

drivers/mmc/host/sdhci-esdhc-imx.c static inline void esdhc_pltfm_set_clock(struct sdhci_host *host, · line 595

Quoted 6 lines
drivers/mmc/host/sdhci-esdhc-imx.c +1 -1
@@ -594,7 +592,7 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	unsigned int host_clock = pltfm_host->clock;
+	unsigned int host_clock = clk_get_rate(imx_data->clk_per);

Will this bring the issue that commit a974862faee1 (mmc: sdhci-esdhc-imx:
fix access hardirq-unsafe lock in atomic context) fixed back to us?

Probably, yes, because the ?%$&"$$ sdhc driver still uses spinlocks.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
author maintainer
#4 lore raw

PATCH 2/5 mmc: sdhci-esdhc-imx: introduce function for parsing platform_data

In reply to Sascha Hauer

As a first step to separate the platform_data code from the devicetree code add a function to parse the boarddata. Also change the argument of the dt probe function from struct esdhc_platform_data * to struct pltfm_imx_data *imx_data which makes it possible to get struct esdhc_platform_data out of the devicetree probing path.

Signed-off-by Sascha Hauer <[email protected]>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +22 -9
index d530820..f6f7a45 100644
@@ -901,8 +901,9 @@ static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
 #ifdef CONFIG_OF
 static int
 sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
-			 struct esdhc_platform_data *boarddata)
+			 struct pltfm_imx_data *imx_data)
 {
+	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 	struct device_node *np = pdev->dev.of_node;
 
 	if (!np)

@@ -942,12 +943,28 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 #else
 static inline int
 sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
-			 struct esdhc_platform_data *boarddata)
+			 struct pltfm_imx_data *imx_data)
 {
 	return -ENODEV;
 }
 #endif
 
+static int
+sdhci_esdhc_imx_probe_boarddata(struct platform_device *pdev,
+			 struct pltfm_imx_data *imx_data)
+{
+	struct esdhc_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "no boarddata\n");
+		return -EINVAL;
+	}
+
+	imx_data->boarddata = *pdata;
+
+	return 0;
+}
+
 static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =

@@ -1037,14 +1054,10 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 			host->ioaddr + ESDHC_TUNING_CTRL);
 
 	boarddata = &imx_data->boarddata;
-	if (sdhci_esdhc_imx_probe_dt(pdev, boarddata) < 0) {
-		if (!host->mmc->parent->platform_data) {
-			dev_err(mmc_dev(host->mmc), "no board data!\n");
-			err = -EINVAL;
+	if (sdhci_esdhc_imx_probe_dt(pdev, imx_data) < 0) {
+		err = sdhci_esdhc_imx_probe_boarddata(pdev, imx_data);
+		if (err)
 			goto disable_clk;
-		}
-		imx_data->boarddata = *((struct esdhc_platform_data *)
-					host->mmc->parent->platform_data);
 	}
 
 	imx_data->f_max = clk_get_rate(imx_data->clk_per);
Signature · 1 line
2.0.0.rc0
author maintainer
#5 lore raw

PATCH 3/5 mmc: sdhci-esdhc-imx: straighten SDHCI_QUIRK_BROKEN_CARD_DETECTION flag

In reply to Sascha Hauer

The driver always sets the flag and clears it afterwards in some cases. Instead, leave the flag cleared and set it when needed which makes the code easier to read.

Signed-off-by Sascha Hauer <[email protected]>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +4 -5
index f6f7a45..620cbcd 100644
@@ -893,8 +893,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
 	.quirks = ESDHC_DEFAULT_QUIRKS | SDHCI_QUIRK_NO_HISPD_BIT
 			| SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC
-			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC
-			| SDHCI_QUIRK_BROKEN_CARD_DETECTION,
+			| SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
 	.ops = &sdhci_esdhc_ops,
 };
 

@@ -1084,18 +1083,18 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 				"failed to request card-detect gpio!\n");
 			goto disable_clk;
 		}
-		/* fall through */
+		break;
 
 	case ESDHC_CD_CONTROLLER:
-		/* we have a working card_detect back */
-		host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 		break;
 
 	case ESDHC_CD_PERMANENT:
 		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 		break;
 
 	case ESDHC_CD_NONE:
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 		break;
 	}
Signature · 1 line
2.0.0.rc0
author maintainer
#6 lore raw

PATCH 4/5 mmc: sdhci-esdhc-imx: use mmc_of_parse

In reply to Sascha Hauer

mmc_of_parse allows us to use the generic devicetree bindings instead of parsing the properties manually.
To do so we have to refactor the driver. The driver currently parses the dt properties into platform_data which is then translated into struct mmc_host caps. This changes the driver so that it
- parses dt data into mmc_host caps directly via mmc_of_parse
- parses platform_data into mmc_host caps directly
- parses the flags for which no mmc_host caps exist into driver private data
This makes platform_data used for its original purpose and irrelevant for the devicetree case.

Signed-off-by Sascha Hauer <[email protected]>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 175 +++++++++++++++++++------------------
 1 file changed, 90 insertions(+), 85 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +90 -85
index 620cbcd..8c0af44 100644
@@ -151,8 +151,9 @@ struct pltfm_imx_data {
 	struct pinctrl_state *pins_100mhz;
 	struct pinctrl_state *pins_200mhz;
 	const struct esdhc_soc_data *socdata;
-	struct esdhc_platform_data boarddata;
 	unsigned long f_max;
+	unsigned int delay_line;
+	bool support_vsel;
 	struct clk *clk_ipg;
 	struct clk *clk_ahb;
 	struct clk *clk_per;

@@ -161,6 +162,7 @@ struct pltfm_imx_data {
 		MULTIBLK_IN_PROCESS, /* exact multiblock cmd in process */
 		WAIT_FOR_INT,        /* sent CMD12, waiting for response INT */
 	} multiblock_status;
+	enum wp_types wp_type;
 	u32 uhs_mode;
 	u32 is_ddr;
 };

@@ -651,9 +653,8 @@ static unsigned int esdhc_pltfm_get_ro(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
-	switch (boarddata->wp_type) {
+	switch (imx_data->wp_type) {
 	case ESDHC_WP_GPIO:
 		return mmc_gpio_get_ro(host->mmc);
 	case ESDHC_WP_CONTROLLER:

@@ -839,7 +840,6 @@ static int esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
 	switch (uhs) {
 	case MMC_TIMING_UHS_SDR12:

@@ -861,9 +861,9 @@ static int esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned int uhs)
 				ESDHC_MIX_CTRL_DDREN,
 				host->ioaddr + ESDHC_MIX_CTRL);
 		imx_data->is_ddr = 1;
-		if (boarddata->delay_line) {
+		if (imx_data->delay_line) {
 			u32 v;
-			v = boarddata->delay_line <<
+			v = imx_data->delay_line <<
 				ESDHC_DLL_OVERRIDE_VAL_SHIFT |
 				(1 << ESDHC_DLL_OVERRIDE_EN_SHIFT);
 			if (is_imx53_esdhc(imx_data))

@@ -900,49 +900,50 @@ static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
 #ifdef CONFIG_OF
 static int
 sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
-			 struct pltfm_imx_data *imx_data)
+			 struct sdhci_host *host, struct pltfm_imx_data *imx_data)
 {
-	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 	struct device_node *np = pdev->dev.of_node;
+	int ret;
 
 	if (!np)
 		return -ENODEV;
 
-	if (of_get_property(np, "non-removable", NULL))
-		boarddata->cd_type = ESDHC_CD_PERMANENT;
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		return ret;
 
-	if (of_get_property(np, "fsl,cd-controller", NULL))
-		boarddata->cd_type = ESDHC_CD_CONTROLLER;
+	if (host->mmc->caps & MMC_CAP_NONREMOVABLE)
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 
 	if (of_get_property(np, "fsl,wp-controller", NULL))
-		boarddata->wp_type = ESDHC_WP_CONTROLLER;
+		imx_data->wp_type = ESDHC_WP_CONTROLLER;
 
-	boarddata->cd_gpio = of_get_named_gpio(np, "cd-gpios", 0);
-	if (gpio_is_valid(boarddata->cd_gpio))
-		boarddata->cd_type = ESDHC_CD_GPIO;
+	if (mmc_gpio_get_ro(host->mmc) >= 0)
+		imx_data->wp_type = ESDHC_WP_GPIO;
 
-	boarddata->wp_gpio = of_get_named_gpio(np, "wp-gpios", 0);
-	if (gpio_is_valid(boarddata->wp_gpio))
-		boarddata->wp_type = ESDHC_WP_GPIO;
+	/* This flag only means: Do not set the MMC_CAP_4_BIT_DATA flag */
+	host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
 
-	of_property_read_u32(np, "bus-width", &boarddata->max_bus_width);
-
-	of_property_read_u32(np, "max-frequency", &boarddata->f_max);
+	/*
+	 * mmc_of_parse sets host->mmc->f_max, but it gets overwritten
+	 * by the sdhci driver later, so we have to save the value
+	 */
+	imx_data->f_max = host->mmc->f_max;
 
 	if (of_find_property(np, "no-1-8-v", NULL))
-		boarddata->support_vsel = false;
+		imx_data->support_vsel = false;
 	else
-		boarddata->support_vsel = true;
+		imx_data->support_vsel = true;
 
-	if (of_property_read_u32(np, "fsl,delay-line", &boarddata->delay_line))
-		boarddata->delay_line = 0;
+	if (of_property_read_u32(np, "fsl,delay-line", &imx_data->delay_line))
+		imx_data->delay_line = 0;
 
 	return 0;
 }
 #else
 static inline int
 sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
-			 struct pltfm_imx_data *imx_data)
+			 struct sdhci_host *host, struct pltfm_imx_data *imx_data)
 {
 	return -ENODEV;
 }

@@ -950,16 +951,67 @@ sdhci_esdhc_imx_probe_dt(struct platform_device *pdev,
 
 static int
 sdhci_esdhc_imx_probe_boarddata(struct platform_device *pdev,
-			 struct pltfm_imx_data *imx_data)
+			 struct sdhci_host *host, struct pltfm_imx_data *imx_data)
 {
 	struct esdhc_platform_data *pdata = pdev->dev.platform_data;
+	int err;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "no boarddata\n");
 		return -EINVAL;
 	}
 
-	imx_data->boarddata = *pdata;
+	/* card_detect */
+	switch (pdata->cd_type) {
+	case ESDHC_CD_GPIO:
+		err = mmc_gpio_request_cd(host->mmc, pdata->cd_gpio, 0);
+		if (err) {
+			dev_err(mmc_dev(host->mmc),
+				"failed to request card-detect gpio!\n");
+			return err;
+		}
+		break;
+
+	case ESDHC_CD_CONTROLLER:
+		break;
+
+	case ESDHC_CD_PERMANENT:
+		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+		break;
+
+	case ESDHC_CD_NONE:
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+		break;
+	}
+
+	/* write_protect */
+	imx_data->wp_type = pdata->wp_type;
+	if (pdata->wp_type == ESDHC_WP_GPIO) {
+		err = mmc_gpio_request_ro(host->mmc, pdata->wp_gpio);
+		if (err) {
+			dev_err(mmc_dev(host->mmc),
+				"failed to request write-protect gpio!\n");
+			return err;
+		}
+		host->mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+	}
+
+	switch (pdata->max_bus_width) {
+	case 8:
+		host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_4_BIT_DATA;
+		break;
+	case 4:
+		host->mmc->caps |= MMC_CAP_4_BIT_DATA;
+		break;
+	case 1:
+	default:
+		host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
+		break;
+	}
+
+	imx_data->delay_line = pdata->delay_line;
+	imx_data->support_vsel = pdata->support_vsel;
 
 	return 0;
 }

@@ -970,9 +1022,9 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 			of_match_device(imx_esdhc_dt_ids, &pdev->dev);
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_host *host;
-	struct esdhc_platform_data *boarddata;
 	int err;
 	struct pltfm_imx_data *imx_data;
+	unsigned long f_max;
 
 	host = sdhci_pltfm_init(pdev, &sdhci_esdhc_imx_pdata, 0);
 	if (IS_ERR(host))

@@ -1052,67 +1104,20 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 			ESDHC_STD_TUNING_EN | ESDHC_TUNING_START_TAP,
 			host->ioaddr + ESDHC_TUNING_CTRL);
 
-	boarddata = &imx_data->boarddata;
-	if (sdhci_esdhc_imx_probe_dt(pdev, imx_data) < 0) {
-		err = sdhci_esdhc_imx_probe_boarddata(pdev, imx_data);
+	if (sdhci_esdhc_imx_probe_dt(pdev, host, imx_data) < 0) {
+		err = sdhci_esdhc_imx_probe_boarddata(pdev, host, imx_data);
 		if (err)
 			goto disable_clk;
 	}
 
-	imx_data->f_max = clk_get_rate(imx_data->clk_per);
-	if (boarddata->f_max && boarddata->f_max < imx_data->f_max)
-		imx_data->f_max = boarddata->f_max;
-
-	/* write_protect */
-	if (boarddata->wp_type == ESDHC_WP_GPIO) {
-		err = mmc_gpio_request_ro(host->mmc, boarddata->wp_gpio);
-		if (err) {
-			dev_err(mmc_dev(host->mmc),
-				"failed to request write-protect gpio!\n");
-			goto disable_clk;
-		}
-		host->mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
-	}
-
-	/* card_detect */
-	switch (boarddata->cd_type) {
-	case ESDHC_CD_GPIO:
-		err = mmc_gpio_request_cd(host->mmc, boarddata->cd_gpio, 0);
-		if (err) {
-			dev_err(mmc_dev(host->mmc),
-				"failed to request card-detect gpio!\n");
-			goto disable_clk;
-		}
-		break;
-
-	case ESDHC_CD_CONTROLLER:
-		break;
-
-	case ESDHC_CD_PERMANENT:
-		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
-		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
-		break;
-
-	case ESDHC_CD_NONE:
-		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
-		break;
-	}
-
-	switch (boarddata->max_bus_width) {
-	case 8:
-		host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_4_BIT_DATA;
-		break;
-	case 4:
-		host->mmc->caps |= MMC_CAP_4_BIT_DATA;
-		break;
-	case 1:
-	default:
-		host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
-		break;
-	}
+	f_max = clk_get_rate(imx_data->clk_per);
+	if (imx_data->f_max)
+		imx_data->f_max = min(f_max, imx_data->f_max);
+	else
+		imx_data->f_max = f_max;
 
 	/* sdr50 and sdr104 needs work on 1.8v signal voltage */
-	if ((boarddata->support_vsel) && esdhc_is_usdhc(imx_data)) {
+	if ((imx_data->support_vsel) && esdhc_is_usdhc(imx_data)) {
 		imx_data->pins_100mhz = pinctrl_lookup_state(imx_data->pinctrl,
 						ESDHC_PINCTRL_STATE_100MHZ);
 		imx_data->pins_200mhz = pinctrl_lookup_state(imx_data->pinctrl,
Signature · 1 line
2.0.0.rc0
author maintainer
#7 lore raw

PATCH 5/5 ARM: dts: imx51-babbage: Fix esdhc setup

In reply to Sascha Hauer

- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since the controller based detection does not work
- Fix cd gpio polarity for esdhc1. This is wrong and currently only works because the imx esdhc driver ignores the polarity.

Signed-off-by Sascha Hauer <[email protected]>
---
 arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
arch/arm/boot/dts/imx51-babbage.dts +7 -7
index 9e9deb2..28d4553 100644
@@ -134,15 +134,15 @@
 &esdhc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc1>;
-	fsl,cd-controller;
-	fsl,wp-controller;
+	cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
 
 &esdhc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc2>;
-	cd-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
+	cd-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
 	wp-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };

@@ -300,10 +300,6 @@
 	imx51-babbage {
 		pinctrl_hog: hoggrp {
 			fsl,pins = <
-				MX51_PAD_GPIO1_0__SD1_CD     0x20d5
-				MX51_PAD_GPIO1_1__SD1_WP     0x20d5
-				MX51_PAD_GPIO1_5__GPIO1_5    0x100
-				MX51_PAD_GPIO1_6__GPIO1_6    0x100
 				MX51_PAD_EIM_A27__GPIO2_21   0x5
 				MX51_PAD_CSPI1_SS0__GPIO4_24 0x85
 				MX51_PAD_CSPI1_SS1__GPIO4_25 0x85

@@ -330,6 +326,8 @@
 
 		pinctrl_esdhc1: esdhc1grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
+				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5
 				MX51_PAD_SD1_CMD__SD1_CMD		0x400020d5
 				MX51_PAD_SD1_CLK__SD1_CLK		0x20d5
 				MX51_PAD_SD1_DATA0__SD1_DATA0		0x20d5

@@ -341,6 +339,8 @@
 
 		pinctrl_esdhc2: esdhc2grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_5__GPIO1_5		0x100
+				MX51_PAD_GPIO1_6__GPIO1_6		0x100
 				MX51_PAD_SD2_CMD__SD2_CMD		0x400020d5
 				MX51_PAD_SD2_CLK__SD2_CLK		0x20d5
 				MX51_PAD_SD2_DATA0__SD2_DATA0		0x20d5
Signature · 1 line
2.0.0.rc0
#8 lore raw

PATCH 5/5 ARM: dts: imx51-babbage: Fix esdhc setup

Quoted 3 lines · Sascha Hauer

- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since the controller based detection does not work

I tracked it a little bit and found that the controller based detection works fine with v3.13 and starts being broken from v3.14-rc1. The offending commit seems to be 89d7e5c13122 (mmc: sdhci-esdhc-imx: add runtime pm support).

I will probably apply the patch as it is anyway. But I'm wondering if we should fix the regression and keep maintaining the support of controller based cd/wp in sdhci-esdhc-imx driver.

Shawn

Quoted 63 lines

- Fix cd gpio polarity for esdhc1. This is wrong and currently only works because the imx esdhc driver ignores the polarity.

Signed-off-by Sascha Hauer <[email protected]>
---
 arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
arch/arm/boot/dts/imx51-babbage.dts +7 -7
index 9e9deb2..28d4553 100644
@@ -134,15 +134,15 @@
 &esdhc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc1>;
-	fsl,cd-controller;
-	fsl,wp-controller;
+	cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
 
 &esdhc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc2>;
-	cd-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
+	cd-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
 	wp-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };

@@ -300,10 +300,6 @@
 	imx51-babbage {
 		pinctrl_hog: hoggrp {
 			fsl,pins = <
-				MX51_PAD_GPIO1_0__SD1_CD     0x20d5
-				MX51_PAD_GPIO1_1__SD1_WP     0x20d5
-				MX51_PAD_GPIO1_5__GPIO1_5    0x100
-				MX51_PAD_GPIO1_6__GPIO1_6    0x100
 				MX51_PAD_EIM_A27__GPIO2_21   0x5
 				MX51_PAD_CSPI1_SS0__GPIO4_24 0x85
 				MX51_PAD_CSPI1_SS1__GPIO4_25 0x85

@@ -330,6 +326,8 @@
 
 		pinctrl_esdhc1: esdhc1grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
+				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5
 				MX51_PAD_SD1_CMD__SD1_CMD		0x400020d5
 				MX51_PAD_SD1_CLK__SD1_CLK		0x20d5
 				MX51_PAD_SD1_DATA0__SD1_DATA0		0x20d5

@@ -341,6 +339,8 @@
 
 		pinctrl_esdhc2: esdhc2grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_5__GPIO1_5		0x100
+				MX51_PAD_GPIO1_6__GPIO1_6		0x100
 				MX51_PAD_SD2_CMD__SD2_CMD		0x400020d5
 				MX51_PAD_SD2_CLK__SD2_CLK		0x20d5
 				MX51_PAD_SD2_DATA0__SD2_DATA0		0x20d5
Signature · 1 line
2.0.0.rc0
author maintainer
#9 lore raw

PATCH 5/5 ARM: dts: imx51-babbage: Fix esdhc setup

Quoted 9 lines · Shawn Guo
Quoted 3 lines · Sascha Hauer

- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since the controller based detection does not work

I tracked it a little bit and found that the controller based detection works fine with v3.13 and starts being broken from v3.14-rc1. The offending commit seems to be 89d7e5c13122 (mmc: sdhci-esdhc-imx: add runtime pm support).

This makes sense. When the controller is disabled it obviously can't detect card insertion/removal events anymore. So you can only have one:
controller based card detection or runtime pm for the controller. So it probably makes sense to use gpio card detection whenever possible to get additional power savings.

Sascha

Signature · 4 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
#10 lore raw

PATCH 5/5 ARM: dts: imx51-babbage: Fix esdhc setup

Quoted 11 lines · Sascha Hauer
Quoted 9 lines · Shawn Guo
Quoted 3 lines · Sascha Hauer

- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since the controller based detection does not work

I tracked it a little bit and found that the controller based detection works fine with v3.13 and starts being broken from v3.14-rc1. The offending commit seems to be 89d7e5c13122 (mmc: sdhci-esdhc-imx: add runtime pm support).

Quoted 5 lines

This makes sense. When the controller is disabled it obviously can't detect card insertion/removal events anymore. So you can only have one:
controller based card detection or runtime pm for the controller. So it probably makes sense to use gpio card detection whenever possible to get additional power savings.

Can we have something like that in commit log to explain the breakage? Also, the patch does not apply to my branch, so you may need to rebase anyway.

Lothar, Denis,

FYI. The controller based card detection / write protection are broken right now, and I see your boards (imx53-tx53 and imx51-eukrea-mbimxsd51-baseboard) are using them. You may want to switch to use GPIO if available.

Shawn

#11 lore raw

PATCH 5/5 ARM: dts: imx51-babbage: Fix esdhc setup

In reply to Sascha Hauer
Quoted 46 lines · Sascha Hauer

- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since the controller based detection does not work
- Fix cd gpio polarity for esdhc1. This is wrong and currently only works because the imx esdhc driver ignores the polarity.

Signed-off-by Sascha Hauer <[email protected]>
---
 arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
arch/arm/boot/dts/imx51-babbage.dts +3 -7
index 9e9deb2..28d4553 100644
@@ -134,15 +134,15 @@
 &esdhc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc1>;
-	fsl,cd-controller;
-	fsl,wp-controller;
+	cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
 
 &esdhc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc2>;
-	cd-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
+	cd-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
 	wp-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };

@@ -300,10 +300,6 @@
 	imx51-babbage {
 		pinctrl_hog: hoggrp {
 			fsl,pins = <
-				MX51_PAD_GPIO1_0__SD1_CD     0x20d5
-				MX51_PAD_GPIO1_1__SD1_WP     0x20d5
-				MX51_PAD_GPIO1_5__GPIO1_5    0x100
-				MX51_PAD_GPIO1_6__GPIO1_6    0x100
 				MX51_PAD_EIM_A27__GPIO2_21   0x5
 				MX51_PAD_CSPI1_SS0__GPIO4_24 0x85
 				MX51_PAD_CSPI1_SS1__GPIO4_25 0x85

arch/arm/boot/dts/imx51-babbage.dts line 330

Quoted 5 lines
arch/arm/boot/dts/imx51-babbage.dts +2 -0
@@ -330,6 +326,8 @@
 
 		pinctrl_esdhc1: esdhc1grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
+				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5

The pad config value 0x20d5 still makes sense here when switching to GPIO function? Probably should be 0x100 like what we have in esdhc2grp?

Shawn

Quoted 15 lines
arch/arm/boot/dts/imx51-babbage.dts +2 -0
 				MX51_PAD_SD1_CMD__SD1_CMD		0x400020d5
 				MX51_PAD_SD1_CLK__SD1_CLK		0x20d5
 				MX51_PAD_SD1_DATA0__SD1_DATA0		0x20d5

@@ -341,6 +339,8 @@
 
 		pinctrl_esdhc2: esdhc2grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_5__GPIO1_5		0x100
+				MX51_PAD_GPIO1_6__GPIO1_6		0x100
 				MX51_PAD_SD2_CMD__SD2_CMD		0x400020d5
 				MX51_PAD_SD2_CLK__SD2_CLK		0x20d5
 				MX51_PAD_SD2_DATA0__SD2_DATA0		0x20d5
Signature · 1 line
2.0.0.rc0
#12 lore raw

PATCH 5/5 ARM: dts: imx51-babbage: Fix esdhc setup

Hi,

Quoted 7 lines · Shawn Guo
Quoted 6 lines · Sascha Hauer

- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since the controller based detection does not work
- Fix cd gpio polarity for esdhc1. This is wrong and currently only works because the imx esdhc driver ignores the polarity.

[...]

Quoted 10 lines

arch/arm/boot/dts/imx51-babbage.dts line 330

Quoted 6 lines
arch/arm/boot/dts/imx51-babbage.dts +2 -0
@@ -330,6 +326,8 @@
 
 		pinctrl_esdhc1: esdhc1grp {
 			fsl,pins = <
+				MX51_PAD_GPIO1_0__GPIO1_0		0x20d5
+				MX51_PAD_GPIO1_1__GPIO1_1		0x20d5

The pad config value 0x20d5 still makes sense here when switching to GPIO function? Probably should be 0x100 like what we have in esdhc2grp?

The pad config 0x20d5 doesn't make any sense at all because according to the Ref. Manual only bits 0..2 and 4..8 (mask 0x01f7) are defined for those pads. Thus 0x2000 doesn't serve any purpose at all.

Lothar Wa?mann

Signature · 9 lines
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
#13 lore raw

PATCH 5/5 ARM: dts: imx51-babbage: Fix esdhc setup

In reply to Sascha Hauer
Quoted 7 lines · Sascha Hauer

- Move cd/wp pinctrl from the hog group to the esdhc groups
- use gpio for card detection / write protection on esdhc2 since the controller based detection does not work
- Fix cd gpio polarity for esdhc1. This is wrong and currently only works because the imx esdhc driver ignores the polarity.

Signed-off-by Sascha Hauer <[email protected]>

Applied it after fixing up the commit log and pad value.

Shawn