[PATCH 1/1]sdhci-pxa: support tune_timming for various cards

[PATCH 1/1]sdhci-pxa: support tune_timming for various cards

[zhangfei gao]

[-- Attachment #1: Type: text/plain, Size: 4367 bytes --]

Haojian,

Would you help review, since arch/arm/plat-pxa/include/plat/sdhci.h is
updated, thanks

>From 6619812b749aa12ebf0efbe057366c2f99051bfe Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Tue, 2 Nov 2010 07:37:44 -0400
Subject: [PATCH] sdhci-pxa: support tune_timming for various cards

	1. Add pdata check, in case pdata is NULL
	2. Add tune_timming to adjust read data/command timming without
performance impact when crc error, as a result
		a, sd could work at 50M
		b, emmc could work at ddr50 mode
	3. Remove clock_enable checking, since clock gating is still on-going
in core stack.

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |    2 +
 drivers/mmc/host/sdhci-pxa.c           |   51 +++++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
b/arch/arm/plat-pxa/include/plat/sdhci.h
index fc5ceab..8c340b1 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -24,11 +24,13 @@
  * @max_speed: the maximum speed supported
  * @quirks: quirks of specific device
  * @flags: flags for platform requirement
+ * @clk_delay: clk_delay vlaue to tunning various cards
  */
 struct sdhci_pxa_platdata {
 	unsigned int	max_speed;
 	unsigned int	quirks;
 	unsigned int	flags;
+	unsigned int	clk_delay;
 };

 #endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
index 8455c46..cf514f8 100644
--- a/drivers/mmc/host/sdhci-pxa.c
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -32,6 +32,11 @@
 #define SD_FIFO_PARAM		0x104
 #define DIS_PAD_SD_CLK_GATE	0x400

+#define SD_CLOCK_AND_BURST_SIZE_SETUP		0x10A
+#define SDCLK_SEL	0x100
+#define SDCLK_DELAY_SHIFT	9
+#define SDCLK_DELAY_MASK	0x1f
+
 struct sdhci_pxa {
 	struct sdhci_host		*host;
 	struct sdhci_pxa_platdata	*pdata;
@@ -46,6 +51,26 @@ struct sdhci_pxa {
  * SDHCI core callbacks                                                      *
  *                                                                           *
 \*****************************************************************************/
+static void tune_timming(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	/*
+	 * tune timming of read data/command when crc error happen
+	 * No perfermance impact
+	 */
+	if (pxa->pdata && 0 != pxa->pdata->clk_delay) {
+		u16 tmp;
+
+		tmp = readw(host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
+		tmp |= (pxa->pdata->clk_delay & SDCLK_DELAY_MASK)
+					<< SDCLK_DELAY_SHIFT;
+		tmp |= SDCLK_SEL;
+		writew(tmp, host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
+	}
+
+}
+
 static void set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pxa *pxa = sdhci_priv(host);
@@ -57,15 +82,16 @@ static void set_clock(struct sdhci_host *host,
unsigned int clock)
 			pxa->clk_enable = 0;
 		}
 	} else {
-		if (0 == pxa->clk_enable) {
-			if (pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
-				tmp = readl(host->ioaddr + SD_FIFO_PARAM);
-				tmp |= DIS_PAD_SD_CLK_GATE;
-				writel(tmp, host->ioaddr + SD_FIFO_PARAM);
-			}
-			clk_enable(pxa->clk);
-			pxa->clk_enable = 1;
+		tune_timming(host);
+		if (pxa->pdata &&
+			pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
+
+			tmp = readl(host->ioaddr + SD_FIFO_PARAM);
+			tmp |= DIS_PAD_SD_CLK_GATE;
+			writel(tmp, host->ioaddr + SD_FIFO_PARAM);
 		}
+		clk_enable(pxa->clk);
+		pxa->clk_enable = 1;
 	}
 }

@@ -138,13 +164,13 @@ static int __devinit sdhci_pxa_probe(struct
platform_device *pdev)
 	host->irq = irq;
 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;

-	if (pxa->pdata->flags & PXA_FLAG_CARD_PERMANENT) {
+	if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
 		/* on-chip device */
 		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
 	}

-	if (pdata->quirks)
+	if (pdata && pdata->quirks)
 		host->quirks |= pdata->quirks;

 	ret = sdhci_add_host(host);
@@ -153,9 +179,8 @@ static int __devinit sdhci_pxa_probe(struct
platform_device *pdev)
 		goto out;
 	}

-	if (pxa->pdata->max_speed)
-		host->mmc->f_max = pxa->pdata->max_speed;
-
+	if (pdata && pdata->max_speed)
+		host->mmc->f_max = pdata->max_speed;
 	platform_set_drvdata(pdev, host);

 	return 0;
-- 
1.7.0.4

[-- Attachment #2: 0001-sdhci-pxa-support-tune_timming-for-various-cards.patch --]
[-- Type: text/x-patch, Size: 4277 bytes --]

From 6619812b749aa12ebf0efbe057366c2f99051bfe Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Tue, 2 Nov 2010 07:37:44 -0400
Subject: [PATCH] sdhci-pxa: support tune_timming for various cards

	1. Add pdata check, in case pdata is NULL
	2. Add tune_timming to adjust read data/command timming without performance impact when crc error, as a result
		a, sd could work at 50M
		b, emmc could work at ddr50 mode
	3. Remove clock_enable checking, since clock gating is still on-going in core stack.

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |    2 +
 drivers/mmc/host/sdhci-pxa.c           |   51 +++++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h b/arch/arm/plat-pxa/include/plat/sdhci.h
index fc5ceab..8c340b1 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -24,11 +24,13 @@
  * @max_speed: the maximum speed supported
  * @quirks: quirks of specific device
  * @flags: flags for platform requirement
+ * @clk_delay: clk_delay vlaue to tunning various cards
  */
 struct sdhci_pxa_platdata {
 	unsigned int	max_speed;
 	unsigned int	quirks;
 	unsigned int	flags;
+	unsigned int	clk_delay;
 };
 
 #endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
index 8455c46..cf514f8 100644
--- a/drivers/mmc/host/sdhci-pxa.c
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -32,6 +32,11 @@
 #define SD_FIFO_PARAM		0x104
 #define DIS_PAD_SD_CLK_GATE	0x400
 
+#define SD_CLOCK_AND_BURST_SIZE_SETUP		0x10A
+#define SDCLK_SEL	0x100
+#define SDCLK_DELAY_SHIFT	9
+#define SDCLK_DELAY_MASK	0x1f
+
 struct sdhci_pxa {
 	struct sdhci_host		*host;
 	struct sdhci_pxa_platdata	*pdata;
@@ -46,6 +51,26 @@ struct sdhci_pxa {
  * SDHCI core callbacks                                                      *
  *                                                                           *
 \*****************************************************************************/
+static void tune_timming(struct sdhci_host *host)
+{
+	struct sdhci_pxa *pxa = sdhci_priv(host);
+
+	/*
+	 * tune timming of read data/command when crc error happen
+	 * No perfermance impact
+	 */
+	if (pxa->pdata && 0 != pxa->pdata->clk_delay) {
+		u16 tmp;
+
+		tmp = readw(host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
+		tmp |= (pxa->pdata->clk_delay & SDCLK_DELAY_MASK)
+					<< SDCLK_DELAY_SHIFT;
+		tmp |= SDCLK_SEL;
+		writew(tmp, host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
+	}
+
+}
+
 static void set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pxa *pxa = sdhci_priv(host);
@@ -57,15 +82,16 @@ static void set_clock(struct sdhci_host *host, unsigned int clock)
 			pxa->clk_enable = 0;
 		}
 	} else {
-		if (0 == pxa->clk_enable) {
-			if (pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
-				tmp = readl(host->ioaddr + SD_FIFO_PARAM);
-				tmp |= DIS_PAD_SD_CLK_GATE;
-				writel(tmp, host->ioaddr + SD_FIFO_PARAM);
-			}
-			clk_enable(pxa->clk);
-			pxa->clk_enable = 1;
+		tune_timming(host);
+		if (pxa->pdata &&
+			pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
+
+			tmp = readl(host->ioaddr + SD_FIFO_PARAM);
+			tmp |= DIS_PAD_SD_CLK_GATE;
+			writel(tmp, host->ioaddr + SD_FIFO_PARAM);
 		}
+		clk_enable(pxa->clk);
+		pxa->clk_enable = 1;
 	}
 }
 
@@ -138,13 +164,13 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
 	host->irq = irq;
 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
-	if (pxa->pdata->flags & PXA_FLAG_CARD_PERMANENT) {
+	if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
 		/* on-chip device */
 		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
 	}
 
-	if (pdata->quirks)
+	if (pdata && pdata->quirks)
 		host->quirks |= pdata->quirks;
 
 	ret = sdhci_add_host(host);
@@ -153,9 +179,8 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	if (pxa->pdata->max_speed)
-		host->mmc->f_max = pxa->pdata->max_speed;
-
+	if (pdata && pdata->max_speed)
+		host->mmc->f_max = pdata->max_speed;
 	platform_set_drvdata(pdev, host);
 
 	return 0;
-- 
1.7.0.4

Re: [PATCH 1/1]sdhci-pxa: support tune_timming for various cards

[Chris Ball]

Hi,

Here are some obvious changes:

On Tue, Nov 09, 2010 at 09:17:21AM -0500, zhangfei gao wrote:
> Haojian,
> 
> Would you help review, since arch/arm/plat-pxa/include/plat/sdhci.h is
> updated, thanks
> 
> >From 6619812b749aa12ebf0efbe057366c2f99051bfe Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Tue, 2 Nov 2010 07:37:44 -0400
> Subject: [PATCH] sdhci-pxa: support tune_timming for various cards
> 
> 	1. Add pdata check, in case pdata is NULL
> 	2. Add tune_timming to adjust read data/command timming without
> performance impact when crc error, as a result
> 		a, sd could work at 50M
> 		b, emmc could work at ddr50 mode

timming -> timing, throughout the patch

> 	3. Remove clock_enable checking, since clock gating is still on-going
> in core stack.
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
>  arch/arm/plat-pxa/include/plat/sdhci.h |    2 +
>  drivers/mmc/host/sdhci-pxa.c           |   51 +++++++++++++++++++++++--------
>  2 files changed, 40 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> index fc5ceab..8c340b1 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -24,11 +24,13 @@
>   * @max_speed: the maximum speed supported
>   * @quirks: quirks of specific device
>   * @flags: flags for platform requirement
> + * @clk_delay: clk_delay vlaue to tunning various cards

vlaue -> value
tunning -> tuning

>   */
>  struct sdhci_pxa_platdata {
>  	unsigned int	max_speed;
>  	unsigned int	quirks;
>  	unsigned int	flags;
> +	unsigned int	clk_delay;
>  };
> 
>  #endif /* __PLAT_PXA_SDHCI_H */
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> index 8455c46..cf514f8 100644
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -32,6 +32,11 @@
>  #define SD_FIFO_PARAM		0x104
>  #define DIS_PAD_SD_CLK_GATE	0x400
> 
> +#define SD_CLOCK_AND_BURST_SIZE_SETUP		0x10A
> +#define SDCLK_SEL	0x100
> +#define SDCLK_DELAY_SHIFT	9
> +#define SDCLK_DELAY_MASK	0x1f
> +
>  struct sdhci_pxa {
>  	struct sdhci_host		*host;
>  	struct sdhci_pxa_platdata	*pdata;
> @@ -46,6 +51,26 @@ struct sdhci_pxa {
>   * SDHCI core callbacks                                                      *
>   *                                                                           *
>  \*****************************************************************************/
> +static void tune_timming(struct sdhci_host *host)
> +{
> +	struct sdhci_pxa *pxa = sdhci_priv(host);
> +
> +	/*
> +	 * tune timming of read data/command when crc error happen
> +	 * No perfermance impact

perfermance -> performance

> +	 */
> +	if (pxa->pdata && 0 != pxa->pdata->clk_delay) {
> +		u16 tmp;
> +
> +		tmp = readw(host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
> +		tmp |= (pxa->pdata->clk_delay & SDCLK_DELAY_MASK)
> +					<< SDCLK_DELAY_SHIFT;
> +		tmp |= SDCLK_SEL;
> +		writew(tmp, host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
> +	}
> +
> +}
> +
>  static void set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>  	struct sdhci_pxa *pxa = sdhci_priv(host);
> @@ -57,15 +82,16 @@ static void set_clock(struct sdhci_host *host,
> unsigned int clock)
>  			pxa->clk_enable = 0;
>  		}
>  	} else {
> -		if (0 == pxa->clk_enable) {
> -			if (pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
> -				tmp = readl(host->ioaddr + SD_FIFO_PARAM);
> -				tmp |= DIS_PAD_SD_CLK_GATE;
> -				writel(tmp, host->ioaddr + SD_FIFO_PARAM);
> -			}
> -			clk_enable(pxa->clk);
> -			pxa->clk_enable = 1;
> +		tune_timming(host);

It looks like tune_timing() is a void function that only gets called
once.  Perhaps it should just happen inline here instead?

> +		if (pxa->pdata &&
> +			pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
> +
> +			tmp = readl(host->ioaddr + SD_FIFO_PARAM);
> +			tmp |= DIS_PAD_SD_CLK_GATE;
> +			writel(tmp, host->ioaddr + SD_FIFO_PARAM);
>  		}
> +		clk_enable(pxa->clk);
> +		pxa->clk_enable = 1;
>  	}
>  }
> 
> @@ -138,13 +164,13 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
>  	host->irq = irq;
>  	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> 
> -	if (pxa->pdata->flags & PXA_FLAG_CARD_PERMANENT) {
> +	if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
>  		/* on-chip device */
>  		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>  		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>  	}
> 
> -	if (pdata->quirks)
> +	if (pdata && pdata->quirks)
>  		host->quirks |= pdata->quirks;
> 
>  	ret = sdhci_add_host(host);
> @@ -153,9 +179,8 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
>  		goto out;
>  	}
> 
> -	if (pxa->pdata->max_speed)
> -		host->mmc->f_max = pxa->pdata->max_speed;
> -
> +	if (pdata && pdata->max_speed)
> +		host->mmc->f_max = pdata->max_speed;
>  	platform_set_drvdata(pdev, host);

No reason to remove the newline here.

> 
>  	return 0;
> -- 

Thanks,

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 1/1]sdhci-pxa: support tune_timming for various cards

[Eric Miao]

On Tue, Nov 9, 2010 at 10:17 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
> Haojian,
>
> Would you help review, since arch/arm/plat-pxa/include/plat/sdhci.h is
> updated, thanks
>
> From 6619812b749aa12ebf0efbe057366c2f99051bfe Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Tue, 2 Nov 2010 07:37:44 -0400
> Subject: [PATCH] sdhci-pxa: support tune_timming for various cards
>
>        1. Add pdata check, in case pdata is NULL
>        2. Add tune_timming to adjust read data/command timming without
> performance impact when crc error, as a result
>                a, sd could work at 50M
>                b, emmc could work at ddr50 mode
>        3. Remove clock_enable checking, since clock gating is still on-going
> in core stack.
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
>  arch/arm/plat-pxa/include/plat/sdhci.h |    2 +
>  drivers/mmc/host/sdhci-pxa.c           |   51 +++++++++++++++++++++++--------
>  2 files changed, 40 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> index fc5ceab..8c340b1 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -24,11 +24,13 @@
>  * @max_speed: the maximum speed supported
>  * @quirks: quirks of specific device
>  * @flags: flags for platform requirement
> + * @clk_delay: clk_delay vlaue to tunning various cards
>  */
>  struct sdhci_pxa_platdata {
>        unsigned int    max_speed;
>        unsigned int    quirks;
>        unsigned int    flags;
> +       unsigned int    clk_delay;
>  };
>
>  #endif /* __PLAT_PXA_SDHCI_H */
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> index 8455c46..cf514f8 100644
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -32,6 +32,11 @@
>  #define SD_FIFO_PARAM          0x104
>  #define DIS_PAD_SD_CLK_GATE    0x400
>
> +#define SD_CLOCK_AND_BURST_SIZE_SETUP          0x10A
> +#define SDCLK_SEL      0x100
> +#define SDCLK_DELAY_SHIFT      9
> +#define SDCLK_DELAY_MASK       0x1f
> +
>  struct sdhci_pxa {
>        struct sdhci_host               *host;
>        struct sdhci_pxa_platdata       *pdata;
> @@ -46,6 +51,26 @@ struct sdhci_pxa {
>  * SDHCI core callbacks                                                      *
>  *                                                                           *
>  \*****************************************************************************/
> +static void tune_timming(struct sdhci_host *host)
> +{
> +       struct sdhci_pxa *pxa = sdhci_priv(host);
> +
> +       /*
> +        * tune timming of read data/command when crc error happen
> +        * No perfermance impact
> +        */
> +       if (pxa->pdata && 0 != pxa->pdata->clk_delay) {
> +               u16 tmp;
> +
> +               tmp = readw(host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
> +               tmp |= (pxa->pdata->clk_delay & SDCLK_DELAY_MASK)
> +                                       << SDCLK_DELAY_SHIFT;

delay of cycles or nanoseconds or microseconds? I think it would be more
readable to have something like:

.clk_delay_cycles or .clk_delay_ns or .clk_delay_ms and do a bit
translation here,
or you may want to add some comments to the unit of this delay.

And is this necessary to adjust the timing every time a set_clock is done? This
looks like a non-set-clock related action. Wondering whether it's
feasible to put
this in initialization.

> +               tmp |= SDCLK_SEL;
> +               writew(tmp, host->ioaddr + SD_CLOCK_AND_BURST_SIZE_SETUP);
> +       }
> +

unnecessary new line here

> +}
> +
>  static void set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>        struct sdhci_pxa *pxa = sdhci_priv(host);
> @@ -57,15 +82,16 @@ static void set_clock(struct sdhci_host *host,
> unsigned int clock)
>                        pxa->clk_enable = 0;
>                }
>        } else {
> -               if (0 == pxa->clk_enable) {
> -                       if (pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {
> -                               tmp = readl(host->ioaddr + SD_FIFO_PARAM);
> -                               tmp |= DIS_PAD_SD_CLK_GATE;
> -                               writel(tmp, host->ioaddr + SD_FIFO_PARAM);
> -                       }
> -                       clk_enable(pxa->clk);
> -                       pxa->clk_enable = 1;
> +               tune_timming(host);
> +               if (pxa->pdata &&
> +                       pxa->pdata->flags & PXA_FLAG_DISABLE_CLOCK_GATING) {

I guess instead of having this if () statement everywhere, you may
want to encode
the information from pdata to 'struct sdhci_pxa' instead. And you also need to
consider the case if pxa->pdata == NULL is allowed, and if it's allowed, what
is the default action.

> +
> +                       tmp = readl(host->ioaddr + SD_FIFO_PARAM);
> +                       tmp |= DIS_PAD_SD_CLK_GATE;
> +                       writel(tmp, host->ioaddr + SD_FIFO_PARAM);
>                }
> +               clk_enable(pxa->clk);
> +               pxa->clk_enable = 1;
>        }
>  }
>
> @@ -138,13 +164,13 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
>        host->irq = irq;
>        host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> -       if (pxa->pdata->flags & PXA_FLAG_CARD_PERMANENT) {
> +       if (pdata && pdata->flags & PXA_FLAG_CARD_PERMANENT) {
>                /* on-chip device */
>                host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>                host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>        }
>
> -       if (pdata->quirks)
> +       if (pdata && pdata->quirks)
>                host->quirks |= pdata->quirks;
>
>        ret = sdhci_add_host(host);
> @@ -153,9 +179,8 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
>                goto out;
>        }
>
> -       if (pxa->pdata->max_speed)
> -               host->mmc->f_max = pxa->pdata->max_speed;
> -
> +       if (pdata && pdata->max_speed)
> +               host->mmc->f_max = pdata->max_speed;
>        platform_set_drvdata(pdev, host);
>
>        return 0;
> --
> 1.7.0.4
>

[PATCH 1/1]sdhci-pxa: support tune_timming for various cards

[Philip Rakity]

<snip>

> +#define SD_CLOCK_AND_BURST_SIZE_SETUP          0x10A

This offset is only for MMP2.  On other pxa processors use a  different offset.  sdhci-pxa.c needs changing to support other pxa soc chips.

The value only needs setting at initialization time or sdhci reset (RESET_ALL)

Re: [PATCH 1/1]sdhci-pxa: support tune_timming for various cards

[zhangfei gao]

On Wed, Nov 10, 2010 at 4:58 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> <snip>
>
>> +#define SD_CLOCK_AND_BURST_SIZE_SETUP          0x10A
>
> This offset is only for MMP2.  On other pxa processors use a  different offset.  sdhci-pxa.c needs changing to support other pxa soc chips.

The first stage is only considering MMP2, the next step wil consider
other processor, for example move private register to arch/

>
> The value only needs setting at initialization time or sdhci reset (RESET_ALL)
it is OK to put here, then no call back is needed.
>
>
>

Re: [PATCH 1/1]sdhci-pxa: support tune_timming for various cards

[Philip Rakity]

We should rename sdhci-pxa.c to sdhci-mmp2.c and adjust Kconfig and the makefile accordingly since it is clear code is clearly processor dependent 

or

The additional changes to support the pxa family of chips should be posted to the web.


On Nov 10, 2010, at 6:08 PM, zhangfei gao wrote:

> On Wed, Nov 10, 2010 at 4:58 PM, Philip Rakity <prakity@marvell.com> wrote:
>> 
>> <snip>
>> 
>>> +#define SD_CLOCK_AND_BURST_SIZE_SETUP          0x10A
>> 
>> This offset is only for MMP2.  On other pxa processors use a  different offset.  sdhci-pxa.c needs changing to support other pxa soc chips.
> 
> The first stage is only considering MMP2, the next step wil consider
> other processor, for example move private register to arch/
> 
>> 
>> The value only needs setting at initialization time or sdhci reset (RESET_ALL)
> it is OK to put here, then no call back is needed.
>> 
>> 
>>