public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: add SDHCI driver for STM platforms (V2)
@ 2010-09-20  8:20 Giuseppe CAVALLARO
  2010-09-20  8:38 ` Wolfram Sang
  0 siblings, 1 reply; 12+ messages in thread
From: Giuseppe CAVALLARO @ 2010-09-20  8:20 UTC (permalink / raw)
  To: linux-mmc; +Cc: akpm, matt, Giuseppe Cavallaro

This patch adds the Arasan MMC/SD/SDIO driver
for the STM ST40 platforms. It is based on the
SDHCI driver.
It has been tested on the STx7106/STx7108/STx5289
platforms.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/mmc/host/Kconfig      |   12 +++
 drivers/mmc/host/Makefile     |    1 +
 drivers/mmc/host/sdhci-stm.c  |  185 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/sdhci-stm.h |   23 +++++
 4 files changed, 221 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-stm.c
 create mode 100644 include/linux/mmc/sdhci-stm.h

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 68d1279..dbf71b7 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -157,6 +157,18 @@ config MMC_SDHCI_SPEAR
 
 	  If unsure, say N.
 
+config MMC_SDHCI_STM
+	tristate "SDHCI support on STM platforms"
+	depends on MMC_SDHCI && CPU_SUBTYPE_ST40
+	help
+	  This selects the Secure Digital Host Controller Interface (SDHCI)
+	  often referrered to as the HSMMC block in some of the STM range
+	  of SoC (e.g. STx7106/STx7108/STx5289).
+
+	  If you have a controller with this interface, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_S3C_DMA
 	bool "DMA support on S3C SDHCI"
 	depends on MMC_SDHCI_S3C && EXPERIMENTAL
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 840bcb5..35bb030 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
 obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
 obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
+obj-$(CONFIG_MMC_SDHCI_STM)	+= sdhci-stm.o
 obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
 obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
 obj-$(CONFIG_MMC_OMAP)		+= omap.o
diff --git a/drivers/mmc/host/sdhci-stm.c b/drivers/mmc/host/sdhci-stm.c
new file mode 100644
index 0000000..2744e67
--- /dev/null
+++ b/drivers/mmc/host/sdhci-stm.c
@@ -0,0 +1,185 @@
+/* linux/drivers/mmc/host/sdhci-stm.c
+ *
+ * Copyright (C) 2010 STMicroelectronics Ltd
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * SDHCI support for STM platforms
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mmc/host.h>
+#include <linux/io.h>
+
+#include <linux/mmc/sdhci-stm.h>
+
+#include "sdhci.h"
+
+static struct sdhci_ops sdhci_pltfm_ops = {
+	/* Nothing to do for now. */
+};
+
+struct sdhci_stm {
+	struct sdhci_host *host;
+	struct platform_device *pdev;
+	struct resource *ioarea;
+	struct sdhci_platform_data *pdata;
+};
+
+static int __devinit sdhci_probe(struct platform_device *pdev)
+{
+	struct sdhci_platform_data *pdata;
+	struct device *dev = &pdev->dev;
+	struct sdhci_host *host;
+	struct sdhci_stm *sc;
+	struct resource *res;
+	int ret, irq;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "no memory specified\n");
+		return -ENOENT;
+	}
+
+	irq = platform_get_irq_byname(pdev, "mmcirq");
+	if (irq < 0) {
+		dev_err(dev, "no irq specified\n");
+		return irq;
+	}
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_err(dev, "no device data specified\n");
+		return -ENOENT;
+	}
+
+	host = sdhci_alloc_host(&pdev->dev, 0);
+	if (IS_ERR(host)) {
+		ret = PTR_ERR(host);
+		dev_dbg(&pdev->dev, "error allocating host\n");
+		return PTR_ERR(host);
+	}
+
+	sc = sdhci_priv(host);
+	sc->host = host;
+	sc->pdev = pdev;
+	sc->pdata = pdata;
+
+	platform_set_drvdata(pdev, host);
+
+	sc->ioarea = request_mem_region(res->start, resource_size(res),
+					pdev->name);
+	if (!sc->ioarea) {
+		dev_err(dev, "failed to reserve register area\n");
+		ret = -ENXIO;
+		goto out;
+	}
+
+	/* Claim the SDHCI_STM resources */
+	ret = pdata->claim_resource(pdev);
+	if (ret < 0)
+		goto out;
+
+	host->hw_name = "sdhci-stm";
+	host->ops = &sdhci_pltfm_ops;
+	host->irq = irq;
+	host->quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
+
+	host->ioaddr = ioremap(res->start, resource_size(res));
+	if (!host->ioaddr) {
+		dev_err(dev, "failed to map registers\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = sdhci_add_host(host);
+	if (ret) {
+		dev_dbg(&pdev->dev, "error adding host\n");
+		goto out;
+	}
+
+	return 0;
+
+out:
+	if (host->ioaddr)
+		iounmap(host->ioaddr);
+
+	if (sc->ioarea) {
+		release_resource(sc->ioarea);
+		kfree(sc->ioarea);
+	}
+
+	sdhci_free_host(host);
+
+	return ret;
+}
+
+static int __devexit sdhci_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_stm *sc = sdhci_priv(host);
+
+	sdhci_remove_host(host, 1);
+
+	iounmap(host->ioaddr);
+	release_resource(sc->ioarea);
+	kfree(sc->ioarea);
+
+	sdhci_free_host(host);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int sdhci_stm_suspend(struct platform_device *dev, pm_message_t pm)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+
+	sdhci_suspend_host(host, pm);
+	return 0;
+}
+
+static int sdhci_stm_resume(struct platform_device *dev)
+{
+	struct sdhci_host *host = platform_get_drvdata(dev);
+
+	sdhci_resume_host(host);
+	return 0;
+}
+#endif
+
+static struct platform_driver sdhci_driver = {
+	.driver = {
+		   .name = "sdhci-stm",
+		   .owner = THIS_MODULE,
+		   },
+	.probe = sdhci_probe,
+	.remove = __devexit_p(sdhci_remove),
+#ifdef CONFIG_PM
+	.suspend = sdhci_stm_suspend,
+	.resume = sdhci_stm_resume,
+#endif
+};
+
+static int __init sdhci_stm_init(void)
+{
+	return platform_driver_register(&sdhci_driver);
+}
+
+module_init(sdhci_stm_init);
+
+static void __exit sdhci_stm_exit(void)
+{
+	platform_driver_unregister(&sdhci_driver);
+}
+
+module_exit(sdhci_stm_exit);
+
+MODULE_DESCRIPTION("STM Secure Digital Host Controller Interface driver");
+MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mmc/sdhci-stm.h b/include/linux/mmc/sdhci-stm.h
new file mode 100644
index 0000000..3cb2258
--- /dev/null
+++ b/include/linux/mmc/sdhci-stm.h
@@ -0,0 +1,23 @@
+/*
+ * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *
+ * include/linux/mmc/sdhci-stm.h
+ *
+ * platform data for the sdhci stm driver.
+ *
+ * Copyright (C) 2010  STMicroelectronics Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ */
+
+#ifndef __SDHCI_STM_PLAT_H__
+#define __SDHCI_STM_PLAT_H__
+
+struct sdhci_platform_data {
+	int (*claim_resource)(struct platform_device *pdev);
+};
+
+#endif /* __SDHCI_STM_PLAT_H__ */
-- 
1.5.5.6


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

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-20  8:20 [PATCH] mmc: add SDHCI driver for STM platforms (V2) Giuseppe CAVALLARO
@ 2010-09-20  8:38 ` Wolfram Sang
  2010-09-21  9:21   ` Peppe CAVALLARO
  0 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2010-09-20  8:38 UTC (permalink / raw)
  To: Giuseppe CAVALLARO; +Cc: linux-mmc, akpm, matt

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

On Mon, Sep 20, 2010 at 10:20:17AM +0200, Giuseppe CAVALLARO wrote:
> This patch adds the Arasan MMC/SD/SDIO driver
> for the STM ST40 platforms. It is based on the
> SDHCI driver.
> It has been tested on the STx7106/STx7108/STx5289
> platforms.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

Looks to me that you could just use the sdhci-pltfm driver?

-- 
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] 12+ messages in thread

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-20  8:38 ` Wolfram Sang
@ 2010-09-21  9:21   ` Peppe CAVALLARO
  2010-09-21  9:44     ` Wolfram Sang
  0 siblings, 1 reply; 12+ messages in thread
From: Peppe CAVALLARO @ 2010-09-21  9:21 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

On 09/20/2010 10:38 AM, Wolfram Sang wrote:
> On Mon, Sep 20, 2010 at 10:20:17AM +0200, Giuseppe CAVALLARO wrote:
>> This patch adds the Arasan MMC/SD/SDIO driver
>> for the STM ST40 platforms. It is based on the
>> SDHCI driver.
>> It has been tested on the STx7106/STx7108/STx5289
>> platforms.
>>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
> Looks to me that you could just use the sdhci-pltfm driver?
> 

Hello Wolfram,
some weeks ago I discussed about this driver and I reworked it according
to the changes required. This is the meaning of this patch.

At any rate, I can look at the sdhci-pltfm driver: at first glance, it
could actually help on our platforms. This could take a while.

Regards
Peppe

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

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-21  9:21   ` Peppe CAVALLARO
@ 2010-09-21  9:44     ` Wolfram Sang
  2010-09-22 13:31       ` Peppe CAVALLARO
  0 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2010-09-21  9:44 UTC (permalink / raw)
  To: Peppe CAVALLARO
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

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

On Tue, Sep 21, 2010 at 11:21:48AM +0200, Peppe CAVALLARO wrote:
> On 09/20/2010 10:38 AM, Wolfram Sang wrote:
> > On Mon, Sep 20, 2010 at 10:20:17AM +0200, Giuseppe CAVALLARO wrote:
> >> This patch adds the Arasan MMC/SD/SDIO driver
> >> for the STM ST40 platforms. It is based on the
> >> SDHCI driver.
> >> It has been tested on the STx7106/STx7108/STx5289
> >> platforms.
> >>
> >> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> > 
> > Looks to me that you could just use the sdhci-pltfm driver?
> > 
> 
> Hello Wolfram,
> some weeks ago I discussed about this driver and I reworked it according
> to the changes required. This is the meaning of this patch.
> 
> At any rate, I can look at the sdhci-pltfm driver: at first glance, it
> could actually help on our platforms. This could take a while.

Sorry, I didn't spot the first version of your patch. I would have said
the same then, simply to avoid the code duplication. Oh, and no need to
hurry from my side :)

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] 12+ messages in thread

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-21  9:44     ` Wolfram Sang
@ 2010-09-22 13:31       ` Peppe CAVALLARO
  2010-09-22 14:12         ` Wolfram Sang
  0 siblings, 1 reply; 12+ messages in thread
From: Peppe CAVALLARO @ 2010-09-22 13:31 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

On 09/21/2010 11:44 AM, Wolfram Sang wrote:
> On Tue, Sep 21, 2010 at 11:21:48AM +0200, Peppe CAVALLARO wrote:
>> On 09/20/2010 10:38 AM, Wolfram Sang wrote:
>>> On Mon, Sep 20, 2010 at 10:20:17AM +0200, Giuseppe CAVALLARO wrote:
>>>> This patch adds the Arasan MMC/SD/SDIO driver
>>>> for the STM ST40 platforms. It is based on the
>>>> SDHCI driver.
>>>> It has been tested on the STx7106/STx7108/STx5289
>>>> platforms.
>>>>
>>>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>>
>>> Looks to me that you could just use the sdhci-pltfm driver?
>>>
>>
>> Hello Wolfram,
>> some weeks ago I discussed about this driver and I reworked it according
>> to the changes required. This is the meaning of this patch.
>>
>> At any rate, I can look at the sdhci-pltfm driver: at first glance, it
>> could actually help on our platforms. This could take a while.
> 
> Sorry, I didn't spot the first version of your patch. I would have said
> the same then, simply to avoid the code duplication. Oh, and no need to
> hurry from my side :)

Hi Wolfran,
I agree that it's a good idea to reduce duplicated code when possible
(i.e. the probe function that, at first glance, is almost equal for
several sdhci drivers based).

Maybe, I could use on our ST boxes the sdhci_pltfm but I have the
following questions and ideas. Welcome advice and feedback.

1) I've already a patch to add the suspend/resume in the sdhci_pltfm
   driver. Please note this is mandatory for me.

   Note: I'd like to look at the wake-up on card that should be nice to
   have in the future. IIUC, it is missing in the sdhci. Please correct
   me if I'm wrong.

2) sdhci_pltfm_data has a "quirk" flag but IMO the quirk macros, that
   currently are in linux-2.6/drivers/mmc/host/sdhci.h, should be
   moved in a separate file:

    include/linux/mmc/sdhci.h or
    include/linux/mmc/sdhci-quirk.h or ...

   I don't know if it has been already done but I could create a patch
   for this too. Let me know the name convention you like, eventually.

   Otherwise, in my platforms, where I need to set this flag (e.g. the
   sdhci-stm needs: SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC), I should include
   drivers/mmc/host/sdhci.h?!? I don't like it :-(
   Please, correct me if I've missed something.

3) In the end, another hook could be added in the sdhci_pltfm_data to
   invoke specific own functions for claiming resources etc.
   For example, I need an extra callback to invoke the STM pad manager
   that's used for managing clocks, PIO lines and syscfg registers.

   I'm thinking about something like this:

   struct sdhci_pltfm_data {
        struct sdhci_ops *ops;
        unsigned int quirks;
        int (*init)(struct sdhci_host *host);
        void (*exit)(struct sdhci_host *host);
        int (*claim_resource)(struct platform_device *pdev);
                 |
                 |_ we can use another name.
   };

What do you think?

Regards
Peppe

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

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-22 13:31       ` Peppe CAVALLARO
@ 2010-09-22 14:12         ` Wolfram Sang
  2010-09-22 14:35           ` Peppe CAVALLARO
  0 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2010-09-22 14:12 UTC (permalink / raw)
  To: Peppe CAVALLARO
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

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

Hi Peppe,

> 1) I've already a patch to add the suspend/resume in the sdhci_pltfm
>    driver. Please note this is mandatory for me.

Great, improvements to the generic pltfm-driver are most welcome!

>    Note: I'd like to look at the wake-up on card that should be nice to
>    have in the future. IIUC, it is missing in the sdhci. Please correct
>    me if I'm wrong.

It is not in sdhci yet. Please make sure to send very early RFC-patches
here, so we can see what you are aiming for.

> 2) sdhci_pltfm_data has a "quirk" flag but IMO the quirk macros, that
>    currently are in linux-2.6/drivers/mmc/host/sdhci.h, should be
>    moved in a separate file:
> 
>     include/linux/mmc/sdhci.h or
>     include/linux/mmc/sdhci-quirk.h or ...
> 
>    I don't know if it has been already done but I could create a patch
>    for this too. Let me know the name convention you like, eventually.
> 
>    Otherwise, in my platforms, where I need to set this flag (e.g. the
>    sdhci-stm needs: SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC), I should include
>    drivers/mmc/host/sdhci.h?!? I don't like it :-(
>    Please, correct me if I've missed something.

You are correct. So far, all users of the quirk-flags happened to be
inside the host-directory. If you happen to have a controller which only
needs quirk-flags and no custom functions (congrats! ;)), then those
quirk-flags really need to be moved, so your platform code can find it.

I'd go for sdhci.h as the name. Be sure to catch all users of the quirks
to include your new file.

> 3) In the end, another hook could be added in the sdhci_pltfm_data to
>    invoke specific own functions for claiming resources etc.
>    For example, I need an extra callback to invoke the STM pad manager
>    that's used for managing clocks, PIO lines and syscfg registers.
> 
>    I'm thinking about something like this:
> 
>    struct sdhci_pltfm_data {
>         struct sdhci_ops *ops;
>         unsigned int quirks;
>         int (*init)(struct sdhci_host *host);
>         void (*exit)(struct sdhci_host *host);
>         int (*claim_resource)(struct platform_device *pdev);
>                  |
>                  |_ we can use another name.
>    };

And init() is too late?

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] 12+ messages in thread

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-22 14:12         ` Wolfram Sang
@ 2010-09-22 14:35           ` Peppe CAVALLARO
  2010-09-23  6:48             ` Peppe CAVALLARO
  2010-09-23  8:48             ` Wolfram Sang
  0 siblings, 2 replies; 12+ messages in thread
From: Peppe CAVALLARO @ 2010-09-22 14:35 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

On 09/22/2010 04:12 PM, Wolfram Sang wrote:
> Hi Peppe,
> 
>> 1) I've already a patch to add the suspend/resume in the sdhci_pltfm
>>    driver. Please note this is mandatory for me.
> 
> Great, improvements to the generic pltfm-driver are most welcome!

Good! I'm happy to contribute on it. I'll start soon and post the
patches asap.

> 
>>    Note: I'd like to look at the wake-up on card that should be nice to
>>    have in the future. IIUC, it is missing in the sdhci. Please correct
>>    me if I'm wrong.
> 
> It is not in sdhci yet. Please make sure to send very early RFC-patches
> here, so we can see what you are aiming for.

OK!

> 
>> 2) sdhci_pltfm_data has a "quirk" flag but IMO the quirk macros, that
>>    currently are in linux-2.6/drivers/mmc/host/sdhci.h, should be
>>    moved in a separate file:
>>
>>     include/linux/mmc/sdhci.h or
>>     include/linux/mmc/sdhci-quirk.h or ...
>>
>>    I don't know if it has been already done but I could create a patch
>>    for this too. Let me know the name convention you like, eventually.
>>
>>    Otherwise, in my platforms, where I need to set this flag (e.g. the
>>    sdhci-stm needs: SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC), I should include
>>    drivers/mmc/host/sdhci.h?!? I don't like it :-(
>>    Please, correct me if I've missed something.
> 
> You are correct. So far, all users of the quirk-flags happened to be
> inside the host-directory. If you happen to have a controller which only
> needs quirk-flags and no custom functions (congrats! ;)), then those
> quirk-flags really need to be moved, so your platform code can find it.
> 
> I'd go for sdhci.h as the name. Be sure to catch all users of the quirks
> to include your new file.

OK! I like shdci.h too ;-)
I'll pay attention to add this header file in the other drivers based on
sdhci.

> 
>> 3) In the end, another hook could be added in the sdhci_pltfm_data to
>>    invoke specific own functions for claiming resources etc.
>>    For example, I need an extra callback to invoke the STM pad manager
>>    that's used for managing clocks, PIO lines and syscfg registers.
>>
>>    I'm thinking about something like this:
>>
>>    struct sdhci_pltfm_data {
>>         struct sdhci_ops *ops;
>>         unsigned int quirks;
>>         int (*init)(struct sdhci_host *host);
>>         void (*exit)(struct sdhci_host *host);
>>         int (*claim_resource)(struct platform_device *pdev);
>>                  |
>>                  |_ we can use another name.
>>    };
> 
> And init() is too late?

No it's not late but I need the dev structure from the platform_device.
We could add the "struct device dev;" in the sdhci_host structure
instead of.
In this case the sdhci_host should be moved within the new
include/linux/mmc/sdhci.h file.
Indeed, this could make sense because the drivers/mmc/host/sdhci.h will
only have the HW register configuration. Shared macros (quirks) and
structures among sdhci driver based could be moved in
include/linux/mmc/sdhci.h header.

What do you think?

Regards,
Peppe

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

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-22 14:35           ` Peppe CAVALLARO
@ 2010-09-23  6:48             ` Peppe CAVALLARO
  2010-09-23  8:48             ` Wolfram Sang
  1 sibling, 0 replies; 12+ messages in thread
From: Peppe CAVALLARO @ 2010-09-23  6:48 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

On 09/22/2010 04:35 PM, Peppe CAVALLARO wrote:
> No it's not late but I need the dev structure from the platform_device.
> We could add the "struct device dev;" in the sdhci_host structure
> instead of.
> In this case the sdhci_host should be moved within the new
> include/linux/mmc/sdhci.h file.
> Indeed, this could make sense because the drivers/mmc/host/sdhci.h will
> only have the HW register configuration. Shared macros (quirks) and
> structures among sdhci driver based could be moved in
> include/linux/mmc/sdhci.h header.
> 
> What do you think?


Hello!
I've just something working on my ST Linux box (ST40 CPU).
I mean that I'm using the sdhci-pltfm driver with some of the
modifications discussed in this thread.

I'll post some patches asap.

Pepep

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

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-22 14:35           ` Peppe CAVALLARO
  2010-09-23  6:48             ` Peppe CAVALLARO
@ 2010-09-23  8:48             ` Wolfram Sang
  2010-09-23  9:30               ` Peppe CAVALLARO
  1 sibling, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2010-09-23  8:48 UTC (permalink / raw)
  To: Peppe CAVALLARO
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

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

> >> 3) In the end, another hook could be added in the sdhci_pltfm_data to
> >>    invoke specific own functions for claiming resources etc.
> >>    For example, I need an extra callback to invoke the STM pad manager
> >>    that's used for managing clocks, PIO lines and syscfg registers.
> >>
> >>    I'm thinking about something like this:
> >>
> >>    struct sdhci_pltfm_data {
> >>         struct sdhci_ops *ops;
> >>         unsigned int quirks;
> >>         int (*init)(struct sdhci_host *host);
> >>         void (*exit)(struct sdhci_host *host);
> >>         int (*claim_resource)(struct platform_device *pdev);
> >>                  |
> >>                  |_ we can use another name.
> >>    };
> > 
> > And init() is too late?
> 
> No it's not late but I need the dev structure from the platform_device.
> We could add the "struct device dev;" in the sdhci_host structure
> instead of.
> In this case the sdhci_host should be moved within the new
> include/linux/mmc/sdhci.h file.
> Indeed, this could make sense because the drivers/mmc/host/sdhci.h will
> only have the HW register configuration. Shared macros (quirks) and
> structures among sdhci driver based could be moved in
> include/linux/mmc/sdhci.h header.
> 
> What do you think?

I think it will be better to give init() access to all information
needed than to implement another hook. I tried with this patch

https://patchwork.kernel.org/patch/196992/

I am still not sure what kind of information you need from the pdev. Can
you create a branch somewhere with your current work?

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] 12+ messages in thread

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-23  8:48             ` Wolfram Sang
@ 2010-09-23  9:30               ` Peppe CAVALLARO
  2010-09-23  9:51                 ` Wolfram Sang
  0 siblings, 1 reply; 12+ messages in thread
From: Peppe CAVALLARO @ 2010-09-23  9:30 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

On 09/23/2010 10:48 AM, Wolfram Sang wrote:
> I think it will be better to give init() access to all information
> needed than to implement another hook. I tried with this patch
> 
> https://patchwork.kernel.org/patch/196992/
> 
> I am still not sure what kind of information you need from the pdev. Can
> you create a branch somewhere with your current work?


Hi Wolfram,
I've just sent some patches and I used the .init as you wanted.

Fortunately the problem can be resolved without modifying .init within
the sdhci_pltfm_data structure.

The sdhci_host structure has the mmc field that also has the pointer to
the device parent.
This can be used for getting the device structure pointer in my platform.
For example, below there is the piece of code added, in my platform, to
use the sdhci-pltfm driver.

It works fine without modifying other files.

What do you think?

Regards,
Peppe

[snip]

static int mmc_pad_resources(struct sdhci_host *sdhci)
{
        if (!devm_stm_pad_claim(sdhci->mmc->parent,
                                &stx7108_mmc_pad_config,
                                dev_name(sdhci->mmc->parent)))
                return -ENODEV;

        return 0;
}

static struct sdhci_pltfm_data stx7108_mmc_platform_data = {
                .init = &mmc_pad_resources,
                .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
};

static struct platform_device stx7108_mmc_device = {
                .name = "sdhci",

[snip]

> 
> Regards,
> 
>    Wolfram
> 

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

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-23  9:30               ` Peppe CAVALLARO
@ 2010-09-23  9:51                 ` Wolfram Sang
  2010-09-23 10:05                   ` Peppe CAVALLARO
  0 siblings, 1 reply; 12+ messages in thread
From: Wolfram Sang @ 2010-09-23  9:51 UTC (permalink / raw)
  To: Peppe CAVALLARO
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

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


> static int mmc_pad_resources(struct sdhci_host *sdhci)
> {
>         if (!devm_stm_pad_claim(sdhci->mmc->parent,
>                                 &stx7108_mmc_pad_config,
>                                 dev_name(sdhci->mmc->parent)))
>                 return -ENODEV;
> 
>         return 0;
> }
> 
> static struct sdhci_pltfm_data stx7108_mmc_platform_data = {
>                 .init = &mmc_pad_resources,

You can drop the &

>                 .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> };
> 
> static struct platform_device stx7108_mmc_device = {
>                 .name = "sdhci",
> 
> [snip]

All in all, this looks like the right way! A good example how to
slightly update and then use the pltfm-driver. Redundant code saved and
benefits for all added :)

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] 12+ messages in thread

* Re: [PATCH] mmc: add SDHCI driver for STM platforms (V2)
  2010-09-23  9:51                 ` Wolfram Sang
@ 2010-09-23 10:05                   ` Peppe CAVALLARO
  0 siblings, 0 replies; 12+ messages in thread
From: Peppe CAVALLARO @ 2010-09-23 10:05 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-mmc@vger.kernel.org, akpm@linux-foundation.org,
	matt@console-pimps.org

On 09/23/2010 11:51 AM, Wolfram Sang wrote:
> 
>> static int mmc_pad_resources(struct sdhci_host *sdhci)
>> {
>>         if (!devm_stm_pad_claim(sdhci->mmc->parent,
>>                                 &stx7108_mmc_pad_config,
>>                                 dev_name(sdhci->mmc->parent)))
>>                 return -ENODEV;
>>
>>         return 0;
>> }
>>
>> static struct sdhci_pltfm_data stx7108_mmc_platform_data = {
>>                 .init = &mmc_pad_resources,
> 
> You can drop the &

Thx ;-)

> 
>>                 .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>> };
>>
>> static struct platform_device stx7108_mmc_device = {
>>                 .name = "sdhci",
>>
>> [snip]
> 
> All in all, this looks like the right way! A good example how to
> slightly update and then use the pltfm-driver. Redundant code saved and
> benefits for all added :)

Regards,
Peppe

> Regards,
> 
>    Wolfram
> 

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

end of thread, other threads:[~2010-09-23 10:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-20  8:20 [PATCH] mmc: add SDHCI driver for STM platforms (V2) Giuseppe CAVALLARO
2010-09-20  8:38 ` Wolfram Sang
2010-09-21  9:21   ` Peppe CAVALLARO
2010-09-21  9:44     ` Wolfram Sang
2010-09-22 13:31       ` Peppe CAVALLARO
2010-09-22 14:12         ` Wolfram Sang
2010-09-22 14:35           ` Peppe CAVALLARO
2010-09-23  6:48             ` Peppe CAVALLARO
2010-09-23  8:48             ` Wolfram Sang
2010-09-23  9:30               ` Peppe CAVALLARO
2010-09-23  9:51                 ` Wolfram Sang
2010-09-23 10:05                   ` Peppe CAVALLARO

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