[PATCH] MMC: use mmc_of_parse in sdhci-esdhc-imx

[PATCH] MMC: use mmc_of_parse in sdhci-esdhc-imx

[Sascha Hauer]

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(-)

[PATCH] MMC: use mmc_of_parse in sdhci-esdhc-imx

[Sascha Hauer]

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(-)

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index b841bb7..d530820 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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);
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index b841bb7..d530820 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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);
-- 
2.0.0.rc0

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:00PM +0200, Sascha Hauer wrote:
> 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 <s.hauer@pengutronix.de>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index b841bb7..d530820 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -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);

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

>  	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);
> -- 
> 2.0.0.rc0
> 

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:00PM +0200, Sascha Hauer wrote:
> 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 <s.hauer@pengutronix.de>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index b841bb7..d530820 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -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);

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

>  	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);
> -- 
> 2.0.0.rc0
> 

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

[Sascha Hauer]

On Sun, Jun 01, 2014 at 03:45:43PM +0800, Shawn Guo wrote:
> > @@ -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

-- 
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 |

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

[Sascha Hauer]

On Sun, Jun 01, 2014 at 03:45:43PM +0800, Shawn Guo wrote:
> > @@ -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

-- 
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 |

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index d530820..f6f7a45 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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);
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index d530820..f6f7a45 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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);
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f6f7a45..620cbcd 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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;
 	}
 
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index f6f7a45..620cbcd 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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;
 	}
 
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 175 +++++++++++++++++++------------------
 1 file changed, 90 insertions(+), 85 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 620cbcd..8c0af44 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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,
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 175 +++++++++++++++++++------------------
 1 file changed, 90 insertions(+), 85 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 620cbcd..8c0af44 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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,
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index 9e9deb2..28d4553 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -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
-- 
2.0.0.rc0

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

[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 <s.hauer@pengutronix.de>
---
 arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index 9e9deb2..28d4553 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -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
-- 
2.0.0.rc0

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - 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

> - 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 <s.hauer@pengutronix.de>
> ---
>  arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index 9e9deb2..28d4553 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -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
> -- 
> 2.0.0.rc0
> 

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - 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

> - 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 <s.hauer@pengutronix.de>
> ---
>  arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index 9e9deb2..28d4553 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -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
> -- 
> 2.0.0.rc0
> 

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

[Sascha Hauer]

On Sun, Jun 01, 2014 at 11:22:19PM +0800, Shawn Guo wrote:
> On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > - 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

-- 
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 |

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

[Sascha Hauer]

On Sun, Jun 01, 2014 at 11:22:19PM +0800, Shawn Guo wrote:
> On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > - 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

-- 
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 |

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

[Shawn Guo]

On Thu, Jun 05, 2014 at 12:39:49PM +0200, Sascha Hauer wrote:
> On Sun, Jun 01, 2014 at 11:22:19PM +0800, Shawn Guo wrote:
> > On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > > - 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.

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

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

[Shawn Guo]

On Thu, Jun 05, 2014 at 12:39:49PM +0200, Sascha Hauer wrote:
> On Sun, Jun 01, 2014 at 11:22:19PM +0800, Shawn Guo wrote:
> > On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > > - 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.

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

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - 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 <s.hauer@pengutronix.de>
> ---
>  arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index 9e9deb2..28d4553 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -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

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

>  				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
> -- 
> 2.0.0.rc0
> 

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - 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 <s.hauer@pengutronix.de>
> ---
>  arch/arm/boot/dts/imx51-babbage.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> index 9e9deb2..28d4553 100644
> --- a/arch/arm/boot/dts/imx51-babbage.dts
> +++ b/arch/arm/boot/dts/imx51-babbage.dts
> @@ -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

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

>  				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
> -- 
> 2.0.0.rc0
> 

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

[Lothar Waßmann]

Hi,

Shawn Guo wrote:
> On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > - 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.
> > 
[...]
> > @@ -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
-- 
___________________________________________________________

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@karo-electronics.de
___________________________________________________________

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

[Lothar Waßmann]

Hi,

Shawn Guo wrote:
> On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> > - 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.
> > 
[...]
> > @@ -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
-- 
___________________________________________________________

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
___________________________________________________________

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - 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 <s.hauer@pengutronix.de>

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

Shawn

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

[Shawn Guo]

On Fri, May 23, 2014 at 02:33:04PM +0200, Sascha Hauer wrote:
> - 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 <s.hauer@pengutronix.de>

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

Shawn