public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] sdhci-pltfm: add call back function
@ 2010-09-26  8:52 zhangfei gao
  2010-09-26 16:09 ` Wolfram Sang
  0 siblings, 1 reply; 5+ messages in thread
From: zhangfei gao @ 2010-09-26  8:52 UTC (permalink / raw)
  To: linux-mmc
  Cc: Chris Ball, Wolfram Sang, eric.y.miao, Haojian Zhuang,
	Richard Zhu

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

Add several call back to sdhci-pltfm.c, help give suggestion
1. struct sdhci_host *(*alloc_host)(struct device *dev), since
specific driver need some private variable to allocate and free,
including clk.
2. unsigned int  (*get_quirk)(struct sdhci_host *host); add this
because one driver may serve several device, each one may require
different quirk, and specific driver is right one to know.
3. void (*set_max_speed)(struct sdhci_host *host); this should be done
after add_host, which impact f_max.
4. int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data
*pdata, void* priv_pdata); copy from Wolfram Sang's patch to transfer
platform data, copy here for test.

>From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Sun, 26 Sep 2010 16:37:11 -0400
Subject: [PATCH 2/2] sdhci-pltfm: add call back function


Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
 include/linux/sdhci-pltfm.h    |    5 ++++-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index e045e3c..e06f047 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct
platform_device *pdev)
 {
 	struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
 	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	void *priv_pdata = NULL;
 	struct sdhci_host *host;
 	struct resource *iomem;
 	int ret;

-	if (!pdata && platid && platid->driver_data)
+	if (platid && platid->driver_data) {
 		pdata = (void *)platid->driver_data;
+		priv_pdata = pdev->dev.platform_data;
+	} else {
+		pdata = pdev->dev.platform_data;
+	}

 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!iomem) {
@@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct
platform_device *pdev)
 		dev_err(&pdev->dev, "Invalid iomem size. You may "
 			"experience problems.\n");

-	if (pdev->dev.parent)
-		host = sdhci_alloc_host(pdev->dev.parent, 0);
+	if (pdata && pdata->alloc_host)
+		host = pdata->alloc_host(&pdev->dev);
 	else
 		host = sdhci_alloc_host(&pdev->dev, 0);

@@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct
platform_device *pdev)
 		host->ops = pdata->ops;
 	else
 		host->ops = &sdhci_pltfm_ops;
-	if (pdata)
-		host->quirks = pdata->quirks;
+
 	host->irq = platform_get_irq(pdev, 0);

 	if (!request_mem_region(iomem->start, resource_size(iomem),
@@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct
platform_device *pdev)
 	}

 	if (pdata && pdata->init) {
-		ret = pdata->init(host);
+		ret = pdata->init(host, pdata, priv_pdata);
 		if (ret)
 			goto err_plat_init;
 	}

+	if (pdata && pdata->get_quirk)
+		host->quirks = pdata->get_quirk(host);
+	else if (pdata)
+		host->quirks = pdata->quirks;
+
 	ret = sdhci_add_host(host);
 	if (ret)
 		goto err_add_host;

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

 	return 0;
diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
index 0239bd7..0d8e8f6 100644
--- a/include/linux/sdhci-pltfm.h
+++ b/include/linux/sdhci-pltfm.h
@@ -28,8 +28,11 @@ struct sdhci_host;
 struct sdhci_pltfm_data {
 	struct sdhci_ops *ops;
 	unsigned int quirks;
-	int (*init)(struct sdhci_host *host);
+	int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata,
void* priv_pdata);
 	void (*exit)(struct sdhci_host *host);
+	void (*set_max_speed)(struct sdhci_host *host);
+	unsigned int  (*get_quirk)(struct sdhci_host *host);
+	struct sdhci_host *(*alloc_host)(struct device *dev);
 };

 #endif /* _SDHCI_PLTFM_H */
-- 
1.7.0.4

[-- Attachment #2: 0002-sdhci-pltfm-add-call-back-function.patch --]
[-- Type: text/x-patch, Size: 3128 bytes --]

From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
From: Zhangfei Gao <zhangfei.gao@marvell.com>
Date: Sun, 26 Sep 2010 16:37:11 -0400
Subject: [PATCH 2/2] sdhci-pltfm: add call back function


Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
---
 drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
 include/linux/sdhci-pltfm.h    |    5 ++++-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index e045e3c..e06f047 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 {
 	struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
 	const struct platform_device_id *platid = platform_get_device_id(pdev);
+	void *priv_pdata = NULL;
 	struct sdhci_host *host;
 	struct resource *iomem;
 	int ret;
 
-	if (!pdata && platid && platid->driver_data)
+	if (platid && platid->driver_data) {
 		pdata = (void *)platid->driver_data;
+		priv_pdata = pdev->dev.platform_data;
+	} else {
+		pdata = pdev->dev.platform_data;
+	}
 
 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!iomem) {
@@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Invalid iomem size. You may "
 			"experience problems.\n");
 
-	if (pdev->dev.parent)
-		host = sdhci_alloc_host(pdev->dev.parent, 0);
+	if (pdata && pdata->alloc_host)
+		host = pdata->alloc_host(&pdev->dev);
 	else
 		host = sdhci_alloc_host(&pdev->dev, 0);
 
@@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 		host->ops = pdata->ops;
 	else
 		host->ops = &sdhci_pltfm_ops;
-	if (pdata)
-		host->quirks = pdata->quirks;
+
 	host->irq = platform_get_irq(pdev, 0);
 
 	if (!request_mem_region(iomem->start, resource_size(iomem),
@@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 	}
 
 	if (pdata && pdata->init) {
-		ret = pdata->init(host);
+		ret = pdata->init(host, pdata, priv_pdata);
 		if (ret)
 			goto err_plat_init;
 	}
 
+	if (pdata && pdata->get_quirk)
+		host->quirks = pdata->get_quirk(host);
+	else if (pdata)
+		host->quirks = pdata->quirks;
+
 	ret = sdhci_add_host(host);
 	if (ret)
 		goto err_add_host;
 
+	if (pdata && pdata->set_max_speed)
+		pdata->set_max_speed(host);
+
 	platform_set_drvdata(pdev, host);
 
 	return 0;
diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
index 0239bd7..0d8e8f6 100644
--- a/include/linux/sdhci-pltfm.h
+++ b/include/linux/sdhci-pltfm.h
@@ -28,8 +28,11 @@ struct sdhci_host;
 struct sdhci_pltfm_data {
 	struct sdhci_ops *ops;
 	unsigned int quirks;
-	int (*init)(struct sdhci_host *host);
+	int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata, void* priv_pdata);
 	void (*exit)(struct sdhci_host *host);
+	void (*set_max_speed)(struct sdhci_host *host);
+	unsigned int  (*get_quirk)(struct sdhci_host *host);
+	struct sdhci_host *(*alloc_host)(struct device *dev);
 };
 
 #endif /* _SDHCI_PLTFM_H */
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [patch] sdhci-pltfm: add call back function
  2010-09-26  8:52 [patch] sdhci-pltfm: add call back function zhangfei gao
@ 2010-09-26 16:09 ` Wolfram Sang
  2010-09-27  1:06   ` Eric Miao
  2010-09-27  2:50   ` zhangfei gao
  0 siblings, 2 replies; 5+ messages in thread
From: Wolfram Sang @ 2010-09-26 16:09 UTC (permalink / raw)
  To: zhangfei gao
  Cc: linux-mmc, Chris Ball, eric.y.miao, Haojian Zhuang, Richard Zhu

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

On Sun, Sep 26, 2010 at 04:52:06AM -0400, zhangfei gao wrote:
> Add several call back to sdhci-pltfm.c, help give suggestion

Just formal things, without looking at the code yet.

Make seperate patches out of these, everyone with a proper description.

> 1. struct sdhci_host *(*alloc_host)(struct device *dev), since
> specific driver need some private variable to allocate and free,
> including clk.
> 2. unsigned int  (*get_quirk)(struct sdhci_host *host); add this
> because one driver may serve several device, each one may require
> different quirk, and specific driver is right one to know.
> 3. void (*set_max_speed)(struct sdhci_host *host); this should be done
> after add_host, which impact f_max.
> 4. int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data
> *pdata, void* priv_pdata); copy from Wolfram Sang's patch to transfer
> platform data, copy here for test.

No copying. If you think my patch is useful, add an Acked-by and say your
patches depend on mine.

> 
> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Sun, 26 Sep 2010 16:37:11 -0400
> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
> 
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
>  drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
>  include/linux/sdhci-pltfm.h    |    5 ++++-
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index e045e3c..e06f047 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct
> platform_device *pdev)
>  {
>  	struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>  	const struct platform_device_id *platid = platform_get_device_id(pdev);
> +	void *priv_pdata = NULL;
>  	struct sdhci_host *host;
>  	struct resource *iomem;
>  	int ret;
> 
> -	if (!pdata && platid && platid->driver_data)
> +	if (platid && platid->driver_data) {
>  		pdata = (void *)platid->driver_data;
> +		priv_pdata = pdev->dev.platform_data;
> +	} else {
> +		pdata = pdev->dev.platform_data;
> +	}
> 
>  	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!iomem) {
> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct
> platform_device *pdev)
>  		dev_err(&pdev->dev, "Invalid iomem size. You may "
>  			"experience problems.\n");
> 
> -	if (pdev->dev.parent)
> -		host = sdhci_alloc_host(pdev->dev.parent, 0);
> +	if (pdata && pdata->alloc_host)
> +		host = pdata->alloc_host(&pdev->dev);
>  	else
>  		host = sdhci_alloc_host(&pdev->dev, 0);
> 
> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct
> platform_device *pdev)
>  		host->ops = pdata->ops;
>  	else
>  		host->ops = &sdhci_pltfm_ops;
> -	if (pdata)
> -		host->quirks = pdata->quirks;
> +
>  	host->irq = platform_get_irq(pdev, 0);
> 
>  	if (!request_mem_region(iomem->start, resource_size(iomem),
> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct
> platform_device *pdev)
>  	}
> 
>  	if (pdata && pdata->init) {
> -		ret = pdata->init(host);
> +		ret = pdata->init(host, pdata, priv_pdata);
>  		if (ret)
>  			goto err_plat_init;
>  	}
> 
> +	if (pdata && pdata->get_quirk)
> +		host->quirks = pdata->get_quirk(host);
> +	else if (pdata)
> +		host->quirks = pdata->quirks;
> +
>  	ret = sdhci_add_host(host);
>  	if (ret)
>  		goto err_add_host;
> 
> +	if (pdata && pdata->set_max_speed)
> +		pdata->set_max_speed(host);
> +
>  	platform_set_drvdata(pdev, host);
> 
>  	return 0;
> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
> index 0239bd7..0d8e8f6 100644
> --- a/include/linux/sdhci-pltfm.h
> +++ b/include/linux/sdhci-pltfm.h
> @@ -28,8 +28,11 @@ struct sdhci_host;
>  struct sdhci_pltfm_data {
>  	struct sdhci_ops *ops;
>  	unsigned int quirks;
> -	int (*init)(struct sdhci_host *host);
> +	int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata,
> void* priv_pdata);
>  	void (*exit)(struct sdhci_host *host);
> +	void (*set_max_speed)(struct sdhci_host *host);
> +	unsigned int  (*get_quirk)(struct sdhci_host *host);
> +	struct sdhci_host *(*alloc_host)(struct device *dev);
>  };
> 
>  #endif /* _SDHCI_PLTFM_H */
> -- 
> 1.7.0.4

> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
> From: Zhangfei Gao <zhangfei.gao@marvell.com>
> Date: Sun, 26 Sep 2010 16:37:11 -0400
> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
> 
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
> ---
>  drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
>  include/linux/sdhci-pltfm.h    |    5 ++++-
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index e045e3c..e06f047 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>  {
>  	struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>  	const struct platform_device_id *platid = platform_get_device_id(pdev);
> +	void *priv_pdata = NULL;
>  	struct sdhci_host *host;
>  	struct resource *iomem;
>  	int ret;
>  
> -	if (!pdata && platid && platid->driver_data)
> +	if (platid && platid->driver_data) {
>  		pdata = (void *)platid->driver_data;
> +		priv_pdata = pdev->dev.platform_data;
> +	} else {
> +		pdata = pdev->dev.platform_data;
> +	}
>  
>  	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!iomem) {
> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "Invalid iomem size. You may "
>  			"experience problems.\n");
>  
> -	if (pdev->dev.parent)
> -		host = sdhci_alloc_host(pdev->dev.parent, 0);
> +	if (pdata && pdata->alloc_host)
> +		host = pdata->alloc_host(&pdev->dev);
>  	else
>  		host = sdhci_alloc_host(&pdev->dev, 0);
>  
> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>  		host->ops = pdata->ops;
>  	else
>  		host->ops = &sdhci_pltfm_ops;
> -	if (pdata)
> -		host->quirks = pdata->quirks;
> +
>  	host->irq = platform_get_irq(pdev, 0);
>  
>  	if (!request_mem_region(iomem->start, resource_size(iomem),
> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>  	}
>  
>  	if (pdata && pdata->init) {
> -		ret = pdata->init(host);
> +		ret = pdata->init(host, pdata, priv_pdata);
>  		if (ret)
>  			goto err_plat_init;
>  	}
>  
> +	if (pdata && pdata->get_quirk)
> +		host->quirks = pdata->get_quirk(host);
> +	else if (pdata)
> +		host->quirks = pdata->quirks;
> +
>  	ret = sdhci_add_host(host);
>  	if (ret)
>  		goto err_add_host;
>  
> +	if (pdata && pdata->set_max_speed)
> +		pdata->set_max_speed(host);
> +
>  	platform_set_drvdata(pdev, host);
>  
>  	return 0;
> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
> index 0239bd7..0d8e8f6 100644
> --- a/include/linux/sdhci-pltfm.h
> +++ b/include/linux/sdhci-pltfm.h
> @@ -28,8 +28,11 @@ struct sdhci_host;
>  struct sdhci_pltfm_data {
>  	struct sdhci_ops *ops;
>  	unsigned int quirks;
> -	int (*init)(struct sdhci_host *host);
> +	int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata, void* priv_pdata);
>  	void (*exit)(struct sdhci_host *host);
> +	void (*set_max_speed)(struct sdhci_host *host);
> +	unsigned int  (*get_quirk)(struct sdhci_host *host);
> +	struct sdhci_host *(*alloc_host)(struct device *dev);
>  };
>  
>  #endif /* _SDHCI_PLTFM_H */
> -- 
> 1.7.0.4
> 


-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] sdhci-pltfm: add call back function
  2010-09-26 16:09 ` Wolfram Sang
@ 2010-09-27  1:06   ` Eric Miao
  2010-09-27 12:41     ` Wolfram Sang
  2010-09-27  2:50   ` zhangfei gao
  1 sibling, 1 reply; 5+ messages in thread
From: Eric Miao @ 2010-09-27  1:06 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: zhangfei gao, linux-mmc, Chris Ball, Haojian Zhuang, Richard Zhu

On Mon, Sep 27, 2010 at 12:09 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Sun, Sep 26, 2010 at 04:52:06AM -0400, zhangfei gao wrote:
>> Add several call back to sdhci-pltfm.c, help give suggestion
>
> Just formal things, without looking at the code yet.
>
> Make seperate patches out of these, everyone with a proper description.
>
>> 1. struct sdhci_host *(*alloc_host)(struct device *dev), since
>> specific driver need some private variable to allocate and free,
>> including clk.
>> 2. unsigned int  (*get_quirk)(struct sdhci_host *host); add this
>> because one driver may serve several device, each one may require
>> different quirk, and specific driver is right one to know.
>> 3. void (*set_max_speed)(struct sdhci_host *host); this should be done
>> after add_host, which impact f_max.
>> 4. int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data
>> *pdata, void* priv_pdata); copy from Wolfram Sang's patch to transfer
>> platform data, copy here for test.
>

Just a rough idea, considering the potential differences (I believe
there will be
more in the future) between the SDHCI of different SoCs, would it make more
sense to make sdhci-pltfm.c as a common function library for sdhci-<soc>.c?

Or from OO point of view (though I don't quite like OO), the sdhci-<soc> is
actually a derivative of sdhci-pltfm, which by concept, sdhci-<soc> could be
an individual driver, and can call its super functions in sdhci-pltfm. E.g. in
sdhci_<soc>_probe(), one can call sdhci_pltfm_probe()?

The platform driver version may not be able to resemble the pci version so
just provide an alternate way of solving this issue.

> No copying. If you think my patch is useful, add an Acked-by and say your
> patches depend on mine.
>
>>
>> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Sun, 26 Sep 2010 16:37:11 -0400
>> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
>>
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
>>  include/linux/sdhci-pltfm.h    |    5 ++++-
>>  2 files changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> index e045e3c..e06f047 100644
>> --- a/drivers/mmc/host/sdhci-pltfm.c
>> +++ b/drivers/mmc/host/sdhci-pltfm.c
>> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>  {
>>       struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>>       const struct platform_device_id *platid = platform_get_device_id(pdev);
>> +     void *priv_pdata = NULL;
>>       struct sdhci_host *host;
>>       struct resource *iomem;
>>       int ret;
>>
>> -     if (!pdata && platid && platid->driver_data)
>> +     if (platid && platid->driver_data) {
>>               pdata = (void *)platid->driver_data;
>> +             priv_pdata = pdev->dev.platform_data;
>> +     } else {
>> +             pdata = pdev->dev.platform_data;
>> +     }
>>
>>       iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       if (!iomem) {
>> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>               dev_err(&pdev->dev, "Invalid iomem size. You may "
>>                       "experience problems.\n");
>>
>> -     if (pdev->dev.parent)
>> -             host = sdhci_alloc_host(pdev->dev.parent, 0);
>> +     if (pdata && pdata->alloc_host)
>> +             host = pdata->alloc_host(&pdev->dev);
>>       else
>>               host = sdhci_alloc_host(&pdev->dev, 0);
>>
>> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>               host->ops = pdata->ops;
>>       else
>>               host->ops = &sdhci_pltfm_ops;
>> -     if (pdata)
>> -             host->quirks = pdata->quirks;
>> +
>>       host->irq = platform_get_irq(pdev, 0);
>>
>>       if (!request_mem_region(iomem->start, resource_size(iomem),
>> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>       }
>>
>>       if (pdata && pdata->init) {
>> -             ret = pdata->init(host);
>> +             ret = pdata->init(host, pdata, priv_pdata);
>>               if (ret)
>>                       goto err_plat_init;
>>       }
>>
>> +     if (pdata && pdata->get_quirk)
>> +             host->quirks = pdata->get_quirk(host);
>> +     else if (pdata)
>> +             host->quirks = pdata->quirks;
>> +
>>       ret = sdhci_add_host(host);
>>       if (ret)
>>               goto err_add_host;
>>
>> +     if (pdata && pdata->set_max_speed)
>> +             pdata->set_max_speed(host);
>> +
>>       platform_set_drvdata(pdev, host);
>>
>>       return 0;
>> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
>> index 0239bd7..0d8e8f6 100644
>> --- a/include/linux/sdhci-pltfm.h
>> +++ b/include/linux/sdhci-pltfm.h
>> @@ -28,8 +28,11 @@ struct sdhci_host;
>>  struct sdhci_pltfm_data {
>>       struct sdhci_ops *ops;
>>       unsigned int quirks;
>> -     int (*init)(struct sdhci_host *host);
>> +     int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata,
>> void* priv_pdata);
>>       void (*exit)(struct sdhci_host *host);
>> +     void (*set_max_speed)(struct sdhci_host *host);
>> +     unsigned int  (*get_quirk)(struct sdhci_host *host);
>> +     struct sdhci_host *(*alloc_host)(struct device *dev);
>>  };
>>
>>  #endif /* _SDHCI_PLTFM_H */
>> --
>> 1.7.0.4
>
>> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Sun, 26 Sep 2010 16:37:11 -0400
>> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
>>
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
>>  include/linux/sdhci-pltfm.h    |    5 ++++-
>>  2 files changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> index e045e3c..e06f047 100644
>> --- a/drivers/mmc/host/sdhci-pltfm.c
>> +++ b/drivers/mmc/host/sdhci-pltfm.c
>> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>  {
>>       struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>>       const struct platform_device_id *platid = platform_get_device_id(pdev);
>> +     void *priv_pdata = NULL;
>>       struct sdhci_host *host;
>>       struct resource *iomem;
>>       int ret;
>>
>> -     if (!pdata && platid && platid->driver_data)
>> +     if (platid && platid->driver_data) {
>>               pdata = (void *)platid->driver_data;
>> +             priv_pdata = pdev->dev.platform_data;
>> +     } else {
>> +             pdata = pdev->dev.platform_data;
>> +     }
>>
>>       iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       if (!iomem) {
>> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>               dev_err(&pdev->dev, "Invalid iomem size. You may "
>>                       "experience problems.\n");
>>
>> -     if (pdev->dev.parent)
>> -             host = sdhci_alloc_host(pdev->dev.parent, 0);
>> +     if (pdata && pdata->alloc_host)
>> +             host = pdata->alloc_host(&pdev->dev);
>>       else
>>               host = sdhci_alloc_host(&pdev->dev, 0);
>>
>> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>               host->ops = pdata->ops;
>>       else
>>               host->ops = &sdhci_pltfm_ops;
>> -     if (pdata)
>> -             host->quirks = pdata->quirks;
>> +
>>       host->irq = platform_get_irq(pdev, 0);
>>
>>       if (!request_mem_region(iomem->start, resource_size(iomem),
>> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>       }
>>
>>       if (pdata && pdata->init) {
>> -             ret = pdata->init(host);
>> +             ret = pdata->init(host, pdata, priv_pdata);
>>               if (ret)
>>                       goto err_plat_init;
>>       }
>>
>> +     if (pdata && pdata->get_quirk)
>> +             host->quirks = pdata->get_quirk(host);
>> +     else if (pdata)
>> +             host->quirks = pdata->quirks;
>> +
>>       ret = sdhci_add_host(host);
>>       if (ret)
>>               goto err_add_host;
>>
>> +     if (pdata && pdata->set_max_speed)
>> +             pdata->set_max_speed(host);
>> +
>>       platform_set_drvdata(pdev, host);
>>
>>       return 0;
>> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
>> index 0239bd7..0d8e8f6 100644
>> --- a/include/linux/sdhci-pltfm.h
>> +++ b/include/linux/sdhci-pltfm.h
>> @@ -28,8 +28,11 @@ struct sdhci_host;
>>  struct sdhci_pltfm_data {
>>       struct sdhci_ops *ops;
>>       unsigned int quirks;
>> -     int (*init)(struct sdhci_host *host);
>> +     int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata, void* priv_pdata);
>>       void (*exit)(struct sdhci_host *host);
>> +     void (*set_max_speed)(struct sdhci_host *host);
>> +     unsigned int  (*get_quirk)(struct sdhci_host *host);
>> +     struct sdhci_host *(*alloc_host)(struct device *dev);
>>  };
>>
>>  #endif /* _SDHCI_PLTFM_H */
>> --
>> 1.7.0.4
>>
>
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkyfcD0ACgkQD27XaX1/VRt1rgCdHfpBgfsUinbhf4wXCvIMreul
> tCwAoIvA7nFDgH4jBIZgLX6xRstcENaG
> =3A6W
> -----END PGP SIGNATURE-----
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] sdhci-pltfm: add call back function
  2010-09-26 16:09 ` Wolfram Sang
  2010-09-27  1:06   ` Eric Miao
@ 2010-09-27  2:50   ` zhangfei gao
  1 sibling, 0 replies; 5+ messages in thread
From: zhangfei gao @ 2010-09-27  2:50 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc, Chris Ball, eric.y.miao, Haojian Zhuang, Richard Zhu

On Mon, Sep 27, 2010 at 12:09 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Sun, Sep 26, 2010 at 04:52:06AM -0400, zhangfei gao wrote:
>> Add several call back to sdhci-pltfm.c, help give suggestion
>
> Just formal things, without looking at the code yet.
>
> Make seperate patches out of these, everyone with a proper description.

>
>> 1. struct sdhci_host *(*alloc_host)(struct device *dev), since
>> specific driver need some private variable to allocate and free,
>> including clk.
>> 2. unsigned int  (*get_quirk)(struct sdhci_host *host); add this
>> because one driver may serve several device, each one may require
>> different quirk, and specific driver is right one to know.
>> 3. void (*set_max_speed)(struct sdhci_host *host); this should be done
>> after add_host, which impact f_max.
>> 4. int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data
>> *pdata, void* priv_pdata); copy from Wolfram Sang's patch to transfer
>> platform data, copy here for test.
>
> No copying. If you think my patch is useful, add an Acked-by and say your
> patches depend on mine.

No problem, here is just draft for discussion, will submit another
three call back functions.

>
>>
>> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Sun, 26 Sep 2010 16:37:11 -0400
>> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
>>
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
>>  include/linux/sdhci-pltfm.h    |    5 ++++-
>>  2 files changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> index e045e3c..e06f047 100644
>> --- a/drivers/mmc/host/sdhci-pltfm.c
>> +++ b/drivers/mmc/host/sdhci-pltfm.c
>> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>  {
>>       struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>>       const struct platform_device_id *platid = platform_get_device_id(pdev);
>> +     void *priv_pdata = NULL;
>>       struct sdhci_host *host;
>>       struct resource *iomem;
>>       int ret;
>>
>> -     if (!pdata && platid && platid->driver_data)
>> +     if (platid && platid->driver_data) {
>>               pdata = (void *)platid->driver_data;
>> +             priv_pdata = pdev->dev.platform_data;
>> +     } else {
>> +             pdata = pdev->dev.platform_data;
>> +     }
>>
>>       iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       if (!iomem) {
>> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>               dev_err(&pdev->dev, "Invalid iomem size. You may "
>>                       "experience problems.\n");
>>
>> -     if (pdev->dev.parent)
>> -             host = sdhci_alloc_host(pdev->dev.parent, 0);
>> +     if (pdata && pdata->alloc_host)
>> +             host = pdata->alloc_host(&pdev->dev);
>>       else
>>               host = sdhci_alloc_host(&pdev->dev, 0);
>>
>> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>               host->ops = pdata->ops;
>>       else
>>               host->ops = &sdhci_pltfm_ops;
>> -     if (pdata)
>> -             host->quirks = pdata->quirks;
>> +
>>       host->irq = platform_get_irq(pdev, 0);
>>
>>       if (!request_mem_region(iomem->start, resource_size(iomem),
>> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct
>> platform_device *pdev)
>>       }
>>
>>       if (pdata && pdata->init) {
>> -             ret = pdata->init(host);
>> +             ret = pdata->init(host, pdata, priv_pdata);
>>               if (ret)
>>                       goto err_plat_init;
>>       }
>>
>> +     if (pdata && pdata->get_quirk)
>> +             host->quirks = pdata->get_quirk(host);
>> +     else if (pdata)
>> +             host->quirks = pdata->quirks;
>> +
>>       ret = sdhci_add_host(host);
>>       if (ret)
>>               goto err_add_host;
>>
>> +     if (pdata && pdata->set_max_speed)
>> +             pdata->set_max_speed(host);
>> +
>>       platform_set_drvdata(pdev, host);
>>
>>       return 0;
>> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
>> index 0239bd7..0d8e8f6 100644
>> --- a/include/linux/sdhci-pltfm.h
>> +++ b/include/linux/sdhci-pltfm.h
>> @@ -28,8 +28,11 @@ struct sdhci_host;
>>  struct sdhci_pltfm_data {
>>       struct sdhci_ops *ops;
>>       unsigned int quirks;
>> -     int (*init)(struct sdhci_host *host);
>> +     int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata,
>> void* priv_pdata);
>>       void (*exit)(struct sdhci_host *host);
>> +     void (*set_max_speed)(struct sdhci_host *host);
>> +     unsigned int  (*get_quirk)(struct sdhci_host *host);
>> +     struct sdhci_host *(*alloc_host)(struct device *dev);
>>  };
>>
>>  #endif /* _SDHCI_PLTFM_H */
>> --
>> 1.7.0.4
>
>> From 8c59d0b917f434d7c424ce1e0896af45c3e5fbba Mon Sep 17 00:00:00 2001
>> From: Zhangfei Gao <zhangfei.gao@marvell.com>
>> Date: Sun, 26 Sep 2010 16:37:11 -0400
>> Subject: [PATCH 2/2] sdhci-pltfm: add call back function
>>
>>
>> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com>
>> ---
>>  drivers/mmc/host/sdhci-pltfm.c |   24 ++++++++++++++++++------
>>  include/linux/sdhci-pltfm.h    |    5 ++++-
>>  2 files changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
>> index e045e3c..e06f047 100644
>> --- a/drivers/mmc/host/sdhci-pltfm.c
>> +++ b/drivers/mmc/host/sdhci-pltfm.c
>> @@ -54,12 +54,17 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>  {
>>       struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
>>       const struct platform_device_id *platid = platform_get_device_id(pdev);
>> +     void *priv_pdata = NULL;
>>       struct sdhci_host *host;
>>       struct resource *iomem;
>>       int ret;
>>
>> -     if (!pdata && platid && platid->driver_data)
>> +     if (platid && platid->driver_data) {
>>               pdata = (void *)platid->driver_data;
>> +             priv_pdata = pdev->dev.platform_data;
>> +     } else {
>> +             pdata = pdev->dev.platform_data;
>> +     }
>>
>>       iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       if (!iomem) {
>> @@ -71,8 +76,8 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>               dev_err(&pdev->dev, "Invalid iomem size. You may "
>>                       "experience problems.\n");
>>
>> -     if (pdev->dev.parent)
>> -             host = sdhci_alloc_host(pdev->dev.parent, 0);
>> +     if (pdata && pdata->alloc_host)
>> +             host = pdata->alloc_host(&pdev->dev);
>>       else
>>               host = sdhci_alloc_host(&pdev->dev, 0);
>>
>> @@ -86,8 +91,7 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>               host->ops = pdata->ops;
>>       else
>>               host->ops = &sdhci_pltfm_ops;
>> -     if (pdata)
>> -             host->quirks = pdata->quirks;
>> +
>>       host->irq = platform_get_irq(pdev, 0);
>>
>>       if (!request_mem_region(iomem->start, resource_size(iomem),
>> @@ -105,15 +109,23 @@ static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
>>       }
>>
>>       if (pdata && pdata->init) {
>> -             ret = pdata->init(host);
>> +             ret = pdata->init(host, pdata, priv_pdata);
>>               if (ret)
>>                       goto err_plat_init;
>>       }
>>
>> +     if (pdata && pdata->get_quirk)
>> +             host->quirks = pdata->get_quirk(host);
>> +     else if (pdata)
>> +             host->quirks = pdata->quirks;
>> +
>>       ret = sdhci_add_host(host);
>>       if (ret)
>>               goto err_add_host;
>>
>> +     if (pdata && pdata->set_max_speed)
>> +             pdata->set_max_speed(host);
>> +
>>       platform_set_drvdata(pdev, host);
>>
>>       return 0;
>> diff --git a/include/linux/sdhci-pltfm.h b/include/linux/sdhci-pltfm.h
>> index 0239bd7..0d8e8f6 100644
>> --- a/include/linux/sdhci-pltfm.h
>> +++ b/include/linux/sdhci-pltfm.h
>> @@ -28,8 +28,11 @@ struct sdhci_host;
>>  struct sdhci_pltfm_data {
>>       struct sdhci_ops *ops;
>>       unsigned int quirks;
>> -     int (*init)(struct sdhci_host *host);
>> +     int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data *pdata, void* priv_pdata);
>>       void (*exit)(struct sdhci_host *host);
>> +     void (*set_max_speed)(struct sdhci_host *host);
>> +     unsigned int  (*get_quirk)(struct sdhci_host *host);
>> +     struct sdhci_host *(*alloc_host)(struct device *dev);
>>  };
>>
>>  #endif /* _SDHCI_PLTFM_H */
>> --
>> 1.7.0.4
>>
>
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkyfcD0ACgkQD27XaX1/VRt1rgCdHfpBgfsUinbhf4wXCvIMreul
> tCwAoIvA7nFDgH4jBIZgLX6xRstcENaG
> =3A6W
> -----END PGP SIGNATURE-----
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] sdhci-pltfm: add call back function
  2010-09-27  1:06   ` Eric Miao
@ 2010-09-27 12:41     ` Wolfram Sang
  0 siblings, 0 replies; 5+ messages in thread
From: Wolfram Sang @ 2010-09-27 12:41 UTC (permalink / raw)
  To: Eric Miao
  Cc: zhangfei gao, linux-mmc, Chris Ball, Haojian Zhuang, Richard Zhu

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

On Mon, Sep 27, 2010 at 09:06:57AM +0800, Eric Miao wrote:
> On Mon, Sep 27, 2010 at 12:09 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> > On Sun, Sep 26, 2010 at 04:52:06AM -0400, zhangfei gao wrote:
> >> Add several call back to sdhci-pltfm.c, help give suggestion
> >
> > Just formal things, without looking at the code yet.
> >
> > Make seperate patches out of these, everyone with a proper description.
> >
> >> 1. struct sdhci_host *(*alloc_host)(struct device *dev), since
> >> specific driver need some private variable to allocate and free,
> >> including clk.
> >> 2. unsigned int  (*get_quirk)(struct sdhci_host *host); add this
> >> because one driver may serve several device, each one may require
> >> different quirk, and specific driver is right one to know.
> >> 3. void (*set_max_speed)(struct sdhci_host *host); this should be done
> >> after add_host, which impact f_max.
> >> 4. int (*init)(struct sdhci_host *host, struct sdhci_pltfm_data
> >> *pdata, void* priv_pdata); copy from Wolfram Sang's patch to transfer
> >> platform data, copy here for test.
> >
> 
> Just a rough idea, considering the potential differences (I believe
> there will be
> more in the future) between the SDHCI of different SoCs, would it make more
> sense to make sdhci-pltfm.c as a common function library for sdhci-<soc>.c?

Yeah, I think Alan Cox mentioned this idea, too. My guess is that it
will be well received, if somebody actually does it ;) Up to that point,
it probably makes sense to keep redundancy low by the means we have
today, i.e. pltfm. That should help a later migration process.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-09-27 12:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-26  8:52 [patch] sdhci-pltfm: add call back function zhangfei gao
2010-09-26 16:09 ` Wolfram Sang
2010-09-27  1:06   ` Eric Miao
2010-09-27 12:41     ` Wolfram Sang
2010-09-27  2:50   ` zhangfei gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox