[PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

	Call back functions is added to support mmp2, pxa910, and pxa955

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
Signed-off-by: RaymondWu <xywu@marvell.com>
Signed-off-by: Jun Nie <njun@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |   46 +++++++++++++++++
 drivers/mmc/host/sdhci-pxa.c           |   88 +++++++++++++++++++++-----------
 2 files changed, 104 insertions(+), 30 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
b/arch/arm/plat-pxa/include/plat/sdhci.h
index 1ab332e..269dbe6 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -13,9 +13,14 @@
 #ifndef __PLAT_PXA_SDHCI_H
 #define __PLAT_PXA_SDHCI_H

+#include <linux/mmc/sdhci.h>
+#include <linux/platform_device.h>
+
 /* pxa specific flag */
 /* Require clock free running */
 #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
+/* card alwayes wired to host, like on-chip emmc */
+#define PXA_FLAG_CARD_PERMANENT	(1<<1)

 /* Board design supports 8-bit data on SD/SDIO BUS */
 #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
@@ -23,13 +28,54 @@
 /*
  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
  * @max_speed: the maximum speed supported
+ * @host_caps: Standard MMC host capabilities bit field.
  * @quirks: quirks of specific device
  * @flags: flags for platform requirement
+ * @clk_delay_cycles:
+ *	mmp2: each step is roughly 100ps, 5bits width
+ *	pxa910: each step is 1ns, 4bits width
+ * @clk_delay_enable: enable clk_delay or not, used on pxa910
+ * @clk_delay_sel: select clk_delay, used on pxa910
+ *	0: choose feedback clk
+ *	1: choose feedback clk + delay value
+ *	2: choose internal clk
+ * @ext_cd_gpio: gpio pin used for external CD line
+ * @ext_cd_gpio_invert: invert values for external CD gpio line
+ * @soc_set_timing: set timing for specific soc
+ * @ext_cd_init: Initialize external card detect subsystem
+ * @ext_cd_cleanup: Cleanup external card detect subsystem
+ * @soc_set_ops: overwrite host ops with soc specific ops
  */
+struct sdhci_pxa;
 struct sdhci_pxa_platdata {
 	unsigned int	max_speed;
+	unsigned int	host_caps;
 	unsigned int	quirks;
 	unsigned int	flags;
+	unsigned int	clk_delay_cycles;
+	unsigned int	clk_delay_sel;
+	unsigned int	ext_cd_gpio;
+	bool		ext_cd_gpio_invert;
+	bool		clk_delay_enable;
+
+	void (*soc_set_timing)(struct sdhci_host *host,
+			struct sdhci_pxa_platdata *pdata);
+	int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
+						   int state), void *data);
+	int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
+						      int state), void *data);
+	void (*soc_set_ops)(struct sdhci_pxa *pxa);
+};
+
+struct sdhci_pxa {
+	struct sdhci_host		*host;
+	struct sdhci_pxa_platdata	*pdata;
+	struct clk			*clk;
+	struct resource			*res;
+	struct sdhci_ops		*ops;
+
+	u8	clk_enable;
+	u8	power_mode;
 };

 #endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
index 5a61208..4a1b7f7 100644
--- a/drivers/mmc/host/sdhci-pxa.c
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -24,32 +24,22 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/slab.h>
 #include <plat/sdhci.h>
 #include "sdhci.h"

 #define DRIVER_NAME	"sdhci-pxa"

-#define SD_FIFO_PARAM		0x104
-#define DIS_PAD_SD_CLK_GATE	0x400
-
-struct sdhci_pxa {
-	struct sdhci_host		*host;
-	struct sdhci_pxa_platdata	*pdata;
-	struct clk			*clk;
-	struct resource			*res;
-
-	u8 clk_enable;
-};
-
 /*****************************************************************************\
  *                                                                           *
  * SDHCI core callbacks                                                      *
  *                                                                           *
 \*****************************************************************************/
+
 static void set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pxa *pxa = sdhci_priv(host);
-	u32 tmp = 0;
+	struct sdhci_pxa_platdata *pdata = pxa->pdata;

 	if (clock == 0) {
 		if (pxa->clk_enable) {
@@ -57,21 +47,31 @@ 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;
-		}
+		if (pdata && pdata->soc_set_timing)
+			pdata->soc_set_timing(host, pdata);
+		clk_enable(pxa->clk);
+		pxa->clk_enable = 1;
 	}
 }

-static struct sdhci_ops sdhci_pxa_ops = {
-	.set_clock = set_clock,
-};
+static void sdhci_pxa_notify_change(struct platform_device *dev, int state)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+	unsigned long flags;
+
+	if (host) {
+		spin_lock_irqsave(&host->lock, flags);
+		if (state) {
+			dev_dbg(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+		} else {
+			dev_dbg(&dev->dev, "card removed.\n");
+			host->flags |= SDHCI_DEVICE_DEAD;
+		}
+		tasklet_schedule(&host->card_tasklet);
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+}

 /*****************************************************************************\
  *                                                                           *
@@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct
platform_device *pdev)
 	pxa->pdata = pdata;
 	pxa->clk_enable = 0;

+	pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
+	if (!pxa->ops) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	pxa->clk = clk_get(dev, "PXA-SDHCLK");
 	if (IS_ERR(pxa->clk)) {
 		dev_err(dev, "failed to get io clock\n");
@@ -134,32 +140,49 @@ static int __devinit sdhci_pxa_probe(struct
platform_device *pdev)
 	}

 	host->hw_name = "MMC";
-	host->ops = &sdhci_pxa_ops;
 	host->irq = irq;
-	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
+	host->quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
+
+	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;

 	/* If slot design supports 8 bit data, indicate this to MMC. */
 	if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
 		host->mmc->caps |= MMC_CAP_8_BIT_DATA;

+	if (pdata && pdata->host_caps)
+		host->mmc->caps |= pdata->host_caps;
+
+	if (pdata && pdata->soc_set_ops)
+		pdata->soc_set_ops(pxa);
+
+	pxa->ops->set_clock = set_clock;
+	host->ops = pxa->ops;
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to add host\n");
 		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;

+	if (pdata && pdata->ext_cd_init)
+		pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
 	platform_set_drvdata(pdev, host);

 	return 0;
 out:
 	if (host) {
 		clk_put(pxa->clk);
+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
@@ -173,18 +196,23 @@ out:

 static int __devexit sdhci_pxa_remove(struct platform_device *pdev)
 {
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pxa *pxa = sdhci_priv(host);
 	int dead = 0;
 	u32 scratch;

 	if (host) {
+		if (pdata && pdata->ext_cd_cleanup)
+			pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
+
 		scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
 		if (scratch == (u32)-1)
 			dead = 1;

 		sdhci_remove_host(host, dead);

+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
-- 
1.7.0.4

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Chris Ball]

Hi,

On Thu, Apr 28 2011, zhangfei gao wrote:
> Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
>
> 	Call back functions is added to support mmp2, pxa910, and pxa955
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> Signed-off-by: RaymondWu <xywu@marvell.com>
> Signed-off-by: Jun Nie <njun@marvell.com>

This patchset looks good to me.  Philip, any objections?

Thanks,

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

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Philip Rakity]

All other platform specific code is in the host/ directory.

This moves it to arch/arm

If that is the direction the group wants to go in --> then the patch is fine provided the mmc group can review the patches.
Otherwise they are handled by the arm maintainer.


On May 12, 2011, at 3:25 PM, Chris Ball wrote:

> Hi,
> 
> On Thu, Apr 28 2011, zhangfei gao wrote:
>> Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
>> 
>> 	Call back functions is added to support mmp2, pxa910, and pxa955
>> 
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Signed-off-by: RaymondWu <xywu@marvell.com>
>> Signed-off-by: Jun Nie <njun@marvell.com>
> 
> This patchset looks good to me.  Philip, any objections?
> 
> Thanks,
> 
> - Chris.
> -- 
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Chris Ball]

Hi,

On Thu, May 12 2011, Philip Rakity wrote:
> All other platform specific code is in the host/ directory.
>
> This moves it to arch/arm
>
> If that is the direction the group wants to go in --> then the patch
> is fine provided the mmc group can review the patches.  Otherwise they
> are handled by the arm maintainer.

Thanks.  Wolfram, do you have any ideas on what the best design is for
these SoC-specific changes to sdhci-pxa?

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

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Thu, May 12 2011, Philip Rakity wrote:
>> All other platform specific code is in the host/ directory.
>>
>> This moves it to arch/arm
>>
>> If that is the direction the group wants to go in --> then the patch
>> is fine provided the mmc group can review the patches.  Otherwise they
>> are handled by the arm maintainer.
>
> Thanks.  Wolfram, do you have any ideas on what the best design is for
> these SoC-specific changes to sdhci-pxa?
>
> - Chris.

The code in arch/arm is
1. Accessing private register, take pxa910 and mmp2 we want to support
as example, there are several private registers differece, though they
are same ip, with same issues and quirk.
2. Handle platform difference, for example, mmp2 used in two different
platform, one use wp pin, the other does not.

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

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Philip Rakity]

On May 13, 2011, at 10:11 PM, zhangfei gao wrote:

> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>> Hi,
>> 
>> On Thu, May 12 2011, Philip Rakity wrote:
>>> All other platform specific code is in the host/ directory.
>>> 
>>> This moves it to arch/arm
>>> 
>>> If that is the direction the group wants to go in --> then the patch
>>> is fine provided the mmc group can review the patches.  Otherwise they
>>> are handled by the arm maintainer.
>> 
>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>> these SoC-specific changes to sdhci-pxa?
>> 
>> - Chris.
> 
> The code in arch/arm is
> 1. Accessing private register, take pxa910 and mmp2 we want to support
> as example, there are several private registers differece, though they
> are same ip, with same issues and quirk.
> 2. Handle platform difference, for example, mmp2 used in two different
> platform, one use wp pin, the other does not.

The situation is a little more complicated.

pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
controller spec with extensions.  The pxa910 controller has fixes to the
pxa168 controller.  They share the same private registers that allow support
for clock gating and timing adjustments.  

mmp2 is based on SD 3.0 spec.  The private register space is different.

mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
cannot co-exist.   What is currently submitted does not work.  One cannot
compile mmp2 and pxa910  nor would they work if one could.  

Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
that took into account these differences.   It provided a common interface layer
that then used platform specific code in the host/ directory to handle the different
behavior.  

Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
pxa familty controller and board.  Based on this comments we submitted a patch
to allow selection if the appropriate SoC. 

These are two approaches. 

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

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Philip Rakity]

resend

On May 14, 2011, at 10:01 AM, Philip Rakity wrote:

> 
> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
> 
>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>>> Hi,
>>> 
>>> On Thu, May 12 2011, Philip Rakity wrote:
>>>> All other platform specific code is in the host/ directory.
>>>> 
>>>> This moves it to arch/arm
>>>> 
>>>> If that is the direction the group wants to go in --> then the patch
>>>> is fine provided the mmc group can review the patches.  Otherwise they
>>>> are handled by the arm maintainer.
>>> 
>>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>>> these SoC-specific changes to sdhci-pxa?
>>> 
>>> - Chris.
>> 
>> The code in arch/arm is
>> 1. Accessing private register, take pxa910 and mmp2 we want to support
>> as example, there are several private registers differece, though they
>> are same ip, with same issues and quirk.
>> 2. Handle platform difference, for example, mmp2 used in two different
>> platform, one use wp pin, the other does not.
> 
> The situation is a little more complicated.
> 
> pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
> controller spec with extensions.  The pxa910 controller has fixes to the
> pxa168 controller.  They share the same private registers that allow support
> for clock gating and timing adjustments.  
> 
> mmp2 is based on SD 3.0 spec.  The private register space is different.
> 
> mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
> cannot co-exist.   What is currently submitted does not work.  One cannot
> compile mmp2 and pxa910  nor would they work if one could.  
> 
> Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
> that took into account these differences.   It provided a common interface layer
> that then used platform specific code in the host/ directory to handle the different
> behavior.  
> 
> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
> pxa familty controller and board.  Based on this comments we submitted a patch
> to allow selection if the appropriate SoC. 
> 
> These are two approaches. 
> 
>> 
>> Thanks
>>> --
>>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>>> One Laptop Per Child
>>> 
> 

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
>
>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>>> Hi,
>>>
>>> On Thu, May 12 2011, Philip Rakity wrote:
>>>> All other platform specific code is in the host/ directory.
>>>>
>>>> This moves it to arch/arm
>>>>
>>>> If that is the direction the group wants to go in --> then the patch
>>>> is fine provided the mmc group can review the patches.  Otherwise they
>>>> are handled by the arm maintainer.
>>>
>>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>>> these SoC-specific changes to sdhci-pxa?
>>>
>>> - Chris.
>>
>> The code in arch/arm is
>> 1. Accessing private register, take pxa910 and mmp2 we want to support
>> as example, there are several private registers differece, though they
>> are same ip, with same issues and quirk.
>> 2. Handle platform difference, for example, mmp2 used in two different
>> platform, one use wp pin, the other does not.
>
> The situation is a little more complicated.

The interface is used for long time among mmp2, pxa910 and mmp3.
Also could be used for new controller with minor register difference
but same ip, without adding new specific driver.

>
> pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
> controller spec with extensions.  The pxa910 controller has fixes to the
> pxa168 controller.  They share the same private registers that allow support
> for clock gating and timing adjustments.
>
> mmp2 is based on SD 3.0 spec.  The private register space is different.
>
> mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
> cannot co-exist.   What is currently submitted does not work.  One cannot
> compile mmp2 and pxa910  nor would they work if one could.
>
> Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
> that took into account these differences.   It provided a common interface layer
> that then used platform specific code in the host/ directory to handle the different
> behavior.
>
> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
> pxa familty controller and board.  Based on this comments we submitted a patch
> to allow selection if the appropriate SoC.
>
> These are two approaches.
>
>>
>> Thanks
>>> --
>>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>>> One Laptop Per Child
>>>
>
>

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Philip Rakity]

On May 15, 2011, at 11:26 PM, zhangfei gao wrote:

> On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@marvell.com> wrote:
>> 
>> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
>> 
>>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>>>> Hi,
>>>> 
>>>> On Thu, May 12 2011, Philip Rakity wrote:
>>>>> All other platform specific code is in the host/ directory.
>>>>> 
>>>>> This moves it to arch/arm
>>>>> 
>>>>> If that is the direction the group wants to go in --> then the patch
>>>>> is fine provided the mmc group can review the patches.  Otherwise they
>>>>> are handled by the arm maintainer.
>>>> 
>>>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>>>> these SoC-specific changes to sdhci-pxa?
>>>> 
>>>> - Chris.
>>> 
>>> The code in arch/arm is
>>> 1. Accessing private register, take pxa910 and mmp2 we want to support
>>> as example, there are several private registers differece, though they
>>> are same ip, with same issues and quirk.
>>> 2. Handle platform difference, for example, mmp2 used in two different
>>> platform, one use wp pin, the other does not.
>> 
>> The situation is a little more complicated.
> 
> The interface is used for long time among mmp2, pxa910 and mmp3.
> Also could be used for new controller with minor register difference
> but same ip, without adding new specific driver.

applies to both approaches.  The mmp2 specific code can be applied to other marvell
platforms that share the same controller.  Just change Kconfig and the Makefile to use
the mmp2 code.  The name of the file is not the important thing.  It is what it does.

The pxa168 and pxa910 code are a little less sharable due to io accessors and some
other quirks. 

For other following the discussion we are probably talking about 100 liens of code.

The philosophical location of where the code belongs for host specific drivers is 
to me more important. 

Is it in arch/arm and not directly visible to the mmc list or is it like all other platform drivers
in drivers/mmc/host.



> 
>> 
>> pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
>> controller spec with extensions.  The pxa910 controller has fixes to the
>> pxa168 controller.  They share the same private registers that allow support
>> for clock gating and timing adjustments.
>> 
>> mmp2 is based on SD 3.0 spec.  The private register space is different.
>> 
>> mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
>> cannot co-exist.   What is currently submitted does not work.  One cannot
>> compile mmp2 and pxa910  nor would they work if one could.
>> 
>> Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
>> that took into account these differences.   It provided a common interface layer
>> that then used platform specific code in the host/ directory to handle the different
>> behavior.
>> 
>> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
>> pxa familty controller and board.  Based on this comments we submitted a patch
>> to allow selection if the appropriate SoC.
>> 
>> These are two approaches.
>> 
>>> 
>>> Thanks
>>>> --
>>>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>>>> One Laptop Per Child
>>>> 
>> 
>> 

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

On Mon, May 16, 2011 at 9:51 AM, Philip Rakity <prakity@marvell.com> wrote:
>
> On May 15, 2011, at 11:26 PM, zhangfei gao wrote:
>
>> On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@marvell.com> wrote:
>>>
>>> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
>>>
>>>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, May 12 2011, Philip Rakity wrote:
>>>>>> All other platform specific code is in the host/ directory.
>>>>>>
>>>>>> This moves it to arch/arm
>>>>>>
>>>>>> If that is the direction the group wants to go in --> then the patch
>>>>>> is fine provided the mmc group can review the patches.  Otherwise they
>>>>>> are handled by the arm maintainer.
>>>>>
>>>>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>>>>> these SoC-specific changes to sdhci-pxa?
>>>>>
>>>>> - Chris.
>>>>
>>>> The code in arch/arm is
>>>> 1. Accessing private register, take pxa910 and mmp2 we want to support
>>>> as example, there are several private registers differece, though they
>>>> are same ip, with same issues and quirk.
>>>> 2. Handle platform difference, for example, mmp2 used in two different
>>>> platform, one use wp pin, the other does not.
>>>
>>> The situation is a little more complicated.
>>
>> The interface is used for long time among mmp2, pxa910 and mmp3.
>> Also could be used for new controller with minor register difference
>> but same ip, without adding new specific driver.
>
> applies to both approaches.  The mmp2 specific code can be applied to other marvell
> platforms that share the same controller.  Just change Kconfig and the Makefile to use
> the mmp2 code.  The name of the file is not the important thing.  It is what it does.
>
> The pxa168 and pxa910 code are a little less sharable due to io accessors and some
> other quirks.
>
> For other following the discussion we are probably talking about 100 liens of code.
>
> The philosophical location of where the code belongs for host specific drivers is
> to me more important.
>
> Is it in arch/arm and not directly visible to the mmc list or is it like all other platform drivers
> in drivers/mmc/host.

The code handle several register difference are located at
arch/arm/mach-mmp/mmp2.c is for mmp2
arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
still registers changing.

The board difference are directly put in board.c
For example ttc_dkb do not use wp pin, so get_ro is provided.

>
>
>
>>
>>>
>>> pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
>>> controller spec with extensions.  The pxa910 controller has fixes to the
>>> pxa168 controller.  They share the same private registers that allow support
>>> for clock gating and timing adjustments.
>>>
>>> mmp2 is based on SD 3.0 spec.  The private register space is different.
>>>
>>> mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
>>> cannot co-exist.   What is currently submitted does not work.  One cannot
>>> compile mmp2 and pxa910  nor would they work if one could.
>>>
>>> Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
>>> that took into account these differences.   It provided a common interface layer
>>> that then used platform specific code in the host/ directory to handle the different
>>> behavior.
>>>
>>> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
>>> pxa familty controller and board.  Based on this comments we submitted a patch
>>> to allow selection if the appropriate SoC.
>>>
>>> These are two approaches.
>>>
>>>>
>>>> Thanks
>>>>> --
>>>>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>>>>> One Laptop Per Child
>>>>>
>>>
>>>
>
>

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Philip Rakity]

On May 16, 2011, at 7:02 PM, zhangfei gao wrote:

> On Mon, May 16, 2011 at 9:51 AM, Philip Rakity <prakity@marvell.com> wrote:
>> 
>> On May 15, 2011, at 11:26 PM, zhangfei gao wrote:
>> 
>>> On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@marvell.com> wrote:
>>>> 
>>>> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
>>>> 
>>>>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> On Thu, May 12 2011, Philip Rakity wrote:
>>>>>>> All other platform specific code is in the host/ directory.
>>>>>>> 
>>>>>>> This moves it to arch/arm
>>>>>>> 
>>>>>>> If that is the direction the group wants to go in --> then the patch
>>>>>>> is fine provided the mmc group can review the patches.  Otherwise they
>>>>>>> are handled by the arm maintainer.
>>>>>> 
>>>>>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>>>>>> these SoC-specific changes to sdhci-pxa?
>>>>>> 
>>>>>> - Chris.
>>>>> 
>>>>> The code in arch/arm is
>>>>> 1. Accessing private register, take pxa910 and mmp2 we want to support
>>>>> as example, there are several private registers differece, though they
>>>>> are same ip, with same issues and quirk.
>>>>> 2. Handle platform difference, for example, mmp2 used in two different
>>>>> platform, one use wp pin, the other does not.
>>>> 
>>>> The situation is a little more complicated.
>>> 
>>> The interface is used for long time among mmp2, pxa910 and mmp3.
>>> Also could be used for new controller with minor register difference
>>> but same ip, without adding new specific driver.
>> 
>> applies to both approaches.  The mmp2 specific code can be applied to other marvell
>> platforms that share the same controller.  Just change Kconfig and the Makefile to use
>> the mmp2 code.  The name of the file is not the important thing.  It is what it does.
>> 
>> The pxa168 and pxa910 code are a little less sharable due to io accessors and some
>> other quirks.
>> 
>> For other following the discussion we are probably talking about 100 liens of code.
>> 
>> The philosophical location of where the code belongs for host specific drivers is
>> to me more important.
>> 
>> Is it in arch/arm and not directly visible to the mmc list or is it like all other platform drivers
>> in drivers/mmc/host.
> 
> The code handle several register difference are located at
> arch/arm/mach-mmp/mmp2.c is for mmp2
> arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
> still registers changing.
> 
> The board difference are directly put in board.c
> For example ttc_dkb do not use wp pin, so get_ro is provided.

embedding the code in these chip files stops code sharing.  For example,
mmp3 has same controller as mmp2. 
> 
>> 
>> 
>> 
>>> 
>>>> 
>>>> pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
>>>> controller spec with extensions.  The pxa910 controller has fixes to the
>>>> pxa168 controller.  They share the same private registers that allow support
>>>> for clock gating and timing adjustments.
>>>> 
>>>> mmp2 is based on SD 3.0 spec.  The private register space is different.
>>>> 
>>>> mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
>>>> cannot co-exist.   What is currently submitted does not work.  One cannot
>>>> compile mmp2 and pxa910  nor would they work if one could.
>>>> 
>>>> Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
>>>> that took into account these differences.   It provided a common interface layer
>>>> that then used platform specific code in the host/ directory to handle the different
>>>> behavior.
>>>> 
>>>> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
>>>> pxa familty controller and board.  Based on this comments we submitted a patch
>>>> to allow selection if the appropriate SoC.
>>>> 
>>>> These are two approaches.
>>>> 
>>>>> 
>>>>> Thanks
>>>>>> --
>>>>>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>>>>>> One Laptop Per Child
>>>>>> 
>>>> 
>>>> 
>> 
>> 

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

On Tue, May 17, 2011 at 12:27 AM, Philip Rakity <prakity@marvell.com> wrote:
>
> On May 16, 2011, at 7:02 PM, zhangfei gao wrote:
>
>> On Mon, May 16, 2011 at 9:51 AM, Philip Rakity <prakity@marvell.com> wrote:
>>>
>>> On May 15, 2011, at 11:26 PM, zhangfei gao wrote:
>>>
>>>> On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@marvell.com> wrote:
>>>>>
>>>>> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
>>>>>
>>>>>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Thu, May 12 2011, Philip Rakity wrote:
>>>>>>>> All other platform specific code is in the host/ directory.
>>>>>>>>
>>>>>>>> This moves it to arch/arm
>>>>>>>>
>>>>>>>> If that is the direction the group wants to go in --> then the patch
>>>>>>>> is fine provided the mmc group can review the patches.  Otherwise they
>>>>>>>> are handled by the arm maintainer.
>>>>>>>
>>>>>>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>>>>>>> these SoC-specific changes to sdhci-pxa?
>>>>>>>
>>>>>>> - Chris.
>>>>>>
>>>>>> The code in arch/arm is
>>>>>> 1. Accessing private register, take pxa910 and mmp2 we want to support
>>>>>> as example, there are several private registers differece, though they
>>>>>> are same ip, with same issues and quirk.
>>>>>> 2. Handle platform difference, for example, mmp2 used in two different
>>>>>> platform, one use wp pin, the other does not.
>>>>>
>>>>> The situation is a little more complicated.
>>>>
>>>> The interface is used for long time among mmp2, pxa910 and mmp3.
>>>> Also could be used for new controller with minor register difference
>>>> but same ip, without adding new specific driver.
>>>
>>> applies to both approaches.  The mmp2 specific code can be applied to other marvell
>>> platforms that share the same controller.  Just change Kconfig and the Makefile to use
>>> the mmp2 code.  The name of the file is not the important thing.  It is what it does.
>>>
>>> The pxa168 and pxa910 code are a little less sharable due to io accessors and some
>>> other quirks.
>>>
>>> For other following the discussion we are probably talking about 100 liens of code.
>>>
>>> The philosophical location of where the code belongs for host specific drivers is
>>> to me more important.
>>>
>>> Is it in arch/arm and not directly visible to the mmc list or is it like all other platform drivers
>>> in drivers/mmc/host.
>>
>> The code handle several register difference are located at
>> arch/arm/mach-mmp/mmp2.c is for mmp2
>> arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
>> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
>> still registers changing.
>>
>> The board difference are directly put in board.c
>> For example ttc_dkb do not use wp pin, so get_ro is provided.
>
> embedding the code in these chip files stops code sharing.  For example,
> mmp3 has same controller as mmp2.

If register are totally same, they can share.
If there is minor difference with chip upgrading, they can put in
different file.

>>
>>>
>>>
>>>
>>>>
>>>>>
>>>>> pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
>>>>> controller spec with extensions.  The pxa910 controller has fixes to the
>>>>> pxa168 controller.  They share the same private registers that allow support
>>>>> for clock gating and timing adjustments.
>>>>>
>>>>> mmp2 is based on SD 3.0 spec.  The private register space is different.
>>>>>
>>>>> mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
>>>>> cannot co-exist.   What is currently submitted does not work.  One cannot
>>>>> compile mmp2 and pxa910  nor would they work if one could.
>>>>>
>>>>> Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
>>>>> that took into account these differences.   It provided a common interface layer
>>>>> that then used platform specific code in the host/ directory to handle the different
>>>>> behavior.
>>>>>
>>>>> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
>>>>> pxa familty controller and board.  Based on this comments we submitted a patch
>>>>> to allow selection if the appropriate SoC.
>>>>>
>>>>> These are two approaches.
>>>>>
>>>>>>
>>>>>> Thanks
>>>>>>> --
>>>>>>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>>>>>>> One Laptop Per Child
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Arnd Bergmann]

On Tuesday 17 May 2011, zhangfei gao wrote:
> >> The code handle several register difference are located at
> >> arch/arm/mach-mmp/mmp2.c is for mmp2
> >> arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
> >> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
> >> still registers changing.
> >>
> >> The board difference are directly put in board.c
> >> For example ttc_dkb do not use wp pin, so get_ro is provided.
> >
> > embedding the code in these chip files stops code sharing.  For example,
> > mmp3 has same controller as mmp2.
> 
> If register are totally same, they can share.
> If there is minor difference with chip upgrading, they can put in
> different file.

When we talked about similar problems in drivers during the Linaro Developer
Summit, the broad consensus was to move code from arch/arm into individual
device drivers, and I think this should be done for this driver too.

My recommendation here would be to have no callbacks in the platform
data at all, but instead have the code for all variants in the
sdhci-pxa driver itself. You can still have a structure to describe
the differences using function pointers, but it's better if that is
part of the driver itself. In the platform data, provide a way
to identify the variant of the controller (e.g. using an enum)
and point to the base addresses as required.

	Arnd

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

On Wed, May 18, 2011 at 4:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 17 May 2011, zhangfei gao wrote:
>> >> The code handle several register difference are located at
>> >> arch/arm/mach-mmp/mmp2.c is for mmp2
>> >> arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
>> >> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
>> >> still registers changing.
>> >>
>> >> The board difference are directly put in board.c
>> >> For example ttc_dkb do not use wp pin, so get_ro is provided.
>> >
>> > embedding the code in these chip files stops code sharing.  For example,
>> > mmp3 has same controller as mmp2.
>>
>> If register are totally same, they can share.
>> If there is minor difference with chip upgrading, they can put in
>> different file.
>
> When we talked about similar problems in drivers during the Linaro Developer
> Summit, the broad consensus was to move code from arch/arm into individual
> device drivers, and I think this should be done for this driver too.
>
> My recommendation here would be to have no callbacks in the platform
> data at all, but instead have the code for all variants in the
> sdhci-pxa driver itself. You can still have a structure to describe
> the differences using function pointers, but it's better if that is
> part of the driver itself. In the platform data, provide a way
> to identify the variant of the controller (e.g. using an enum)
> and point to the base addresses as required.
>
>        Arnd
>
Hi, Arnd

Thanks for your suggestion.

To make sdhci-pxa easy to maintain, we still prefer put platform
specific difference under platform code.

"Identify controller from platform data" looks to me is same as using #ifdef.
The method is used in our another driver before.
More and more workaround comes when more and more platform need to
support, making the driver bigger and bigger and not easy to maintain.
Nobody remember what's the workaround purpose several years later.
So the result is we have to re-write the driver again to make it simple :(

Currently, the ip is shared among mmp2, pxa910, pxa168 etc, pxa910
maintainer does not care what workaround is used on pxa168.
When mmp3 wants to reuse the ip, it's easier to not care what's the
history at all.

So we still prefer keep driver as simple as possible, while specific
platform self-maintain specific workaround , which is not aware to
other platform.

Thanks
>

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Arnd Bergmann]

On Thursday 19 May 2011, zhangfei gao wrote:
> To make sdhci-pxa easy to maintain, we still prefer put platform
> specific difference under platform code.
> 
> "Identify controller from platform data" looks to me is same as using #ifdef.
> The method is used in our another driver before.

I meant run-time code, not compile-time #ifdef. In the long run, we
want to have a kernel that is able to work on many different systems,
and that means the drivers should not impose hardcoded limitations.

> More and more workaround comes when more and more platform need to
> support, making the driver bigger and bigger and not easy to maintain.
> Nobody remember what's the workaround purpose several years later.
> So the result is we have to re-write the driver again to make it simple :(
> 
> Currently, the ip is shared among mmp2, pxa910, pxa168 etc, pxa910
> maintainer does not care what workaround is used on pxa168.
> When mmp3 wants to reuse the ip, it's easier to not care what's the
> history at all.
> 
> So we still prefer keep driver as simple as possible, while specific
> platform self-maintain specific workaround , which is not aware to
> other platform.

There are a lot of different ways to do the same thing, but splitting
out code into the subarchitecture is not a good one. Please have a look
at how Shawn Guo has reworked the sdhci-pltfm drivers to have common
part as a library and then multiple users of that. You can probably
do the same with the different versions of the sdhci-pxa driver.

	Arnd

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2645 bytes --]

On Thu, 19 May 2011, zhangfei gao wrote:

> On Wed, May 18, 2011 at 4:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 17 May 2011, zhangfei gao wrote:
> >> >> The code handle several register difference are located at
> >> >> arch/arm/mach-mmp/mmp2.c is for mmp2
> >> >> arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
> >> >> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
> >> >> still registers changing.
> >> >>
> >> >> The board difference are directly put in board.c
> >> >> For example ttc_dkb do not use wp pin, so get_ro is provided.
> >> >
> >> > embedding the code in these chip files stops code sharing.  For example,
> >> > mmp3 has same controller as mmp2.
> >>
> >> If register are totally same, they can share.
> >> If there is minor difference with chip upgrading, they can put in
> >> different file.
> >
> > When we talked about similar problems in drivers during the Linaro Developer
> > Summit, the broad consensus was to move code from arch/arm into individual
> > device drivers, and I think this should be done for this driver too.
> >
> > My recommendation here would be to have no callbacks in the platform
> > data at all, but instead have the code for all variants in the
> > sdhci-pxa driver itself. You can still have a structure to describe
> > the differences using function pointers, but it's better if that is
> > part of the driver itself. In the platform data, provide a way
> > to identify the variant of the controller (e.g. using an enum)
> > and point to the base addresses as required.
> >
> >        Arnd
> >
> Hi, Arnd
> 
> Thanks for your suggestion.
> 
> To make sdhci-pxa easy to maintain, we still prefer put platform
> specific difference under platform code.

That's fine to split out platform differences.  However please put those 
differences, maybe in different files, but alongside the driver itself 
in drivers/mmc/host/sdhci-pxa-pxa910.c, sdhci-pxa-mmp2.c, etc. We don't 
want to see driver specific stuff hidden in arch/arm/mach-pxa/ or the 
like.

> More and more workaround comes when more and more platform need to
> support, making the driver bigger and bigger and not easy to maintain.
> Nobody remember what's the workaround purpose several years later.

You really really should spend more time documenting those workarounds 
in the source directly in that case.  Anyone not familiar with the 
issues should be able to understand why a given workaround is there, 
whether it is today or in several years.  And this is why we want driver 
stuff go in the driver directory where more people specifically 
interested in MMC/SD are looking.


Nicolas

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

On Fri, May 20, 2011 at 2:24 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Thu, 19 May 2011, zhangfei gao wrote:
>
>> On Wed, May 18, 2011 at 4:38 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Tuesday 17 May 2011, zhangfei gao wrote:
>> >> >> The code handle several register difference are located at
>> >> >> arch/arm/mach-mmp/mmp2.c is for mmp2
>> >> >> arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
>> >> >> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
>> >> >> still registers changing.
>> >> >>
>> >> >> The board difference are directly put in board.c
>> >> >> For example ttc_dkb do not use wp pin, so get_ro is provided.
>> >> >
>> >> > embedding the code in these chip files stops code sharing.  For example,
>> >> > mmp3 has same controller as mmp2.
>> >>
>> >> If register are totally same, they can share.
>> >> If there is minor difference with chip upgrading, they can put in
>> >> different file.
>> >
>> > When we talked about similar problems in drivers during the Linaro Developer
>> > Summit, the broad consensus was to move code from arch/arm into individual
>> > device drivers, and I think this should be done for this driver too.
>> >
>> > My recommendation here would be to have no callbacks in the platform
>> > data at all, but instead have the code for all variants in the
>> > sdhci-pxa driver itself. You can still have a structure to describe
>> > the differences using function pointers, but it's better if that is
>> > part of the driver itself. In the platform data, provide a way
>> > to identify the variant of the controller (e.g. using an enum)
>> > and point to the base addresses as required.
>> >
>> >        Arnd
>> >
>> Hi, Arnd
>>
>> Thanks for your suggestion.
>>
>> To make sdhci-pxa easy to maintain, we still prefer put platform
>> specific difference under platform code.
>
> That's fine to split out platform differences.  However please put those
> differences, maybe in different files, but alongside the driver itself
> in drivers/mmc/host/sdhci-pxa-pxa910.c, sdhci-pxa-mmp2.c, etc. We don't
> want to see driver specific stuff hidden in arch/arm/mach-pxa/ or the
> like.
>
>> More and more workaround comes when more and more platform need to
>> support, making the driver bigger and bigger and not easy to maintain.
>> Nobody remember what's the workaround purpose several years later.
>
> You really really should spend more time documenting those workarounds
> in the source directly in that case.  Anyone not familiar with the
> issues should be able to understand why a given workaround is there,
> whether it is today or in several years.  And this is why we want driver
> stuff go in the driver directory where more people specifically
> interested in MMC/SD are looking.

Thanks for explanation, will update accordingly.
>
>
> Nicolas
>

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[zhangfei gao]

On Thu, May 19, 2011 at 9:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 19 May 2011, zhangfei gao wrote:
>> To make sdhci-pxa easy to maintain, we still prefer put platform
>> specific difference under platform code.
>>
>> "Identify controller from platform data" looks to me is same as using #ifdef.
>> The method is used in our another driver before.
>
> I meant run-time code, not compile-time #ifdef. In the long run, we
> want to have a kernel that is able to work on many different systems,
> and that means the drivers should not impose hardcoded limitations.
>
>> More and more workaround comes when more and more platform need to
>> support, making the driver bigger and bigger and not easy to maintain.
>> Nobody remember what's the workaround purpose several years later.
>> So the result is we have to re-write the driver again to make it simple :(
>>
>> Currently, the ip is shared among mmp2, pxa910, pxa168 etc, pxa910
>> maintainer does not care what workaround is used on pxa168.
>> When mmp3 wants to reuse the ip, it's easier to not care what's the
>> history at all.
>>
>> So we still prefer keep driver as simple as possible, while specific
>> platform self-maintain specific workaround , which is not aware to
>> other platform.
>
> There are a lot of different ways to do the same thing, but splitting
> out code into the subarchitecture is not a good one. Please have a look
> at how Shawn Guo has reworked the sdhci-pltfm drivers to have common
> part as a library and then multiple users of that. You can probably
> do the same with the different versions of the sdhci-pxa driver.
>
>        Arnd
>
Hi, Arnd

When we start to share sdhci-pxa, we find another sdhci-platfm occurs.
While in Shawn's patch, driver self-register is recommended.
If this is new trend, it maybe a better choice to use separate driver
for each device based on updated sdhci-platfm.c by Shawn.
Will update the sdhci-mmp2.c for mmp2 only based on Shawn's modification.

Thanks

Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

[Arnd Bergmann]

On Monday 23 May 2011, zhangfei gao wrote:
> When we start to share sdhci-pxa, we find another sdhci-platfm occurs.
> While in Shawn's patch, driver self-register is recommended.
> If this is new trend, it maybe a better choice to use separate driver
> for each device based on updated sdhci-platfm.c by Shawn.
> Will update the sdhci-mmp2.c for mmp2 only based on Shawn's modification.

Having each driver register its own platform_driver sounds like
the best solution to me. Whether you do that on top of sdhci-pltfm.c or
model sdhci-pxa.c to be similar to sdhci-pltfm.c is a different
question. I don't care much either way, so just do whichever works
best for you. Maybe Shawn or Chris can recommend what they think would
work better.

	Arnd

Re: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

[zhangfei gao]

On Fri, Dec 3, 2010 at 1:31 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
> From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Tue, 30 Nov 2010 05:43:29 -0500
> Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
>
>        Call back functions is added to support mmp2, pxa910, pxa168 and pxa955
>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> CC: RaymondWu <xywu@marvell.com>
> CC: Jun Nie <njun@marvell.com>
> CC: Philip Rakity <prakity@marvell.com>
> ---
>  arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
>  drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
>  2 files changed, 98 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> index 1ab332e..4ce3b05 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -13,9 +13,14 @@
>  #ifndef __PLAT_PXA_SDHCI_H
>  #define __PLAT_PXA_SDHCI_H
>
> +#include <linux/mmc/sdhci.h>
> +#include <linux/platform_device.h>
> +
>  /* pxa specific flag */
>  /* Require clock free running */
>  #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
> +/* card alwayes wired to host, like on-chip emmc */
> +#define PXA_FLAG_CARD_PERMANENT        (1<<1)
>
>  /* Board design supports 8-bit data on SD/SDIO BUS */
>  #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
> @@ -25,11 +30,50 @@
>  * @max_speed: the maximum speed supported
>  * @quirks: quirks of specific device
>  * @flags: flags for platform requirement
> + * @clk_delay_cycles:
> + *     mmp2: each step is roughly 100ps, 5bits width
> + *     pxa910 & pxa168: each step is 1ns, 4bits width
> + * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
> + * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
> + *     0: choose feedback clk
> + *     1: choose feedback clk + delay value
> + *     2: choose internal clk
> + * @ext_cd_gpio: gpio pin used for external CD line
> + * @ext_cd_gpio_invert: invert values for external CD gpio line
> + * @soc_set_timing: set timing for specific soc
> + * @ext_cd_init: Initialize external card detect subsystem
> + * @ext_cd_cleanup: Cleanup external card detect subsystem
> + * @soc_set_ops: overwrite host ops with soc specific ops
>  */
> +struct sdhci_pxa;
>  struct sdhci_pxa_platdata {
>        unsigned int    max_speed;
>        unsigned int    quirks;
>        unsigned int    flags;
> +       unsigned int    clk_delay_cycles;
> +       unsigned int    clk_delay_sel;
> +       unsigned int    ext_cd_gpio;
> +       bool            ext_cd_gpio_invert;
> +       bool            clk_delay_enable;
> +
> +       void (*soc_set_timing)(struct sdhci_host *host,
> +                       struct sdhci_pxa_platdata *pdata);
> +       int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
> +                                                  int state), void *data);
> +       int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
> +                                                     int state), void *data);
> +       void (*soc_set_ops)(struct sdhci_pxa *pxa);
> +};
> +
> +struct sdhci_pxa {
> +       struct sdhci_host               *host;
> +       struct sdhci_pxa_platdata       *pdata;
> +       struct clk                      *clk;
> +       struct resource                 *res;
> +       struct sdhci_ops                *ops;
> +
> +       u8      clk_enable;
> +       u8      power_mode;
>  };
>
>  #endif /* __PLAT_PXA_SDHCI_H */
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> index 5a61208..f79dccf 100644
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -24,32 +24,22 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/err.h>
> +#include <linux/slab.h>
>  #include <plat/sdhci.h>
>  #include "sdhci.h"
>
>  #define DRIVER_NAME    "sdhci-pxa"
>
> -#define SD_FIFO_PARAM          0x104
> -#define DIS_PAD_SD_CLK_GATE    0x400
> -
> -struct sdhci_pxa {
> -       struct sdhci_host               *host;
> -       struct sdhci_pxa_platdata       *pdata;
> -       struct clk                      *clk;
> -       struct resource                 *res;
> -
> -       u8 clk_enable;
> -};
> -
>  /*****************************************************************************\
>  *                                                                           *
>  * SDHCI core callbacks                                                      *
>  *                                                                           *
>  \*****************************************************************************/
> +
>  static void set_clock(struct sdhci_host *host, unsigned int clock)
>  {
>        struct sdhci_pxa *pxa = sdhci_priv(host);
> -       u32 tmp = 0;
> +       struct sdhci_pxa_platdata *pdata = pxa->pdata;
>
>        if (clock == 0) {
>                if (pxa->clk_enable) {
> @@ -57,21 +47,31 @@ 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;
> -               }
> +               if (pdata && pdata->soc_set_timing)
> +                       pdata->soc_set_timing(host, pdata);
> +               clk_enable(pxa->clk);
> +               pxa->clk_enable = 1;
>        }
>  }
>
> -static struct sdhci_ops sdhci_pxa_ops = {
> -       .set_clock = set_clock,
> -};
> +static void sdhci_pxa_notify_change(struct platform_device *dev, int state)
> +{
> +       struct sdhci_host *host = platform_get_drvdata(dev);
> +       unsigned long flags;
> +
> +       if (host) {
> +               spin_lock_irqsave(&host->lock, flags);
> +               if (state) {
> +                       dev_dbg(&dev->dev, "card inserted.\n");
> +                       host->flags &= ~SDHCI_DEVICE_DEAD;
> +               } else {
> +                       dev_dbg(&dev->dev, "card removed.\n");
> +                       host->flags |= SDHCI_DEVICE_DEAD;
> +               }
> +               tasklet_schedule(&host->card_tasklet);
> +               spin_unlock_irqrestore(&host->lock, flags);
> +       }
> +}
>
>  /*****************************************************************************\
>  *                                                                           *
> @@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
>        pxa->pdata = pdata;
>        pxa->clk_enable = 0;
>
> +       pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
> +       if (!pxa->ops) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
> +
>        pxa->clk = clk_get(dev, "PXA-SDHCLK");
>        if (IS_ERR(pxa->clk)) {
>                dev_err(dev, "failed to get io clock\n");
> @@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
>        }
>
>        host->hw_name = "MMC";
> -       host->ops = &sdhci_pxa_ops;
>        host->irq = irq;
>        host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> -       if (pdata->quirks)
> +       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 && pdata->quirks)
>                host->quirks |= pdata->quirks;
>
>        /* If slot design supports 8 bit data, indicate this to MMC. */
>        if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>                host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>
> +       if (pdata && pdata->soc_set_ops)
> +               pdata->soc_set_ops(pxa);
> +
> +       pxa->ops->set_clock = set_clock;
> +       host->ops = pxa->ops;
> +
>        ret = sdhci_add_host(host);
>        if (ret) {
>                dev_err(&pdev->dev, "failed to add host\n");
>                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;
>
> +       if (pdata && pdata->ext_cd_init)
> +               pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
>        platform_set_drvdata(pdev, host);
>
>        return 0;
>  out:
>        if (host) {
>                clk_put(pxa->clk);
> +               kfree(pxa->ops);
>                if (host->ioaddr)
>                        iounmap(host->ioaddr);
>                if (pxa->res)
> @@ -173,18 +193,23 @@ out:
>
>  static int __devexit sdhci_pxa_remove(struct platform_device *pdev)
>  {
> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>        struct sdhci_host *host = platform_get_drvdata(pdev);
>        struct sdhci_pxa *pxa = sdhci_priv(host);
>        int dead = 0;
>        u32 scratch;
>
>        if (host) {
> +               if (pdata && pdata->ext_cd_cleanup)
> +                       pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
> +
>                scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>                if (scratch == (u32)-1)
>                        dead = 1;
>
>                sdhci_remove_host(host, dead);
>
> +               kfree(pxa->ops);
>                if (host->ioaddr)
>                        iounmap(host->ioaddr);
>                if (pxa->res)
> --
> 1.7.0.4
>

Re: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

[zhangfei gao]

2010/12/7 Philip Rakity <prakity@marvell.com>:
>
> was interested in the 9xx code.

You can ask Jun for pxa910 code, and Raymond for pxa950 code via
internal email system, thanks
>
>
> On Dec 7, 2010, at 7:38 AM, zhangfei gao wrote:
>
>> 2010/12/7 Philip Rakity <prakity@marvell.com>:
>>>
>>> Where is the code that implements
>>>
>>>> +     unsigned int    clk_delay_cycles;
>>>> +     unsigned int    clk_delay_sel;
>>
>> The code is located under arch/arm/, it is used differently depends on
>> different platfrom.
>> For example, aspen could realize under arch/arm/mach-pxa/pxa168.c.
>> Could you refer the code for mmp2.
>>
>>>
>>>
>>>
>>>
>>> On Dec 6, 2010, at 9:56 PM, Raymond Wu wrote:
>>>
>>>> Hi,
>>>>
>>>> This patch is verified by: Raymond Wu(xywu@marvell.com) on pxa955 SAARB platform.
>>>>
>>>> Thanks,
>>>> Raymond
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
>>>> Sent: 2010年12月3日 14:31
>>>> To: linux-mmc@vger.kernel.org
>>>> Cc: Chris Ball; Eric Miao; Haojian Zhuang; niej0001@gmail.com; Philip Rakity; Mark Brown; Raymond Wu; Jun Nie
>>>> Subject: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa
>>>>
>>>> From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
>>>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>>>> Date: Tue, 30 Nov 2010 05:43:29 -0500
>>>> Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
>>>>
>>>>      Call back functions is added to support mmp2, pxa910, pxa168 and pxa955
>>>>
>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>>>> CC: RaymondWu <xywu@marvell.com>
>>>> CC: Jun Nie <njun@marvell.com>
>>>> CC: Philip Rakity <prakity@marvell.com>
>>>> ---
>>>> arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
>>>> drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
>>>> 2 files changed, 98 insertions(+), 29 deletions(-)
>>>>
>>>> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
>>>> b/arch/arm/plat-pxa/include/plat/sdhci.h
>>>> index 1ab332e..4ce3b05 100644
>>>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>>>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>>>> @@ -13,9 +13,14 @@
>>>> #ifndef __PLAT_PXA_SDHCI_H
>>>> #define __PLAT_PXA_SDHCI_H
>>>>
>>>> +#include <linux/mmc/sdhci.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> /* pxa specific flag */
>>>> /* Require clock free running */
>>>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>>>> +/* card alwayes wired to host, like on-chip emmc */
>>>> +#define PXA_FLAG_CARD_PERMANENT      (1<<1)
>>>>
>>>> /* Board design supports 8-bit data on SD/SDIO BUS */  #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2) @@ -25,11 +30,50 @@
>>>> * @max_speed: the maximum speed supported
>>>> * @quirks: quirks of specific device
>>>> * @flags: flags for platform requirement
>>>> + * @clk_delay_cycles:
>>>> + *   mmp2: each step is roughly 100ps, 5bits width
>>>> + *   pxa910 & pxa168: each step is 1ns, 4bits width
>>>> + * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
>>>> + * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
>>>> + *   0: choose feedback clk
>>>> + *   1: choose feedback clk + delay value
>>>> + *   2: choose internal clk
>>>> + * @ext_cd_gpio: gpio pin used for external CD line
>>>> + * @ext_cd_gpio_invert: invert values for external CD gpio line
>>>> + * @soc_set_timing: set timing for specific soc
>>>> + * @ext_cd_init: Initialize external card detect subsystem
>>>> + * @ext_cd_cleanup: Cleanup external card detect subsystem
>>>> + * @soc_set_ops: overwrite host ops with soc specific ops
>>>> */
>>>> +struct sdhci_pxa;
>>>> struct sdhci_pxa_platdata {
>>>>      unsigned int    max_speed;
>>>>      unsigned int    quirks;
>>>>      unsigned int    flags;
>>>> +     unsigned int    clk_delay_cycles;
>>>> +     unsigned int    clk_delay_sel;
>>>> +     unsigned int    ext_cd_gpio;
>>>> +     bool            ext_cd_gpio_invert;
>>>> +     bool            clk_delay_enable;
>>>> +
>>>> +     void (*soc_set_timing)(struct sdhci_host *host,
>>>> +                     struct sdhci_pxa_platdata *pdata);
>>>> +     int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
>>>> +                                                int state), void *data);
>>>> +     int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
>>>> +                                                   int state), void *data);
>>>> +     void (*soc_set_ops)(struct sdhci_pxa *pxa); };
>>>> +
>>>> +struct sdhci_pxa {
>>>> +     struct sdhci_host               *host;
>>>> +     struct sdhci_pxa_platdata       *pdata;
>>>> +     struct clk                      *clk;
>>>> +     struct resource                 *res;
>>>> +     struct sdhci_ops                *ops;
>>>> +
>>>> +     u8      clk_enable;
>>>> +     u8      power_mode;
>>>> };
>>>>
>>>> #endif /* __PLAT_PXA_SDHCI_H */
>>>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c index 5a61208..f79dccf 100644
>>>> --- a/drivers/mmc/host/sdhci-pxa.c
>>>> +++ b/drivers/mmc/host/sdhci-pxa.c
>>>> @@ -24,32 +24,22 @@
>>>> #include <linux/clk.h>
>>>> #include <linux/io.h>
>>>> #include <linux/err.h>
>>>> +#include <linux/slab.h>
>>>> #include <plat/sdhci.h>
>>>> #include "sdhci.h"
>>>>
>>>> #define DRIVER_NAME   "sdhci-pxa"
>>>>
>>>> -#define SD_FIFO_PARAM                0x104
>>>> -#define DIS_PAD_SD_CLK_GATE  0x400
>>>> -
>>>> -struct sdhci_pxa {
>>>> -     struct sdhci_host               *host;
>>>> -     struct sdhci_pxa_platdata       *pdata;
>>>> -     struct clk                      *clk;
>>>> -     struct resource                 *res;
>>>> -
>>>> -     u8 clk_enable;
>>>> -};
>>>> -
>>>> /*****************************************************************************\
>>>> *                                                                           *
>>>> * SDHCI core callbacks                                                      *
>>>> *                                                                           *
>>>> \*****************************************************************************/
>>>> +
>>>> static void set_clock(struct sdhci_host *host, unsigned int clock)  {
>>>>      struct sdhci_pxa *pxa = sdhci_priv(host);
>>>> -     u32 tmp = 0;
>>>> +     struct sdhci_pxa_platdata *pdata = pxa->pdata;
>>>>
>>>>      if (clock == 0) {
>>>>              if (pxa->clk_enable) {
>>>> @@ -57,21 +47,31 @@ 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;
>>>> -             }
>>>> +             if (pdata && pdata->soc_set_timing)
>>>> +                     pdata->soc_set_timing(host, pdata);
>>>> +             clk_enable(pxa->clk);
>>>> +             pxa->clk_enable = 1;
>>>>      }
>>>> }
>>>>
>>>> -static struct sdhci_ops sdhci_pxa_ops = {
>>>> -     .set_clock = set_clock,
>>>> -};
>>>> +static void sdhci_pxa_notify_change(struct platform_device *dev, int
>>>> +state) {
>>>> +     struct sdhci_host *host = platform_get_drvdata(dev);
>>>> +     unsigned long flags;
>>>> +
>>>> +     if (host) {
>>>> +             spin_lock_irqsave(&host->lock, flags);
>>>> +             if (state) {
>>>> +                     dev_dbg(&dev->dev, "card inserted.\n");
>>>> +                     host->flags &= ~SDHCI_DEVICE_DEAD;
>>>> +             } else {
>>>> +                     dev_dbg(&dev->dev, "card removed.\n");
>>>> +                     host->flags |= SDHCI_DEVICE_DEAD;
>>>> +             }
>>>> +             tasklet_schedule(&host->card_tasklet);
>>>> +             spin_unlock_irqrestore(&host->lock, flags);
>>>> +     }
>>>> +}
>>>>
>>>> /*****************************************************************************\
>>>> *                                                                           *
>>>> @@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
>>>>      pxa->pdata = pdata;
>>>>      pxa->clk_enable = 0;
>>>>
>>>> +     pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>>>> +     if (!pxa->ops) {
>>>> +             ret = -ENOMEM;
>>>> +             goto out;
>>>> +     }
>>>> +
>>>>      pxa->clk = clk_get(dev, "PXA-SDHCLK");
>>>>      if (IS_ERR(pxa->clk)) {
>>>>              dev_err(dev, "failed to get io clock\n"); @@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
>>>>      }
>>>>
>>>>      host->hw_name = "MMC";
>>>> -     host->ops = &sdhci_pxa_ops;
>>>>      host->irq = irq;
>>>>      host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>>>>
>>>> -     if (pdata->quirks)
>>>> +     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 && pdata->quirks)
>>>>              host->quirks |= pdata->quirks;
>>>>
>>>>      /* If slot design supports 8 bit data, indicate this to MMC. */
>>>>      if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>>>>              host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>>>
>>>> +     if (pdata && pdata->soc_set_ops)
>>>> +             pdata->soc_set_ops(pxa);
>>>> +
>>>> +     pxa->ops->set_clock = set_clock;
>>>> +     host->ops = pxa->ops;
>>>> +
>>>>      ret = sdhci_add_host(host);
>>>>      if (ret) {
>>>>              dev_err(&pdev->dev, "failed to add host\n");
>>>>              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;
>>>>
>>>> +     if (pdata && pdata->ext_cd_init)
>>>> +             pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
>>>>      platform_set_drvdata(pdev, host);
>>>>
>>>>      return 0;
>>>> out:
>>>>      if (host) {
>>>>              clk_put(pxa->clk);
>>>> +             kfree(pxa->ops);
>>>>              if (host->ioaddr)
>>>>                      iounmap(host->ioaddr);
>>>>              if (pxa->res)
>>>> @@ -173,18 +193,23 @@ out:
>>>>
>>>> static int __devexit sdhci_pxa_remove(struct platform_device *pdev)  {
>>>> +     struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>>>>      struct sdhci_host *host = platform_get_drvdata(pdev);
>>>>      struct sdhci_pxa *pxa = sdhci_priv(host);
>>>>      int dead = 0;
>>>>      u32 scratch;
>>>>
>>>>      if (host) {
>>>> +             if (pdata && pdata->ext_cd_cleanup)
>>>> +                     pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
>>>> +
>>>>              scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>>>>              if (scratch == (u32)-1)
>>>>                      dead = 1;
>>>>
>>>>              sdhci_remove_host(host, dead);
>>>>
>>>> +             kfree(pxa->ops);
>>>>              if (host->ioaddr)
>>>>                      iounmap(host->ioaddr);
>>>>              if (pxa->res)
>>>> --
>>>> 1.7.0.4
>>>
>>>
>
>

Re: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

[Philip Rakity]

was interested in the 9xx code. 


On Dec 7, 2010, at 7:38 AM, zhangfei gao wrote:

> 2010/12/7 Philip Rakity <prakity@marvell.com>:
>> 
>> Where is the code that implements
>> 
>>> +     unsigned int    clk_delay_cycles;
>>> +     unsigned int    clk_delay_sel;
> 
> The code is located under arch/arm/, it is used differently depends on
> different platfrom.
> For example, aspen could realize under arch/arm/mach-pxa/pxa168.c.
> Could you refer the code for mmp2.
> 
>> 
>> 
>> 
>> 
>> On Dec 6, 2010, at 9:56 PM, Raymond Wu wrote:
>> 
>>> Hi,
>>> 
>>> This patch is verified by: Raymond Wu(xywu@marvell.com) on pxa955 SAARB platform.
>>> 
>>> Thanks,
>>> Raymond
>>> 
>>> 
>>> -----Original Message-----
>>> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
>>> Sent: 2010年12月3日 14:31
>>> To: linux-mmc@vger.kernel.org
>>> Cc: Chris Ball; Eric Miao; Haojian Zhuang; niej0001@gmail.com; Philip Rakity; Mark Brown; Raymond Wu; Jun Nie
>>> Subject: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa
>>> 
>>> From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
>>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>>> Date: Tue, 30 Nov 2010 05:43:29 -0500
>>> Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
>>> 
>>>      Call back functions is added to support mmp2, pxa910, pxa168 and pxa955
>>> 
>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>>> CC: RaymondWu <xywu@marvell.com>
>>> CC: Jun Nie <njun@marvell.com>
>>> CC: Philip Rakity <prakity@marvell.com>
>>> ---
>>> arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
>>> drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
>>> 2 files changed, 98 insertions(+), 29 deletions(-)
>>> 
>>> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
>>> b/arch/arm/plat-pxa/include/plat/sdhci.h
>>> index 1ab332e..4ce3b05 100644
>>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>>> @@ -13,9 +13,14 @@
>>> #ifndef __PLAT_PXA_SDHCI_H
>>> #define __PLAT_PXA_SDHCI_H
>>> 
>>> +#include <linux/mmc/sdhci.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> /* pxa specific flag */
>>> /* Require clock free running */
>>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>>> +/* card alwayes wired to host, like on-chip emmc */
>>> +#define PXA_FLAG_CARD_PERMANENT      (1<<1)
>>> 
>>> /* Board design supports 8-bit data on SD/SDIO BUS */  #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2) @@ -25,11 +30,50 @@
>>> * @max_speed: the maximum speed supported
>>> * @quirks: quirks of specific device
>>> * @flags: flags for platform requirement
>>> + * @clk_delay_cycles:
>>> + *   mmp2: each step is roughly 100ps, 5bits width
>>> + *   pxa910 & pxa168: each step is 1ns, 4bits width
>>> + * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
>>> + * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
>>> + *   0: choose feedback clk
>>> + *   1: choose feedback clk + delay value
>>> + *   2: choose internal clk
>>> + * @ext_cd_gpio: gpio pin used for external CD line
>>> + * @ext_cd_gpio_invert: invert values for external CD gpio line
>>> + * @soc_set_timing: set timing for specific soc
>>> + * @ext_cd_init: Initialize external card detect subsystem
>>> + * @ext_cd_cleanup: Cleanup external card detect subsystem
>>> + * @soc_set_ops: overwrite host ops with soc specific ops
>>> */
>>> +struct sdhci_pxa;
>>> struct sdhci_pxa_platdata {
>>>      unsigned int    max_speed;
>>>      unsigned int    quirks;
>>>      unsigned int    flags;
>>> +     unsigned int    clk_delay_cycles;
>>> +     unsigned int    clk_delay_sel;
>>> +     unsigned int    ext_cd_gpio;
>>> +     bool            ext_cd_gpio_invert;
>>> +     bool            clk_delay_enable;
>>> +
>>> +     void (*soc_set_timing)(struct sdhci_host *host,
>>> +                     struct sdhci_pxa_platdata *pdata);
>>> +     int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
>>> +                                                int state), void *data);
>>> +     int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
>>> +                                                   int state), void *data);
>>> +     void (*soc_set_ops)(struct sdhci_pxa *pxa); };
>>> +
>>> +struct sdhci_pxa {
>>> +     struct sdhci_host               *host;
>>> +     struct sdhci_pxa_platdata       *pdata;
>>> +     struct clk                      *clk;
>>> +     struct resource                 *res;
>>> +     struct sdhci_ops                *ops;
>>> +
>>> +     u8      clk_enable;
>>> +     u8      power_mode;
>>> };
>>> 
>>> #endif /* __PLAT_PXA_SDHCI_H */
>>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c index 5a61208..f79dccf 100644
>>> --- a/drivers/mmc/host/sdhci-pxa.c
>>> +++ b/drivers/mmc/host/sdhci-pxa.c
>>> @@ -24,32 +24,22 @@
>>> #include <linux/clk.h>
>>> #include <linux/io.h>
>>> #include <linux/err.h>
>>> +#include <linux/slab.h>
>>> #include <plat/sdhci.h>
>>> #include "sdhci.h"
>>> 
>>> #define DRIVER_NAME   "sdhci-pxa"
>>> 
>>> -#define SD_FIFO_PARAM                0x104
>>> -#define DIS_PAD_SD_CLK_GATE  0x400
>>> -
>>> -struct sdhci_pxa {
>>> -     struct sdhci_host               *host;
>>> -     struct sdhci_pxa_platdata       *pdata;
>>> -     struct clk                      *clk;
>>> -     struct resource                 *res;
>>> -
>>> -     u8 clk_enable;
>>> -};
>>> -
>>> /*****************************************************************************\
>>> *                                                                           *
>>> * SDHCI core callbacks                                                      *
>>> *                                                                           *
>>> \*****************************************************************************/
>>> +
>>> static void set_clock(struct sdhci_host *host, unsigned int clock)  {
>>>      struct sdhci_pxa *pxa = sdhci_priv(host);
>>> -     u32 tmp = 0;
>>> +     struct sdhci_pxa_platdata *pdata = pxa->pdata;
>>> 
>>>      if (clock == 0) {
>>>              if (pxa->clk_enable) {
>>> @@ -57,21 +47,31 @@ 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;
>>> -             }
>>> +             if (pdata && pdata->soc_set_timing)
>>> +                     pdata->soc_set_timing(host, pdata);
>>> +             clk_enable(pxa->clk);
>>> +             pxa->clk_enable = 1;
>>>      }
>>> }
>>> 
>>> -static struct sdhci_ops sdhci_pxa_ops = {
>>> -     .set_clock = set_clock,
>>> -};
>>> +static void sdhci_pxa_notify_change(struct platform_device *dev, int
>>> +state) {
>>> +     struct sdhci_host *host = platform_get_drvdata(dev);
>>> +     unsigned long flags;
>>> +
>>> +     if (host) {
>>> +             spin_lock_irqsave(&host->lock, flags);
>>> +             if (state) {
>>> +                     dev_dbg(&dev->dev, "card inserted.\n");
>>> +                     host->flags &= ~SDHCI_DEVICE_DEAD;
>>> +             } else {
>>> +                     dev_dbg(&dev->dev, "card removed.\n");
>>> +                     host->flags |= SDHCI_DEVICE_DEAD;
>>> +             }
>>> +             tasklet_schedule(&host->card_tasklet);
>>> +             spin_unlock_irqrestore(&host->lock, flags);
>>> +     }
>>> +}
>>> 
>>> /*****************************************************************************\
>>> *                                                                           *
>>> @@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
>>>      pxa->pdata = pdata;
>>>      pxa->clk_enable = 0;
>>> 
>>> +     pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>>> +     if (!pxa->ops) {
>>> +             ret = -ENOMEM;
>>> +             goto out;
>>> +     }
>>> +
>>>      pxa->clk = clk_get(dev, "PXA-SDHCLK");
>>>      if (IS_ERR(pxa->clk)) {
>>>              dev_err(dev, "failed to get io clock\n"); @@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
>>>      }
>>> 
>>>      host->hw_name = "MMC";
>>> -     host->ops = &sdhci_pxa_ops;
>>>      host->irq = irq;
>>>      host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>>> 
>>> -     if (pdata->quirks)
>>> +     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 && pdata->quirks)
>>>              host->quirks |= pdata->quirks;
>>> 
>>>      /* If slot design supports 8 bit data, indicate this to MMC. */
>>>      if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>>>              host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>> 
>>> +     if (pdata && pdata->soc_set_ops)
>>> +             pdata->soc_set_ops(pxa);
>>> +
>>> +     pxa->ops->set_clock = set_clock;
>>> +     host->ops = pxa->ops;
>>> +
>>>      ret = sdhci_add_host(host);
>>>      if (ret) {
>>>              dev_err(&pdev->dev, "failed to add host\n");
>>>              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;
>>> 
>>> +     if (pdata && pdata->ext_cd_init)
>>> +             pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
>>>      platform_set_drvdata(pdev, host);
>>> 
>>>      return 0;
>>> out:
>>>      if (host) {
>>>              clk_put(pxa->clk);
>>> +             kfree(pxa->ops);
>>>              if (host->ioaddr)
>>>                      iounmap(host->ioaddr);
>>>              if (pxa->res)
>>> @@ -173,18 +193,23 @@ out:
>>> 
>>> static int __devexit sdhci_pxa_remove(struct platform_device *pdev)  {
>>> +     struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>>>      struct sdhci_host *host = platform_get_drvdata(pdev);
>>>      struct sdhci_pxa *pxa = sdhci_priv(host);
>>>      int dead = 0;
>>>      u32 scratch;
>>> 
>>>      if (host) {
>>> +             if (pdata && pdata->ext_cd_cleanup)
>>> +                     pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
>>> +
>>>              scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>>>              if (scratch == (u32)-1)
>>>                      dead = 1;
>>> 
>>>              sdhci_remove_host(host, dead);
>>> 
>>> +             kfree(pxa->ops);
>>>              if (host->ioaddr)
>>>                      iounmap(host->ioaddr);
>>>              if (pxa->res)
>>> --
>>> 1.7.0.4
>> 
>> 

Re: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

[zhangfei gao]

2010/12/7 Philip Rakity <prakity@marvell.com>:
>
> Where is the code that implements
>
>> +     unsigned int    clk_delay_cycles;
>> +     unsigned int    clk_delay_sel;

The code is located under arch/arm/, it is used differently depends on
different platfrom.
For example, aspen could realize under arch/arm/mach-pxa/pxa168.c.
Could you refer the code for mmp2.

>
>
>
>
> On Dec 6, 2010, at 9:56 PM, Raymond Wu wrote:
>
>> Hi,
>>
>> This patch is verified by: Raymond Wu(xywu@marvell.com) on pxa955 SAARB platform.
>>
>> Thanks,
>> Raymond
>>
>>
>> -----Original Message-----
>> From: zhangfei gao [mailto:zhangfei.gao@gmail.com]
>> Sent: 2010年12月3日 14:31
>> To: linux-mmc@vger.kernel.org
>> Cc: Chris Ball; Eric Miao; Haojian Zhuang; niej0001@gmail.com; Philip Rakity; Mark Brown; Raymond Wu; Jun Nie
>> Subject: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa
>>
>> From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Tue, 30 Nov 2010 05:43:29 -0500
>> Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
>>
>>       Call back functions is added to support mmp2, pxa910, pxa168 and pxa955
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> CC: RaymondWu <xywu@marvell.com>
>> CC: Jun Nie <njun@marvell.com>
>> CC: Philip Rakity <prakity@marvell.com>
>> ---
>> arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
>> drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
>> 2 files changed, 98 insertions(+), 29 deletions(-)
>>
>> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
>> b/arch/arm/plat-pxa/include/plat/sdhci.h
>> index 1ab332e..4ce3b05 100644
>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>> @@ -13,9 +13,14 @@
>> #ifndef __PLAT_PXA_SDHCI_H
>> #define __PLAT_PXA_SDHCI_H
>>
>> +#include <linux/mmc/sdhci.h>
>> +#include <linux/platform_device.h>
>> +
>> /* pxa specific flag */
>> /* Require clock free running */
>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>> +/* card alwayes wired to host, like on-chip emmc */
>> +#define PXA_FLAG_CARD_PERMANENT      (1<<1)
>>
>> /* Board design supports 8-bit data on SD/SDIO BUS */  #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2) @@ -25,11 +30,50 @@
>>  * @max_speed: the maximum speed supported
>>  * @quirks: quirks of specific device
>>  * @flags: flags for platform requirement
>> + * @clk_delay_cycles:
>> + *   mmp2: each step is roughly 100ps, 5bits width
>> + *   pxa910 & pxa168: each step is 1ns, 4bits width
>> + * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
>> + * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
>> + *   0: choose feedback clk
>> + *   1: choose feedback clk + delay value
>> + *   2: choose internal clk
>> + * @ext_cd_gpio: gpio pin used for external CD line
>> + * @ext_cd_gpio_invert: invert values for external CD gpio line
>> + * @soc_set_timing: set timing for specific soc
>> + * @ext_cd_init: Initialize external card detect subsystem
>> + * @ext_cd_cleanup: Cleanup external card detect subsystem
>> + * @soc_set_ops: overwrite host ops with soc specific ops
>>  */
>> +struct sdhci_pxa;
>> struct sdhci_pxa_platdata {
>>       unsigned int    max_speed;
>>       unsigned int    quirks;
>>       unsigned int    flags;
>> +     unsigned int    clk_delay_cycles;
>> +     unsigned int    clk_delay_sel;
>> +     unsigned int    ext_cd_gpio;
>> +     bool            ext_cd_gpio_invert;
>> +     bool            clk_delay_enable;
>> +
>> +     void (*soc_set_timing)(struct sdhci_host *host,
>> +                     struct sdhci_pxa_platdata *pdata);
>> +     int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
>> +                                                int state), void *data);
>> +     int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
>> +                                                   int state), void *data);
>> +     void (*soc_set_ops)(struct sdhci_pxa *pxa); };
>> +
>> +struct sdhci_pxa {
>> +     struct sdhci_host               *host;
>> +     struct sdhci_pxa_platdata       *pdata;
>> +     struct clk                      *clk;
>> +     struct resource                 *res;
>> +     struct sdhci_ops                *ops;
>> +
>> +     u8      clk_enable;
>> +     u8      power_mode;
>> };
>>
>> #endif /* __PLAT_PXA_SDHCI_H */
>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c index 5a61208..f79dccf 100644
>> --- a/drivers/mmc/host/sdhci-pxa.c
>> +++ b/drivers/mmc/host/sdhci-pxa.c
>> @@ -24,32 +24,22 @@
>> #include <linux/clk.h>
>> #include <linux/io.h>
>> #include <linux/err.h>
>> +#include <linux/slab.h>
>> #include <plat/sdhci.h>
>> #include "sdhci.h"
>>
>> #define DRIVER_NAME   "sdhci-pxa"
>>
>> -#define SD_FIFO_PARAM                0x104
>> -#define DIS_PAD_SD_CLK_GATE  0x400
>> -
>> -struct sdhci_pxa {
>> -     struct sdhci_host               *host;
>> -     struct sdhci_pxa_platdata       *pdata;
>> -     struct clk                      *clk;
>> -     struct resource                 *res;
>> -
>> -     u8 clk_enable;
>> -};
>> -
>> /*****************************************************************************\
>>  *                                                                           *
>>  * SDHCI core callbacks                                                      *
>>  *                                                                           *
>> \*****************************************************************************/
>> +
>> static void set_clock(struct sdhci_host *host, unsigned int clock)  {
>>       struct sdhci_pxa *pxa = sdhci_priv(host);
>> -     u32 tmp = 0;
>> +     struct sdhci_pxa_platdata *pdata = pxa->pdata;
>>
>>       if (clock == 0) {
>>               if (pxa->clk_enable) {
>> @@ -57,21 +47,31 @@ 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;
>> -             }
>> +             if (pdata && pdata->soc_set_timing)
>> +                     pdata->soc_set_timing(host, pdata);
>> +             clk_enable(pxa->clk);
>> +             pxa->clk_enable = 1;
>>       }
>> }
>>
>> -static struct sdhci_ops sdhci_pxa_ops = {
>> -     .set_clock = set_clock,
>> -};
>> +static void sdhci_pxa_notify_change(struct platform_device *dev, int
>> +state) {
>> +     struct sdhci_host *host = platform_get_drvdata(dev);
>> +     unsigned long flags;
>> +
>> +     if (host) {
>> +             spin_lock_irqsave(&host->lock, flags);
>> +             if (state) {
>> +                     dev_dbg(&dev->dev, "card inserted.\n");
>> +                     host->flags &= ~SDHCI_DEVICE_DEAD;
>> +             } else {
>> +                     dev_dbg(&dev->dev, "card removed.\n");
>> +                     host->flags |= SDHCI_DEVICE_DEAD;
>> +             }
>> +             tasklet_schedule(&host->card_tasklet);
>> +             spin_unlock_irqrestore(&host->lock, flags);
>> +     }
>> +}
>>
>> /*****************************************************************************\
>>  *                                                                           *
>> @@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
>>       pxa->pdata = pdata;
>>       pxa->clk_enable = 0;
>>
>> +     pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>> +     if (!pxa->ops) {
>> +             ret = -ENOMEM;
>> +             goto out;
>> +     }
>> +
>>       pxa->clk = clk_get(dev, "PXA-SDHCLK");
>>       if (IS_ERR(pxa->clk)) {
>>               dev_err(dev, "failed to get io clock\n"); @@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
>>       }
>>
>>       host->hw_name = "MMC";
>> -     host->ops = &sdhci_pxa_ops;
>>       host->irq = irq;
>>       host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>>
>> -     if (pdata->quirks)
>> +     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 && pdata->quirks)
>>               host->quirks |= pdata->quirks;
>>
>>       /* If slot design supports 8 bit data, indicate this to MMC. */
>>       if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
>>               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>
>> +     if (pdata && pdata->soc_set_ops)
>> +             pdata->soc_set_ops(pxa);
>> +
>> +     pxa->ops->set_clock = set_clock;
>> +     host->ops = pxa->ops;
>> +
>>       ret = sdhci_add_host(host);
>>       if (ret) {
>>               dev_err(&pdev->dev, "failed to add host\n");
>>               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;
>>
>> +     if (pdata && pdata->ext_cd_init)
>> +             pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
>>       platform_set_drvdata(pdev, host);
>>
>>       return 0;
>> out:
>>       if (host) {
>>               clk_put(pxa->clk);
>> +             kfree(pxa->ops);
>>               if (host->ioaddr)
>>                       iounmap(host->ioaddr);
>>               if (pxa->res)
>> @@ -173,18 +193,23 @@ out:
>>
>> static int __devexit sdhci_pxa_remove(struct platform_device *pdev)  {
>> +     struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
>>       struct sdhci_host *host = platform_get_drvdata(pdev);
>>       struct sdhci_pxa *pxa = sdhci_priv(host);
>>       int dead = 0;
>>       u32 scratch;
>>
>>       if (host) {
>> +             if (pdata && pdata->ext_cd_cleanup)
>> +                     pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
>> +
>>               scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>>               if (scratch == (u32)-1)
>>                       dead = 1;
>>
>>               sdhci_remove_host(host, dead);
>>
>> +             kfree(pxa->ops);
>>               if (host->ioaddr)
>>                       iounmap(host->ioaddr);
>>               if (pxa->res)
>> --
>> 1.7.0.4
>
>

Re: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

[Philip Rakity]

Where is the code that implements  

> +	unsigned int	clk_delay_cycles;
> +	unsigned int	clk_delay_sel;




On Dec 6, 2010, at 9:56 PM, Raymond Wu wrote:

> Hi,
> 
> This patch is verified by: Raymond Wu(xywu@marvell.com) on pxa955 SAARB platform.
> 
> Thanks,
> Raymond
> 
> 
> -----Original Message-----
> From: zhangfei gao [mailto:zhangfei.gao@gmail.com] 
> Sent: 2010年12月3日 14:31
> To: linux-mmc@vger.kernel.org
> Cc: Chris Ball; Eric Miao; Haojian Zhuang; niej0001@gmail.com; Philip Rakity; Mark Brown; Raymond Wu; Jun Nie
> Subject: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa
> 
> From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Tue, 30 Nov 2010 05:43:29 -0500
> Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
> 
> 	Call back functions is added to support mmp2, pxa910, pxa168 and pxa955
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> CC: RaymondWu <xywu@marvell.com>
> CC: Jun Nie <njun@marvell.com>
> CC: Philip Rakity <prakity@marvell.com>
> ---
> arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
> drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
> 2 files changed, 98 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> index 1ab332e..4ce3b05 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -13,9 +13,14 @@
> #ifndef __PLAT_PXA_SDHCI_H
> #define __PLAT_PXA_SDHCI_H
> 
> +#include <linux/mmc/sdhci.h>
> +#include <linux/platform_device.h>
> +
> /* pxa specific flag */
> /* Require clock free running */
> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
> +/* card alwayes wired to host, like on-chip emmc */
> +#define PXA_FLAG_CARD_PERMANENT	(1<<1)
> 
> /* Board design supports 8-bit data on SD/SDIO BUS */  #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2) @@ -25,11 +30,50 @@
>  * @max_speed: the maximum speed supported
>  * @quirks: quirks of specific device
>  * @flags: flags for platform requirement
> + * @clk_delay_cycles:
> + *	mmp2: each step is roughly 100ps, 5bits width
> + *	pxa910 & pxa168: each step is 1ns, 4bits width
> + * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
> + * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
> + *	0: choose feedback clk
> + *	1: choose feedback clk + delay value
> + *	2: choose internal clk
> + * @ext_cd_gpio: gpio pin used for external CD line
> + * @ext_cd_gpio_invert: invert values for external CD gpio line
> + * @soc_set_timing: set timing for specific soc
> + * @ext_cd_init: Initialize external card detect subsystem
> + * @ext_cd_cleanup: Cleanup external card detect subsystem
> + * @soc_set_ops: overwrite host ops with soc specific ops
>  */
> +struct sdhci_pxa;
> struct sdhci_pxa_platdata {
> 	unsigned int	max_speed;
> 	unsigned int	quirks;
> 	unsigned int	flags;
> +	unsigned int	clk_delay_cycles;
> +	unsigned int	clk_delay_sel;
> +	unsigned int	ext_cd_gpio;
> +	bool		ext_cd_gpio_invert;
> +	bool		clk_delay_enable;
> +
> +	void (*soc_set_timing)(struct sdhci_host *host,
> +			struct sdhci_pxa_platdata *pdata);
> +	int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
> +						   int state), void *data);
> +	int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
> +						      int state), void *data);
> +	void (*soc_set_ops)(struct sdhci_pxa *pxa); };
> +
> +struct sdhci_pxa {
> +	struct sdhci_host		*host;
> +	struct sdhci_pxa_platdata	*pdata;
> +	struct clk			*clk;
> +	struct resource			*res;
> +	struct sdhci_ops		*ops;
> +
> +	u8	clk_enable;
> +	u8	power_mode;
> };
> 
> #endif /* __PLAT_PXA_SDHCI_H */
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c index 5a61208..f79dccf 100644
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -24,32 +24,22 @@
> #include <linux/clk.h>
> #include <linux/io.h>
> #include <linux/err.h>
> +#include <linux/slab.h>
> #include <plat/sdhci.h>
> #include "sdhci.h"
> 
> #define DRIVER_NAME	"sdhci-pxa"
> 
> -#define SD_FIFO_PARAM		0x104
> -#define DIS_PAD_SD_CLK_GATE	0x400
> -
> -struct sdhci_pxa {
> -	struct sdhci_host		*host;
> -	struct sdhci_pxa_platdata	*pdata;
> -	struct clk			*clk;
> -	struct resource			*res;
> -
> -	u8 clk_enable;
> -};
> -
> /*****************************************************************************\
>  *                                                                           *
>  * SDHCI core callbacks                                                      *
>  *                                                                           *
> \*****************************************************************************/
> +
> static void set_clock(struct sdhci_host *host, unsigned int clock)  {
> 	struct sdhci_pxa *pxa = sdhci_priv(host);
> -	u32 tmp = 0;
> +	struct sdhci_pxa_platdata *pdata = pxa->pdata;
> 
> 	if (clock == 0) {
> 		if (pxa->clk_enable) {
> @@ -57,21 +47,31 @@ 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;
> -		}
> +		if (pdata && pdata->soc_set_timing)
> +			pdata->soc_set_timing(host, pdata);
> +		clk_enable(pxa->clk);
> +		pxa->clk_enable = 1;
> 	}
> }
> 
> -static struct sdhci_ops sdhci_pxa_ops = {
> -	.set_clock = set_clock,
> -};
> +static void sdhci_pxa_notify_change(struct platform_device *dev, int 
> +state) {
> +	struct sdhci_host *host = platform_get_drvdata(dev);
> +	unsigned long flags;
> +
> +	if (host) {
> +		spin_lock_irqsave(&host->lock, flags);
> +		if (state) {
> +			dev_dbg(&dev->dev, "card inserted.\n");
> +			host->flags &= ~SDHCI_DEVICE_DEAD;
> +		} else {
> +			dev_dbg(&dev->dev, "card removed.\n");
> +			host->flags |= SDHCI_DEVICE_DEAD;
> +		}
> +		tasklet_schedule(&host->card_tasklet);
> +		spin_unlock_irqrestore(&host->lock, flags);
> +	}
> +}
> 
> /*****************************************************************************\
>  *                                                                           *
> @@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
> 	pxa->pdata = pdata;
> 	pxa->clk_enable = 0;
> 
> +	pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
> +	if (!pxa->ops) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> 	pxa->clk = clk_get(dev, "PXA-SDHCLK");
> 	if (IS_ERR(pxa->clk)) {
> 		dev_err(dev, "failed to get io clock\n"); @@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
> 	}
> 
> 	host->hw_name = "MMC";
> -	host->ops = &sdhci_pxa_ops;
> 	host->irq = irq;
> 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> 
> -	if (pdata->quirks)
> +	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 && pdata->quirks)
> 		host->quirks |= pdata->quirks;
> 
> 	/* If slot design supports 8 bit data, indicate this to MMC. */
> 	if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
> 		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> 
> +	if (pdata && pdata->soc_set_ops)
> +		pdata->soc_set_ops(pxa);
> +
> +	pxa->ops->set_clock = set_clock;
> +	host->ops = pxa->ops;
> +
> 	ret = sdhci_add_host(host);
> 	if (ret) {
> 		dev_err(&pdev->dev, "failed to add host\n");
> 		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;
> 
> +	if (pdata && pdata->ext_cd_init)
> +		pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
> 	platform_set_drvdata(pdev, host);
> 
> 	return 0;
> out:
> 	if (host) {
> 		clk_put(pxa->clk);
> +		kfree(pxa->ops);
> 		if (host->ioaddr)
> 			iounmap(host->ioaddr);
> 		if (pxa->res)
> @@ -173,18 +193,23 @@ out:
> 
> static int __devexit sdhci_pxa_remove(struct platform_device *pdev)  {
> +	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> 	struct sdhci_host *host = platform_get_drvdata(pdev);
> 	struct sdhci_pxa *pxa = sdhci_priv(host);
> 	int dead = 0;
> 	u32 scratch;
> 
> 	if (host) {
> +		if (pdata && pdata->ext_cd_cleanup)
> +			pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
> +
> 		scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> 		if (scratch == (u32)-1)
> 			dead = 1;
> 
> 		sdhci_remove_host(host, dead);
> 
> +		kfree(pxa->ops);
> 		if (host->ioaddr)
> 			iounmap(host->ioaddr);
> 		if (pxa->res)
> --
> 1.7.0.4

RE: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

[Raymond Wu]

Hi,

This patch is verified by: Raymond Wu(xywu@marvell.com) on pxa955 SAARB platform.

Thanks,
Raymond


-----Original Message-----
From: zhangfei gao [mailto:zhangfei.gao@gmail.com] 
Sent: 2010年12月3日 14:31
To: linux-mmc@vger.kernel.org
Cc: Chris Ball; Eric Miao; Haojian Zhuang; niej0001@gmail.com; Philip Rakity; Mark Brown; Raymond Wu; Jun Nie
Subject: [PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Tue, 30 Nov 2010 05:43:29 -0500
Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

	Call back functions is added to support mmp2, pxa910, pxa168 and pxa955

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
CC: RaymondWu <xywu@marvell.com>
CC: Jun Nie <njun@marvell.com>
CC: Philip Rakity <prakity@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
 drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
 2 files changed, 98 insertions(+), 29 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
b/arch/arm/plat-pxa/include/plat/sdhci.h
index 1ab332e..4ce3b05 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -13,9 +13,14 @@
 #ifndef __PLAT_PXA_SDHCI_H
 #define __PLAT_PXA_SDHCI_H

+#include <linux/mmc/sdhci.h>
+#include <linux/platform_device.h>
+
 /* pxa specific flag */
 /* Require clock free running */
 #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
+/* card alwayes wired to host, like on-chip emmc */
+#define PXA_FLAG_CARD_PERMANENT	(1<<1)

 /* Board design supports 8-bit data on SD/SDIO BUS */  #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2) @@ -25,11 +30,50 @@
  * @max_speed: the maximum speed supported
  * @quirks: quirks of specific device
  * @flags: flags for platform requirement
+ * @clk_delay_cycles:
+ *	mmp2: each step is roughly 100ps, 5bits width
+ *	pxa910 & pxa168: each step is 1ns, 4bits width
+ * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
+ * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
+ *	0: choose feedback clk
+ *	1: choose feedback clk + delay value
+ *	2: choose internal clk
+ * @ext_cd_gpio: gpio pin used for external CD line
+ * @ext_cd_gpio_invert: invert values for external CD gpio line
+ * @soc_set_timing: set timing for specific soc
+ * @ext_cd_init: Initialize external card detect subsystem
+ * @ext_cd_cleanup: Cleanup external card detect subsystem
+ * @soc_set_ops: overwrite host ops with soc specific ops
  */
+struct sdhci_pxa;
 struct sdhci_pxa_platdata {
 	unsigned int	max_speed;
 	unsigned int	quirks;
 	unsigned int	flags;
+	unsigned int	clk_delay_cycles;
+	unsigned int	clk_delay_sel;
+	unsigned int	ext_cd_gpio;
+	bool		ext_cd_gpio_invert;
+	bool		clk_delay_enable;
+
+	void (*soc_set_timing)(struct sdhci_host *host,
+			struct sdhci_pxa_platdata *pdata);
+	int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
+						   int state), void *data);
+	int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
+						      int state), void *data);
+	void (*soc_set_ops)(struct sdhci_pxa *pxa); };
+
+struct sdhci_pxa {
+	struct sdhci_host		*host;
+	struct sdhci_pxa_platdata	*pdata;
+	struct clk			*clk;
+	struct resource			*res;
+	struct sdhci_ops		*ops;
+
+	u8	clk_enable;
+	u8	power_mode;
 };

 #endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c index 5a61208..f79dccf 100644
--- a/drivers/mmc/host/sdhci-pxa.c
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -24,32 +24,22 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/slab.h>
 #include <plat/sdhci.h>
 #include "sdhci.h"

 #define DRIVER_NAME	"sdhci-pxa"

-#define SD_FIFO_PARAM		0x104
-#define DIS_PAD_SD_CLK_GATE	0x400
-
-struct sdhci_pxa {
-	struct sdhci_host		*host;
-	struct sdhci_pxa_platdata	*pdata;
-	struct clk			*clk;
-	struct resource			*res;
-
-	u8 clk_enable;
-};
-
 /*****************************************************************************\
  *                                                                           *
  * SDHCI core callbacks                                                      *
  *                                                                           *
 \*****************************************************************************/
+
 static void set_clock(struct sdhci_host *host, unsigned int clock)  {
 	struct sdhci_pxa *pxa = sdhci_priv(host);
-	u32 tmp = 0;
+	struct sdhci_pxa_platdata *pdata = pxa->pdata;

 	if (clock == 0) {
 		if (pxa->clk_enable) {
@@ -57,21 +47,31 @@ 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;
-		}
+		if (pdata && pdata->soc_set_timing)
+			pdata->soc_set_timing(host, pdata);
+		clk_enable(pxa->clk);
+		pxa->clk_enable = 1;
 	}
 }

-static struct sdhci_ops sdhci_pxa_ops = {
-	.set_clock = set_clock,
-};
+static void sdhci_pxa_notify_change(struct platform_device *dev, int 
+state) {
+	struct sdhci_host *host = platform_get_drvdata(dev);
+	unsigned long flags;
+
+	if (host) {
+		spin_lock_irqsave(&host->lock, flags);
+		if (state) {
+			dev_dbg(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+		} else {
+			dev_dbg(&dev->dev, "card removed.\n");
+			host->flags |= SDHCI_DEVICE_DEAD;
+		}
+		tasklet_schedule(&host->card_tasklet);
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+}

 /*****************************************************************************\
  *                                                                           *
@@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
 	pxa->pdata = pdata;
 	pxa->clk_enable = 0;

+	pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
+	if (!pxa->ops) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	pxa->clk = clk_get(dev, "PXA-SDHCLK");
 	if (IS_ERR(pxa->clk)) {
 		dev_err(dev, "failed to get io clock\n"); @@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
 	}

 	host->hw_name = "MMC";
-	host->ops = &sdhci_pxa_ops;
 	host->irq = irq;
 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;

-	if (pdata->quirks)
+	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 && pdata->quirks)
 		host->quirks |= pdata->quirks;

 	/* If slot design supports 8 bit data, indicate this to MMC. */
 	if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
 		host->mmc->caps |= MMC_CAP_8_BIT_DATA;

+	if (pdata && pdata->soc_set_ops)
+		pdata->soc_set_ops(pxa);
+
+	pxa->ops->set_clock = set_clock;
+	host->ops = pxa->ops;
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to add host\n");
 		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;

+	if (pdata && pdata->ext_cd_init)
+		pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
 	platform_set_drvdata(pdev, host);

 	return 0;
 out:
 	if (host) {
 		clk_put(pxa->clk);
+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
@@ -173,18 +193,23 @@ out:

 static int __devexit sdhci_pxa_remove(struct platform_device *pdev)  {
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pxa *pxa = sdhci_priv(host);
 	int dead = 0;
 	u32 scratch;

 	if (host) {
+		if (pdata && pdata->ext_cd_cleanup)
+			pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
+
 		scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
 		if (scratch == (u32)-1)
 			dead = 1;

 		sdhci_remove_host(host, dead);

+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
--
1.7.0.4

[PATCH 2/2] sdhci-pxa add call back interface to share sdhci-pxa

[zhangfei gao]

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

>From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Tue, 30 Nov 2010 05:43:29 -0500
Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

	Call back functions is added to support mmp2, pxa910, pxa168 and pxa955

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
CC: RaymondWu <xywu@marvell.com>
CC: Jun Nie <njun@marvell.com>
CC: Philip Rakity <prakity@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
 drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
 2 files changed, 98 insertions(+), 29 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
b/arch/arm/plat-pxa/include/plat/sdhci.h
index 1ab332e..4ce3b05 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -13,9 +13,14 @@
 #ifndef __PLAT_PXA_SDHCI_H
 #define __PLAT_PXA_SDHCI_H

+#include <linux/mmc/sdhci.h>
+#include <linux/platform_device.h>
+
 /* pxa specific flag */
 /* Require clock free running */
 #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
+/* card alwayes wired to host, like on-chip emmc */
+#define PXA_FLAG_CARD_PERMANENT	(1<<1)

 /* Board design supports 8-bit data on SD/SDIO BUS */
 #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
@@ -25,11 +30,50 @@
  * @max_speed: the maximum speed supported
  * @quirks: quirks of specific device
  * @flags: flags for platform requirement
+ * @clk_delay_cycles:
+ *	mmp2: each step is roughly 100ps, 5bits width
+ *	pxa910 & pxa168: each step is 1ns, 4bits width
+ * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
+ * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
+ *	0: choose feedback clk
+ *	1: choose feedback clk + delay value
+ *	2: choose internal clk
+ * @ext_cd_gpio: gpio pin used for external CD line
+ * @ext_cd_gpio_invert: invert values for external CD gpio line
+ * @soc_set_timing: set timing for specific soc
+ * @ext_cd_init: Initialize external card detect subsystem
+ * @ext_cd_cleanup: Cleanup external card detect subsystem
+ * @soc_set_ops: overwrite host ops with soc specific ops
  */
+struct sdhci_pxa;
 struct sdhci_pxa_platdata {
 	unsigned int	max_speed;
 	unsigned int	quirks;
 	unsigned int	flags;
+	unsigned int	clk_delay_cycles;
+	unsigned int	clk_delay_sel;
+	unsigned int	ext_cd_gpio;
+	bool		ext_cd_gpio_invert;
+	bool		clk_delay_enable;
+
+	void (*soc_set_timing)(struct sdhci_host *host,
+			struct sdhci_pxa_platdata *pdata);
+	int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
+						   int state), void *data);
+	int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
+						      int state), void *data);
+	void (*soc_set_ops)(struct sdhci_pxa *pxa);
+};
+
+struct sdhci_pxa {
+	struct sdhci_host		*host;
+	struct sdhci_pxa_platdata	*pdata;
+	struct clk			*clk;
+	struct resource			*res;
+	struct sdhci_ops		*ops;
+
+	u8	clk_enable;
+	u8	power_mode;
 };

 #endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
index 5a61208..f79dccf 100644
--- a/drivers/mmc/host/sdhci-pxa.c
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -24,32 +24,22 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/slab.h>
 #include <plat/sdhci.h>
 #include "sdhci.h"

 #define DRIVER_NAME	"sdhci-pxa"

-#define SD_FIFO_PARAM		0x104
-#define DIS_PAD_SD_CLK_GATE	0x400
-
-struct sdhci_pxa {
-	struct sdhci_host		*host;
-	struct sdhci_pxa_platdata	*pdata;
-	struct clk			*clk;
-	struct resource			*res;
-
-	u8 clk_enable;
-};
-
 /*****************************************************************************\
  *                                                                           *
  * SDHCI core callbacks                                                      *
  *                                                                           *
 \*****************************************************************************/
+
 static void set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pxa *pxa = sdhci_priv(host);
-	u32 tmp = 0;
+	struct sdhci_pxa_platdata *pdata = pxa->pdata;

 	if (clock == 0) {
 		if (pxa->clk_enable) {
@@ -57,21 +47,31 @@ 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;
-		}
+		if (pdata && pdata->soc_set_timing)
+			pdata->soc_set_timing(host, pdata);
+		clk_enable(pxa->clk);
+		pxa->clk_enable = 1;
 	}
 }

-static struct sdhci_ops sdhci_pxa_ops = {
-	.set_clock = set_clock,
-};
+static void sdhci_pxa_notify_change(struct platform_device *dev, int state)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+	unsigned long flags;
+
+	if (host) {
+		spin_lock_irqsave(&host->lock, flags);
+		if (state) {
+			dev_dbg(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+		} else {
+			dev_dbg(&dev->dev, "card removed.\n");
+			host->flags |= SDHCI_DEVICE_DEAD;
+		}
+		tasklet_schedule(&host->card_tasklet);
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+}

 /*****************************************************************************\
  *                                                                           *
@@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct
platform_device *pdev)
 	pxa->pdata = pdata;
 	pxa->clk_enable = 0;

+	pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
+	if (!pxa->ops) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	pxa->clk = clk_get(dev, "PXA-SDHCLK");
 	if (IS_ERR(pxa->clk)) {
 		dev_err(dev, "failed to get io clock\n");
@@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct
platform_device *pdev)
 	}

 	host->hw_name = "MMC";
-	host->ops = &sdhci_pxa_ops;
 	host->irq = irq;
 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;

-	if (pdata->quirks)
+	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 && pdata->quirks)
 		host->quirks |= pdata->quirks;

 	/* If slot design supports 8 bit data, indicate this to MMC. */
 	if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
 		host->mmc->caps |= MMC_CAP_8_BIT_DATA;

+	if (pdata && pdata->soc_set_ops)
+		pdata->soc_set_ops(pxa);
+
+	pxa->ops->set_clock = set_clock;
+	host->ops = pxa->ops;
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to add host\n");
 		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;

+	if (pdata && pdata->ext_cd_init)
+		pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
 	platform_set_drvdata(pdev, host);

 	return 0;
 out:
 	if (host) {
 		clk_put(pxa->clk);
+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
@@ -173,18 +193,23 @@ out:

 static int __devexit sdhci_pxa_remove(struct platform_device *pdev)
 {
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pxa *pxa = sdhci_priv(host);
 	int dead = 0;
 	u32 scratch;

 	if (host) {
+		if (pdata && pdata->ext_cd_cleanup)
+			pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
+
 		scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
 		if (scratch == (u32)-1)
 			dead = 1;

 		sdhci_remove_host(host, dead);

+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
-- 
1.7.0.4

[-- Attachment #2: 0002-sdhci-pxa-add-call-back-interface-to-share-sdhci-pxa.patch --]
[-- Type: text/x-patch, Size: 7887 bytes --]

From fb5927afac7cbb44805093816bea51dde33da1e5 Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Tue, 30 Nov 2010 05:43:29 -0500
Subject: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa

	Call back functions is added to support mmp2, pxa910, pxa168 and pxa955

Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
CC: RaymondWu <xywu@marvell.com>
CC: Jun Nie <njun@marvell.com>
CC: Philip Rakity <prakity@marvell.com>
---
 arch/arm/plat-pxa/include/plat/sdhci.h |   44 +++++++++++++++++
 drivers/mmc/host/sdhci-pxa.c           |   83 +++++++++++++++++++++-----------
 2 files changed, 98 insertions(+), 29 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h b/arch/arm/plat-pxa/include/plat/sdhci.h
index 1ab332e..4ce3b05 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -13,9 +13,14 @@
 #ifndef __PLAT_PXA_SDHCI_H
 #define __PLAT_PXA_SDHCI_H
 
+#include <linux/mmc/sdhci.h>
+#include <linux/platform_device.h>
+
 /* pxa specific flag */
 /* Require clock free running */
 #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
+/* card alwayes wired to host, like on-chip emmc */
+#define PXA_FLAG_CARD_PERMANENT	(1<<1)
 
 /* Board design supports 8-bit data on SD/SDIO BUS */
 #define PXA_FLAG_SD_8_BIT_CAPABLE_SLOT (1<<2)
@@ -25,11 +30,50 @@
  * @max_speed: the maximum speed supported
  * @quirks: quirks of specific device
  * @flags: flags for platform requirement
+ * @clk_delay_cycles:
+ *	mmp2: each step is roughly 100ps, 5bits width
+ *	pxa910 & pxa168: each step is 1ns, 4bits width
+ * @clk_delay_enable: enable clk_delay or not, used on pxa910 & pxa168
+ * @clk_delay_sel: select clk_delay, used on pxa910 & pxa168
+ *	0: choose feedback clk
+ *	1: choose feedback clk + delay value
+ *	2: choose internal clk
+ * @ext_cd_gpio: gpio pin used for external CD line
+ * @ext_cd_gpio_invert: invert values for external CD gpio line
+ * @soc_set_timing: set timing for specific soc
+ * @ext_cd_init: Initialize external card detect subsystem
+ * @ext_cd_cleanup: Cleanup external card detect subsystem
+ * @soc_set_ops: overwrite host ops with soc specific ops
  */
+struct sdhci_pxa;
 struct sdhci_pxa_platdata {
 	unsigned int	max_speed;
 	unsigned int	quirks;
 	unsigned int	flags;
+	unsigned int	clk_delay_cycles;
+	unsigned int	clk_delay_sel;
+	unsigned int	ext_cd_gpio;
+	bool		ext_cd_gpio_invert;
+	bool		clk_delay_enable;
+
+	void (*soc_set_timing)(struct sdhci_host *host,
+			struct sdhci_pxa_platdata *pdata);
+	int (*ext_cd_init)(void (*notify_func)(struct platform_device *dev,
+						   int state), void *data);
+	int (*ext_cd_cleanup)(void (*notify_func)(struct platform_device *dev,
+						      int state), void *data);
+	void (*soc_set_ops)(struct sdhci_pxa *pxa);
+};
+
+struct sdhci_pxa {
+	struct sdhci_host		*host;
+	struct sdhci_pxa_platdata	*pdata;
+	struct clk			*clk;
+	struct resource			*res;
+	struct sdhci_ops		*ops;
+
+	u8	clk_enable;
+	u8	power_mode;
 };
 
 #endif /* __PLAT_PXA_SDHCI_H */
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
index 5a61208..f79dccf 100644
--- a/drivers/mmc/host/sdhci-pxa.c
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -24,32 +24,22 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/slab.h>
 #include <plat/sdhci.h>
 #include "sdhci.h"
 
 #define DRIVER_NAME	"sdhci-pxa"
 
-#define SD_FIFO_PARAM		0x104
-#define DIS_PAD_SD_CLK_GATE	0x400
-
-struct sdhci_pxa {
-	struct sdhci_host		*host;
-	struct sdhci_pxa_platdata	*pdata;
-	struct clk			*clk;
-	struct resource			*res;
-
-	u8 clk_enable;
-};
-
 /*****************************************************************************\
  *                                                                           *
  * SDHCI core callbacks                                                      *
  *                                                                           *
 \*****************************************************************************/
+
 static void set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	struct sdhci_pxa *pxa = sdhci_priv(host);
-	u32 tmp = 0;
+	struct sdhci_pxa_platdata *pdata = pxa->pdata;
 
 	if (clock == 0) {
 		if (pxa->clk_enable) {
@@ -57,21 +47,31 @@ 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;
-		}
+		if (pdata && pdata->soc_set_timing)
+			pdata->soc_set_timing(host, pdata);
+		clk_enable(pxa->clk);
+		pxa->clk_enable = 1;
 	}
 }
 
-static struct sdhci_ops sdhci_pxa_ops = {
-	.set_clock = set_clock,
-};
+static void sdhci_pxa_notify_change(struct platform_device *dev, int state)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+	unsigned long flags;
+
+	if (host) {
+		spin_lock_irqsave(&host->lock, flags);
+		if (state) {
+			dev_dbg(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+		} else {
+			dev_dbg(&dev->dev, "card removed.\n");
+			host->flags |= SDHCI_DEVICE_DEAD;
+		}
+		tasklet_schedule(&host->card_tasklet);
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+}
 
 /*****************************************************************************\
  *                                                                           *
@@ -111,6 +111,12 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
 	pxa->pdata = pdata;
 	pxa->clk_enable = 0;
 
+	pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
+	if (!pxa->ops) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	pxa->clk = clk_get(dev, "PXA-SDHCLK");
 	if (IS_ERR(pxa->clk)) {
 		dev_err(dev, "failed to get io clock\n");
@@ -134,32 +140,46 @@ static int __devinit sdhci_pxa_probe(struct platform_device *pdev)
 	}
 
 	host->hw_name = "MMC";
-	host->ops = &sdhci_pxa_ops;
 	host->irq = irq;
 	host->quirks = SDHCI_QUIRK_BROKEN_ADMA | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
-	if (pdata->quirks)
+	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 && pdata->quirks)
 		host->quirks |= pdata->quirks;
 
 	/* If slot design supports 8 bit data, indicate this to MMC. */
 	if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
 		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
 
+	if (pdata && pdata->soc_set_ops)
+		pdata->soc_set_ops(pxa);
+
+	pxa->ops->set_clock = set_clock;
+	host->ops = pxa->ops;
+
 	ret = sdhci_add_host(host);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to add host\n");
 		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;
 
+	if (pdata && pdata->ext_cd_init)
+		pdata->ext_cd_init(&sdhci_pxa_notify_change, pdata);
 	platform_set_drvdata(pdev, host);
 
 	return 0;
 out:
 	if (host) {
 		clk_put(pxa->clk);
+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
@@ -173,18 +193,23 @@ out:
 
 static int __devexit sdhci_pxa_remove(struct platform_device *pdev)
 {
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pxa *pxa = sdhci_priv(host);
 	int dead = 0;
 	u32 scratch;
 
 	if (host) {
+		if (pdata && pdata->ext_cd_cleanup)
+			pdata->ext_cd_cleanup(&sdhci_pxa_notify_change, pdata);
+
 		scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
 		if (scratch == (u32)-1)
 			dead = 1;
 
 		sdhci_remove_host(host, dead);
 
+		kfree(pxa->ops);
 		if (host->ioaddr)
 			iounmap(host->ioaddr);
 		if (pxa->res)
-- 
1.7.0.4